RE: [PATCH v2 1/6] xen: introduce XEN_DOMCTL_CDF_directmap

[Penny Zheng <Penny.Zheng@xxxxxxx>]

Hi Julien

> -----Original Message-----
> From: Julien Grall <julien@xxxxxxx>
> Sent: Friday, October 15, 2021 4:57 PM
> To: Penny Zheng <Penny.Zheng@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> sstabellini@xxxxxxxxxx
> Cc: Wei Chen <Wei.Chen@xxxxxxx>; Bertrand Marquis
> <Bertrand.Marquis@xxxxxxx>
> Subject: Re: [PATCH v2 1/6] xen: introduce XEN_DOMCTL_CDF_directmap
> 
> Hi Penny,
> 
> On 15/10/2021 04:09, Penny Zheng wrote:
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 0167731ab0..37e2d62d47 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -3069,8 +3069,10 @@ static int __init construct_dom0(struct domain *d)
> >   void __init create_dom0(void)
> >   {
> >       struct domain *dom0;
> > +    /* DOM0 has always its memory directly mapped. */
> >       struct xen_domctl_createdomain dom0_cfg = {
> > -        .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap,
> > +        .flags = XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
> > +                 XEN_DOMCTL_CDF_directmap,
> >           .max_evtchn_port = -1,
> >           .max_grant_frames = gnttab_dom0_frames(),
> >           .max_maptrack_frames = -1,
> > diff --git a/xen/common/domain.c b/xen/common/domain.c index
> > 8b53c49d1e..7a6131db74 100644
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -486,7 +486,8 @@ static int sanitise_domain_config(struct
> xen_domctl_createdomain *config)
> >            ~(XEN_DOMCTL_CDF_hvm | XEN_DOMCTL_CDF_hap |
> >              XEN_DOMCTL_CDF_s3_integrity | XEN_DOMCTL_CDF_oos_off |
> >              XEN_DOMCTL_CDF_xs_domain | XEN_DOMCTL_CDF_iommu |
> > -           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu) )
> > +           XEN_DOMCTL_CDF_nested_virt | XEN_DOMCTL_CDF_vpmu |
> > +           XEN_DOMCTL_CDF_directmap) )
> >       {
> >           dprintk(XENLOG_INFO, "Unknown CDF flags %#x\n", config->flags);
> >           return -EINVAL;
> > diff --git a/xen/include/asm-arm/domain.h
> > b/xen/include/asm-arm/domain.h index 14e575288f..fc42c6a310 100644
> > --- a/xen/include/asm-arm/domain.h
> > +++ b/xen/include/asm-arm/domain.h
> > @@ -29,8 +29,8 @@ enum domain_type {
> >   #define is_64bit_domain(d) (0)
> >   #endif
> >
> > -/* The hardware domain has always its memory direct mapped. */
> > -#define is_domain_direct_mapped(d) is_hardware_domain(d)
> > +/* Check if domain is direct-map memory map. */ #define
> > +is_domain_direct_mapped(d) (d->options & XEN_DOMCTL_CDF_directmap)
> >
> >   struct vtimer {
> >       struct vcpu *v;
> > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > index 238384b5ae..b505a0db51 100644
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -72,9 +72,11 @@ struct xen_domctl_createdomain {
> >   #define XEN_DOMCTL_CDF_nested_virt    (1U <<
> _XEN_DOMCTL_CDF_nested_virt)
> >   /* Should we expose the vPMU to the guest? */
> >   #define XEN_DOMCTL_CDF_vpmu           (1U << 7)
> > +/* If this domain has its memory directly mapped? (ARM only) */
> > +#define XEN_DOMCTL_CDF_directmap      (1U << 8)
> >
> >   /* Max XEN_DOMCTL_CDF_* constant.  Used for ABI checking. */
> > -#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu
> > +#define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_directmap
> 
> In the previous version, this flag was only settable for domain created by 
> Xen.
> Now this is also settable by the toolstack. I don't think the toolstack can
> sensibly use this flag (at least in the current state).
> 
> So can you explain why this flag is exposed to the toolstack?
> 

if I moved XEN_DOMCTL_CDF_directmap to the domain.h, and let it hold the
bit 8, in case later another developer wants to introduce a new flag to 
accidently
hold 8 bit too, I would like to add some explanatory comments here and maybe 
rename
the XEN_DOMCTL_CDF_directmap to XEN_DOMCTL_CDF_Internal_directmap, wdyt?

> Cheers,
> 
> --

Cheers

Penny Zheng
> Julien Grall