qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [kvm-unit-tests PATCH v5 10/13] arm/arm64: ITS: INT functional tests


From: Zenghui Yu
Subject: Re: [kvm-unit-tests PATCH v5 10/13] arm/arm64: ITS: INT functional tests
Date: Thu, 12 Mar 2020 17:19:51 +0800
User-agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.2.0

On 2020/3/11 22:00, Marc Zyngier wrote:
That is still a problem with the ITS. There is no architectural way
to report an error, even if the error numbers are architected...

One thing we could do though is to implement the stall model (as described
in 5.3.2). It still doesn't give us the error, but at least the command
queue would stop on detecting an error.

It would be interesting to see the buggy guest's behavior under the
stall mode. I've used the following diff (absolutely *not* a formal
patch, don't handle CREADR.Stalled and CWRITER.Retry at all) to have
a try, and caught another command error in the 'its-trigger' test.

logs/its-trigger.log:
" INT dev_id=2 event_id=20
lib/arm64/gic-v3-its-cmd.c:194: assert failed: false: INT timeout! "

dmesg:
[13297.711958] ------------[ cut here ]------------
[13297.711964] ITS command error encoding 0x10307

It's the last INT test in test_its_trigger() who has triggered this
error, Eric?


Thanks.

---8<---
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 9d53f545a3d5..5717f5da0f22 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -179,6 +179,7 @@ struct vgic_its {
        u64                     cbaser;
        u32                     creadr;
        u32                     cwriter;
+       bool                    stalled;

        /* migration ABI revision in use */
        u32                     abi_rev;
diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index d53d34a33e35..72422b75e627 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -1519,6 +1519,9 @@ static void vgic_its_process_commands(struct kvm *kvm, struct vgic_its *its)
        if (!its->enabled)
                return;

+       if (unlikely(its->stalled))
+               return;
+
        cbaser = GITS_CBASER_ADDRESS(its->cbaser);

        while (its->cwriter != its->creadr) {
@@ -1531,9 +1534,34 @@ static void vgic_its_process_commands(struct kvm *kvm, struct vgic_its *its)
                 * According to section 6.3.2 in the GICv3 spec we can just
                 * ignore that command then.
                 */
-               if (!ret)
-                       vgic_its_handle_command(kvm, its, cmd_buf);
+               if (ret)
+                       goto done;
+
+               ret = vgic_its_handle_command(kvm, its, cmd_buf);
+
+               /*
+                * Choose the stall mode on detection of command errors.
+                * Guest still can't get the architected error numbers though...
+                */
+               if (ret) {
+                       /* GITS_CREADR.Stalled is set to 1. */
+                       its->stalled = true;
+
+                       /*
+                        * GITS_TYPER.SEIS is 0 atm, no System error will be
+                        * generated.  Instead report error encodings at ITS
+                        * level.
+                        */
+                       WARN_RATELIMIT(ret, "ITS command error encoding 0x%x", 
ret);
+
+                       /*
+                        * GITS_CREADR is not incremented and continues to
+                        * point to the entry that triggered the error.
+                        */
+                       break;
+               }

+done:
                its->creadr += ITS_CMD_SIZE;
                if (its->creadr == ITS_CMD_BUFFER_SIZE(its->cbaser))
                        its->creadr = 0;




reply via email to

[Prev in Thread] Current Thread [Next in Thread]