| Message ID | 20091012132025.GA13022@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 12, 2009 at 03:20:27PM +0200, Michael S. Tsirkin wrote: > Right. And there's another bug that I see and that is that size for I/O > BAR was calculated incorrectly: val & ~0xf is wrong for I/O as the size > could be 4 bytes. So here's a patch that addresses all 3 issues: > Ack? > Pleas do whatever spec recommends and use 0xffffffff to size the BAR. Specs says this, linux does this, lets not be different. > > ----> > > Subject: [PATCH] seabios: fix low bits in ROM and I/O sizing > > This cleans up handling of low bits during BAR sizing, > to match PCI spec requirements, and to use symbolic > constants from pci_regs.h > > Issues fixed: > For ROM BARs, bit 0 is writeable (enable bit), which we not > only don't want to set, but it will stick and make us think > it's an I/O port resource. > Further, PCI spec defines the following bits as reserved: > - bit 1 in I/O BAR > - bits 10:1 in ROM BAR > and we should be careful and preserve any values there. > Bits 3:2 in I/O BAR might be writeable, so it > is wrong to mask them when calculating BAR size. > > Spec references: > See 6.2.5.1 for I/O and memory, and 6.2.5.2 for ROM, > 6.1 for reserved bit handling; > pages 225, 228 and 214 in PCI spec revision 3.0. > > See also Qemu pcbios commit 6ddb9f5c742b2b82b1755d7ec2a127f6e20e3806 > > Original-by: Gleb Natapov <gleb@redhat.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > diff --git a/src/pciinit.c b/src/pciinit.c > index 7d2ea00..f9ebd61 100644 > --- a/src/pciinit.c > +++ b/src/pciinit.c > @@ -136,16 +136,23 @@ static void pci_bios_init_device(u16 bdf) > /* default memory mappings */ > for (i = 0; i < PCI_NUM_REGIONS; i++) { > int ofs; > - u32 val, size; > - > + u32 val, mask, size; > if (i == PCI_ROM_SLOT) > ofs = PCI_ROM_ADDRESS; > else > ofs = PCI_BASE_ADDRESS_0 + i * 4; > - pci_config_writel(bdf, ofs, 0xffffffff); > + > + val = pci_config_readl(bdf, ofs); > + if (i == PCI_ROM_SLOT) > + mask = PCI_ROM_ADDRESS_MASK; > + else if (val & PCI_BASE_ADDRESS_SPACE_IO) > + mask = PCI_BASE_ADDRESS_IO_MASK; > + else > + mask = PCI_BASE_ADDRESS_MEM_MASK; > + pci_config_writel(bdf, ofs, val | mask); > val = pci_config_readl(bdf, ofs); > if (val != 0) { > - size = (~(val & ~0xf)) + 1; > + size = (~(val & mask)) + 1; > if (val & PCI_BASE_ADDRESS_SPACE_IO) > paddr = &pci_bios_io_addr; > else if (size >= 0x04000000) > -- > 1.6.5.rc2 > -- Gleb.
On Mon, Oct 12, 2009 at 03:29:25PM +0200, Gleb Natapov wrote: > On Mon, Oct 12, 2009 at 03:20:27PM +0200, Michael S. Tsirkin wrote: > > Right. And there's another bug that I see and that is that size for I/O > > BAR was calculated incorrectly: val & ~0xf is wrong for I/O as the size > > could be 4 bytes. So here's a patch that addresses all 3 issues: > > Ack? > > > Pleas do whatever spec recommends and use 0xffffffff to size the BAR. What we do is equivalent: since next thing we override the BAR value, there's no reason for the save/restore dance. But if you like it this way, OK. Send a patch. Note this applies to I/O and memory only, ROM has to be sized the way I coded it. > Specs says this, linux does this, lets not be different. BTW, note that what linux does for ROM is incorrect: the implementation note you cite only applies to I/O and memory BARs. I'll send a patch to lkml when I have the time. > > > > ----> > > > > Subject: [PATCH] seabios: fix low bits in ROM and I/O sizing > > > > This cleans up handling of low bits during BAR sizing, > > to match PCI spec requirements, and to use symbolic > > constants from pci_regs.h > > > > Issues fixed: > > For ROM BARs, bit 0 is writeable (enable bit), which we not > > only don't want to set, but it will stick and make us think > > it's an I/O port resource. > > Further, PCI spec defines the following bits as reserved: > > - bit 1 in I/O BAR > > - bits 10:1 in ROM BAR > > and we should be careful and preserve any values there. > > Bits 3:2 in I/O BAR might be writeable, so it > > is wrong to mask them when calculating BAR size. > > > > Spec references: > > See 6.2.5.1 for I/O and memory, and 6.2.5.2 for ROM, > > 6.1 for reserved bit handling; > > pages 225, 228 and 214 in PCI spec revision 3.0. > > > > See also Qemu pcbios commit 6ddb9f5c742b2b82b1755d7ec2a127f6e20e3806 > > > > Original-by: Gleb Natapov <gleb@redhat.com> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > diff --git a/src/pciinit.c b/src/pciinit.c > > index 7d2ea00..f9ebd61 100644 > > --- a/src/pciinit.c > > +++ b/src/pciinit.c > > @@ -136,16 +136,23 @@ static void pci_bios_init_device(u16 bdf) > > /* default memory mappings */ > > for (i = 0; i < PCI_NUM_REGIONS; i++) { > > int ofs; > > - u32 val, size; > > - > > + u32 val, mask, size; > > if (i == PCI_ROM_SLOT) > > ofs = PCI_ROM_ADDRESS; > > else > > ofs = PCI_BASE_ADDRESS_0 + i * 4; > > - pci_config_writel(bdf, ofs, 0xffffffff); > > + > > + val = pci_config_readl(bdf, ofs); > > + if (i == PCI_ROM_SLOT) > > + mask = PCI_ROM_ADDRESS_MASK; > > + else if (val & PCI_BASE_ADDRESS_SPACE_IO) > > + mask = PCI_BASE_ADDRESS_IO_MASK; > > + else > > + mask = PCI_BASE_ADDRESS_MEM_MASK; > > + pci_config_writel(bdf, ofs, val | mask); > > val = pci_config_readl(bdf, ofs); > > if (val != 0) { > > - size = (~(val & ~0xf)) + 1; > > + size = (~(val & mask)) + 1; > > if (val & PCI_BASE_ADDRESS_SPACE_IO) > > paddr = &pci_bios_io_addr; > > else if (size >= 0x04000000) > > -- > > 1.6.5.rc2 > > > > -- > Gleb.
On Mon, Oct 12, 2009 at 03:51:12PM +0200, Michael S. Tsirkin wrote: > On Mon, Oct 12, 2009 at 03:29:25PM +0200, Gleb Natapov wrote: > > On Mon, Oct 12, 2009 at 03:20:27PM +0200, Michael S. Tsirkin wrote: > > > Right. And there's another bug that I see and that is that size for I/O > > > BAR was calculated incorrectly: val & ~0xf is wrong for I/O as the size > > > could be 4 bytes. So here's a patch that addresses all 3 issues: > > > Ack? > > > > > Pleas do whatever spec recommends and use 0xffffffff to size the BAR. > > What we do is equivalent: since next thing we override the BAR value, > there's no reason for the save/restore dance. But if you like it this That what spec says if you are not trying to interpret it too hard. And simple is always better. > way, OK. Send a patch. Note this applies to I/O and memory only, > ROM has to be sized the way I coded it. > > > Specs says this, linux does this, lets not be different. > > BTW, note that what linux does for ROM is incorrect: > the implementation note you cite only applies to I/O and memory BARs. > I'll send a patch to lkml when I have the time. > It does mask = type ? ~PCI_ROM_ADDRESS_ENABLE : ~0; so it definitely treats ROM specially, You may argue that it shouldn't set bits 1-10 though. The patch that currently in KVM bios (the one I've sent to seabios initially) uses the same technique to size the BAR as Linux kernel. The only thing missing is restoring of old value. > > > > > > ----> > > > > > > Subject: [PATCH] seabios: fix low bits in ROM and I/O sizing > > > > > > This cleans up handling of low bits during BAR sizing, > > > to match PCI spec requirements, and to use symbolic > > > constants from pci_regs.h > > > > > > Issues fixed: > > > For ROM BARs, bit 0 is writeable (enable bit), which we not > > > only don't want to set, but it will stick and make us think > > > it's an I/O port resource. > > > Further, PCI spec defines the following bits as reserved: > > > - bit 1 in I/O BAR > > > - bits 10:1 in ROM BAR > > > and we should be careful and preserve any values there. > > > Bits 3:2 in I/O BAR might be writeable, so it > > > is wrong to mask them when calculating BAR size. > > > > > > Spec references: > > > See 6.2.5.1 for I/O and memory, and 6.2.5.2 for ROM, > > > 6.1 for reserved bit handling; > > > pages 225, 228 and 214 in PCI spec revision 3.0. > > > > > > See also Qemu pcbios commit 6ddb9f5c742b2b82b1755d7ec2a127f6e20e3806 > > > > > > Original-by: Gleb Natapov <gleb@redhat.com> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > diff --git a/src/pciinit.c b/src/pciinit.c > > > index 7d2ea00..f9ebd61 100644 > > > --- a/src/pciinit.c > > > +++ b/src/pciinit.c > > > @@ -136,16 +136,23 @@ static void pci_bios_init_device(u16 bdf) > > > /* default memory mappings */ > > > for (i = 0; i < PCI_NUM_REGIONS; i++) { > > > int ofs; > > > - u32 val, size; > > > - > > > + u32 val, mask, size; > > > if (i == PCI_ROM_SLOT) > > > ofs = PCI_ROM_ADDRESS; > > > else > > > ofs = PCI_BASE_ADDRESS_0 + i * 4; > > > - pci_config_writel(bdf, ofs, 0xffffffff); > > > + > > > + val = pci_config_readl(bdf, ofs); > > > + if (i == PCI_ROM_SLOT) > > > + mask = PCI_ROM_ADDRESS_MASK; > > > + else if (val & PCI_BASE_ADDRESS_SPACE_IO) > > > + mask = PCI_BASE_ADDRESS_IO_MASK; > > > + else > > > + mask = PCI_BASE_ADDRESS_MEM_MASK; > > > + pci_config_writel(bdf, ofs, val | mask); > > > val = pci_config_readl(bdf, ofs); > > > if (val != 0) { > > > - size = (~(val & ~0xf)) + 1; > > > + size = (~(val & mask)) + 1; > > > if (val & PCI_BASE_ADDRESS_SPACE_IO) > > > paddr = &pci_bios_io_addr; > > > else if (size >= 0x04000000) > > > -- > > > 1.6.5.rc2 > > > > > > > -- > > Gleb. -- Gleb.
On Mon, Oct 12, 2009 at 04:04:25PM +0200, Gleb Natapov wrote: > On Mon, Oct 12, 2009 at 03:51:12PM +0200, Michael S. Tsirkin wrote: > > On Mon, Oct 12, 2009 at 03:29:25PM +0200, Gleb Natapov wrote: > > > On Mon, Oct 12, 2009 at 03:20:27PM +0200, Michael S. Tsirkin wrote: > > > > Right. And there's another bug that I see and that is that size for I/O > > > > BAR was calculated incorrectly: val & ~0xf is wrong for I/O as the size > > > > could be 4 bytes. So here's a patch that addresses all 3 issues: > > > > Ack? > > > > > > > Pleas do whatever spec recommends and use 0xffffffff to size the BAR. > > > > What we do is equivalent: since next thing we override the BAR value, > > there's no reason for the save/restore dance. But if you like it this > That what spec says if you are not trying to interpret it too hard. > And simple is always better. Implementation note is not a recommentation, just one way to do things. We don't need to restore old BAR so no reason to do this, but I won't lose sleep if we do this either. So go ahead. > > > way, OK. Send a patch. Note this applies to I/O and memory only, > > ROM has to be sized the way I coded it. > > > > > Specs says this, linux does this, lets not be different. > > > > BTW, note that what linux does for ROM is incorrect: > > the implementation note you cite only applies to I/O and memory BARs. > > I'll send a patch to lkml when I have the time. > > > It does mask = type ? ~PCI_ROM_ADDRESS_ENABLE : ~0; so it definitely > treats ROM specially, You may argue that it shouldn't set bits 1-10 > though. Right. > The patch that currently in KVM bios (the one I've sent to seabios > initially) uses the same technique to size the BAR as Linux kernel. The only > thing missing is restoring of old value. That, too. Also let's use symbolic constants, and fix size math: (val & ~0xf) is wrong for both ROM and I/O. > > > > > > > > ----> > > > > > > > > Subject: [PATCH] seabios: fix low bits in ROM and I/O sizing > > > > > > > > This cleans up handling of low bits during BAR sizing, > > > > to match PCI spec requirements, and to use symbolic > > > > constants from pci_regs.h > > > > > > > > Issues fixed: > > > > For ROM BARs, bit 0 is writeable (enable bit), which we not > > > > only don't want to set, but it will stick and make us think > > > > it's an I/O port resource. > > > > Further, PCI spec defines the following bits as reserved: > > > > - bit 1 in I/O BAR > > > > - bits 10:1 in ROM BAR > > > > and we should be careful and preserve any values there. > > > > Bits 3:2 in I/O BAR might be writeable, so it > > > > is wrong to mask them when calculating BAR size. > > > > > > > > Spec references: > > > > See 6.2.5.1 for I/O and memory, and 6.2.5.2 for ROM, > > > > 6.1 for reserved bit handling; > > > > pages 225, 228 and 214 in PCI spec revision 3.0. > > > > > > > > See also Qemu pcbios commit 6ddb9f5c742b2b82b1755d7ec2a127f6e20e3806 > > > > > > > > Original-by: Gleb Natapov <gleb@redhat.com> > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > > > diff --git a/src/pciinit.c b/src/pciinit.c > > > > index 7d2ea00..f9ebd61 100644 > > > > --- a/src/pciinit.c > > > > +++ b/src/pciinit.c > > > > @@ -136,16 +136,23 @@ static void pci_bios_init_device(u16 bdf) > > > > /* default memory mappings */ > > > > for (i = 0; i < PCI_NUM_REGIONS; i++) { > > > > int ofs; > > > > - u32 val, size; > > > > - > > > > + u32 val, mask, size; > > > > if (i == PCI_ROM_SLOT) > > > > ofs = PCI_ROM_ADDRESS; > > > > else > > > > ofs = PCI_BASE_ADDRESS_0 + i * 4; > > > > - pci_config_writel(bdf, ofs, 0xffffffff); > > > > + > > > > + val = pci_config_readl(bdf, ofs); > > > > + if (i == PCI_ROM_SLOT) > > > > + mask = PCI_ROM_ADDRESS_MASK; > > > > + else if (val & PCI_BASE_ADDRESS_SPACE_IO) > > > > + mask = PCI_BASE_ADDRESS_IO_MASK; > > > > + else > > > > + mask = PCI_BASE_ADDRESS_MEM_MASK; > > > > + pci_config_writel(bdf, ofs, val | mask); > > > > val = pci_config_readl(bdf, ofs); > > > > if (val != 0) { > > > > - size = (~(val & ~0xf)) + 1; > > > > + size = (~(val & mask)) + 1; > > > > if (val & PCI_BASE_ADDRESS_SPACE_IO) > > > > paddr = &pci_bios_io_addr; > > > > else if (size >= 0x04000000) > > > > -- > > > > 1.6.5.rc2 > > > > > > > > > > -- > > > Gleb. > > -- > Gleb.
On Mon, Oct 12, 2009 at 04:11:29PM +0200, Michael S. Tsirkin wrote: > On Mon, Oct 12, 2009 at 04:04:25PM +0200, Gleb Natapov wrote: > > On Mon, Oct 12, 2009 at 03:51:12PM +0200, Michael S. Tsirkin wrote: > > > On Mon, Oct 12, 2009 at 03:29:25PM +0200, Gleb Natapov wrote: > > > > On Mon, Oct 12, 2009 at 03:20:27PM +0200, Michael S. Tsirkin wrote: > > > > > Right. And there's another bug that I see and that is that size for I/O > > > > > BAR was calculated incorrectly: val & ~0xf is wrong for I/O as the size > > > > > could be 4 bytes. So here's a patch that addresses all 3 issues: > > > > > Ack? > > > > > > > > > Pleas do whatever spec recommends and use 0xffffffff to size the BAR. > > > > > > What we do is equivalent: since next thing we override the BAR value, > > > there's no reason for the save/restore dance. But if you like it this > > That what spec says if you are not trying to interpret it too hard. > > And simple is always better. > > Implementation note is not a recommentation, just one way to do things. Agree. But I prefer to use a way recommended by spec. It much less likely to break. > We don't need to restore old BAR so no reason to do this, but We need to restore lower bits. > I won't lose sleep if we do this either. So go ahead. > > > > > > way, OK. Send a patch. Note this applies to I/O and memory only, > > > ROM has to be sized the way I coded it. > > > > > > > Specs says this, linux does this, lets not be different. > > > > > > BTW, note that what linux does for ROM is incorrect: > > > the implementation note you cite only applies to I/O and memory BARs. > > > I'll send a patch to lkml when I have the time. > > > > > It does mask = type ? ~PCI_ROM_ADDRESS_ENABLE : ~0; so it definitely > > treats ROM specially, You may argue that it shouldn't set bits 1-10 > > though. > > Right. > > > The patch that currently in KVM bios (the one I've sent to seabios > > initially) uses the same technique to size the BAR as Linux kernel. The only > > thing missing is restoring of old value. > > That, too. Also let's use symbolic constants, and fix size math: > (val & ~0xf) is wrong for both ROM and I/O. > 0xf fix should be really in a separate patch. > > > > > > > > > > ----> > > > > > > > > > > Subject: [PATCH] seabios: fix low bits in ROM and I/O sizing > > > > > > > > > > This cleans up handling of low bits during BAR sizing, > > > > > to match PCI spec requirements, and to use symbolic > > > > > constants from pci_regs.h > > > > > > > > > > Issues fixed: > > > > > For ROM BARs, bit 0 is writeable (enable bit), which we not > > > > > only don't want to set, but it will stick and make us think > > > > > it's an I/O port resource. > > > > > Further, PCI spec defines the following bits as reserved: > > > > > - bit 1 in I/O BAR > > > > > - bits 10:1 in ROM BAR > > > > > and we should be careful and preserve any values there. > > > > > Bits 3:2 in I/O BAR might be writeable, so it > > > > > is wrong to mask them when calculating BAR size. > > > > > > > > > > Spec references: > > > > > See 6.2.5.1 for I/O and memory, and 6.2.5.2 for ROM, > > > > > 6.1 for reserved bit handling; > > > > > pages 225, 228 and 214 in PCI spec revision 3.0. > > > > > > > > > > See also Qemu pcbios commit 6ddb9f5c742b2b82b1755d7ec2a127f6e20e3806 > > > > > > > > > > Original-by: Gleb Natapov <gleb@redhat.com> > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > > > > > diff --git a/src/pciinit.c b/src/pciinit.c > > > > > index 7d2ea00..f9ebd61 100644 > > > > > --- a/src/pciinit.c > > > > > +++ b/src/pciinit.c > > > > > @@ -136,16 +136,23 @@ static void pci_bios_init_device(u16 bdf) > > > > > /* default memory mappings */ > > > > > for (i = 0; i < PCI_NUM_REGIONS; i++) { > > > > > int ofs; > > > > > - u32 val, size; > > > > > - > > > > > + u32 val, mask, size; > > > > > if (i == PCI_ROM_SLOT) > > > > > ofs = PCI_ROM_ADDRESS; > > > > > else > > > > > ofs = PCI_BASE_ADDRESS_0 + i * 4; > > > > > - pci_config_writel(bdf, ofs, 0xffffffff); > > > > > + > > > > > + val = pci_config_readl(bdf, ofs); > > > > > + if (i == PCI_ROM_SLOT) > > > > > + mask = PCI_ROM_ADDRESS_MASK; > > > > > + else if (val & PCI_BASE_ADDRESS_SPACE_IO) > > > > > + mask = PCI_BASE_ADDRESS_IO_MASK; > > > > > + else > > > > > + mask = PCI_BASE_ADDRESS_MEM_MASK; > > > > > + pci_config_writel(bdf, ofs, val | mask); > > > > > val = pci_config_readl(bdf, ofs); > > > > > if (val != 0) { > > > > > - size = (~(val & ~0xf)) + 1; > > > > > + size = (~(val & mask)) + 1; > > > > > if (val & PCI_BASE_ADDRESS_SPACE_IO) > > > > > paddr = &pci_bios_io_addr; > > > > > else if (size >= 0x04000000) > > > > > -- > > > > > 1.6.5.rc2 > > > > > > > > > > > > > -- > > > > Gleb. > > > > -- > > Gleb. -- Gleb.
On Mon, Oct 12, 2009 at 04:17:11PM +0200, Gleb Natapov wrote: > On Mon, Oct 12, 2009 at 04:11:29PM +0200, Michael S. Tsirkin wrote: > > On Mon, Oct 12, 2009 at 04:04:25PM +0200, Gleb Natapov wrote: > > > On Mon, Oct 12, 2009 at 03:51:12PM +0200, Michael S. Tsirkin wrote: > > > > On Mon, Oct 12, 2009 at 03:29:25PM +0200, Gleb Natapov wrote: > > > > > On Mon, Oct 12, 2009 at 03:20:27PM +0200, Michael S. Tsirkin wrote: > > > > > > Right. And there's another bug that I see and that is that size for I/O > > > > > > BAR was calculated incorrectly: val & ~0xf is wrong for I/O as the size > > > > > > could be 4 bytes. So here's a patch that addresses all 3 issues: > > > > > > Ack? > > > > > > > > > > > Pleas do whatever spec recommends and use 0xffffffff to size the BAR. > > > > > > > > What we do is equivalent: since next thing we override the BAR value, > > > > there's no reason for the save/restore dance. But if you like it this > > > That what spec says if you are not trying to interpret it too hard. > > > And simple is always better. > > > > Implementation note is not a recommentation, just one way to do things. > Agree. But I prefer to use a way recommended by spec. It much less > likely to break. > > > We don't need to restore old BAR so no reason to do this, but > We need to restore lower bits. The way I did it was always keeping original values for the lower bits. > > I won't lose sleep if we do this either. So go ahead. > > > > > > > > > way, OK. Send a patch. Note this applies to I/O and memory only, > > > > ROM has to be sized the way I coded it. > > > > > > > > > Specs says this, linux does this, lets not be different. > > > > > > > > BTW, note that what linux does for ROM is incorrect: > > > > the implementation note you cite only applies to I/O and memory BARs. > > > > I'll send a patch to lkml when I have the time. > > > > > > > It does mask = type ? ~PCI_ROM_ADDRESS_ENABLE : ~0; so it definitely > > > treats ROM specially, You may argue that it shouldn't set bits 1-10 > > > though. > > > > Right. > > > > > The patch that currently in KVM bios (the one I've sent to seabios > > > initially) uses the same technique to size the BAR as Linux kernel. The only > > > thing missing is restoring of old value. > > > > That, too. Also let's use symbolic constants, and fix size math: > > (val & ~0xf) is wrong for both ROM and I/O. > > > 0xf fix should be really in a separate patch. It could be, but it won't be an independent patch: I'm using the same mask value all over the code. > > > > > > > > > > > > ----> > > > > > > > > > > > > Subject: [PATCH] seabios: fix low bits in ROM and I/O sizing > > > > > > > > > > > > This cleans up handling of low bits during BAR sizing, > > > > > > to match PCI spec requirements, and to use symbolic > > > > > > constants from pci_regs.h > > > > > > > > > > > > Issues fixed: > > > > > > For ROM BARs, bit 0 is writeable (enable bit), which we not > > > > > > only don't want to set, but it will stick and make us think > > > > > > it's an I/O port resource. > > > > > > Further, PCI spec defines the following bits as reserved: > > > > > > - bit 1 in I/O BAR > > > > > > - bits 10:1 in ROM BAR > > > > > > and we should be careful and preserve any values there. > > > > > > Bits 3:2 in I/O BAR might be writeable, so it > > > > > > is wrong to mask them when calculating BAR size. > > > > > > > > > > > > Spec references: > > > > > > See 6.2.5.1 for I/O and memory, and 6.2.5.2 for ROM, > > > > > > 6.1 for reserved bit handling; > > > > > > pages 225, 228 and 214 in PCI spec revision 3.0. > > > > > > > > > > > > See also Qemu pcbios commit 6ddb9f5c742b2b82b1755d7ec2a127f6e20e3806 > > > > > > > > > > > > Original-by: Gleb Natapov <gleb@redhat.com> > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > > > > > > > > > diff --git a/src/pciinit.c b/src/pciinit.c > > > > > > index 7d2ea00..f9ebd61 100644 > > > > > > --- a/src/pciinit.c > > > > > > +++ b/src/pciinit.c > > > > > > @@ -136,16 +136,23 @@ static void pci_bios_init_device(u16 bdf) > > > > > > /* default memory mappings */ > > > > > > for (i = 0; i < PCI_NUM_REGIONS; i++) { > > > > > > int ofs; > > > > > > - u32 val, size; > > > > > > - > > > > > > + u32 val, mask, size; > > > > > > if (i == PCI_ROM_SLOT) > > > > > > ofs = PCI_ROM_ADDRESS; > > > > > > else > > > > > > ofs = PCI_BASE_ADDRESS_0 + i * 4; > > > > > > - pci_config_writel(bdf, ofs, 0xffffffff); > > > > > > + > > > > > > + val = pci_config_readl(bdf, ofs); > > > > > > + if (i == PCI_ROM_SLOT) > > > > > > + mask = PCI_ROM_ADDRESS_MASK; > > > > > > + else if (val & PCI_BASE_ADDRESS_SPACE_IO) > > > > > > + mask = PCI_BASE_ADDRESS_IO_MASK; > > > > > > + else > > > > > > + mask = PCI_BASE_ADDRESS_MEM_MASK; > > > > > > + pci_config_writel(bdf, ofs, val | mask); > > > > > > val = pci_config_readl(bdf, ofs); > > > > > > if (val != 0) { > > > > > > - size = (~(val & ~0xf)) + 1; > > > > > > + size = (~(val & mask)) + 1; > > > > > > if (val & PCI_BASE_ADDRESS_SPACE_IO) > > > > > > paddr = &pci_bios_io_addr; > > > > > > else if (size >= 0x04000000) > > > > > > -- > > > > > > 1.6.5.rc2 > > > > > > > > > > > > > > > > -- > > > > > Gleb. > > > > > > -- > > > Gleb. > > -- > Gleb.
diff --git a/src/pciinit.c b/src/pciinit.c index 7d2ea00..f9ebd61 100644 --- a/src/pciinit.c +++ b/src/pciinit.c @@ -136,16 +136,23 @@ static void pci_bios_init_device(u16 bdf) /* default memory mappings */ for (i = 0; i < PCI_NUM_REGIONS; i++) { int ofs; - u32 val, size; - + u32 val, mask, size; if (i == PCI_ROM_SLOT) ofs = PCI_ROM_ADDRESS; else ofs = PCI_BASE_ADDRESS_0 + i * 4; - pci_config_writel(bdf, ofs, 0xffffffff); + + val = pci_config_readl(bdf, ofs); + if (i == PCI_ROM_SLOT) + mask = PCI_ROM_ADDRESS_MASK; + else if (val & PCI_BASE_ADDRESS_SPACE_IO) + mask = PCI_BASE_ADDRESS_IO_MASK; + else + mask = PCI_BASE_ADDRESS_MEM_MASK; + pci_config_writel(bdf, ofs, val | mask); val = pci_config_readl(bdf, ofs); if (val != 0) { - size = (~(val & ~0xf)) + 1; + size = (~(val & mask)) + 1; if (val & PCI_BASE_ADDRESS_SPACE_IO) paddr = &pci_bios_io_addr; else if (size >= 0x04000000)