On 7/14/2017 7:37 PM, Andrew Cooper wrote:
On 13/07/17 07:42, Huang, Kai wrote:
On 7/12/2017 10:56 PM, Andrew Cooper wrote:
On 09/07/17 10:10, Kai Huang wrote:
Why do we need this hide_epc parameter? If we aren't providing any
epc resource to the guest, the entire sgx union should be zero and
the SGX feature bit should be hidden.
My intention was to hide physical EPC info for pv_max_policy and
hvm_max_policy (recalculate_sgx is also called by
calculate_pv_max_policy and calculate_hvm_max_policy), as they are for
guest and don't need physical EPC info. But keeping physical EPC info
in them does no harm so I think we can simply remove hide_epc.
It is my experience that providing half the information is worse than
providing none or all of it, because developers are notorious for taking
shortcuts when looking for features.
Patch 1 means that a PV guest will never have p->feat.sgx set.
Therefore, we will hit the memset() below, and zero the whole of the SGX
union.
Yes I'll remove hide_epc. It is not absolutely needed.
IMO we cannot check whether EPC is valid and zero sgx union in
recalculate_sgx, as it is called for each CPUID. For example, it is
called for SGX subleaf 0, and 1, and then 2, and when subleaf 0 and 1
are called, the EPC resource is 0 (hasn't been configured).
recalculate_*() only get called when the toolstack makes updates to the
policy. It is an unfortunate side effect of the current implementation,
but will be going away with my CPUID work.
The intended flow will be this:
At Xen boot:
* Calculates the raw, host and max policies (as we do today)
At domain create:
* Appropriate policy gets copied to make the default domain policy.
* Toolstack gets the whole policy at one with a new
DOMCTL_get_cpuid_policy hypercall.
* Toolstack makes all adjustments (locally) that it wants to, based on
configuration, etc.
* Toolstack makes a single DOMCTL_set_cpuid_policy hypercall.
* Xen audits the new policy proposed by the toolstack, resulting in a
single yes/no decision.
** If not, the toolstack is told to try again. This will likely result
in xl asking the user to modify their .cfg file.
** If yes, the proposed policy becomes the actual policy.
This scheme will fix the current problem we have where the toolstack
blindly proposes changes (one leaf at a time), and Xen has to zero the
bits it doesn't like (because the toolstack has never traditionally
checked the return value of the hypercall :( )
This is actually what I was looking for when implementing CPUID support
for SGX. I think I'll wait for your work to be merged to Xen and then do
my work above your work. :)
Thanks,
-Kai
+
+ /* Subleaf 2. */
+ uint32_t base_valid:1, :11, base_pfn_low:20;
+ uint32_t base_pfn_high:20, :12;
+ uint32_t size_valid:1, :11, npages_low:20;
+ uint32_t npages_high:20, :12;
+ };
Are the {base,size}_valid fields correct? The manual says the are
4-bit fields rather than single bit fields.
They are 4 bits in SDM but actually currently only bit 1 is valid
(other values are reserved). I think for now bool base_valid should be
enough. We can extend when new values come out. What's your suggestion?
Ok. That can work for now.
I would also drop the _pfn from the base names. The fields still
need shifting to get a sensible value.
OK. Will do.
As a further thought, what about uint64_t base:40 and size:40? That
would reduce the complexity of calculating the values.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
