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

take care,
  Gerd


Reply via email to