> -----Original Message-----
> From: Marc Zyngier <maz@xxxxxxxxxx>
> Sent: Thursday, January 28, 2021 5:07 PM
> To: Jianyong Wu <Jianyong.Wu@xxxxxxx>
> Cc: James Morse <James.Morse@xxxxxxx>; will@xxxxxxxxxx; Suzuki
> Poulose <Suzuki.Poulose@xxxxxxx>; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> kvmarm@xxxxxxxxxxxxxxxxxxxxx; Steve Capper <Steve.Capper@xxxxxxx>;
> Justin He <Justin.He@xxxxxxx>; nd <nd@xxxxxxx>
> Subject: Re: [PATCH] arm64/kvm: correct the error report in
> kvm_handle_guest_abort
>
> On 2021-01-28 03:01, Jianyong Wu wrote:
> > Hi Marc,
> >
> >>
> >> From 8f2a919d6f13d36445974794c76821fbb6b40f88 Mon Sep 17 00:00:00
> >> 2001
> >> From: Marc Zyngier <maz@xxxxxxxxxx>
> >> Date: Sat, 16 Jan 2021 10:53:21 +0000
> >> Subject: [PATCH] CMO on RO memslot
> >>
> >> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> >> ---
> >> arch/arm64/kvm/mmu.c | 51
> +++++++++++++++++++++++++++++++++----
> >> -------
> >> 1 file changed, 39 insertions(+), 12 deletions(-)
> >>
> >> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index
> >> 7d2257cc5438..3c176b5b0a28 100644
> >> --- a/arch/arm64/kvm/mmu.c
> >> +++ b/arch/arm64/kvm/mmu.c
> >> @@ -760,7 +760,15 @@ static int user_mem_abort(struct kvm_vcpu
> *vcpu,
> >> phys_addr_t fault_ipa,
> >> struct kvm_pgtable *pgt;
> >>
> >> fault_granule = 1UL <<
> >> ARM64_HW_PGTABLE_LEVEL_SHIFT(fault_level);
> >> - write_fault = kvm_is_write_fault(vcpu);
> >> + /*
> >> + * Treat translation faults on CMOs as read faults. Should
> >> + * this further generate a permission fault, it will be caught
> >> + * in kvm_handle_guest_abort(), with prejudice...
> >> + */
> >> + if (fault_status == FSC_FAULT && kvm_vcpu_dabt_is_cm(vcpu))
> >> + write_fault = false;
> >> + else
> >> + write_fault = kvm_is_write_fault(vcpu);
> >> exec_fault = kvm_vcpu_trap_is_exec_fault(vcpu);
> >> VM_BUG_ON(write_fault && exec_fault);
> >>
> >> @@ -1013,19 +1021,37 @@ 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:
> >> + *
> >> + * - 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, check whether this is a permission fault.
> >> + * If so, that's a DC IVAC on a R/O memslot, which is a
> >> + * pretty bad idea, and we tell the guest so.
> >> *
> >> - * So let's assume that the guest is just being
> >> - * cautious, and skip the instruction.
> >> + * - If this wasn't a permission fault, pass it along for
> >> + * further handling (including faulting the page in
> >> if it
> >> + * was a translation fault).
> >> */
> >> - if (kvm_is_error_hva(hva) && kvm_vcpu_dabt_is_cm(vcpu))
> >> {
> >> - kvm_incr_pc(vcpu);
> >> - ret = 1;
> >> - goto out_unlock;
> >> + if (kvm_vcpu_dabt_is_cm(vcpu)) {
> >> + if (kvm_is_error_hva(hva)) {
> >> + kvm_incr_pc(vcpu);
> >> + ret = 1;
> >> + goto out_unlock;
> >> + }
> >> +
> >> + if (fault_status == FSC_PERM) {
> >> + /* DC IVAC on a R/O memslot */
> >> + kvm_inject_dabt(vcpu,
> >> kvm_vcpu_get_hfar(vcpu));
> >
> > One question:
> > In general, the "DC" ops show up very early in guest. So what if the
> > guest do this before interrupt initialization? If so, the guest may
> > stuck here.
>
> I don't understand your question. Do you mean "what if the guest does this
> without being able to handle an exception"?
>
> If that's your question, then the answer is "don't do that".
> The architecture is clear that DC IVAC needs write permission, and will result
> in an abort being delivered if there is no writable mapping (and there can't be,
> the memslot is R/O).
>
> DC CIVAC doesn't have that requirement, and will not generate an exception.
>
OK, get it.
I have tested the patch above using my test case. It works well for "dc civac" and for "dc ivac" , a "Synchronous External Abort" occurs in guest as expected.
Thanks
Jianyong
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
