Re: [QEMU][PATCH v2 10/11] hw/arm: introduce xenpv machine

[Alex Bennée]

Julien Grall <julien@xen.org> writes:

> Hi,
>
> On 02/12/2022 22:36, Stefano Stabellini wrote:
>>> Do you know what Xen version your build env has?
>> I think Alex is just building against upstream Xen. GUEST_TPM_BASE
>> is
>> not defined there yet. I think we would need to introduce in
>> xen_common.h something like:
>> #ifndef GUEST_TPM_BASE
>> #define GUEST_TPM_BASE 0x0c000000
>> #endif
>
> I think this would be a big mistake to add the two lines above in QEMU.
>
> Libxl is responsible for creating the domain and generating the
> firwmare tables. Any mismatch of values will be a real pain to debug.
>
> Even if...
>
>> We already have similar code in xen_common.h for other things.
>> Also, it
>> would be best to get GUEST_TPM_BASE defined upstream in Xen first.
>
> ... we introduce upstream first, the guest layout is not part of the
> stable ABI and therefore could change from release to release.
>
>> 
>>> Another way to fix this(as Julien suggested) is by setting this 
>>> GUEST_TPM_BASE
>>> value via a property or something and user can set it via command line.
>>>
>>> @sstabellini@kernel.org, do you think of any other fix?
>> Setting the TPM address from the command line is nice and preferable
>> to
>> hardcoding the value in xen_common.h. It comes with the challenge that
>> it is not very scalable (imagine we have a dozen emulated devices) but
>> for now it is fine and a good way to start if you can arrange it.
>
> It is not clear which one you think is not scalable. If this is the
> command line option approach, then I think this is unrealistic to ask
> every user to rebuild there QEMU just because the guest layout has
> changed.
>
> Today the rebuild may only be necessary when switching to a new
> release. But in the future we may imagine a per-domain layout (e.g.
> for legacy purpose). So you will now need to request the user to have
> one QEMU built per domain.

I agree if this is something that can change it needs to be configured
from the command line or somehow otherwise gleaned from the source of
truth. Isn't this information available via XenStore? Isn't that what
Viresh has to do for all the VirtIO devices he's adding to libxl?

A default value for the option could be done I guess.

>
> How is that scalable?
>
> Cheers,


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro