From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:38719) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qi627-00014L-HN for qemu-devel@nongnu.org; Sat, 16 Jul 2011 10:39:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Qi626-0003Fz-66 for qemu-devel@nongnu.org; Sat, 16 Jul 2011 10:39:55 -0400 Received: from mail-gw0-f45.google.com ([74.125.83.45]:52702) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Qi625-0003FS-9Y for qemu-devel@nongnu.org; Sat, 16 Jul 2011 10:39:53 -0400 Received: by gwb19 with SMTP id 19so974450gwb.4 for ; Sat, 16 Jul 2011 07:39:52 -0700 (PDT) Message-ID: <4E21A2B6.50100@codemonkey.ws> Date: Sat, 16 Jul 2011 09:39:50 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1310742615-23901-1-git-send-email-pbonzini@redhat.com> In-Reply-To: <1310742615-23901-1-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH] report serial devices created with -device in the PIIX4 config space List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org, armbru@redhat.com On 07/15/2011 10:10 AM, Paolo Bonzini wrote: > Serial and parallel devices created with -device are not reported in > the PIIX4 configuration space, and are hence not picked up by the DSDT. > This upsets Windows, which hides them altogether from the guest. > > To avoid this, check at the end of machine initialization whether the > corresponding I/O ports have been registered. The new function in > ioport.c does this; this also requires a tweak to isa_unassign_ioport. > > I left the comment in piix4_pm_initfn since the registers I moved do > seem to match the 82371AB datasheet. There are some quirks though. > We are setting this bit: > > "Device 8 EIO Enable (EIO_EN_DEV8)—R/W. 1=Enable PCI access to the > device 8 enabled I/O ranges to be claimed by PIIX4 and forwarded > to the ISA/EIO bus. 0=Disable. The LPT_MON_EN must be set to enable > the decode." > > but not LPT_MON_EN (bit 18 at 50h): > > LPT Port Enable (LPT_MON_EN)—R/W. 1=Enable accesses to parallel > port address range (LPT_DEC_SEL) to generate a device 8 (parallel > port) decode event. 0=Disable. > > We're also setting the LPT_DEC_SEL field (that's the 0x60 written to > 63h) to 11, which means reserved, rather than to 01 (378h-37Fh). > > Likewise we're not setting SA_MON_EN, SB_MON_EN (respectively bit 14 > and bit 16 at address 50h) for the serial ports. However, we're setting > COMA_DEC_SEL and COMB_DEC_SEL correctly, unlike the corresponding register > for the parallel port. > > All these fields are left as they are, since they are probably only > meant to be used in the DSDT. > > Signed-off-by: Paolo Bonzini Instead of checking for a port assignment, couldn't we do a device tree transversal and look for isa-serial devices? We could then look at the iobase property to figure out which serial device is configured. Regards, Anthony Liguori > --- > hw/acpi_piix4.c | 23 ++++++++++++++++++----- > ioport.c | 19 +++++++++++++------ > ioport.h | 2 +- > 3 files changed, 32 insertions(+), 12 deletions(-) > > diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c > index 350558b..03de3ad 100644 > --- a/hw/acpi_piix4.c > +++ b/hw/acpi_piix4.c > @@ -23,6 +23,7 @@ > #include "acpi.h" > #include "sysemu.h" > #include "range.h" > +#include "ioport.h" > > //#define DEBUG > > @@ -63,6 +64,7 @@ typedef struct PIIX4PMState { > qemu_irq irq; > qemu_irq smi_irq; > int kvm_enabled; > + Notifier machine_ready; > > /* for pci hotplug */ > ACPIGPE gpe; > @@ -311,6 +313,19 @@ static void piix4_powerdown(void *opaque, int irq, int power_failing) > acpi_pm1_evt_power_down(pm1a, tmr); > } > > +static void piix4_pm_machine_ready(struct Notifier* n) > +{ > + PIIX4PMState *s = container_of(n, PIIX4PMState, machine_ready); > + uint8_t *pci_conf; > + > + pci_conf = s->dev.config; > + pci_conf[0x5f] = (isa_is_ioport_assigned(0x378) ? 0x80 : 0) | 0x10; > + pci_conf[0x63] = 0x60; > + pci_conf[0x67] = (isa_is_ioport_assigned(0x3f8) ? 0x08 : 0) | > + (isa_is_ioport_assigned(0x2f8) ? 0x90 : 0); > + > +} > + > static int piix4_pm_initfn(PCIDevice *dev) > { > PIIX4PMState *s = DO_UPCAST(PIIX4PMState, dev, dev); > @@ -337,11 +352,6 @@ static int piix4_pm_initfn(PCIDevice *dev) > > /* XXX: which specification is used ? The i82731AB has different > mappings */ > - pci_conf[0x5f] = (parallel_hds[0] != NULL ? 0x80 : 0) | 0x10; > - pci_conf[0x63] = 0x60; > - pci_conf[0x67] = (serial_hds[0] != NULL ? 0x08 : 0) | > - (serial_hds[1] != NULL ? 0x90 : 0); > - > pci_conf[0x90] = s->smb_io_base | 1; > pci_conf[0x91] = s->smb_io_base>> 8; > pci_conf[0xd2] = 0x09; > @@ -354,12 +364,14 @@ static int piix4_pm_initfn(PCIDevice *dev) > qemu_system_powerdown = *qemu_allocate_irqs(piix4_powerdown, s, 1); > > pm_smbus_init(&s->dev.qdev,&s->smb); > + s->machine_ready.notify = piix4_pm_machine_ready; > + qemu_add_machine_init_done_notifier(&s->machine_ready); > qemu_register_reset(piix4_reset, s); > piix4_acpi_system_hot_add_init(dev->bus, s); > > return 0; > } > > i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base, > qemu_irq sci_irq, qemu_irq cmos_s3, qemu_irq smi_irq, > int kvm_enabled) > diff --git a/ioport.c b/ioport.c > index 2e971fa..0d2611d 100644 > --- a/ioport.c > +++ b/ioport.c > @@ -245,18 +245,25 @@ void isa_unassign_ioport(pio_addr_t start, int length) > int i; > > for(i = start; i< start + length; i++) { > - ioport_read_table[0][i] = default_ioport_readb; > - ioport_read_table[1][i] = default_ioport_readw; > - ioport_read_table[2][i] = default_ioport_readl; > + ioport_read_table[0][i] = NULL; > + ioport_read_table[1][i] = NULL; > + ioport_read_table[2][i] = NULL; > > - ioport_write_table[0][i] = default_ioport_writeb; > - ioport_write_table[1][i] = default_ioport_writew; > - ioport_write_table[2][i] = default_ioport_writel; > + ioport_write_table[0][i] = NULL; > + ioport_write_table[1][i] = NULL; > + ioport_write_table[2][i] = NULL; > > ioport_opaque[i] = NULL; > } > } > > +bool isa_is_ioport_assigned(pio_addr_t start) > +{ > + return (ioport_read_table[0][start] || ioport_write_table[0][start] || > + ioport_read_table[1][start] || ioport_write_table[1][start] || > + ioport_read_table[2][start] || ioport_write_table[2][start]); > +} > + > /***********************************************************/ > > void cpu_outb(pio_addr_t addr, uint8_t val) > diff --git a/ioport.h b/ioport.h > index 5ae62a3..82ffd9d 100644 > --- a/ioport.h > +++ b/ioport.h > @@ -43,7 +43,7 @@ int register_ioport_read(pio_addr_t start, int length, int size, > int register_ioport_write(pio_addr_t start, int length, int size, > IOPortWriteFunc *func, void *opaque); > void isa_unassign_ioport(pio_addr_t start, int length); > - > +bool isa_is_ioport_assigned(pio_addr_t start); > > void cpu_outb(pio_addr_t addr, uint8_t val); > void cpu_outw(pio_addr_t addr, uint16_t val);