Re: [PATCH v2 26/41] arm : acpi add xen environment table

[Julien Grall <julien.grall@citrix.com>]

Hi Jan,

On 21/05/15 07:22, Jan Beulich wrote:
>>>> On 20.05.15 at 19:00, <julien.grall@citrix.com> wrote:
>> On 20/05/15 17:22, Jan Beulich wrote:
>>>>>> On 17.05.15 at 22:03, <parth.dixit@linaro.org> wrote:
>>>> Xen environment table is ACPI table that is used to pass grant table
>>>> and event channel interrupt information to dom0.
>>>
>>> The documents linked to by the uefi.org web site don't look like these
>>> are final, formally acceptable definitions. I'm not sure we want to
>>> include potentially in flux things in the hypervisor when it is not clear
>>> whether by the next release these would get finalized.
>>
>> What do you mean by "formally acceptable definitions"?
> 
> Oops, sorry - s/acceptable/accepted/.
> 
>> The XENV table is final and accepted as a separate table handle by Xen
>> Project.
>>
>> I would have to dig into my mail to find why we decide to only give a URL...
> 
> The linked to document (on our wiki) is versioned 0.<something>,
> which doesn't look like a final stable version. The same applies to
> the other (STAO?) one.

That's a mistake in the version number. Those tables has been reviewed
by Citrix and Linaro people and we agreed about the final tables.

> 
>>> Apart from that I could do with an explanation of why the XENV table
>>> is needed in the first place, considering that on x86 we get away
>>> without.
>>
>> When you boot DOM0 on ARM there is no way to know that we are running on
>> Xen (no cpuid like, no specific boot path,...).
> 
> Iirc ARM has a CPUID like identification mechanism - why can't that
> be used? And if it can't be used, considering that namely ARM64
> basically has virtualization support designed in from the beginning,
> doesn't this look like a design flaw? After all - do you really see
> each hypervisor kind to define their own ACPI table as a proper,
> scalable mechanism?

The ARM CPUID describe the hardware but doesn't offer the opportunity to
extend it as x86 in order to expose Hypervisor specific CPUID.

I know there was some discussion about adding Hypervisor CPUID but so
far it's not in the spec. We have to deal with it.

>> For the device tree, we
>> include a new node. For ACPI, this table allow us to know the we are
>> running on Xen.
> 
> Which seems superseded by 6.0's hypervisor vendor identification
> in FADT. And the OEM IDs in various table headers could have
> served such identification purposes too, as could have "OEMx"
> tables.

ACPI 6.0 has been released few months after Parth and Naresh began to
implement ACPI for Xen. We could take advantage of this new field.

The "OEMx" could have clashed with the one provided by the hardware.

With a separate signature reserved ("XENV" is reserved since ACPI 6.0)
we avoid any clash.

>> Furthermore, on x86 all these informations are passed via the start_info
>> structure which doesn't exits on ARM. And there would be no easy way to
>> pass it to DOM0 at startup (the memory layout is different from every
>> board).
> 
> There's no start_info structure for x86 HVM. And passing a pointer
> to the entry point in a register, or via EFI GUID (as you seem to tie
> together ACPI and EFI presence) could have done the same.

The entry point register is not an option. The current impact of XEN ARM
in the OS is very miminal and mostly self-contained. We would like to
keep the same impact with the ACPI support.

There was some concerns about exposing start_info to ARM. I don't
exactly remember which one. I will let Ian, Stefano answer to this.

Regards,

-- 
Julien Grall