Re: [Qemu-devel] [PATCH] report serial devices created with -device in the PIIX4 config space

[Andreas Färber]

Am 15.07.2011 um 17:10 schrieb Paolo Bonzini:

> 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 <address@hidden>
> ---
> 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

> @@ -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);

DO_UPCAST()? I assume we have it for a reason.

> 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;

Does this make a change as to whether unhandled I/O ports are reported  
in debug mode?

What do you think of Gleb's recent request to convert all ioports to  
IORanges? I briefly looked into it but feared that needlessly using  
uint64_t for all parameters might damage performance on 32-bit hosts.

The ioport changes look okay otherwise.

Andreas