Re: [Qemu-devel] [PATCH 2/5] acpi_piix4: Only allow writes to PCI hotplug eject register

[Alex Williamson <alex.williamson@redhat.com>]

On Thu, 2012-04-05 at 10:21 +0200, Igor Mammedov wrote:
> On 04/05/2012 07:51 AM, Alex Williamson wrote:
> > This is never read.  We can also derive bus from the write handler,
> > making this more inline with the other callbacks.  Note that
> > pciej_write was actually called with (PCIBus *)dev->bus, which is
> > cast as a void* allowing us to pretend it's a BusState*.  Fix this
> > so we don't depend on the BusState location within PCIBus.
> >
> > Signed-off-by: Alex Williamson<alex.williamson@redhat.com>
> > ---
> >
> >   docs/specs/acpi_pci_hotplug.txt |    2 +-
> >   hw/acpi_piix4.c                 |   14 ++++----------
> >   2 files changed, 5 insertions(+), 11 deletions(-)
> >
> > diff --git a/docs/specs/acpi_pci_hotplug.txt b/docs/specs/acpi_pci_hotplug.txt
> > index 1e2c8a2..1e61d19 100644
> > --- a/docs/specs/acpi_pci_hotplug.txt
> > +++ b/docs/specs/acpi_pci_hotplug.txt
> > @@ -28,7 +28,7 @@ PCI device eject (IO port 0xae08-0xae0b, 4-byte access):
> >   ----------------------------------------
> >
> >   Used by ACPI BIOS _EJ0 method to request device removal. One bit per slot.
> > -Reads return 0.
> > +Read-only.
> Write-only perhaps?

Oops, copy/paste.  Will fix.

> >
> >   PCI removability status (IO port 0xae0c-0xae0f, 4-byte access):
> >   -----------------------------------------------
> > diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> > index 44d1423..6ee832a 100644
> > --- a/hw/acpi_piix4.c
> > +++ b/hw/acpi_piix4.c
> > @@ -487,15 +487,11 @@ static uint32_t pci_down_read(void *opaque, uint32_t addr)
> >       return val;
> >   }
> >
> > -static uint32_t pciej_read(void *opaque, uint32_t addr)
> > -{
> > -    PIIX4_DPRINTF("pciej read %x\n", addr);
> > -    return 0;
> > -}
> > -
> what will happen if bios tries to read from reg?

-1.  Thanks,

Alex

> >   static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
> >   {
> > -    BusState *bus = opaque;
> > +    PIIX4PMState *s = opaque;
> > +    PCIDevice *dev =&s->dev;
> > +    BusState *bus = qdev_get_parent_bus(&dev->qdev);
> >       DeviceState *qdev, *next;
> >       int slot = ffs(val) - 1;
> >
> > @@ -507,7 +503,6 @@ static void pciej_write(void *opaque, uint32_t addr, uint32_t val)
> >           }
> >       }
> >
> > -
> >       PIIX4_DPRINTF("pciej write %x<== %d\n", addr, val);
> >   }
> >
> > @@ -547,8 +542,7 @@ static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
> >       register_ioport_read(PCI_UP_BASE, 4, 4, pci_up_read, s);
> >       register_ioport_read(PCI_DOWN_BASE, 4, 4, pci_down_read, s);
> >
> > -    register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, bus);
> > -    register_ioport_read(PCI_EJ_BASE, 4, 4,  pciej_read, bus);
> > +    register_ioport_write(PCI_EJ_BASE, 4, 4, pciej_write, s);
> >
> >       register_ioport_write(PCI_RMV_BASE, 4, 4, pcirmv_write, s);
> >       register_ioport_read(PCI_RMV_BASE, 4, 4,  pcirmv_read, s);
> >
>