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

Re: [RFC PATCH] xen: Introduce arch specific field to XEN_SYSCTL_physinfo

On 16.08.21 10:33, Jan Beulich wrote:

Hi Jan

Sorry for the late response.

On 14.08.2021 01:28, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>

We need to pass info about maximum supported address space size
to the toolstack on Arm in order to properly calculate the base
and size of the safe range for the guest. Use p2m_ipa_bits variable
which purpose is to hold the bit size of IPAs in P2M tables.
What is "the safe range"?

It is unallocated (unused) address space which could be safely used by domain for foreign/grant mappings on Arm, I will update description.

--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -332,6 +332,11 @@ struct xen_arch_domainconfig {
      uint32_t clock_frequency;
+struct arch_physinfo {
+    /* Holds the bit size of IPAs in p2m tables. */
+    uint32_t p2m_ipa_bits;
  #endif /* __XEN__ || __XEN_TOOLS__ */
struct arch_vcpu_info {
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -346,6 +346,8 @@ typedef struct xen_msr_entry {
  } xen_msr_entry_t;
+struct arch_physinfo {
  #endif /* !__ASSEMBLY__ */
While the term "p2m_ipa_bits" surely isn't arch-agnostic, I wonder
whether the expressed information is (the x86 equivalent being
hap_paddr_bits, at a guess), and hence whether this really ought
to live in an arch-specific sub-struct.

Well, on Arm we calculate the number of the IPA bits based on the number of PA bits when setting Stage 2 address translation. I might mistake, but what we currently have on Arm is "ipa_bits == pa_bits". So, this means that information we need at the toolstack side isn't really arch-specific and we could probably avoid introducing arch-specific sub-struct by suitable renaming the field (pa_bits, paddr_bits, whatever).

We could even name the field as hap_paddr_bits. Although, I don't know whether the hap_* is purely x86-ism, but I see there are several mentions in the common code (hap_pt_share, use_hap_pt, etc). Any suggestions?

If indeed so, please name
the struct in a name space clean way, i.e. add xen_ as prefix.


Also please retain a blank line before the #endif. I wonder whether
on Arm you wouldn't want to add one at this occasion.




Oleksandr Tyshchenko



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