Re: [Qemu-devel] [PATCH 6/7] ipmi: Fix SSIF ACPI handling to use the right CRS

[Corey Minyard]

On 05/12/2016 08:39 AM, Michael S. Tsirkin wrote:
> On Thu, May 12, 2016 at 08:29:53AM -0500, Corey Minyard wrote:
> > On 05/12/2016 02:33 AM, Michael S. Tsirkin wrote:
> > > On Wed, May 11, 2016 at 02:46:05PM -0500, address@hidden wrote:
> > > > From: Corey Minyard <address@hidden>
> > > >
> > > > Signed-off-by: Corey Minyard <address@hidden>
> > > > ---
> > > >   hw/acpi/ipmi.c | 4 +++-
> > > >   1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/hw/acpi/ipmi.c b/hw/acpi/ipmi.c
> > > > index 731f4ad..c187fdd 100644
> > > > --- a/hw/acpi/ipmi.c
> > > > +++ b/hw/acpi/ipmi.c
> > > > @@ -49,7 +49,9 @@ static Aml *aml_ipmi_crs(IPMIFwInfo *info)
> > > >                               regspacing, info->register_length));
> > > >           break;
> > > >       case IPMI_MEMSPACE_SMBUS:
> > > > -        aml_append(crs, aml_return(aml_int(info->base_address)));
> > > > +        aml_append(crs, aml_i2c_serial_bus_device(0, 100000,
> > > > +                                                  info->base_address,
> > > > +                                                  info->acpi_parent));
> > > Isn't this fairly new? If so using these opcodes
> > > is likely to break some older guests. Maybe they already don't
> > > work, but I'd like to see some explanation about that,
> > > and what was tested.
> > This is new with the 5.0 specification.
> >
> > I haven't done extensive testing on anything but Linux 3.10 and later.
> > Well, I might have run 2.6.32, but I can't remember.  I don't have the
> > ability to test Windows.
> >
> > But isn't the idea of these definitions that they are ignored if the OS
> > doesn't understand them?
> Not always. It depends, spec does not require it.
>
> You can check which revision does OSPM support but
> you have to decide what to do for an old revision then.
>
> >   Otherwise you could never add anything.
> >
> > -corey
> Question is, what happened before this change?

I could add a "noacpi" option for the device, that way the user could
accommodate an OS that couldn't handle the option.

I also realized that the addition of the SMBus ACPI device required
backwards compatibility so it wouldn't be there for previous machine
versions.  So you could specify a 2.6 machine and not get the ACPI
entry.

-corey