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

[Corey Minyard <tcminyard@gmail.com>]

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, minyard@acm.org wrote:
>>>> From: Corey Minyard <cminyard@mvista.com>
>>>>
>>>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>>>> ---
>>>>   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