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

[Anthony Liguori <anthony@codemonkey.ws>]

On 07/15/2011 04:00 PM, Andreas Färber wrote:
> 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 <pbonzini@redhat.com>
>> ---
>> 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.

NIH is the reason we have it.

Regards,

Anthony Liguori