> On Fri, Jul 29, 2022 at 09:38:35 +0200, Michal Prívozník wrote:
> > 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@intel.com>
> > >>
> > >> 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@intel.com>
> > >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> > >> ---
> > >
> > > [...]
> > >
> > >> 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.
>
> Even this interpretation would not sit well with the semantics of the
> <memory> element.
>
> > 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.
>
> Note that the name expands to "enclave page cache", so even the name
> doesn't really seem to be hinting that the memory declared like this is
> used by the guest OS.
>
> Preferrably the documentation added in this patch will clarify that,
> because if it is not clarified by the documentation, we will be getting
> clarification requests and it's very apparent that we can't answer them
> since neither of us actually knows what's going on.
>
> >
> > >
> > > 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.
Basically sgx-epc memory created here is usable by guest OS. KVM SGX creates
One new misc device, and QEMU will open '/dev/sgx_vepc' device node to
mmap() host EPC memory to guest.
The sgx-epc memory is a separate memory range. Its size will not change the
guest OS size that is initialized by ‘-m size=…’. So it should be excluded from
calculation in virDomainDefGetMemoryInitial. Thanks for pointing this out.
Thanks,
Lin.