On 7/28/22 14:41, Peter Krempa wrote: > On Wed, Jul 27, 2022 at 12:34:57 +0200, Michal Privoznik wrote: >> From: Lin Yang <lin.a.yang@xxxxxxxxx> >> >> With NUMA config: >> >> <devices> >> ... >> <memory model='sgx-epc'> >> <source> >> <nodemask>0-1</nodemask> >> </source> >> <target> >> <size unit='KiB'>512</size> >> <node>0</node> >> </target> >> </memory> >> ... >> </devices> >> >> Without NUMA config: >> >> <devices> >> ... >> <memory model='sgx-epc'> >> <target> >> <size unit='KiB'>512</size> >> </target> >> </memory> >> ... >> </devices> >> >> Signed-off-by: Lin Yang <lin.a.yang@xxxxxxxxx> >> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx> >> --- > > [...] > >> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst >> index 1ed969ac3e..bdd0fcea8e 100644 >> --- a/docs/formatdomain.rst >> +++ b/docs/formatdomain.rst >> @@ -7940,6 +7940,20 @@ Example: usage of the memory devices >> <current unit='KiB'>524288</current> >> </target> >> </memory> >> + <memory model='sgx-epc'> >> + <source> >> + <nodemask>0-1</nodemask> >> + </source> >> + <target> >> + <size unit='KiB'>16384</size> >> + <node>0</node> >> + </target> >> + </memory> >> + <memory model='sgx-epc'> >> + <target> >> + <size unit='KiB'>16384</size> >> + </target> >> + </memory> >> </devices> >> ... >> >> @@ -7948,7 +7962,9 @@ Example: usage of the memory devices >> 1.2.14` Provide ``nvdimm`` model that adds a Non-Volatile DIMM module. >> :since:`Since 3.2.0` Provide ``virtio-pmem`` model to add a paravirtualized >> persistent memory device. :since:`Since 7.1.0` Provide ``virtio-mem`` model >> - to add paravirtualized memory device. :since:`Since 7.9.0` >> + to add paravirtualized memory device. :since:`Since 7.9.0` Provide >> + ``sgx-epc`` model to add a SGX enclave page cache (EPC) memory to the guest. >> + :since:`Since 8.7.0 and QEMU 6.2.0` > > I don't quite understand from this description whether this is real > memory usable by the guest OS or something for internal use by the > hypervisor. > > Specifically which leads me to questioning this is that the example > sizes are very tiny compared to real memory sizing. I admit that I don't know all the details and let somebody from Intel to provide them. But basically, SGX works by reserving a chunk of RAM (on each NUMA node) which is then encrypted and the processor controls access to it. This chunk is referred to as Processor Reserved Memory and to my understanding is the maximum size of an enclave. Thus I can understand why new <memory/> model was used. But I'm not sure how QEMU accounts for this memory, whether say 16KiB worth of SGX is added to whatever current guest has OR it's taken from an existing memory. > > Additionally in qemuDomainDefValidateMemoryHotplug you are changing the > code to specifically exclude the sizing of the 'sgx-epc' memory devices > from the total size of the memory, but this contrasts with the memory > _not_ being excluded from the initial memory calculation in > virDomainDefGetMemoryInitial which is used to format the initial memory > argument ('-m size=...'). Thus at least one of them is wrong. > > If this is not guest usable memory, then the <memory> element should not > be used to bolt this on, but rather add a new element similarly to what > we have when AMD SEV is in use. > Alright, I'm hold off reworking these patches per your review until we are clear on this. Michal