revert commit e4fd0475 ("hvmloader: always include HPET table")

revert commit e4fd0475 ("hvmloader: always include HPET table")

[Jan Beulich]

Windows SVVP tests requiring a HPET ACPI table is in my opinion
not a valid reason to always expose that table - respective tests
should be run with "hpet=1" in the guest config file.

The problem here is that at least with qemu-traditional, which
by default doesn't appear to emulate a HPET, the advertising
here can mislead an OS to believe that there actually is a usable
HPET, which isn't true when neither Xen nor qemu emulate one.
This ie being observed in reality: SLES9, being 2.6.5 based,
doesn't have enough checking to notice that the HPET doesn't
actually work.

In fact, we may want to have an inverse mode (like a lot of
hardware used to behave earlier on): A functional HPET that
isn't exposed in ACPI tables, but that an OS knowing enough
about the chipset can nevertheless find and use.

Jan

Re: revert commit e4fd0475 ("hvmloader: always include HPET table")

[Paolo Bonzini]

Il 03/07/2013 13:43, Jan Beulich ha scritto:
> Windows SVVP tests requiring a HPET ACPI table is in my opinion
> not a valid reason to always expose that table - respective tests
> should be run with "hpet=1" in the guest config file.
> 
> The problem here is that at least with qemu-traditional, which
> by default doesn't appear to emulate a HPET,

Isn't the HPET emulated in the hypervisor anyway?

> the advertising
> here can mislead an OS to believe that there actually is a usable
> HPET, which isn't true when neither Xen nor qemu emulate one.
> This ie being observed in reality: SLES9, being 2.6.5 based,
> doesn't have enough checking to notice that the HPET doesn't
> actually work.

Fair enough, the oldest I tested at the time was 2.6.9.

Paolo

> In fact, we may want to have an inverse mode (like a lot of
> hardware used to behave earlier on): A functional HPET that
> isn't exposed in ACPI tables, but that an OS knowing enough
> about the chipset can nevertheless find and use.

Re: revert commit e4fd0475 ("hvmloader: always include HPET table")

[Jan Beulich]

>>> On 03.07.13 at 13:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 03/07/2013 13:43, Jan Beulich ha scritto:
>> Windows SVVP tests requiring a HPET ACPI table is in my opinion
>> not a valid reason to always expose that table - respective tests
>> should be run with "hpet=1" in the guest config file.
>> 
>> The problem here is that at least with qemu-traditional, which
>> by default doesn't appear to emulate a HPET,
> 
> Isn't the HPET emulated in the hypervisor anyway?

No:

static int hpet_range(struct vcpu *v, unsigned long addr)
{
    return (v->domain->arch.hvm_domain.params[HVM_PARAM_HPET_ENABLED] &&
            (addr >= HPET_BASE_ADDRESS) &&
            (addr < (HPET_BASE_ADDRESS + HPET_MMAP_SIZE)));
}

Jan

Re: revert commit e4fd0475 ("hvmloader: always include HPET table")

[Paolo Bonzini]

Il 03/07/2013 14:03, Jan Beulich ha scritto:
>>>> On 03.07.13 at 13:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 03/07/2013 13:43, Jan Beulich ha scritto:
>>> Windows SVVP tests requiring a HPET ACPI table is in my opinion
>>> not a valid reason to always expose that table - respective tests
>>> should be run with "hpet=1" in the guest config file.
>>>
>>> The problem here is that at least with qemu-traditional, which
>>> by default doesn't appear to emulate a HPET,
>>
>> Isn't the HPET emulated in the hypervisor anyway?
> 
> No:
> 
> static int hpet_range(struct vcpu *v, unsigned long addr)
> {
>     return (v->domain->arch.hvm_domain.params[HVM_PARAM_HPET_ENABLED] &&
>             (addr >= HPET_BASE_ADDRESS) &&
>             (addr < (HPET_BASE_ADDRESS + HPET_MMAP_SIZE)));
> }

Ah, I meant with hpet=1.  Of course if hpet=0 qemu shouldn't emulate it
or should be told not to emulate it.

The idea of the patch was that anyway Device(HPET) is disabled in the
ACPI table if the hpet is disabled.  But if old Linux guests cannot cope
with this, too bad...

Paolo

Re: revert commit e4fd0475 ("hvmloader: always include HPET table")

[Jan Beulich]

>>> On 03.07.13 at 14:07, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The idea of the patch was that anyway Device(HPET) is disabled in the
> ACPI table if the hpet is disabled.  But if old Linux guests cannot cope
> with this, too bad...

The thing is that HPET is needed / gets set up by Linux (and likely
by other OSes too) long before ACPI CA is ready to parse the
DSDT. In fact I don't think Linux even looks at the DSDT object in
order to determine whether HPET is usable (and as per the spec
the intention anyway is to have these objects mainly/exclusively
for secondary HPETs, not the primary one replacing 8254/RTC).

Jan

Ping: revert commit e4fd0475 ("hvmloader: always include HPET table")

[Jan Beulich]

>>> On 03.07.13 at 13:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 03/07/2013 13:43, Jan Beulich ha scritto:
>> Windows SVVP tests requiring a HPET ACPI table is in my opinion
>> not a valid reason to always expose that table - respective tests
>> should be run with "hpet=1" in the guest config file.
>> 
>> The problem here is that at least with qemu-traditional, which
>> by default doesn't appear to emulate a HPET,
> 
> Isn't the HPET emulated in the hypervisor anyway?
> 
>> the advertising
>> here can mislead an OS to believe that there actually is a usable
>> HPET, which isn't true when neither Xen nor qemu emulate one.
>> This ie being observed in reality: SLES9, being 2.6.5 based,
>> doesn't have enough checking to notice that the HPET doesn't
>> actually work.
> 
> Fair enough, the oldest I tested at the time was 2.6.9.

Any opinion on this, Keir? You're the defacto maintainer of
hvmloader (perhaps worth an explicit entry in MAINTAINERS), so
I'd need your ack for reverting said change.

Alternatively, Ian, Ian, Stefano - any opinion (as this is formally
living under tools/)?

Jan

Re: Ping: revert commit e4fd0475 ("hvmloader: always include HPET table")

[Keir Fraser]

On 08/07/2013 11:05, "Jan Beulich" <JBeulich@suse.com> wrote:

>>>> On 03.07.13 at 13:45, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 03/07/2013 13:43, Jan Beulich ha scritto:
>>> Windows SVVP tests requiring a HPET ACPI table is in my opinion
>>> not a valid reason to always expose that table - respective tests
>>> should be run with "hpet=1" in the guest config file.
>>> 
>>> The problem here is that at least with qemu-traditional, which
>>> by default doesn't appear to emulate a HPET,
>> 
>> Isn't the HPET emulated in the hypervisor anyway?
>> 
>>> the advertising
>>> here can mislead an OS to believe that there actually is a usable
>>> HPET, which isn't true when neither Xen nor qemu emulate one.
>>> This ie being observed in reality: SLES9, being 2.6.5 based,
>>> doesn't have enough checking to notice that the HPET doesn't
>>> actually work.
>> 
>> Fair enough, the oldest I tested at the time was 2.6.9.
> 
> Any opinion on this, Keir? You're the defacto maintainer of
> hvmloader (perhaps worth an explicit entry in MAINTAINERS), so
> I'd need your ack for reverting said change.

You're right, this patch was a horrible mistake. Please do revert it.

 -- Keir

> Alternatively, Ian, Ian, Stefano - any opinion (as this is formally
> living under tools/)?
> 
> Jan
>