Re: [PATCH v3 08/15] acpi: serial: don't use _STA method

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 5/4/20 3:19 PM, Gerd Hoffmann wrote:
>>> -static Aml *build_com_device_aml(uint8_t uid)
>>> +static void build_com_device_aml(Aml *scope, uint8_t uid)
>>>    {
>>>        Aml *dev;
>>>        Aml *crs;
>>> -    Aml *method;
>>> -    Aml *if_ctx;
>>> -    Aml *else_ctx;
>>> -    Aml *zero = aml_int(0);
>>> -    Aml *is_present = aml_local(0);
>>> -    const char *enabled_field = "CAEN";
>>>        uint8_t irq = 4;
>>>        uint16_t io_port = 0x03F8;
>>>        assert(uid == 1 || uid == 2);
>>>        if (uid == 2) {
>>> -        enabled_field = "CBEN";
>>>            irq = 3;
>>>            io_port = 0x02F8;
>>>        }
>>> +    if (!memory_region_present(get_system_io(), io_port)) {
>>> +        return;
>>> +    }
>>
>> The patch looks OK, but an you split this check into a separate (previous?)
>> patch please?
> 
> I don't think this belongs to a separate patch.  It is basically the
> same check the lpc bridge is doing when filling the pci config space
> (see hw/isa/lpc_ich9.c).  So this effectively maintains the existing
> logic, only that we now check directly instead of letting the guest
> check the pci config space bit via _STA method.
> 
> Also note this is only temporary for bisecting, the next patch in the
> series moves the code to a device callback so this kind of "device
> exists" check is not needed any more.

Oh correct.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> take care,
>    Gerd
>