Re: [Xen-devel] [PATCH V5] x86/altp2m: Add a subop for obtaining the mem access of a page

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 9/26/18 2:39 PM, Razvan Cojocaru wrote:
> On 9/26/18 10:34 AM, Razvan Cojocaru wrote:
>> Currently there is a subop for setting the memaccess of a page, but not
>> for consulting it.  The new HVMOP_altp2m_get_mem_access adds this
>> functionality.
>>
>> Both altp2m get/set mem access functions use the struct
>> xen_hvm_altp2m_mem_access which has now dropped the `set' part and has
>> been renamed from xen_hvm_altp2m_set_mem_access.
>>
>> Signed-off-by: Adrian Pop <apop@xxxxxxxxxxxxxxx>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
> 
> Sorry, I've just found that this change will break the build of
> tools/firmware/xen-dir/xen-root/xen/arch/x86/mm/mem_access.c, because -
> while it's building just fine when doing so for the hypervisor (i.e.
> "make" in xen/), building the tools somehow doesn't take into account
> CONFIG_HVM (output of "make dist"):
> 
> gcc -m64 -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall
> -Wstrict-prototypes -Wdeclaration-after-statement
> -Wno-unused-but-set-variable -Wno-unused-local-typedefs   -O2
> -fomit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror
> -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include
> /home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include/xen/config.h
> '-D__OBJECT_FILE__="mem_access.o"' -Wa,--strip-local-absolute -g -MMD
> -MF ./.mem_access.o.d
> -I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include
> -I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include/asm-x86/mach-generic
> -I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include/asm-x86/mach-default
> -DXEN_IMG_OFFSET=0x200000 '-D__OBJECT_LABEL__=arch$x86$mm$mem_access.o'
> -msoft-float -fno-stack-protector -fno-exceptions -Wnested-externs
> -DHAVE_AS_VMX -DHAVE_AS_SSE4_2 -DHAVE_AS_EPT -DHAVE_AS_RDRAND
> -DHAVE_AS_FSGSBASE -DHAVE_AS_RDSEED -U__OBJECT_LABEL__
> -DHAVE_AS_QUOTED_SYM '-D__OBJECT_LABEL__=arch/x86/mm/mem_access.o'
> -DHAVE_AS_INVPCID -DHAVE_AS_NEGATIVE_TRUE  -mno-red-zone -fpic
> -fno-asynchronous-unwind-tables -mno-sse -mskip-rax-setup
> -DGCC_HAS_VISIBILITY_ATTRIBUTE -mindirect-branch=thunk-extern
> -mindirect-branch-register -DCONFIG_INDIRECT_THUNK
> -Wa,-I/home/red/work/pristine/xen.git/tools/firmware/xen-dir/xen-root/xen/include
> -c mem_access.c -o mem_access.o
> mem_access.c: In function ‘p2m_get_mem_access’:
> mem_access.c:504:21: error: ‘struct arch_domain’ has no member named
> ‘altp2m_eptp’
>               d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
>                      ^
> mem_access.c:507:22: error: ‘struct arch_domain’ has no member named
> ‘altp2m_p2m’
>          p2m = d->arch.altp2m_p2m[altp2m_idx];
>                       ^
> 
> Wei, thoughts on how this would be best approached?

To clarify the question, I'll of course do this:

diff --git a/xen/arch/x86/mm/mem_access.c b/xen/arch/x86/mm/mem_access.c
index 67b4a1d..2b5a621 100644
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -489,14 +489,13 @@ long p2m_set_mem_access_multi(struct domain *d,
 int p2m_get_mem_access(struct domain *d, gfn_t gfn, xenmem_access_t
*access,
                        unsigned int altp2m_idx)
 {
-    struct p2m_domain *p2m;
+    struct p2m_domain *p2m = p2m_get_hostp2m(d);

+#ifdef CONFIG_HVM
     if ( !altp2m_active(d) )
     {
         if ( altp2m_idx )
             return -EINVAL;
-
-        p2m = p2m_get_hostp2m(d);
     }
     else
     {
@@ -506,6 +505,9 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn,
xenmem_access_t *access,

         p2m = d->arch.altp2m_p2m[altp2m_idx];
     }
+#else
+    ASSERT(!altp2m_idx);
+#endif

     return _p2m_get_mem_access(p2m, gfn, access);
 }

but is it OK that the hypervisor builds with a set of flags that
includes CONFIG_HVM and the firmware code with a set that doesn't?


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel