Re: [PATCH v1 4/4] KVM: MMU: Expose the LA57 feature to VM.

[Yu Zhang]

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);

And with LA57, may better be changed to:
  *max_size = min_t(u64, ~0u - la, (1ull << ctxt_virt_addr_bits(ctxt)) 
- la);

And for the above
  if (emul_is_noncanonical_address(la, ctxt))
we may just leave it as it is.

Is this understanding correct? Or did I misunderstand your comments? :-)

Thanks
Yu
> Paolo