Hi Marc,
> -----Original Message-----
> From: Marc Zyngier <[email protected]>
> Sent: Friday, January 15, 2021 7:21 PM
> To: Jianyong Wu <[email protected]>
> Cc: James Morse <[email protected]>; [email protected]; Suzuki
> Poulose <[email protected]>; [email protected];
> [email protected]; Steve Capper <[email protected]>;
> Justin He <[email protected]>; nd <[email protected]>
> Subject: Re: [PATCH] arm64/kvm: correct the error report in
> kvm_handle_guest_abort
>
> On 2021-01-15 09:30, Jianyong Wu wrote:
> > Currently, error report when cache maintenance at read-only memory
> > range, like rom, is not clear enough and even not correct. As the
> > specific error is definitely known by kvm, it is obliged to give it
> > out.
> >
> > Fox example, in a qemu/kvm VM, if the guest do dc at the pflash range
> > from
> > 0 to 128M, error is reported by kvm as "Data abort outside memslots
> > with no valid syndrome info" which is not quite correct.
> >
> > Signed-off-by: Jianyong Wu <[email protected]>
> > ---
> > arch/arm64/kvm/mmu.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index
> > 7d2257cc5438..de66b7e38a5b 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -1022,9 +1022,15 @@ int kvm_handle_guest_abort(struct kvm_vcpu
> > *vcpu)
> > * So let's assume that the guest is just being
> > * cautious, and skip the instruction.
> > */
> > - if (kvm_is_error_hva(hva) && kvm_vcpu_dabt_is_cm(vcpu))
> {
> > - kvm_incr_pc(vcpu);
> > - ret = 1;
> > + if (kvm_vcpu_dabt_is_cm(vcpu)) {
> > + if (kvm_is_error_hva(hva)) {
> > + kvm_incr_pc(vcpu);
> > + ret = 1;
> > + goto out_unlock;
> > + }
> > +
> > + kvm_err("Do cache maintenance in the read-only
> memory range\n");
>
> We don't scream on the console for guests bugs.
Ok
>
> > + ret = -EFAULT;
> > goto out_unlock;
> > }
>
> And what is userspace going to do with that? To be honest, I'd rather not
> report it in any case:
>
> - either it isn't mapped, and there is no cache to clean/invalidate
> - or it is mapped read-only:
> - if it is a "DC IVAC", the guest should get the fault as per
> the ARM ARM. But I don't think we can identify the particular CMO
> at this stage, so actually performing an invalidation is the least
> bad thing to do.
>
> How about this (untested)?
>
> M.
>
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index
> 7d2257cc5438..0f497faad131 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -1013,16 +1013,27 @@ int kvm_handle_guest_abort(struct kvm_vcpu
> *vcpu)
> }
>
> /*
> - * Check for a cache maintenance operation. Since we
> - * ended-up here, we know it is outside of any memory
> - * slot. But we can't find out if that is for a device,
> - * or if the guest is just being stupid. The only thing
> - * we know for sure is that this range cannot be cached.
> + * Check for a cache maintenance operation. Two cases:
> *
> - * So let's assume that the guest is just being
> - * cautious, and skip the instruction.
> + * - It is outside of any memory slot. But we can't
> + * find out if that is for a device, or if the guest
> + * is just being stupid. The only thing we know for
> + * sure is that this range cannot be cached. So
> + * let's assume that the guest is just being
> + * cautious, and skip the instruction.
> + *
> + * - Otherwise, clean/invalidate the whole memslot. We
> + * should special-case DC IVAC and inject a
> + * permission fault, but we can't really identify it
> + * in this context.
> */
> - if (kvm_is_error_hva(hva) && kvm_vcpu_dabt_is_cm(vcpu))
> {
> + if (kvm_vcpu_dabt_is_cm(vcpu)) {
> + if (!kvm_is_error_hva(hva)) {
> + spin_lock(&vcpu->kvm->mmu_lock);
> + stage2_flush_memslot(vcpu->kvm,
> memslot);
> + spin_unlock(&vcpu->kvm->mmu_lock);
> + }
> +
> kvm_incr_pc(vcpu);
> ret = 1;
> goto out_unlock;
>
Thanks Marc, it's OK for me and I will do the test for it.
Thanks
Jianyong
> --
> Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
