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

[Oleksandr <olekstysh@xxxxxxxxx>]

On 30.08.21 16:16, Jan Beulich wrote:

Hi Jan

> On 29.08.2021 22:28, Oleksandr wrote:
> > On 16.08.21 10:33, Jan Beulich wrote:
> > > On 14.08.2021 01:28, Oleksandr Tyshchenko wrote:
> > > > --- 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;
> > > >    DEFINE_XEN_GUEST_HANDLE(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?
> Well, HAP or not - there is going to be a limit to a guest's
> address bits. So I don't see why it would matter whether HAP is
> an x86-specific term.
agree


>   If you wanted to express the guest nature
> and distinguish it from "paddr_bits", how about "gaddr_bits" or
> "gpaddr_bits"?
ok, both sound fine to me, with a little preference for gpaddr_bits. So, 
will drop arch-specific sub-struct for the next version.
Thank you.


> Jan
--
Regards,

Oleksandr Tyshchenko