Re: [PATCH v1 4/4] KVM: MMU: Expose the LA57 feature to VM.
From: Yu Zhang
Date: Mon Aug 21 2017 - 08:34:34 EST
On 8/21/2017 6:12 PM, Paolo Bonzini wrote:
On 21/08/2017 09:27, Yu Zhang wrote:
On 8/18/2017 8:50 PM, Paolo Bonzini wrote:
On 18/08/2017 10:28, Yu Zhang wrote:
On 8/17/2017 10:29 PM, Paolo Bonzini wrote:
On 17/08/2017 13:53, Yu Zhang wrote:
On 8/17/2017 7:57 PM, Paolo Bonzini wrote:
On 12/08/2017 15:35, Yu Zhang wrote:
index a98b88a..50107ae 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -694,7 +694,7 @@ static __always_inline int __linearize(struct
x86_emulate_ctxt *ctxt,
switch (mode) {
case X86EMUL_MODE_PROT64:
*linear = la;
- if (is_noncanonical_address(la))
+ if (emul_is_noncanonical_address(la, ctxt))
goto bad;
*max_size = min_t(u64, ~0u, (1ull << 48) - la);
Oops, you missed one here. Probably best to use ctxt_virt_addr_bits
and
then "inline" emul_is_noncanonical_address as "get_canonical(la,
va_bits) != la".
Sorry, I just sent out the v2 patch set without noticing this
reply. :-)
The emul_is_noncanonical() is defined in x86.h so that no
ctxt_virt_addr_bits needed in emulate.c, are you
suggesting to use ctx_virt_addr_bits in this file each time before
emul_is_noncanonical_address() is called?
No, only in this instance which uses "48" after the call to
emul_is_noncanonical_address.
Sorry, Paolo. I still do not quite get it.
Do you mean the
*max_size = min_t(u64, ~0u, (1ull << 48) - la);
also need to be changed?
But I do not understand why this statement is used like this. My
understanding is that
for 64 bit scenario, the *max_size is calculated to guarantee la +
*max_size still falls in
the canonical address space.
And if above understanding is correct, I think it should be something
like below:
*max_size = min_t(u64, ~0u - la, (1ull << 48) - la);
The "~0u" part is simply because max_size has 32-bit size (it's an
unsigned int variable), while (1ull << 48) - la has 64-bit size. It
protects from the overflow.
But what if value of "la" falls in between 0xFFFFFFFFFFFFFFFF and
0xFFFF000000000000? (1ull << 48) - la may result in something between
0x1000000000001 and> 0x2000000000000, and the *max_size would be 4G - 1
in this scenario. For instance, when "la" is 0xFFFFFFFFFFFFFFF0 (unlikely
in practice though), the *max_size we are expecting should be 15, instead
of 4G - 1.
No, it is possible to wrap a memory access from the top half of the
address space to the bottom half, so there's no limit at 0xFFFFFFFFFFFFFFF0.
Oh? So you mean it is allowed for one instruction to reside both in the
top half of
the address space and in the bottom half?
If this is possible, I guess the code should be
*max_size = min_t(u64, ~0u, (1ull << va_bits) - la);
But I wonder, why should such scenario be allowed? :-)
Thanks
Yu
Paolo