qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] Re: [PATCH 3/5] Use the correct mask to size the PCI option


From: Michael S. Tsirkin
Subject: [Qemu-devel] Re: [PATCH 3/5] Use the correct mask to size the PCI option ROM BAR.
Date: Mon, 12 Oct 2009 13:45:37 +0200
User-agent: Mutt/1.5.19 (2009-01-05)

On Mon, Oct 12, 2009 at 01:03:36PM +0200, Michael S. Tsirkin wrote:
> On Mon, Oct 12, 2009 at 12:08:21PM +0200, Gleb Natapov wrote:
> > On Mon, Oct 12, 2009 at 11:52:25AM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Oct 12, 2009 at 08:50:24AM +0200, Gleb Natapov wrote:
> > > > Send patch with your favorite interpretation to qemu pcbios/seabios.
> > > > The regression concern from my previous mail applicable here as well.
> > > 
> > > Okay. Can you ack the following?
> > > 
> > I can if you'll add PCI spec reference for me to double check.
> 
> 
> > Also I prefer strict spec reading :)
> 
> OK, the issue is that reserved bits in BARs are not
> defined as read-only.  So here's a strict one:
> can you ack?

Actually, that's not right either.
It says on page 214:

        Software must take care to deal correctly with bit-encoded fields that 
have
        some bits reserved for future use. On reads, software must use 
appropriate
        masks to extract the defined bits, and may not rely on reserved bits 
being any
        particular value. On writes, software must ensure that the values of 
reserved
        bit positions are preserved; that is, the values of reserved bit 
positions must
        first be read, merged with the new values for other bit positions and 
the data
        then written back.

So let's do this. Patch forthcoming.


> --->
> 
> seabios: fix ROM and I/O sizing
> 
> 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 write 0 there.
> For memory, bits 0-3 are reserved, so it's safe to handle it
> in the same way as I/O.
> 
> See 6.2.5.1 for I/O and memory, and 6.2.5.2 for ROM;
> pages 225 and 228 in PCI spec revision 3.0.
> 
> See also Qemu pcbios commit 6ddb9f5c742b2b82b1755d7ec2a127f6e20e3806
> 
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  src/pciinit.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/pciinit.c b/src/pciinit.c
> index 1d0f784..29b3901 100644
> --- a/src/pciinit.c
> +++ b/src/pciinit.c
> @@ -139,11 +139,13 @@ static void pci_bios_init_device(u16 bdf)
>              int ofs;
>              u32 val, size;
>  
> -            if (i == PCI_ROM_SLOT)
> +            if (i == PCI_ROM_SLOT) {
>                  ofs = PCI_ROM_ADDRESS;
> -            else
> +                pci_config_writel(bdf, ofs, PCI_ROM_ADDRESS_MASK);
> +            } else {
>                  ofs = PCI_BASE_ADDRESS_0 + i * 4;
> -            pci_config_writel(bdf, ofs, 0xffffffff);
> +                pci_config_writel(bdf, ofs, PCI_BASE_ADDRESS_IO_MASK);
> +            }
>              val = pci_config_readl(bdf, ofs);
>              if (val != 0) {
>                  size = (~(val & ~0xf)) + 1;
> -- 
> 1.6.3.3
> 
> 
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]