On 07.12.2021 10:15, Penny Zheng wrote: > Hi guys > >> -----Original Message----- >> From: Penny Zheng <[email protected]> >> Sent: Tuesday, November 16, 2021 1:25 PM >> To: Penny Zheng <[email protected]> >> Cc: nd <[email protected]> >> Subject: [PATCH v3 01/10] xen: introduce >> XEN_DOMCTL_CDF_INTERNAL_directmap >> >> From: Stefano Stabellini <[email protected]> >> >> This commit introduces a new arm-specific flag >> XEN_DOMCTL_CDF_INTERNAL_directmap to specify that a domain should >> have its memory direct-map(guest physical address == physical address) at >> domain creation. >> >> Since this flag is only available for domain created by XEN, not exposed to >> the >> toolstack, we name it with extra "INTERNAL" to distinguish from other public >> XEN_DOMCTL_CDF_xxx flags, and add comments to warn developers not to >> accidently use its bitfield when introducing new XEN_DOMCTL_CDF_xxx flag. >> >> Refine is_domain_direct_mapped to check whether the flag >> XEN_DOMCTL_CDF_INTERNAL_directmap is set. >> >> Signed-off-by: Penny Zheng <[email protected]> >> Signed-off-by: Stefano Stabellini <[email protected]> >> --- >> CC: [email protected] >> CC: [email protected] >> CC: George Dunlap <[email protected]> >> CC: Ian Jackson <[email protected]> >> CC: Wei Liu <[email protected]> >> CC: "Roger Pau Monné" <[email protected]> >> --- >> v2 changes >> - remove the introduce of internal flag >> - remove flag direct_map since we already store this flag in d->options >> - Refine is_domain_direct_mapped to check whether the flag >> XEN_DOMCTL_CDF_directmap is set >> - reword "1:1 direct-map" to just "direct-map" >> --- >> v3 changes >> - move flag back to xen/include/xen/domain.h, to let it be only available for >> domain created by XEN. >> - name it with extra "INTERNAL" and add comments to warn developers not to >> accidently use its bitfield when introducing new XEN_DOMCTL_CDF_xxx flag. >> - reject this flag in x86'es arch_sanitise_domain_config() >> --- >> xen/arch/arm/domain.c | 3 ++- >> xen/arch/arm/domain_build.c | 4 +++- >> xen/arch/x86/domain.c | 6 ++++++ >> xen/common/domain.c | 3 ++- >> xen/include/asm-arm/domain.h | 4 ++-- >> xen/include/public/domctl.h | 4 ++++ >> xen/include/xen/domain.h | 3 +++ >> 7 files changed, 22 insertions(+), 5 deletions(-) >> >> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index >> 96e1b23550..d77265c03f 100644 >> --- a/xen/arch/arm/domain.c >> +++ b/xen/arch/arm/domain.c >> @@ -629,7 +629,8 @@ int arch_sanitise_domain_config(struct >> xen_domctl_createdomain *config) { >> unsigned int max_vcpus; >> unsigned int flags_required = (XEN_DOMCTL_CDF_hvm | >> XEN_DOMCTL_CDF_hap); >> - unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | >> XEN_DOMCTL_CDF_vpmu); >> + unsigned int flags_optional = (XEN_DOMCTL_CDF_iommu | >> XEN_DOMCTL_CDF_vpmu | >> + XEN_DOMCTL_CDF_INTERNAL_directmap); >> >> if ( (config->flags & ~flags_optional) != flags_required ) >> { >> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c >> index 19487c79da..664c88ebe4 100644 >> --- a/xen/arch/arm/domain_build.c >> +++ b/xen/arch/arm/domain_build.c >> @@ -3089,8 +3089,10 @@ static int __init construct_dom0(struct domain *d) >> void __init create_dom0(void) { >> struct domain *dom0; >> + /* DOM0 has always its memory direct-map. */ >> 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_INTERNAL_directmap, >> .max_evtchn_port = -1, >> .max_grant_frames = gnttab_dom0_frames(), >> .max_maptrack_frames = -1, >> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index >> ef1812dc14..eba6502218 100644 >> --- a/xen/arch/x86/domain.c >> +++ b/xen/arch/x86/domain.c >> @@ -692,6 +692,12 @@ int arch_sanitise_domain_config(struct >> xen_domctl_createdomain *config) >> return -EINVAL; >> } >> >> + if ( config->flags & XEN_DOMCTL_CDF_INTERNAL_directmap ) >> + { >> + dprintk(XENLOG_INFO, "direct-map cannot be enabled yet\n"); >> + return -EINVAL; >> + } >> + >> return 0; >> } >> >> diff --git a/xen/common/domain.c b/xen/common/domain.c index >> 56d47dd664..13ac5950bc 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_INTERNAL_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 9b3647587a..4f2c3f09d4 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) >> +#define is_domain_direct_mapped(d) \ >> + (d->options & XEN_DOMCTL_CDF_INTERNAL_directmap) >> >> struct vtimer { >> struct vcpu *v; >> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index >> 1c21d4dc75..054e545c97 100644 >> --- a/xen/include/public/domctl.h >> +++ b/xen/include/public/domctl.h >> @@ -72,6 +72,10 @@ 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) >> +/* >> + * Be aware that bit 8 has already been occupied by flag >> + * XEN_DOMCTL_CDF_INTERNAL_directmap, defined in >> xen/include/xen/domain.h. >> + */ >> >> /* Max XEN_DOMCTL_CDF_* constant. Used for ABI checking. */ #define >> XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_vpmu diff --git >> a/xen/include/xen/domain.h b/xen/include/xen/domain.h index >> 160c8dbdab..2b9edfdcee 100644 >> --- a/xen/include/xen/domain.h >> +++ b/xen/include/xen/domain.h >> @@ -28,6 +28,9 @@ void getdomaininfo(struct domain *d, struct >> xen_domctl_getdomaininfo *info); void arch_get_domain_info(const struct >> domain *d, >> struct xen_domctl_getdomaininfo *info); >> >> +/* Should domain memory be directly mapped? */ >> +#define XEN_DOMCTL_CDF_INTERNAL_directmap (1U << 8) >> + > > I run into some trouble with defining this flag internal in the new serie. > > Let me explain in details here: > > 1. Currently XEN_DOMCTL_CDF_MAX is set to XEN_DOMCTL_CDF_vpmu. > So we can say that XEN_DOMCTL_CDF_MAX knows that there are 8 CDF flags(0 to > 7). > The corresponding ocaml tool has a list of CDF flags and currently it knows > that there are 8 CDF flags: > https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=tools/ocaml/libs/xc/xenctrl.ml;h=7503031d8f61c2dbcd4aa803738c83e10dfb7bb8;hb=HEAD#l64 > > This tool performs a check to see if the XEN_DOMCTL_CDF_MAX is equal to the > number of entries in domain_create_flag. > > 2. Here we are reserving bit 8 for internal flag > XEN_DOMCTL_CDF_INTERNAL_directmap. As this is internal flag, > I do not want to modify XEN_DOMCTL_CDF_MAX. > > 3. Everything is perfect until someone tries to add another global CDF flag: > > #define XEN_DOMCTL_CDF_next_flag (1<<9) > #define XEN_DOMCTL_CDF_MAX XEN_DOMCTL_CDF_next_flag > > XEN_DOMCTL_CDF_MAX shows right now that there are 10 flags but ocaml tool > sees only 9. > then we are getting build error. > > Hmm, would you please help me find a way to fix this dilemma, thx.
This was already outlined, but let me do so again: You do _not_ want to overlay with XEN_DOMCTL_CDF_*. domain_create() already has an internal- only parameter. That's a "bool" right now and wants extending to an "unsigned int" covering both the existing "is_priv" (step 1) and your new "directmap" (step 2). To make visible the relationship, naming the respective constants CDF_* (with no XEN_DOMCTL_ prefix to represent the difference) might be appopriate. Btw, as a result (if that's not the plan already anyway) you then probably also want to decouple is_domain_direct_mapped() from is_hardware_domain(), and hence create Dom0 also with the new flag set. Jan
