[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH 1/3] xen: Introduce "gpaddr_bits" field to XEN_SYSCTL_physinfo




Hi Andrew.


On 08.09.21 00:28, Oleksandr wrote:

On 07.09.21 20:35, Andrew Cooper wrote:

Hi Andrew

On 07/09/2021 18:09, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>

We need to pass info about maximum supported guest address
space size to the toolstack on Arm in order to properly
calculate the base and size of the extended region (safe range)
for the guest. The extended region is unused address space which
could be safely used by domain for foreign/grant mappings on Arm.
The extended region itself will be handled by the subsequents
patch.

Use p2m_ipa_bits variable on Arm, the x86 equivalent is
hap_paddr_bits.

As we change the size of structure bump the interface version.

Suggested-by: Julien Grall <jgrall@xxxxxxxxxx>
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
So while I think this is a suitable way forward, you're painting
yourself into a corner WRT migration.

On x86, the correct value is d->arch.cpuid->extd.maxphysaddr and this
value is under toolstack control, not Xen control.  In particular, it
needs to be min(hostA, hostB) to make migration safe, and yes - this is
currently a hole in x86's migration logic that will cause large VMs to
explode.

The same will be true on ARM as/when you gain migration support.

Oh, I admit, I didn't keep in mind migration support while creating this patch.



I think this would be better as a domctl.  On ARM, it can reference
p2m_ipa_bits for now along with a /* TODO - make per-domain for
migration support */, while on x86 it can take the appropriate value
(which will soon actually be safe in migration scenarios).

OK, could you please clarify, did you mean to introduce new domctl (something like get_maxphysaddr) or reuse some existing?


May I please ask for clarification here ^ and ...




And ...


However, that does change the ordering requirements in the toolstack -
this hypercall would need to be made after the domain is created, and
has been levelled, and before its main memory layout is decided.

... I am not sure I totally understand the ordering requirements in the toolstack.

On Arm this information (gpaddr_bits) should be obtained by the time we call libxl__arch_domain_finalise_hw_description() where we actually calculate extended region and insert it in domain's device-tree. At that time, the domain memory is already populated, so it's layout is known.
Please see the last patch of this series, which demonstrates the usage:

https://lore.kernel.org/xen-devel/1631034578-12598-4-git-send-email-olekstysh@xxxxxxxxx/


Alternatively, the abstraction could be hidden in libxl itself in arch
specific code, with x86 referring to the local cpu policy (as libxl has
the copy it is/has worked on), and ARM making a hypercall.  This does
make the ordering more obvious.

May I please ask what would be the preferred option to move forward? I am fine with both proposed options, with a little preference for the former (common domctl, abstraction is hidden in Xen itself in arch specific code). It is highly appreciated if we could choose the option which would satisfy all parties, this would save me some time.


... here ^ ?


I have just pushed third version [1] of this series without these being addressed. I decided to push V3 to let the reviewers to focus on the main aspects, but the way how the information in question is passed to the toolstack also needs to be clarified and addressed.


[1] https://lore.kernel.org/xen-devel/1632437334-12015-1-git-send-email-olekstysh@xxxxxxxxx/


[snip]


--
Regards,

Oleksandr Tyshchenko




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.