Re: [RFC PATCH 0/5] QEMU v7.2.0 aarch64 Nested Virtualization Support
Hi Eric, > On 8 Feb 2024, at 15:55, Eric Auger wrote: > > Hi Miguel, > > On 2/27/23 17:37, Miguel Luis wrote: >> This series adds ARMv8.3/8.4 nested virtualization support in KVM mode. >> >> To enable nested virtualization for a guest, the host must expose EL2 >> support via QEMU command line switches: >> >> -machine virt,accel=kvm,virtualization=on >> >> Inspired on Haibo Xu's previous work [0][1], Marc Zyngier's kvmtool branch >> [2] >> and kernel patches [3] on nested virtualization for aarch64, this has been >> tested on an Ampere implementation. >> >> This series adapts previous work on top of v7.2.0, it considers comments >> given >> at the time and preserves authorship of the original patches. >> >> [0]: >> https://lore.kernel.org/qemu-devel/cover.1616052889.git.haibo...@linaro.org/ >> [1]: >> https://lore.kernel.org/qemu-devel/cover.1617281290.git.haibo...@linaro.org/ >> [2]: >> https://git.kernel.org/pub/scm/linux/kernel/git/maz/kvmtool.git/log/?h=arm64/nv-5.16 >> [3]: >> https://lore.kernel.org/linux-arm-kernel/20230131092504.2880505-1-...@kernel.org/ > > I rebased the series on top of v8.2. I was able to boot some L2 guests > with it, although it still does not work with guests featuring edk2. > > Do you plan to send a respin or may I do? > I do not have a short-term respin planned. Please, feel free to do. Thanks Miguel > Thanks > > Eric >> >> Miguel Luis (5): >> linux-headers: [kvm,arm64] add the necessary definitions to match host >>kernel >> hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ >> target/arm/kvm: add helper to detect EL2 when using KVM >> target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported >> arm/virt: provide virtualization extensions to the guest >> >> hw/arm/virt.c | 8 +++- >> hw/intc/arm_gicv3_common.c | 1 + >> hw/intc/arm_gicv3_kvm.c| 25 + >> include/hw/intc/arm_gicv3_common.h | 1 + >> linux-headers/asm-arm64/kvm.h | 2 ++ >> linux-headers/linux/kvm.h | 1 + >> target/arm/cpu.h | 2 +- >> target/arm/kvm64.c | 21 + >> target/arm/kvm_arm.h | 12 >> 9 files changed, 71 insertions(+), 2 deletions(-)
Re: [PATCH 00/35] target/arm: Implement emulation of nested virtualization
Hi Peter,
> On 18 Dec 2023, at 10:32, Peter Maydell wrote:
>
> This patchset adds support for emulating the Arm architectural features
> FEAT_NV and FEAT_NV2 which allow nested virtualization, i.e. where a
> hypervisor can run a guest which thinks it is running at EL2.
>
> Nominally FEAT_NV is sufficient for this and FEAT_NV2 merely improves
> the performance in the nested-virt setup, but in practice hypervisors
> such as KVM are going to require FEAT_NV2 and not bother to support
> the FEAT_NV-only case, so I have implemented them one after the other
> in this single patchset.
>
> The feature is essentially a collection of changes that allow the
> hypervisor to lie to the guest so that it thinks it is running in EL2
> when it's really at EL1. The best summary of what all the changes are
> is in section D8.11 "Nested virtualization" in the Arm ARM, but the
> short summary is:
> * EL2 system registers etc trap to EL2 rather than UNDEFing
> * ERET traps to EL2
> * the CurrentEL register reports "EL2" when NV is enabled
> * on exception entry, SPSR_EL1.M may report "EL2" as the EL the
> exception was taken from
> * when HCR_EL1.NV1 is also set, then there are some extra tweaks
> (NV1 == 1 means "guest thinks it is running with HCR_EL2.E2H == 0")
> * some AT S1 address translation insns can be trapped to EL2
> and FEAT_NV2 adds:
> * accesses to some system registers are transformed into memory
> accesses instead of trapping to EL2
> * accesses to a few EL2 system registers are redirected to the
> equivalent EL1 registers
>
> This patchset is sufficient that you can run an L0 guest kernel that
> has support for FEAT_NV/FEAT_NV2 in its KVM code, and then
> inside that start a nested L1 guest that thinks it has EL2 access,
> and then run an inner-nested L2 guest under that that can get
> to running userspace code. To do that you'll need some not-yet-upstream
> patches for both Linux and kvmtool:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/nv-6.8-nv2-only
> https://gitlab.arm.com/linux-arm/kvmtool/-/commits/nv-v6.6
>
> You'll also want to turn off SVE and SME emulation in QEMU
> (-cpu max,sve=off,sme=off) because at the moment the KVM patchset
> doesn't handle SVE and nested-virt together (the other option
> is to hack kvmtool to make it not ask for both at once, but this
> is easier).
>
> (kvmtool is needed here to run the L1 because QEMU itself as a VMM
> doesn't yet support asking KVM for an EL2 guest.)
>
> The first three patches in the series aren't strictly part of FEAT_NV:
> * patch 1 is already reviewed; I put it here to avoid having
> to deal with textual conflicts between it and this series
> * patch 2 sets CTR_EL0.{IDC,DIC} for '-cpu max', which is a good
> idea anyway and also works around what Marc Z and I think is
> a KVM bug that otherwise causes boot of the L2 kernel to hang
> * patch 3 is a GIC bug which is not FEAT_NV specific but for
> some reason only manifests when booting an L1 kernel under NV
>
I've successfully replicated this setup and reached inner-nested L2 guest
userspace.
FWIW, feel free to add
Tested-by: Miguel Luis
(I've been working on QEMU asking KVM for an EL2 guest on top of this series
although there's been yet some debugging to do.)
Thank you!
Miguel
> thanks
> -- PMM
>
> Peter Maydell (35):
> target/arm: Don't implement *32_EL2 registers when EL1 is AArch64 only
> target/arm: Set CTR_EL0.{IDC,DIC} for the 'max' CPU
> hw/intc/arm_gicv3_cpuif: handle LPIs in in the list registers
> target/arm: Handle HCR_EL2 accesses for bits introduced with FEAT_NV
> target/arm: Implement HCR_EL2.AT handling
> target/arm: Enable trapping of ERET for FEAT_NV
> target/arm: Always honour HCR_EL2.TSC when HCR_EL2.NV is set
> target/arm: Allow use of upper 32 bits of TBFLAG_A64
> target/arm: Record correct opcode fields in cpreg for E2H aliases
> target/arm: *_EL12 registers should UNDEF when HCR_EL2.E2H is 0
> target/arm: Make EL2 cpreg accessfns safe for FEAT_NV EL1 accesses
> target/arm: Move FPU/SVE/SME access checks up above
>ARM_CP_SPECIAL_MASK check
> target/arm: Trap sysreg accesses for FEAT_NV
> target/arm: Make NV reads of CurrentEL return EL2
> target/arm: Set SPSR_EL1.M correctly when nested virt is enabled
> target/arm: Trap registers when HCR_EL2.{NV,NV1} == {1,1}
> target/arm: Always use arm_pan_enabled() when checking if PAN is
>enabled
> target/arm: Don't honour PSTATE.PAN when HCR_EL2.{NV,NV1} == {1,1}
> target/arm: Treat LDTR* and STTR* as LDR/STR when NV,NV1 is 1,1
> target/arm: Handle FEAT_NV page table attribute changes
> target/arm: Add FEAT_NV to max, neoverse-n2,
Re: [RFC PATCH-for-8.2?] accel/tcg: Implement tcg_unregister_thread()
> On 6 Dec 2023, at 14:25, Michal Suchánek wrote:
>
> On Wed, Dec 06, 2023 at 01:17:08PM -0100, Miguel Luis wrote:
>> Hi!
>>
>> On 04/12/2023 18:40, Philippe Mathieu-Daudé wrote:
>>> Unplugging vCPU triggers the following assertion in
>>> tcg_register_thread():
>>>
>>> 796 void tcg_register_thread(void)
>>> 797 {
>>> ...
>>> 812 /* Claim an entry in tcg_ctxs */
>>> 813 n = qatomic_fetch_inc(_cur_ctxs);
>>> 814 g_assert(n < tcg_max_ctxs);
>>>
>>> Implement and use tcg_unregister_thread() so when a
>>> vCPU is unplugged, the tcg_cur_ctxs refcount is
>>> decremented.
>>
>>
>> I've had addressed this issue before (posted at [1]) and had exercised
>> it with vCPU hotplug/unplug patches for ARM although I was not sure about
>> what
>> would be needed to be done regarding plugins on the context of
>> tcg_unregister_thread. I guess they would need to be freed also?
>
> Doesn't it have the same problem that it will randomly free some context
> which is not necessarily associated with the unplugged CPU?
>
> Consider machine with 4 CPUs, they are likely added in order - cpu0
> getting context0, cpu1 context1, etc.
>
> Unplug CPU 1. Given that context 3 is top the would be unallocated by
> the decrement, or am I missing something?
>
I think you’re right and I share of the same opinion that matching a tcg thread
to a vCPU would be handy to solve this and maybe sorting tcg_ctxs after
unregistering the thread.
Thank you
Miguel
> Thanks
>
> Michal
>
>>
>> Thank you
>>
>> Miguel
>>
>> [1]:
>> https://lore.kernel.org/qemu-devel/20230926103654.34424-5-salil.me...@huawei.com/
Re: [RFC PATCH-for-8.2?] accel/tcg: Implement tcg_unregister_thread()
Hi!
On 04/12/2023 18:40, Philippe Mathieu-Daudé wrote:
> Unplugging vCPU triggers the following assertion in
> tcg_register_thread():
>
> 796 void tcg_register_thread(void)
> 797 {
> ...
> 812 /* Claim an entry in tcg_ctxs */
> 813 n = qatomic_fetch_inc(_cur_ctxs);
> 814 g_assert(n < tcg_max_ctxs);
>
> Implement and use tcg_unregister_thread() so when a
> vCPU is unplugged, the tcg_cur_ctxs refcount is
> decremented.
I've had addressed this issue before (posted at [1]) and had exercised
it with vCPU hotplug/unplug patches for ARM although I was not sure about what
would be needed to be done regarding plugins on the context of
tcg_unregister_thread. I guess they would need to be freed also?
> Reported-by: Michal Suchánek
> Suggested-by: Stefan Hajnoczi
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> RFC: untested
> Report:
> https://lore.kernel.org/qemu-devel/20231204183638.gz9...@kitsune.suse.cz/
> ---
> include/tcg/startup.h | 5 +
> accel/tcg/tcg-accel-ops-mttcg.c | 1 +
> accel/tcg/tcg-accel-ops-rr.c| 1 +
> tcg/tcg.c | 17 +
> 4 files changed, 24 insertions(+)
>
> diff --git a/include/tcg/startup.h b/include/tcg/startup.h
> index f71305765c..520942a4a1 100644
> --- a/include/tcg/startup.h
> +++ b/include/tcg/startup.h
> @@ -45,6 +45,11 @@ void tcg_init(size_t tb_size, int splitwx, unsigned
> max_cpus);
> */
> void tcg_register_thread(void);
>
> +/**
> + * tcg_unregister_thread: Unregister this thread with the TCG runtime
> + */
> +void tcg_unregister_thread(void);
> +
> /**
> * tcg_prologue_init(): Generate the code for the TCG prologue
> *
> diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
> index fac80095bb..88d7427aad 100644
> --- a/accel/tcg/tcg-accel-ops-mttcg.c
> +++ b/accel/tcg/tcg-accel-ops-mttcg.c
> @@ -120,6 +120,7 @@ static void *mttcg_cpu_thread_fn(void *arg)
>
> tcg_cpus_destroy(cpu);
> qemu_mutex_unlock_iothread();
> +tcg_unregister_thread();
> rcu_remove_force_rcu_notifier(_rcu.notifier);
> rcu_unregister_thread();
> return NULL;
> diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
> index 611932f3c3..c2af3aad21 100644
> --- a/accel/tcg/tcg-accel-ops-rr.c
> +++ b/accel/tcg/tcg-accel-ops-rr.c
> @@ -302,6 +302,7 @@ static void *rr_cpu_thread_fn(void *arg)
> rr_deal_with_unplugged_cpus();
> }
>
> +tcg_unregister_thread();
> rcu_remove_force_rcu_notifier(_rcu);
> rcu_unregister_thread();
> return NULL;
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index d2ea22b397..5125342d70 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -781,11 +781,18 @@ static void alloc_tcg_plugin_context(TCGContext *s)
> * modes.
> */
> #ifdef CONFIG_USER_ONLY
> +
> void tcg_register_thread(void)
> {
> tcg_ctx = _init_ctx;
> }
> +
> +void tcg_unregister_thread(void)
> +{
> +}
> +
> #else
> +
> void tcg_register_thread(void)
> {
> TCGContext *s = g_malloc(sizeof(*s));
> @@ -814,6 +821,16 @@ void tcg_register_thread(void)
>
> tcg_ctx = s;
> }
> +
> +void tcg_unregister_thread(void)
> +{
> +unsigned int n;
> +
> +n = qatomic_fetch_dec(_cur_ctxs);
> +g_free(tcg_ctxs[n]);
Is the above off-by-one?
> +qatomic_set(_ctxs[n], NULL);
> +}
> +
Thank you
Miguel
[1]:
https://lore.kernel.org/qemu-devel/20230926103654.34424-5-salil.me...@huawei.com/
> #endif /* !CONFIG_USER_ONLY */
>
> /* pool based memory allocation */
Re: [PATCH V6 0/9] Add architecture agnostic code to support vCPU Hotplug
> On 16 Oct 2023, at 10:01, Miguel Luis wrote: > > Hi Salil, > >> On 16 Oct 2023, at 09:52, Salil Mehta wrote: >> >> Hi Miguel, >> >>> From: Miguel Luis >>> Sent: Friday, October 13, 2023 5:34 PM >>> To: Salil Mehta >>> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org; Marc Zyngier >>> ; jean-phili...@linaro.org; Jonathan Cameron >>> ; lpieral...@kernel.org; Peter Maydell >>> ; Richard Henderson >>> ; imamm...@redhat.com; >>> andrew.jo...@linux.dev; da...@redhat.com; phi...@linaro.org; >>> eric.au...@redhat.com; oliver.up...@linux.dev; pbonz...@redhat.com; >>> m...@redhat.com; w...@kernel.org; gs...@redhat.com; raf...@kernel.org; >>> alex.ben...@linaro.org; li...@armlinux.org.uk; >>> dar...@os.amperecomputing.com; il...@os.amperecomputing.com; >>> vis...@os.amperecomputing.com; Karl Heubaum ; >>> salil.me...@opnsrc.net; zhukeqian ; wangxiongfeng >>> (C) ; wangyanan (Y) ; >>> jiakern...@gmail.com; maob...@loongson.cn; lixiang...@loongson.cn; Linuxarm >>> >>> Subject: Re: [PATCH V6 0/9] Add architecture agnostic code to support vCPU >>> Hotplug >>> >>> Hi Salil, >>> >>>> On 13 Oct 2023, at 10:51, Salil Mehta wrote: >>>> >>>> Virtual CPU hotplug support is being added across various >>> architectures[1][3]. >>>> This series adds various code bits common across all architectures: >> >> >> [...] >> >> >>> I tested it for Arm64, make check, boot/reboot, live migration and found no >>> issues, >>> so for this, please feel free to add: >>> >>> Tested-by: Miguel Luis >> >> Great. Many thanks for the confirmation. >> >> I guess you are repeating the same for x86 as well? >> > > You are welcome! > > Absolutely, I’m repeating those same tests for x86. > Unfortunately, there's a qtest failing for x86. The failing test is device-introspect-test in which the assert for mc->possible_cpu_arch_ids fails. There’s also a suggestion to fix it here: https://lore.kernel.org/qemu-devel/15e70616-6abb-63a4-17d0-820f4a254...@opnsrc.net/T/#m108f102b2fe92b7dd7218f2f942f7b233a9d6af3 Thanks, Miguel > Thanks > Miguel > >> Salil.
Re: [PATCH V6 0/9] Add architecture agnostic code to support vCPU Hotplug
Hi Salil, > On 16 Oct 2023, at 09:52, Salil Mehta wrote: > > Hi Miguel, > >> From: Miguel Luis >> Sent: Friday, October 13, 2023 5:34 PM >> To: Salil Mehta >> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org; Marc Zyngier >> ; jean-phili...@linaro.org; Jonathan Cameron >> ; lpieral...@kernel.org; Peter Maydell >> ; Richard Henderson >> ; imamm...@redhat.com; >> andrew.jo...@linux.dev; da...@redhat.com; phi...@linaro.org; >> eric.au...@redhat.com; oliver.up...@linux.dev; pbonz...@redhat.com; >> m...@redhat.com; w...@kernel.org; gs...@redhat.com; raf...@kernel.org; >> alex.ben...@linaro.org; li...@armlinux.org.uk; >> dar...@os.amperecomputing.com; il...@os.amperecomputing.com; >> vis...@os.amperecomputing.com; Karl Heubaum ; >> salil.me...@opnsrc.net; zhukeqian ; wangxiongfeng >> (C) ; wangyanan (Y) ; >> jiakern...@gmail.com; maob...@loongson.cn; lixiang...@loongson.cn; Linuxarm >> >> Subject: Re: [PATCH V6 0/9] Add architecture agnostic code to support vCPU >> Hotplug >> >> Hi Salil, >> >>> On 13 Oct 2023, at 10:51, Salil Mehta wrote: >>> >>> Virtual CPU hotplug support is being added across various >> architectures[1][3]. >>> This series adds various code bits common across all architectures: > > > [...] > > >> I tested it for Arm64, make check, boot/reboot, live migration and found no >> issues, >> so for this, please feel free to add: >> >> Tested-by: Miguel Luis > > Great. Many thanks for the confirmation. > > I guess you are repeating the same for x86 as well? > You are welcome! Absolutely, I’m repeating those same tests for x86. Thanks Miguel > Salil.
Re: [PATCH V6 0/9] Add architecture agnostic code to support vCPU Hotplug
8 > include/exec/gdbstub.h | 5 ++ > include/hw/acpi/cpu.h | 5 +- > include/hw/acpi/cpu_hotplug.h | 4 ++ > include/hw/acpi/generic_event_device.h | 5 ++ > include/hw/core/cpu.h | 1 + > include/sysemu/kvm.h | 16 +++ > system/physmem.c | 29 > 15 files changed, 184 insertions(+), 27 deletions(-) > I tested it for Arm64, make check, boot/reboot, live migration and found no issues, so for this, please feel free to add: Tested-by: Miguel Luis Thank you, Miguel > -- > 2.34.1 >
Re: [PATCH RFC V2 00/37] Support of Virtual CPU Hotplug for ARMv8 Arch
Hi Salil, > On 12 Oct 2023, at 17:54, Salil Mehta wrote: > > Hi Miguel, > >> From: Miguel Luis >> Sent: Thursday, October 12, 2023 6:02 PM >> To: Salil Mehta >> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org; Marc Zyngier >> ; jean-phili...@linaro.org; Jonathan Cameron >> ; lpieral...@kernel.org; Peter Maydell >> ; Richard Henderson >> ; imamm...@redhat.com; >> andrew.jo...@linux.dev; da...@redhat.com; phi...@linaro.org; >> eric.au...@redhat.com; w...@kernel.org; a...@kernel.org; >> oliver.up...@linux.dev; pbonz...@redhat.com; m...@redhat.com; >> gs...@redhat.com; raf...@kernel.org; borntrae...@linux.ibm.com; >> alex.ben...@linaro.org; li...@armlinux.org.uk; >> dar...@os.amperecomputing.com; il...@os.amperecomputing.com; >> vis...@os.amperecomputing.com; Karl Heubaum ; >> salil.me...@opnsrc.net; zhukeqian ; wangxiongfeng >> (C) ; wangyanan (Y) ; >> jiakern...@gmail.com; maob...@loongson.cn; lixiang...@loongson.cn >> Subject: Re: [PATCH RFC V2 00/37] Support of Virtual CPU Hotplug for ARMv8 >> Arch >> >> Hi Salil, >> >>> On 26 Sep 2023, at 10:03, Salil Mehta wrote: >>> >>> [ *REPEAT: Sent patches got held at internal server yesterday* ] >>> >>> PROLOGUE >>> > > [...] > > >> Tested on Oracle platforms with Ampere processors. >> vCPU hotplug/unplug features along VM live migration. >> >> Please feel free to add, >> Tested-by: Miguel Luis > > This is a great help. > > Many thanks for your persistent efforts in the past few months. > It has really helped in expediting fixes, reducing many major > bugs and also helping in TCG part. Really appreciate it! > You are welcome! Likewise, really appreciate you driving this forward and being open to suggestions. Makes it easy to collaborate while helping the community coming together. > Will look forward to collaborate to fix the TCG part next. > That is great! Looking forward to it. Cheers Miguel > Cheers > Salil. > >
Re: [PATCH RFC V2 00/37] Support of Virtual CPU Hotplug for ARMv8 Arch
> Salil. > > (X) DISCLAIMER >== > > This work is an attempt to present a proof-of-concept of the ARM64 vCPU > hotplug > implementation to the community. This is *not* a production level code and > might > have bugs. Only a basic testing has been done on HiSilicon Kunpeng920 SoC for > servers. Once the design and core idea behind the implementation has been > verified more efforts can be put to harden the code. > > This work is *mostly* in the lines of the discussions which have happened in > the > previous years[see refs below] across different channels like mailing-list, > Linaro Open Discussions platform, various conferences like KVMFourm etc. This > RFC is being used as a way to verify the idea mentioned in this cover-letter > and > to get community views. Once this has been agreed, a formal patch shall be > posted to the mailing-list for review. > > [The concept being presented has been found to work!] > > (XI) ORGANIZATION OF PATCHES > === > > A. All patches [Architecture 'agnostic' + 'specific']: > > [Patch 1-9, 23, 36] logic required during machine init >(*) Some validation checks >(*) Introduces core-id property and some util functions required later. >(*) Refactors Parking logic of vCPUs >(*) Logic to pre-create vCPUs >(*) GIC initialization pre-sized with possible vCPUs. >(*) Some refactoring to have common hot and cold plug logic together. >(*) Release of disable QOM CPU objects in post_cpu_init() >(*) Support of ACPI _OSC method to negotiate platform hotplug capabilities > [Patch 10-22] logic related to ACPI at machine init time >(*) Changes required to Enable ACPI for cpu hotplug >(*) Initialization ACPI GED framework to cater CPU Hotplug Events >(*) Build ACPI AML related to CPU control dev >(*) ACPI MADT/MAT changes > [Patch 24-35] Logic required during vCPU hot-(un)plug >(*) Basic framework changes to suppport vCPU hot-(un)plug >(*) ACPI GED changes for hot-(un)plug hooks. >(*) wire-unwire the IRQs >(*) GIC notification logic >(*) ARMCPU unrealize logic >(*) Handling of SMCC Hypercall Exits by KVM to Qemu > > B. Architecture *agnostic* patches part of patch-set: > > [Patch 5,9,11,13,16,20,24,31,33] Common logic to support hotplug >(*) Refactors Parking logic of vCPUs >(*) Introduces ACPI GED Support for vCPU Hotplug Events >(*) Introduces ACPI AML change for CPU Control Device > > (XII) REFERENCES > == > > [1] > https://lore.kernel.org/qemu-devel/20200613213629.21984-1-salil.me...@huawei.com/ > [2] > https://lore.kernel.org/linux-arm-kernel/20200625133757.22332-1-salil.me...@huawei.com/ > [3] https://lore.kernel.org/lkml/20230203135043.409192-1-james.mo...@arm.com/ > [4] https://lore.kernel.org/all/20230913163823.7880-1-james.mo...@arm.com/ > [5] > https://lore.kernel.org/all/20230404154050.2270077-1-oliver.up...@linux.dev/ > [6] https://bugzilla.tianocore.org/show_bug.cgi?id=3706 > [7] > https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#gic-cpu-interface-gicc-structure > [8] https://bugzilla.tianocore.org/show_bug.cgi?id=4481#c5 > [9] > https://cloud.google.com/kubernetes-engine/docs/concepts/verticalpodautoscaler > [10] > https://docs.aws.amazon.com/eks/latest/userguide/vertical-pod-autoscaler.html > [11] https://lkml.org/lkml/2019/7/10/235 > [12] https://lists.cs.columbia.edu/pipermail/kvmarm/2018-July/032316.html > [13] https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg06517.html > [14] > https://op-lists.linaro.org/archives/list/linaro-open-discussi...@op-lists.linaro.org/thread/7CGL6JTACPUZEYQC34CZ2ZBWJGSR74WE/ > [15] http://lists.nongnu.org/archive/html/qemu-devel/2018-07/msg01168.html > [16] https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00131.html > [17] > https://op-lists.linaro.org/archives/list/linaro-open-discussi...@op-lists.linaro.org/message/X74JS6P2N4AUWHHATJJVVFDI2EMDZJ74/ > [18] > https://lore.kernel.org/lkml/20210608154805.216869-1-jean-phili...@linaro.org/ > [19] https://lore.kernel.org/all/20230913163823.7880-1-james.mo...@arm.com/ > [20] > https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#gicc-cpu-interface-flags > > (XIII) ACKNOWLEDGEMENTS > > > I would like to take this opportunity to thank below people for various > discussions with me over different channels during the development: > > Marc Zyngier (Google) Catalin Marinas (ARM), > James Morse(ARM), Will Deacon (Google), > Jean-Phillipe Brucker (Linaro), Sudeep Holla (ARM), > Lorenzo Pieralisi (Linaro),
Re: [RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported
Hi Eric,
Thanks in advance for your comment.
> On 6 Jul 2023, at 08:16, Eric Auger wrote:
>
> Hi Miguel,
>
> On 2/27/23 17:37, Miguel Luis wrote:
>> From: Haibo Xu
>>
>> KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
>> EL2 bits on ID_AA64PFR0 state unsupported on the value 0b.
>>
>> Ref:
>> https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo...@linaro.org/
>>
>> Signed-off-by: Haibo Xu
>> [Miguel Luis: use of ID_AA64PFR0 for cpu features]
>> Signed-off-by: Miguel Luis
>> ---
>> target/arm/cpu.h | 2 +-
>> target/arm/kvm64.c | 16
>> 2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 9aeed3c848..de2a88b43e 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -3961,7 +3961,7 @@ static inline bool isar_feature_aa64_aa32_el1(const
>> ARMISARegisters *id)
>>
>> static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
>> {
>> -return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2;
>> +return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) != 0;
>> }
>>
>> static inline bool isar_feature_aa64_ras(const ARMISARegisters *id)
>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
>> index be8144a2b5..f7ebd731aa 100644
>> --- a/target/arm/kvm64.c
>> +++ b/target/arm/kvm64.c
>> @@ -505,6 +505,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures
>> *ahcf)
>> */
>> int fdarray[3];
>> bool sve_supported;
>> +bool el2_supported;
>> bool pmu_supported = false;
>> uint64_t features = 0;
>> int err;
>> @@ -535,6 +536,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures
>> *ahcf)
>> init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
>> }
>>
>> +/*
>> + * Ask for EL2 if supported.
>> + */
>> +el2_supported = kvm_arm_el2_supported();
>> +if (el2_supported) {
>> +init.features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
> This doesn't work if your host both supports SVE & NV.
>
May I ask if this prevented your L1 or L2 guest from booting? I’ve addressed all
the previous comments on the thread for the new RFC version and this topic is
what I’m currently addressing.
So far the L1 guest booted successfully but not the L2 guest.
> The error output by qemu is not straightforward
>
> qemu-system-aarch64: can't apply global host-arm-cpu.sve=off: Property
> 'host-arm-cpu.sve' not found
>
> The problem is that we create a scratch VM with a CPU featuring both SVE
> and NV and this fails at kernel level, I think on vcpu reset.
>
> The trouble is that we do that even if sve=off at qemu level. So I think
> this is a more generic issue related to the way we validate host cpu
> features.
>
OK, I’m having a look into that.
Thank you
Miguel
> Thanks
>
> Eric
>
>
>> +}
>> +
>> /*
>> * Ask for Pointer Authentication if supported, so that we get
>> * the unsanitized field values for AA64ISAR1_EL1.
>> @@ -714,6 +723,10 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures
>> *ahcf)
>> features |= 1ULL << ARM_FEATURE_PMU;
>> features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
>>
>> +if (el2_supported) {
>> +features |= 1ULL << ARM_FEATURE_EL2;
>> +}
>> +
>> ahcf->features = features;
>>
>> return true;
>> @@ -881,6 +894,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
>> assert(kvm_arm_sve_supported());
>> cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
>> }
>> +if (cpu_isar_feature(aa64_aa32_el2, cpu)) {
>> +cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
>> +}
>> if (cpu_isar_feature(aa64_pauth, cpu)) {
>> cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
>> 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
Re: [RFC PATCH 2/5] hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ
Hi Marc, > On 6 Mar 2023, at 13:32, Marc Zyngier wrote: > > On Mon, 06 Mar 2023 14:02:33 +, > Peter Maydell wrote: >> >> On Mon, 27 Feb 2023 at 16:37, Miguel Luis wrote: >>> >>> From: Haibo Xu >>> >>> Use the VGIC maintenance IRQ if VHE is requested. As per the ARM GIC >>> Architecture Specification for GICv3 and GICv4 Arm strongly recommends that >>> maintenance interrupts are configured to use INTID 25 matching the >>> Server Base System Architecture (SBSA) recomendation. >> >> What does this mean for QEMU, though? If the issue is >> "KVM doesn't support the maintenance interrupt being anything >> other than INTID 25" then we should say so (and have our code >> error out if the board tries to use some other value). > > No, KVM doesn't give two hoots about the INTID, as long as this is a > PPI that is otherwise unused. > >> If the >> issue is "the *host* has to be using the right INTID" then I >> would hope that KVM simply doesn't expose the capability if >> the host h/w won't let it work correctly. > > No host maintenance interrupt, no NV. This is specially mandatory as > the L1 guest is in (almost) complete control of the ICH_*_EL2 > registers and expects MIs to be delivered. > >> If KVM can happily >> use any maintenance interrupt ID that the board model wants, >> then we should make that work, rather than hardcoding 25 into >> our gicv3 code. > > +1. > > I'd eliminate any reference to SBSA, as it has no bearing on either > KVM nor the QEMU GIC code. > > I also question the "if VHE is requested". Not having VHE doesn't > preclude virtualisation. Was that supposed to be "virtualisation > extension" instead? > s/VHE/virtualization extension/ I’ve noted it has been a recurring confusion on my part. Will fix. :) Thank you, Miguel > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
Re: [RFC PATCH 2/5] hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ
Hi Peter,
> On 6 Mar 2023, at 13:02, Peter Maydell wrote:
>
> On Mon, 27 Feb 2023 at 16:37, Miguel Luis wrote:
>>
>> From: Haibo Xu
>>
>> Use the VGIC maintenance IRQ if VHE is requested. As per the ARM GIC
>> Architecture Specification for GICv3 and GICv4 Arm strongly recommends that
>> maintenance interrupts are configured to use INTID 25 matching the
>> Server Base System Architecture (SBSA) recomendation.
>
> What does this mean for QEMU, though? If the issue is
> "KVM doesn't support the maintenance interrupt being anything
> other than INTID 25" then we should say so (and have our code
> error out if the board tries to use some other value).
From the previous work I wondered where did the 25 would come from and I'm in
total agreement that this needs a better and meaningful commit message.
> If the
> issue is "the *host* has to be using the right INTID" then I
> would hope that KVM simply doesn't expose the capability if
> the host h/w won't let it work correctly. If KVM can happily
> use any maintenance interrupt ID that the board model wants,
> then we should make that work, rather than hardcoding 25 into
> our gicv3 code.
>
I agree.
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index b871350856..377181e009 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -759,6 +759,11 @@ static void create_gic(VirtMachineState *vms,
>> MemoryRegion *mem)
>> qdev_prop_set_uint32(vms->gic, "redist-region-count[1]",
>> MIN(smp_cpus - redist0_count, redist1_capacity));
>> }
>> +
>> +if (kvm_irqchip_in_kernel()) {
>> +qdev_prop_set_bit(vms->gic, "has-virtualization-extensions",
>> + vms->virt);
>> +}
>> } else {
>> if (!kvm_irqchip_in_kernel()) {
>> qdev_prop_set_bit(vms->gic, "has-virtualization-extensions",
>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>> index 351843db4a..e2a6ff1b49 100644
>> --- a/hw/intc/arm_gicv3_common.c
>> +++ b/hw/intc/arm_gicv3_common.c
>> @@ -563,6 +563,7 @@ static Property arm_gicv3_common_properties[] = {
>> DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
>> DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
>> DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn,
>> 0),
>> +DEFINE_PROP_BOOL("has-virtualization-extensions", GICv3State,
>> virt_extn, 0),
>> /*
>> * Compatibility property: force 8 bits of physical priority, even
>> * if the CPU being emulated should have fewer.
>> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
>> index 3ca643ecba..ce924753bb 100644
>> --- a/hw/intc/arm_gicv3_kvm.c
>> +++ b/hw/intc/arm_gicv3_kvm.c
>> @@ -22,6 +22,7 @@
>> #include "qemu/osdep.h"
>> #include "qapi/error.h"
>> #include "hw/intc/arm_gicv3_common.h"
>> +#include "hw/arm/virt.h"
>> #include "qemu/error-report.h"
>> #include "qemu/module.h"
>> #include "sysemu/kvm.h"
>> @@ -803,6 +804,30 @@ static void kvm_arm_gicv3_realize(DeviceState *dev,
>> Error **errp)
>> return;
>> }
>>
>> +
>> +if (s->virt_extn) {
>> +/*
>> + * Arm strongly recommends that maintenance interrupts are
>> configured
>> + * to use INTID 25. For more information, see Server Base System
>> + * Architecture (SBSA)
>> + */
>> +uint32_t maint_irq = PPI(ARCH_GIC_MAINT_IRQ);
>> +
>> +struct kvm_device_attr kdevattr = {
>> +.group = KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ,
>> +.addr = (uint64_t)_irq
>> +};
>> +
>> +if (!kvm_device_ioctl(s->dev_fd, KVM_GET_DEVICE_ATTR, )) {
>> +error_setg(errp, "VGICv3 setting maintenance IRQ are not "
>> +"supported by this host kernel");
>> +return;
>> +}
>> +
>> +kvm_device_ioctl(s->dev_fd, KVM_SET_DEVICE_ATTR, );
>> +}
>
> So if I understand this correctly, the requirement here is basically
> "tell the kernel which IRQ line is being used by the board code
> for the maintenance interrupt", right?
From the previous statement I understood it would be better to consider the
board code. So, yes.
> It seems to me that the
> straightforward way to do that is to have an integer QOM property on
> the GICv3 device like "maintenance-interrupt-id", and make the
> board code set it (whether using KVM or not).
Thanks, I’ll look into it.
> The TCG implementation
> doesn't care, and the KVM implementation can set it up in
> kvm_arm_gicv3_realize(). Then you don't need to specifically tell
> the GIC device that the guest is using the virtualization extensions.
>
Yes, that seems better suited.
Thank you,
Miguel
> thanks
> -- PMM
Re: [RFC PATCH 5/5] arm/virt: provide virtualization extensions to the guest
Hi Richard,
> On 27 Feb 2023, at 18:26, Richard Henderson
> wrote:
>
> On 2/27/23 06:37, Miguel Luis wrote:
>> -if (vms->virt && (kvm_enabled() || hvf_enabled())) {
>> +if (vms->virt && (kvm_enabled() || hvf_enabled())
>> +&& !kvm_arm_el2_supported()) {
>
> The ordering of the tests isn't right -- shouldn't test kvm_arm_* for hvf.
>
>virt && ((kvm && !kvm_el2) || hvf)
>
Agree. It will be fixed on the next version.
Thank you!
Miguel
>
> r~
Re: [RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported
Hi Richard,
> On 27 Feb 2023, at 18:24, Richard Henderson
> wrote:
>
> On 2/27/23 06:37, Miguel Luis wrote:
>> From: Haibo Xu
>> KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
>> EL2 bits on ID_AA64PFR0 state unsupported on the value 0b.
>> Ref:
>> https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo...@linaro.org/
>> Signed-off-by: Haibo Xu
>> [Miguel Luis: use of ID_AA64PFR0 for cpu features]
>> Signed-off-by: Miguel Luis
>> ---
>> target/arm/cpu.h | 2 +-
>> target/arm/kvm64.c | 16
>> 2 files changed, 17 insertions(+), 1 deletion(-)
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index 9aeed3c848..de2a88b43e 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -3961,7 +3961,7 @@ static inline bool isar_feature_aa64_aa32_el1(const
>> ARMISARegisters *id)
>>static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
>> {
>> -return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2;
>> +return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) != 0;
>> }
>
> No, this predicate is testing if EL2 supports AArch32 more.
>
>> @@ -714,6 +723,10 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures
>> *ahcf)
>> features |= 1ULL << ARM_FEATURE_PMU;
>> features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
>> +if (el2_supported) {
>> +features |= 1ULL << ARM_FEATURE_EL2;
>> +}
>
> This is the test you want...
>
>> @@ -881,6 +894,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
>> assert(kvm_arm_sve_supported());
>> cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
>> }
>> +if (cpu_isar_feature(aa64_aa32_el2, cpu)) {
>> +cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
>> +}
>
> ... here.
>
> While you could add a new isar_feature predicate for EL2 supported in AArch64
> mode, the feature test is equivalent and good enough, and is more obviously
> tied to the required KVM support.
>
>
Thank you for the explanation. I will take it into consideration on the next
version.
Miguel
> r~
Re: [RFC PATCH 1/5] linux-headers: [kvm, arm64] add the necessary definitions to match host kernel
Hi Cornelia, > On 27 Feb 2023, at 15:49, Cornelia Huck wrote: > > On Mon, Feb 27 2023, Miguel Luis wrote: > >> From: Haibo Xu >> >> linux-headers define host properties needed for the VMM to interact with >> KVM, so let's include them *while* they're not available yet on linux >> upstream since aarch64 nested virtualization is still a work in >> progress. >> >> Ref: >> https://lore.kernel.org/qemu-devel/636b5932e4cf061b6f97516e82d4319c1d29b871.1616052889.git.haibo...@linaro.org/ >> >> Signed-off-by: Haibo Xu >> Signed-off-by: Miguel Luis >> --- >> linux-headers/asm-arm64/kvm.h | 2 ++ >> linux-headers/linux/kvm.h | 1 + >> 2 files changed, 3 insertions(+) > > Can you please mark this explicitly as a placeholder for a proper > headers update? Just so that it doesn't get lost :) Sure yes! Thank you. Miguel
[RFC PATCH 3/5] target/arm/kvm: add helper to detect EL2 when using KVM
From: Haibo Xu
Introduce query support for KVM_CAP_ARM_EL2.
Ref:
https://lore.kernel.org/qemu-devel/65b8771bfecada08bf02c9cf87c2f0f9cdf943b3.1617281290.git.haibo...@linaro.org/
Signed-off-by: Haibo Xu
Signed-off-by: Miguel Luis
---
target/arm/kvm64.c | 5 +
target/arm/kvm_arm.h | 12
2 files changed, 17 insertions(+)
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 1197253d12..be8144a2b5 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -754,6 +754,11 @@ bool kvm_arm_aarch32_supported(void)
return kvm_check_extension(kvm_state, KVM_CAP_ARM_EL1_32BIT);
}
+bool kvm_arm_el2_supported(void)
+{
+return kvm_check_extension(kvm_state, KVM_CAP_ARM_EL2);
+}
+
bool kvm_arm_sve_supported(void)
{
return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE);
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 99017b635c..86a0cb4308 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -305,6 +305,13 @@ bool kvm_arm_pmu_supported(void);
*/
bool kvm_arm_sve_supported(void);
+/**
+ * kvm_arm_el2_supported:
+ *
+ * Returns true if KVM can enable EL2 and false otherwise.
+ */
+bool kvm_arm_el2_supported(void);
+
/**
* kvm_arm_get_max_vm_ipa_size:
* @ms: Machine state handle
@@ -395,6 +402,11 @@ static inline bool kvm_arm_steal_time_supported(void)
return false;
}
+static inline bool kvm_arm_el2_supported(void)
+{
+return false;
+}
+
/*
* These functions should never actually be called without KVM support.
*/
--
2.39.2
[RFC PATCH 5/5] arm/virt: provide virtualization extensions to the guest
From: Haibo Xu
VHE enablement if host supports EL2.
Ref:
https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo...@linaro.org/
Signed-off-by: Haibo Xu
Signed-off-by: Miguel Luis
---
hw/arm/virt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 377181e009..7103aecf3f 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2093,7 +2093,8 @@ static void machvirt_init(MachineState *machine)
exit(1);
}
-if (vms->virt && (kvm_enabled() || hvf_enabled())) {
+if (vms->virt && (kvm_enabled() || hvf_enabled())
+&& !kvm_arm_el2_supported()) {
error_report("mach-virt: %s does not support providing "
"Virtualization extensions to the guest CPU",
kvm_enabled() ? "KVM" : "HVF");
--
2.39.2
[RFC PATCH 0/5] QEMU v7.2.0 aarch64 Nested Virtualization Support
This series adds ARMv8.3/8.4 nested virtualization support in KVM mode. To enable nested virtualization for a guest, the host must expose EL2 support via QEMU command line switches: -machine virt,accel=kvm,virtualization=on Inspired on Haibo Xu's previous work [0][1], Marc Zyngier's kvmtool branch [2] and kernel patches [3] on nested virtualization for aarch64, this has been tested on an Ampere implementation. This series adapts previous work on top of v7.2.0, it considers comments given at the time and preserves authorship of the original patches. [0]: https://lore.kernel.org/qemu-devel/cover.1616052889.git.haibo...@linaro.org/ [1]: https://lore.kernel.org/qemu-devel/cover.1617281290.git.haibo...@linaro.org/ [2]: https://git.kernel.org/pub/scm/linux/kernel/git/maz/kvmtool.git/log/?h=arm64/nv-5.16 [3]: https://lore.kernel.org/linux-arm-kernel/20230131092504.2880505-1-...@kernel.org/ Miguel Luis (5): linux-headers: [kvm,arm64] add the necessary definitions to match host kernel hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ target/arm/kvm: add helper to detect EL2 when using KVM target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported arm/virt: provide virtualization extensions to the guest hw/arm/virt.c | 8 +++- hw/intc/arm_gicv3_common.c | 1 + hw/intc/arm_gicv3_kvm.c| 25 + include/hw/intc/arm_gicv3_common.h | 1 + linux-headers/asm-arm64/kvm.h | 2 ++ linux-headers/linux/kvm.h | 1 + target/arm/cpu.h | 2 +- target/arm/kvm64.c | 21 + target/arm/kvm_arm.h | 12 9 files changed, 71 insertions(+), 2 deletions(-) -- 2.39.2
[RFC PATCH 2/5] hw/intc/gicv3: add support for setting KVM vGIC maintenance IRQ
From: Haibo Xu
Use the VGIC maintenance IRQ if VHE is requested. As per the ARM GIC
Architecture Specification for GICv3 and GICv4 Arm strongly recommends that
maintenance interrupts are configured to use INTID 25 matching the
Server Base System Architecture (SBSA) recomendation.
Ref:
https://lore.kernel.org/qemu-devel/49a4944e2f148c56938380b981afe154b7a8b7ee.1617281290.git.haibo...@linaro.org/
Signed-off-by: Haibo Xu
[Miguel Luis: avoid direct usage of helpers (_check_attr(); _access())]
Signed-off-by: Miguel Luis
---
hw/arm/virt.c | 5 +
hw/intc/arm_gicv3_common.c | 1 +
hw/intc/arm_gicv3_kvm.c| 25 +
include/hw/intc/arm_gicv3_common.h | 1 +
4 files changed, 32 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b871350856..377181e009 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -759,6 +759,11 @@ static void create_gic(VirtMachineState *vms, MemoryRegion
*mem)
qdev_prop_set_uint32(vms->gic, "redist-region-count[1]",
MIN(smp_cpus - redist0_count, redist1_capacity));
}
+
+if (kvm_irqchip_in_kernel()) {
+qdev_prop_set_bit(vms->gic, "has-virtualization-extensions",
+ vms->virt);
+}
} else {
if (!kvm_irqchip_in_kernel()) {
qdev_prop_set_bit(vms->gic, "has-virtualization-extensions",
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 351843db4a..e2a6ff1b49 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -563,6 +563,7 @@ static Property arm_gicv3_common_properties[] = {
DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
+DEFINE_PROP_BOOL("has-virtualization-extensions", GICv3State, virt_extn,
0),
/*
* Compatibility property: force 8 bits of physical priority, even
* if the CPU being emulated should have fewer.
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 3ca643ecba..ce924753bb 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -22,6 +22,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/intc/arm_gicv3_common.h"
+#include "hw/arm/virt.h"
#include "qemu/error-report.h"
#include "qemu/module.h"
#include "sysemu/kvm.h"
@@ -803,6 +804,30 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error
**errp)
return;
}
+
+if (s->virt_extn) {
+/*
+ * Arm strongly recommends that maintenance interrupts are configured
+ * to use INTID 25. For more information, see Server Base System
+ * Architecture (SBSA)
+ */
+uint32_t maint_irq = PPI(ARCH_GIC_MAINT_IRQ);
+
+struct kvm_device_attr kdevattr = {
+.group = KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ,
+.addr = (uint64_t)_irq
+};
+
+if (!kvm_device_ioctl(s->dev_fd, KVM_GET_DEVICE_ATTR, )) {
+error_setg(errp, "VGICv3 setting maintenance IRQ are not "
+"supported by this host kernel");
+return;
+}
+
+kvm_device_ioctl(s->dev_fd, KVM_SET_DEVICE_ATTR, );
+}
+
+
gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL);
for (i = 0; i < s->num_cpu; i++) {
diff --git a/include/hw/intc/arm_gicv3_common.h
b/include/hw/intc/arm_gicv3_common.h
index ab5182a28a..91e1c1a45a 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -248,6 +248,7 @@ struct GICv3State {
uint32_t revision;
bool lpi_enable;
bool security_extn;
+bool virt_extn;
bool force_8bit_prio;
bool irq_reset_nonsecure;
bool gicd_no_migration_shift_bug;
--
2.39.2
[RFC PATCH 1/5] linux-headers: [kvm, arm64] add the necessary definitions to match host kernel
From: Haibo Xu
linux-headers define host properties needed for the VMM to interact with
KVM, so let's include them *while* they're not available yet on linux
upstream since aarch64 nested virtualization is still a work in
progress.
Ref:
https://lore.kernel.org/qemu-devel/636b5932e4cf061b6f97516e82d4319c1d29b871.1616052889.git.haibo...@linaro.org/
Signed-off-by: Haibo Xu
Signed-off-by: Miguel Luis
---
linux-headers/asm-arm64/kvm.h | 2 ++
linux-headers/linux/kvm.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/linux-headers/asm-arm64/kvm.h b/linux-headers/asm-arm64/kvm.h
index 4bf2d7246e..7bbcba7418 100644
--- a/linux-headers/asm-arm64/kvm.h
+++ b/linux-headers/asm-arm64/kvm.h
@@ -108,6 +108,7 @@ struct kvm_regs {
#define KVM_ARM_VCPU_SVE 4 /* enable SVE for this CPU */
#define KVM_ARM_VCPU_PTRAUTH_ADDRESS 5 /* VCPU uses address authentication */
#define KVM_ARM_VCPU_PTRAUTH_GENERIC 6 /* VCPU uses generic authentication */
+#define KVM_ARM_VCPU_HAS_EL2 7 /* Support Nested Virtualization */
struct kvm_vcpu_init {
__u32 target;
@@ -379,6 +380,7 @@ enum {
#define KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS 6
#define KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO 7
#define KVM_DEV_ARM_VGIC_GRP_ITS_REGS 8
+#define KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ 9
#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT 10
#define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
(0x3fULL << KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index ebdafa576d..bfd1d73988 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -1175,6 +1175,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_VM_DISABLE_NX_HUGE_PAGES 220
#define KVM_CAP_S390_ZPCI_OP 221
#define KVM_CAP_S390_CPU_TOPOLOGY 222
+#define KVM_CAP_ARM_EL2 226
#ifdef KVM_CAP_IRQ_ROUTING
--
2.39.2
[RFC PATCH 4/5] target/arm: enable feature ARM_FEATURE_EL2 if EL2 is supported
From: Haibo Xu
KVM_CAP_ARM_EL2 must be supported by the cpu to enable ARM_FEATURE_EL2.
EL2 bits on ID_AA64PFR0 state unsupported on the value 0b.
Ref:
https://lore.kernel.org/qemu-devel/b7c2626e6c720ccc43e57197dff3dac72d613640.1616052890.git.haibo...@linaro.org/
Signed-off-by: Haibo Xu
[Miguel Luis: use of ID_AA64PFR0 for cpu features]
Signed-off-by: Miguel Luis
---
target/arm/cpu.h | 2 +-
target/arm/kvm64.c | 16
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 9aeed3c848..de2a88b43e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3961,7 +3961,7 @@ static inline bool isar_feature_aa64_aa32_el1(const
ARMISARegisters *id)
static inline bool isar_feature_aa64_aa32_el2(const ARMISARegisters *id)
{
-return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) >= 2;
+return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, EL2) != 0;
}
static inline bool isar_feature_aa64_ras(const ARMISARegisters *id)
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index be8144a2b5..f7ebd731aa 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -505,6 +505,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
*/
int fdarray[3];
bool sve_supported;
+bool el2_supported;
bool pmu_supported = false;
uint64_t features = 0;
int err;
@@ -535,6 +536,14 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures
*ahcf)
init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
}
+/*
+ * Ask for EL2 if supported.
+ */
+el2_supported = kvm_arm_el2_supported();
+if (el2_supported) {
+init.features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
+}
+
/*
* Ask for Pointer Authentication if supported, so that we get
* the unsanitized field values for AA64ISAR1_EL1.
@@ -714,6 +723,10 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures
*ahcf)
features |= 1ULL << ARM_FEATURE_PMU;
features |= 1ULL << ARM_FEATURE_GENERIC_TIMER;
+if (el2_supported) {
+features |= 1ULL << ARM_FEATURE_EL2;
+}
+
ahcf->features = features;
return true;
@@ -881,6 +894,9 @@ int kvm_arch_init_vcpu(CPUState *cs)
assert(kvm_arm_sve_supported());
cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_SVE;
}
+if (cpu_isar_feature(aa64_aa32_el2, cpu)) {
+cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_HAS_EL2;
+}
if (cpu_isar_feature(aa64_pauth, cpu)) {
cpu->kvm_init_features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
--
2.39.2
[PATCH 4/4] tests/acpi: virt: update ACPI MADT and FADT binaries
Encoded Access Width : 00 [Undefined/Legacy]
[104h 0260 8] Address :
-/ ACPI table terminates in the middle of a data structure! (dump table) */
+[10Ch 0268 8]Hypervisor ID : 554D4551
-Raw Table Data: Length 268 (0x10C)
+Raw Table Data: Length 276 (0x114)
-: 46 41 43 50 0C 01 00 00 05 55 42 4F 43 48 53 20 // FACP.UBOCHS
+: 46 41 43 50 14 01 00 00 06 15 42 4F 43 48 53 20 // FACP..BOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43 // BXPCBXPC
0020: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
@@ -185,7 +185,7 @@ Raw Table Data: Length 268 (0x10C)
0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0070: 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 //
-0080: 00 03 00 01 00 00 00 00 00 00 00 00 00 00 00 00 //
+0080: 00 03 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00A0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00B0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
@@ -193,4 +193,5 @@ Raw Table Data: Length 268 (0x10C)
00D0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00E0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00F0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
-0100: 00 00 00 00 00 00 00 00 00 00 00 00 //
+0100: 00 00 00 00 00 00 00 00 00 00 00 00 51 45 4D 55 // QEMU
+0110: 00 00 00 00 //
Signed-off-by: Miguel Luis
---
tests/data/acpi/virt/APIC | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.memhp | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.numamem | Bin 168 -> 172 bytes
tests/data/acpi/virt/FACP | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.memhp | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.numamem | Bin 268 -> 276 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 6 --
7 files changed, 6 deletions(-)
diff --git a/tests/data/acpi/virt/APIC b/tests/data/acpi/virt/APIC
index
023f15f12e74fb9a3a6d3d9dc994541016947d6a..179d274770a23209b949c90a929525e22368568b
100644
GIT binary patch
delta 26
hcmZ3%xQ3C-F~HM#4FdxMi~B?_YsP?yZeA06WB^*d2KE2|
delta 26
icmZ3(xPp<(F~HM#1p@;EbHGF{Yet`mZeA0oNB{s@&<6Sd
diff --git a/tests/data/acpi/virt/APIC.memhp b/tests/data/acpi/virt/APIC.memhp
index
023f15f12e74fb9a3a6d3d9dc994541016947d6a..179d274770a23209b949c90a929525e22368568b
100644
GIT binary patch
delta 26
hcmZ3%xQ3C-F~HM#4FdxMi~B?_YsP?yZeA06WB^*d2KE2|
delta 26
icmZ3(xPp<(F~HM#1p@;EbHGF{Yet`mZeA0oNB{s@&<6Sd
diff --git a/tests/data/acpi/virt/APIC.numamem
b/tests/data/acpi/virt/APIC.numamem
index
023f15f12e74fb9a3a6d3d9dc994541016947d6a..179d274770a23209b949c90a929525e22368568b
100644
GIT binary patch
delta 26
hcmZ3%xQ3C-F~HM#4FdxMi~B?_YsP?yZeA06WB^*d2KE2|
delta 26
icmZ3(xPp<(F~HM#1p@;EbHGF{Yet`mZeA0oNB{s@&<6Sd
diff --git a/tests/data/acpi/virt/FACP b/tests/data/acpi/virt/FACP
index
1f764220f8533c427168e80ccf298604826a00b4..ac05c35a69451519bd1152c54d1e741af36390f5
100644
GIT binary patch
delta 33
ncmeBSn!?28=I9(C!pOkDCOVO;a^j?_i3a=}fv&!x3_t(?fr|$^
delta 26
hcmbQj)WgK(=I9*2!^ptE8ak1yl96%Z#OjF#yZ}u&1~C8t
diff --git a/tests/data/acpi/virt/FACP.memhp b/tests/data/acpi/virt/FACP.memhp
index
1f764220f8533c427168e80ccf298604826a00b4..ac05c35a69451519bd1152c54d1e741af36390f5
100644
GIT binary patch
delta 33
ncmeBSn!?28=I9(C!pOkDCOVO;a^j?_i3a=}fv&!x3_t(?fr|$^
delta 26
hcmbQj)WgK(=I9*2!^ptE8ak1yl96%Z#OjF#yZ}u&1~C8t
diff --git a/tests/data/acpi/virt/FACP.numamem
b/tests/data/acpi/virt/FACP.numamem
index
1f764220f8533c427168e80ccf298604826a00b4..ac05c35a69451519bd1152c54d1e741af36390f5
100644
GIT binary patch
delta 33
ncmeBSn!?28=I9(C!pOkDCOVO;a^j?_i3a=}fv&!x3_t(?fr|$^
delta 26
hcmbQj)WgK(=I9*2!^ptE8ak1yl96%Z#OjF#yZ}u&1~C8t
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h
b/tests/qtest/bios-tables-test-allowed-diff.h
index 8dc50f7a8a..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,7 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/FACP",
-"tests/data/acpi/virt/FACP.numamem",
-"tests/data/acpi/virt/FACP.memhp",
-"tests/data/acpi/virt/APIC",
-"tests/data/acpi/virt/APIC.memhp",
-"tests/data/acpi/virt/APIC.numamem",
--
2.37.3
[PATCH 3/4] acpi: arm/virt: madt: bump to revision 4 accordingly to ACPI 6.0 Errata A
MADT has been updated with the GIC Structures from ACPI 6.0 Errata A
and so MADT revision and GICC Structure must be updated also.
Fixes: 37f33084ed2e ("acpi: arm/virt: madt: use build_append_int_noprefix() API
to compose MADT table")
Signed-off-by: Miguel Luis
Reviewed-by: Ani Sinha
---
hw/arm/virt-acpi-build.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 72bb6f61a5..2d21e3cec4 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -686,7 +686,7 @@ build_dbg2(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
};
/*
- * ACPI spec, Revision 5.1 Errata A
+ * ACPI spec, Revision 6.0 Errata A
* 5.2.12 Multiple APIC Description Table (MADT)
*/
static void build_append_gicr(GArray *table_data, uint64_t base, uint32_t size)
@@ -705,7 +705,7 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
int i;
VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
const MemMapEntry *memmap = vms->memmap;
-AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = vms->oem_id,
+AcpiTable table = { .sig = "APIC", .rev = 4, .oem_id = vms->oem_id,
.oem_table_id = vms->oem_table_id };
acpi_table_begin(, table_data);
@@ -740,7 +740,7 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
/* 5.2.12.14 GIC Structure */
build_append_int_noprefix(table_data, 0xB, 1); /* Type */
-build_append_int_noprefix(table_data, 76, 1); /* Length */
+build_append_int_noprefix(table_data, 80, 1); /* Length */
build_append_int_noprefix(table_data, 0, 2);/* Reserved */
build_append_int_noprefix(table_data, i, 4);/* GIC ID */
build_append_int_noprefix(table_data, i, 4);/* ACPI Processor UID
*/
@@ -760,6 +760,10 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
build_append_int_noprefix(table_data, 0, 8);/* GICR Base Address*/
/* MPIDR */
build_append_int_noprefix(table_data, armcpu->mp_affinity, 8);
+/* Processor Power Efficiency Class */
+build_append_int_noprefix(table_data, 0, 1);
+/* Reserved */
+build_append_int_noprefix(table_data, 0, 3);
}
if (vms->gic_version != VIRT_GIC_VERSION_2) {
@@ -771,12 +775,6 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
}
if (its_class_name() && !vmc->no_its) {
-/*
- * FIXME: Structure is from Revision 6.0 where 'GIC Structure'
- * has additional fields on top of implemented 5.1 Errata A,
- * to make it consistent with v6.0 we need to bump everything
- * to v6.0
- */
/*
* ACPI spec, Revision 6.0 Errata A
* (original 6.0 definition has invalid Length)
--
2.37.3
[PATCH 0/4] ACPI MADT and FADT update according to the ACPI 6.0 spec
The MADT table structure has been updated in commit 37f33084ed2e
("acpi: arm/virt: madt: use build_append_int_noprefix() API to compose MADT
table")
to include the 5.2.12.18 GIC ITS Structure and so table's revision also needs
to
be updated. MADT and the FADT tables from the same spec need to be in sync and
in
this case also the FADT needs to be updated.
Revision 6.0 of the ACPI FADT table introduces the field "Hypervisor Vendor
Identity" which is missing and must be included.
Ref: https://uefi.org/sites/default/files/resources/ACPI_6_0_Errata_A.PDF
This patch series originates from a previous RFC [1] discussion. Reviewed-by
tags were kept on patches 2/4 and 3/4.
[1]: https://lists.gnu.org/archive/html/qemu-devel/2022-10/msg01326.html
Miguel Luis (4):
tests/acpi: virt: allow acpi MADT and FADT changes
acpi: fadt: support revision 6.0 of the ACPI specification
acpi: arm/virt: madt: bump to revision 4 accordingly to ACPI 6.0
Errata A
tests/acpi: virt: update ACPI MADT and FADT binaries
hw/acpi/aml-build.c | 13 ++---
hw/arm/virt-acpi-build.c | 26 --
tests/data/acpi/virt/APIC | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.memhp | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.numamem | Bin 168 -> 172 bytes
tests/data/acpi/virt/FACP | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.memhp | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.numamem | Bin 268 -> 276 bytes
8 files changed, 22 insertions(+), 17 deletions(-)
--
2.37.3
[PATCH 2/4] acpi: fadt: support revision 6.0 of the ACPI specification
Update the Fixed ACPI Description Table (FADT) to revision 6.0 of the ACPI
specification adding the field "Hypervisor Vendor Identity".
This field's description states the following: "64-bit identifier of hypervisor
vendor. All bytes in this field are considered part of the vendor identity.
These identifiers are defined independently by the vendors themselves,
usually following the name of the hypervisor product. Version information
should NOT be included in this field - this shall simply denote the vendor's
name or identifier. Version information can be communicated through a
supplemental vendor-specific hypervisor API. Firmware implementers would
place zero bytes into this field, denoting that no hypervisor is present in
the actual firmware."
Signed-off-by: Miguel Luis
Reviewed-by: Ani Sinha
---
hw/acpi/aml-build.c | 13 ++---
hw/arm/virt-acpi-build.c | 10 +-
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index e6bfac95c7..42feb4d4d7 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2070,7 +2070,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker,
MachineState *ms,
acpi_table_end(linker, );
}
-/* build rev1/rev3/rev5.1 FADT */
+/* build rev1/rev3/rev5.1/rev6.0 FADT */
void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
const char *oem_id, const char *oem_table_id)
{
@@ -2193,8 +2193,15 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const
AcpiFadtData *f,
/* SLEEP_STATUS_REG */
build_append_gas_from_struct(tbl, >sleep_sts);
-/* TODO: extra fields need to be added to support revisions above rev5 */
-assert(f->rev == 5);
+if (f->rev == 5) {
+goto done;
+}
+
+/* Hypervisor Vendor Identity */
+build_append_padded_str(tbl, "QEMU", 8, '\0');
+
+/* TODO: extra fields need to be added to support revisions above rev6 */
+assert(f->rev == 6);
done:
acpi_table_end(linker, );
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9b3aee01bf..72bb6f61a5 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -809,13 +809,13 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
}
/* FADT */
-static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
+static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms, unsigned dsdt_tbl_offset)
{
-/* ACPI v5.1 */
+/* ACPI v6.0 */
AcpiFadtData fadt = {
-.rev = 5,
-.minor_ver = 1,
+.rev = 6,
+.minor_ver = 0,
.flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
.xdsdt_tbl_offset = _tbl_offset,
};
@@ -945,7 +945,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables
*tables)
/* FADT MADT PPTT GTDT MCFG SPCR DBG2 pointed to by RSDT */
acpi_add_table(table_offsets, tables_blob);
-build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
+build_fadt_rev6(tables_blob, tables->linker, vms, dsdt);
acpi_add_table(table_offsets, tables_blob);
build_madt(tables_blob, tables->linker, vms);
--
2.37.3
[PATCH 1/4] tests/acpi: virt: allow acpi MADT and FADT changes
Step 3 from bios-tables-test.c documented procedure. Signed-off-by: Miguel Luis --- tests/qtest/bios-tables-test-allowed-diff.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..8dc50f7a8a 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,7 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/FACP", +"tests/data/acpi/virt/FACP.numamem", +"tests/data/acpi/virt/FACP.memhp", +"tests/data/acpi/virt/APIC", +"tests/data/acpi/virt/APIC.memhp", +"tests/data/acpi/virt/APIC.numamem", -- 2.37.3
Re: [RFC PATCH v2 2/4] acpi: fadt: support revision 6.0 of the ACPI specification
> On 11 Oct 2022, at 05:02, Ani Sinha wrote:
>
> On Mon, Oct 10, 2022 at 6:53 PM Miguel Luis wrote:
>>
>> Update the Fixed ACPI Description Table (FADT) to revision 6.0 of the ACPI
>> specification adding the field "Hypervisor Vendor Identity" that was missing.
>>
>> This field's description states the following: "64-bit identifier of
>> hypervisor
>> vendor. All bytes in this field are considered part of the vendor identity.
>> These identifiers are defined independently by the vendors themselves,
>> usually following the name of the hypervisor product. Version information
>> should NOT be included in this field - this shall simply denote the vendor's
>> name or identifier. Version information can be communicated through a
>> supplemental vendor-specific hypervisor API. Firmware implementers would
>> place zero bytes into this field, denoting that no hypervisor is present in
>> the actual firmware."
>>
>> Hereupon, what should a valid identifier of an Hypervisor Vendor ID be and
>> where should QEMU provide that information?
>>
>> On the v1 [1] of this RFC there's the suggestion of having this information
>> in sync by the current acceleration name. This also seems to imply that QEMU,
>> which generates the FADT table, and the FADT consumer need to be in sync with
>> the values of this field.
>>
>> This version follows Ani Sinha's suggestion [2] of using "QEMU" for the
>> hypervisor vendor ID.
>>
>> [1]: https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg00911.html
>> [2]: https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg00989.html
>>
>> Signed-off-by: Miguel Luis
>
> Reviewed-by: Ani Sinha
>
Thank you Ani. In the meanwhile, taking the description part of: “Firmware
implementers would place zero bytes into this field, denoting that no
hypervisor is present in the actual firmware.", I reached to something along
the lines below:
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 42feb4d4d7..e719afe0cb 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2198,7 +2198,11 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const
AcpiFadtData *f,
}
/* Hypervisor Vendor Identity */
-build_append_padded_str(tbl, "QEMU", 8, '\0');
+if (f->hyp_is_present) {
+build_append_padded_str(tbl, "QEMU", 8, '\0');
+} else {
+build_append_int_noprefix(tbl, 0, 8);
+}
/* TODO: extra fields need to be added to support revisions above rev6 */
assert(f->rev == 6);
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 72bb6f61a5..d238ce2b88 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -818,6 +818,7 @@ static void build_fadt_rev6(GArray *table_data, BIOSLinker
*linker,
.minor_ver = 0,
.flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
.xdsdt_tbl_offset = _tbl_offset,
+.hyp_is_present = vms->virt && (kvm_enabled() || hvf_enabled()),
};
switch (vms->psci_conduit) {
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index 2b42e4192b..2aff5304af 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -79,7 +79,7 @@ typedef struct AcpiFadtData {
uint16_t arm_boot_arch;/* ARM_BOOT_ARCH */
uint16_t iapc_boot_arch; /* IAPC_BOOT_ARCH */
uint8_t minor_ver; /* FADT Minor Version */
-
+bool hyp_is_present;
/*
* respective tables offsets within ACPI_BUILD_TABLE_FILE,
* NULL if table doesn't exist (in that case field's value
Any thoughts on this?
Thanks
Miguel
>> ---
>> hw/acpi/aml-build.c | 13 ++---
>> hw/arm/virt-acpi-build.c | 10 +-
>> 2 files changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index e6bfac95c7..42feb4d4d7 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -2070,7 +2070,7 @@ void build_pptt(GArray *table_data, BIOSLinker
>> *linker, MachineState *ms,
>> acpi_table_end(linker, );
>> }
>>
>> -/* build rev1/rev3/rev5.1 FADT */
>> +/* build rev1/rev3/rev5.1/rev6.0 FADT */
>> void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>> const char *oem_id, const char *oem_table_id)
>> {
>> @@ -2193,8 +2193,15 @@ void build_fadt(GArray *tbl, BIOSLinker *linker,
>> const AcpiFadtData *f,
>> /* SLEEP_STATUS_REG */
>> build_append_gas_from_struct(tbl, >sleep_sts);
>>
>> -/* TODO: extra fields need to be added to support revisions above rev5
>> */
>> -assert(f-&
[RFC PATCH v2 2/4] acpi: fadt: support revision 6.0 of the ACPI specification
Update the Fixed ACPI Description Table (FADT) to revision 6.0 of the ACPI
specification adding the field "Hypervisor Vendor Identity" that was missing.
This field's description states the following: "64-bit identifier of hypervisor
vendor. All bytes in this field are considered part of the vendor identity.
These identifiers are defined independently by the vendors themselves,
usually following the name of the hypervisor product. Version information
should NOT be included in this field - this shall simply denote the vendor's
name or identifier. Version information can be communicated through a
supplemental vendor-specific hypervisor API. Firmware implementers would
place zero bytes into this field, denoting that no hypervisor is present in
the actual firmware."
Hereupon, what should a valid identifier of an Hypervisor Vendor ID be and
where should QEMU provide that information?
On the v1 [1] of this RFC there's the suggestion of having this information
in sync by the current acceleration name. This also seems to imply that QEMU,
which generates the FADT table, and the FADT consumer need to be in sync with
the values of this field.
This version follows Ani Sinha's suggestion [2] of using "QEMU" for the
hypervisor vendor ID.
[1]: https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg00911.html
[2]: https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg00989.html
Signed-off-by: Miguel Luis
---
hw/acpi/aml-build.c | 13 ++---
hw/arm/virt-acpi-build.c | 10 +-
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index e6bfac95c7..42feb4d4d7 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2070,7 +2070,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker,
MachineState *ms,
acpi_table_end(linker, );
}
-/* build rev1/rev3/rev5.1 FADT */
+/* build rev1/rev3/rev5.1/rev6.0 FADT */
void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
const char *oem_id, const char *oem_table_id)
{
@@ -2193,8 +2193,15 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const
AcpiFadtData *f,
/* SLEEP_STATUS_REG */
build_append_gas_from_struct(tbl, >sleep_sts);
-/* TODO: extra fields need to be added to support revisions above rev5 */
-assert(f->rev == 5);
+if (f->rev == 5) {
+goto done;
+}
+
+/* Hypervisor Vendor Identity */
+build_append_padded_str(tbl, "QEMU", 8, '\0');
+
+/* TODO: extra fields need to be added to support revisions above rev6 */
+assert(f->rev == 6);
done:
acpi_table_end(linker, );
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9b3aee01bf..72bb6f61a5 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -809,13 +809,13 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
}
/* FADT */
-static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
+static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms, unsigned dsdt_tbl_offset)
{
-/* ACPI v5.1 */
+/* ACPI v6.0 */
AcpiFadtData fadt = {
-.rev = 5,
-.minor_ver = 1,
+.rev = 6,
+.minor_ver = 0,
.flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
.xdsdt_tbl_offset = _tbl_offset,
};
@@ -945,7 +945,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables
*tables)
/* FADT MADT PPTT GTDT MCFG SPCR DBG2 pointed to by RSDT */
acpi_add_table(table_offsets, tables_blob);
-build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
+build_fadt_rev6(tables_blob, tables->linker, vms, dsdt);
acpi_add_table(table_offsets, tables_blob);
build_madt(tables_blob, tables->linker, vms);
--
2.37.3
[RFC PATCH v2 4/4] tests/acpi: virt: update ACPI MADT and FADT binaries
Encoded Access Width : 00 [Undefined/Legacy]
[104h 0260 8] Address :
-/ ACPI table terminates in the middle of a data structure! (dump table) */
+[10Ch 0268 8]Hypervisor ID : 554D4551
-Raw Table Data: Length 268 (0x10C)
+Raw Table Data: Length 276 (0x114)
-: 46 41 43 50 0C 01 00 00 05 55 42 4F 43 48 53 20 // FACP.UBOCHS
+: 46 41 43 50 14 01 00 00 06 15 42 4F 43 48 53 20 // FACP..BOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43 // BXPCBXPC
0020: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
@@ -185,7 +185,7 @@ Raw Table Data: Length 268 (0x10C)
0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0070: 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 //
-0080: 00 03 00 01 00 00 00 00 00 00 00 00 00 00 00 00 //
+0080: 00 03 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00A0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00B0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
@@ -193,4 +193,5 @@ Raw Table Data: Length 268 (0x10C)
00D0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00E0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00F0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
-0100: 00 00 00 00 00 00 00 00 00 00 00 00 //
+0100: 00 00 00 00 00 00 00 00 00 00 00 00 51 45 4D 55 // QEMU
+0110: 00 00 00 00 //
Signed-off-by: Miguel Luis
---
tests/data/acpi/virt/APIC | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.memhp | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.numamem | Bin 168 -> 172 bytes
tests/data/acpi/virt/FACP | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.memhp | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.numamem | Bin 268 -> 276 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 6 --
7 files changed, 6 deletions(-)
diff --git a/tests/data/acpi/virt/APIC b/tests/data/acpi/virt/APIC
index
023f15f12e74fb9a3a6d3d9dc994541016947d6a..179d274770a23209b949c90a929525e22368568b
100644
GIT binary patch
delta 26
hcmZ3%xQ3C-F~HM#4FdxMi~B?_YsP?yZeA06WB^*d2KE2|
delta 26
icmZ3(xPp<(F~HM#1p@;EbHGF{Yet`mZeA0oNB{s@&<6Sd
diff --git a/tests/data/acpi/virt/APIC.memhp b/tests/data/acpi/virt/APIC.memhp
index
023f15f12e74fb9a3a6d3d9dc994541016947d6a..179d274770a23209b949c90a929525e22368568b
100644
GIT binary patch
delta 26
hcmZ3%xQ3C-F~HM#4FdxMi~B?_YsP?yZeA06WB^*d2KE2|
delta 26
icmZ3(xPp<(F~HM#1p@;EbHGF{Yet`mZeA0oNB{s@&<6Sd
diff --git a/tests/data/acpi/virt/APIC.numamem
b/tests/data/acpi/virt/APIC.numamem
index
023f15f12e74fb9a3a6d3d9dc994541016947d6a..179d274770a23209b949c90a929525e22368568b
100644
GIT binary patch
delta 26
hcmZ3%xQ3C-F~HM#4FdxMi~B?_YsP?yZeA06WB^*d2KE2|
delta 26
icmZ3(xPp<(F~HM#1p@;EbHGF{Yet`mZeA0oNB{s@&<6Sd
diff --git a/tests/data/acpi/virt/FACP b/tests/data/acpi/virt/FACP
index
1f764220f8533c427168e80ccf298604826a00b4..ac05c35a69451519bd1152c54d1e741af36390f5
100644
GIT binary patch
delta 33
ncmeBSn!?28=I9(C!pOkDCOVO;a^j?_i3a=}fv&!x3_t(?fr|$^
delta 26
hcmbQj)WgK(=I9*2!^ptE8ak1yl96%Z#OjF#yZ}u&1~C8t
diff --git a/tests/data/acpi/virt/FACP.memhp b/tests/data/acpi/virt/FACP.memhp
index
1f764220f8533c427168e80ccf298604826a00b4..ac05c35a69451519bd1152c54d1e741af36390f5
100644
GIT binary patch
delta 33
ncmeBSn!?28=I9(C!pOkDCOVO;a^j?_i3a=}fv&!x3_t(?fr|$^
delta 26
hcmbQj)WgK(=I9*2!^ptE8ak1yl96%Z#OjF#yZ}u&1~C8t
diff --git a/tests/data/acpi/virt/FACP.numamem
b/tests/data/acpi/virt/FACP.numamem
index
1f764220f8533c427168e80ccf298604826a00b4..ac05c35a69451519bd1152c54d1e741af36390f5
100644
GIT binary patch
delta 33
ncmeBSn!?28=I9(C!pOkDCOVO;a^j?_i3a=}fv&!x3_t(?fr|$^
delta 26
hcmbQj)WgK(=I9*2!^ptE8ak1yl96%Z#OjF#yZ}u&1~C8t
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h
b/tests/qtest/bios-tables-test-allowed-diff.h
index 8dc50f7a8a..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,7 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/FACP",
-"tests/data/acpi/virt/FACP.numamem",
-"tests/data/acpi/virt/FACP.memhp",
-"tests/data/acpi/virt/APIC",
-"tests/data/acpi/virt/APIC.memhp",
-"tests/data/acpi/virt/APIC.numamem",
--
2.37.3
[RFC PATCH v2 0/4] ACPI MADT and FADT update according to the ACPI 6.0 spec
The MADT table structure has been updated in commit 37f33084ed2e
("acpi: arm/virt: madt: use build_append_int_noprefix() API to compose MADT
table")
to include the 5.2.12.18 GIC ITS Structure and so table's revision also needs
to
be updated. MADT and the FADT tables from the same spec need to be in sync and
in
this case also the FADT needs to be updated.
Revision 6.0 of the ACPI FADT table introduces the field "Hypervisor Vendor
Identity" which is missing and must be included. Patch 2/4 includes a
suggestion for the value of this field.
Ref: https://uefi.org/sites/default/files/resources/ACPI_6_0_Errata_A.PDF
Changelog:
v2:
patch 2/4:
fix expression that checks for the revision number (Ani Sinha)
use "QEMU" as the Hypervisor Vendor ID [1] (Ani Sinha)
patch 3/4:
add Reviewed-by tag from Ani Sinha
v1: https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg00910.html
Open to discussion, your comments, thoughts and suggestions are very welcome.
Thanks in advance.
Miguel
[1]: https://lists.nongnu.org/archive/html/qemu-devel/2022-10/msg00989.html
Miguel Luis (4):
tests/acpi: virt: allow acpi MADT and FADT changes
acpi: fadt: support revision 6.0 of the ACPI specification
acpi: arm/virt: madt: bump to revision 4 accordingly to ACPI 6.0
Errata A
tests/acpi: virt: update ACPI MADT and FADT binaries
hw/acpi/aml-build.c | 13 ++---
hw/arm/virt-acpi-build.c | 26 --
tests/data/acpi/virt/APIC | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.memhp | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.numamem | Bin 168 -> 172 bytes
tests/data/acpi/virt/FACP | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.memhp | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.numamem | Bin 268 -> 276 bytes
8 files changed, 22 insertions(+), 17 deletions(-)
--
2.37.3
[RFC PATCH v2 3/4] acpi: arm/virt: madt: bump to revision 4 accordingly to ACPI 6.0 Errata A
MADT has been updated with the GIC Structures from ACPI 6.0 Errata A
and so MADT revision and GICC Structure must be updated also.
Fixes: 37f33084ed2e ("acpi: arm/virt: madt: use build_append_int_noprefix() API
to compose MADT table")
Signed-off-by: Miguel Luis
Reviewed-by: Ani Sinha
---
hw/arm/virt-acpi-build.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 72bb6f61a5..2d21e3cec4 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -686,7 +686,7 @@ build_dbg2(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
};
/*
- * ACPI spec, Revision 5.1 Errata A
+ * ACPI spec, Revision 6.0 Errata A
* 5.2.12 Multiple APIC Description Table (MADT)
*/
static void build_append_gicr(GArray *table_data, uint64_t base, uint32_t size)
@@ -705,7 +705,7 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
int i;
VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
const MemMapEntry *memmap = vms->memmap;
-AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = vms->oem_id,
+AcpiTable table = { .sig = "APIC", .rev = 4, .oem_id = vms->oem_id,
.oem_table_id = vms->oem_table_id };
acpi_table_begin(, table_data);
@@ -740,7 +740,7 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
/* 5.2.12.14 GIC Structure */
build_append_int_noprefix(table_data, 0xB, 1); /* Type */
-build_append_int_noprefix(table_data, 76, 1); /* Length */
+build_append_int_noprefix(table_data, 80, 1); /* Length */
build_append_int_noprefix(table_data, 0, 2);/* Reserved */
build_append_int_noprefix(table_data, i, 4);/* GIC ID */
build_append_int_noprefix(table_data, i, 4);/* ACPI Processor UID
*/
@@ -760,6 +760,10 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
build_append_int_noprefix(table_data, 0, 8);/* GICR Base Address*/
/* MPIDR */
build_append_int_noprefix(table_data, armcpu->mp_affinity, 8);
+/* Processor Power Efficiency Class */
+build_append_int_noprefix(table_data, 0, 1);
+/* Reserved */
+build_append_int_noprefix(table_data, 0, 3);
}
if (vms->gic_version != VIRT_GIC_VERSION_2) {
@@ -771,12 +775,6 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
}
if (its_class_name() && !vmc->no_its) {
-/*
- * FIXME: Structure is from Revision 6.0 where 'GIC Structure'
- * has additional fields on top of implemented 5.1 Errata A,
- * to make it consistent with v6.0 we need to bump everything
- * to v6.0
- */
/*
* ACPI spec, Revision 6.0 Errata A
* (original 6.0 definition has invalid Length)
--
2.37.3
[RFC PATCH v2 1/4] tests/acpi: virt: allow acpi MADT and FADT changes
Step 3 from bios-tables-test.c documented procedure. Signed-off-by: Miguel Luis --- tests/qtest/bios-tables-test-allowed-diff.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..8dc50f7a8a 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,7 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/FACP", +"tests/data/acpi/virt/FACP.numamem", +"tests/data/acpi/virt/FACP.memhp", +"tests/data/acpi/virt/APIC", +"tests/data/acpi/virt/APIC.memhp", +"tests/data/acpi/virt/APIC.numamem", -- 2.37.3
Re: [External] : Re: [PATCH v2 0/3] fix for two ACPI GTDT physical addresses
> On 7 Oct 2022, at 15:21, Ani Sinha wrote: > > On Fri, Oct 7, 2022 at 8:16 PM Miguel Luis wrote: >> >> The ACPI GTDT table contains two invalid 64-bit physical addresses according >> to >> the ACPI spec. 6.5 [1]. Those are the Counter Control Base physical address >> and >> the Counter Read Base physical address. Those fields of the GTDT table >> should be >> set to 0x if not provided, rather than 0x0. >> >> [1]: >> https://urldefense.com/v3/__https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html*gtdt-table-structure__;Iw!!ACWV5N9M2RV99hQ!I-YqmAwYNhk19YHxcbjQBMwEE9a8rZOvufvOOonAPEtgTynOYOf5AyYKLTTGJ2RRzsjvkjIuleSubpg$ >> >> >> Changelog: >> >> v2: >>Updated with collected tags from v1. > > For future reference, there is no need to send out a new version with > just the tags added. The tooling make sure that the tags are collected > correctly from the last version. > Great! Thanks for the tip which is very helpful. Miguel >> >> v1: >> https://urldefense.com/v3/__https://lists.nongnu.org/archive/html/qemu-devel/2022-09/msg02847.html__;!!ACWV5N9M2RV99hQ!I-YqmAwYNhk19YHxcbjQBMwEE9a8rZOvufvOOonAPEtgTynOYOf5AyYKLTTGJ2RRzsjvkjIulSis4m4$ >> >> >> Miguel Luis (3): >> tests/acpi: virt: allow acpi GTDT changes >> acpi: arm/virt: build_gtdt: fix invalid 64-bit physical addresses >> tests/acpi: virt: update ACPI GTDT binaries >> >> hw/arm/virt-acpi-build.c | 5 ++--- >> tests/data/acpi/virt/GTDT | Bin 96 -> 96 bytes >> tests/data/acpi/virt/GTDT.memhp | Bin 96 -> 96 bytes >> tests/data/acpi/virt/GTDT.numamem | Bin 96 -> 96 bytes >> 4 files changed, 2 insertions(+), 3 deletions(-) >> >> -- >> 2.37.3 >>
[PATCH v2 3/3] tests/acpi: virt: update ACPI GTDT binaries
Step 6 & 7 of the bios-tables-test.c documented procedure.
Differences between disassembled ASL files for GTDT:
@@ -13,14 +13,14 @@
[000h 4]Signature : "GTDT"[Generic Timer
Description Table]
[004h 0004 4] Table Length : 0060
[008h 0008 1] Revision : 02
-[009h 0009 1] Checksum : 8C
+[009h 0009 1] Checksum : 9C
[00Ah 0010 6] Oem ID : "BOCHS "
[010h 0016 8] Oem Table ID : "BXPC"
[018h 0024 4] Oem Revision : 0001
[01Ch 0028 4] Asl Compiler ID : "BXPC"
[020h 0032 4]Asl Compiler Revision : 0001
-[024h 0036 8]Counter Block Address :
+[024h 0036 8]Counter Block Address :
[02Ch 0044 4] Reserved :
[030h 0048 4] Secure EL1 Interrupt : 001D
@@ -46,16 +46,16 @@
Trigger Mode : 0
Polarity : 0
Always On : 0
-[050h 0080 8] Counter Read Block Address :
+[050h 0080 8] Counter Read Block Address :
[058h 0088 4] Platform Timer Count :
[05Ch 0092 4]Platform Timer Offset :
Raw Table Data: Length 96 (0x60)
-: 47 54 44 54 60 00 00 00 02 8C 42 4F 43 48 53 20 //
GTDT`.BOCHS
+: 47 54 44 54 60 00 00 00 02 9C 42 4F 43 48 53 20 //
GTDT`.BOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43 // BXPC
BXPC
-0020: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
+0020: 01 00 00 00 FF FF FF FF FF FF FF FF 00 00 00 00 //
0030: 1D 00 00 00 00 00 00 00 1E 00 00 00 04 00 00 00 //
0040: 1B 00 00 00 00 00 00 00 1A 00 00 00 00 00 00 00 //
-0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
+0050: FF FF FF FF FF FF FF FF 00 00 00 00 00 00 00 00 //
Signed-off-by: Miguel Luis
Acked-by: Ani Sinha
---
tests/data/acpi/virt/GTDT | Bin 96 -> 96 bytes
tests/data/acpi/virt/GTDT.memhp | Bin 96 -> 96 bytes
tests/data/acpi/virt/GTDT.numamem | Bin 96 -> 96 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 3 ---
4 files changed, 3 deletions(-)
diff --git a/tests/data/acpi/virt/GTDT b/tests/data/acpi/virt/GTDT
index
9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2
100644
GIT binary patch
delta 45
kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*
delta 45
jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8
diff --git a/tests/data/acpi/virt/GTDT.memhp b/tests/data/acpi/virt/GTDT.memhp
index
9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2
100644
GIT binary patch
delta 45
kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*
delta 45
jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8
diff --git a/tests/data/acpi/virt/GTDT.numamem
b/tests/data/acpi/virt/GTDT.numamem
index
9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2
100644
GIT binary patch
delta 45
kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*
delta 45
jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h
b/tests/qtest/bios-tables-test-allowed-diff.h
index 957bd1b4f6..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,4 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/GTDT",
-"tests/data/acpi/virt/GTDT.memhp",
-"tests/data/acpi/virt/GTDT.numamem",
--
2.37.3
[PATCH v2 1/3] tests/acpi: virt: allow acpi GTDT changes
Step 3 from bios-tables-test.c documented procedure. Signed-off-by: Miguel Luis Acked-by: Ani Sinha --- tests/qtest/bios-tables-test-allowed-diff.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..957bd1b4f6 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,4 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/GTDT", +"tests/data/acpi/virt/GTDT.memhp", +"tests/data/acpi/virt/GTDT.numamem", -- 2.37.3
[PATCH v2 0/3] fix for two ACPI GTDT physical addresses
The ACPI GTDT table contains two invalid 64-bit physical addresses according to the ACPI spec. 6.5 [1]. Those are the Counter Control Base physical address and the Counter Read Base physical address. Those fields of the GTDT table should be set to 0x if not provided, rather than 0x0. [1]: https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#gtdt-table-structure Changelog: v2: Updated with collected tags from v1. v1: https://lists.nongnu.org/archive/html/qemu-devel/2022-09/msg02847.html Miguel Luis (3): tests/acpi: virt: allow acpi GTDT changes acpi: arm/virt: build_gtdt: fix invalid 64-bit physical addresses tests/acpi: virt: update ACPI GTDT binaries hw/arm/virt-acpi-build.c | 5 ++--- tests/data/acpi/virt/GTDT | Bin 96 -> 96 bytes tests/data/acpi/virt/GTDT.memhp | Bin 96 -> 96 bytes tests/data/acpi/virt/GTDT.numamem | Bin 96 -> 96 bytes 4 files changed, 2 insertions(+), 3 deletions(-) -- 2.37.3
[PATCH v2 2/3] acpi: arm/virt: build_gtdt: fix invalid 64-bit physical addresses
Per the ACPI 6.5 specification, on the GTDT Table Structure, the Counter Control
Block Address and Counter Read Block Address fields of the GTDT table should be
set to 0x if not provided, rather than 0x0.
Fixes: 41041e57085 ("acpi: arm/virt: build_gtdt: use
acpi_table_begin()/acpi_table_end() instead of build_header()")
Signed-off-by: Miguel Luis
Reviewed-by: Ani Sinha
---
hw/arm/virt-acpi-build.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9b3aee01bf..13c6e3e468 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -592,8 +592,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
acpi_table_begin(, table_data);
/* CntControlBase Physical Address */
-/* FIXME: invalid value, should be 0x if not impl. ? */
-build_append_int_noprefix(table_data, 0, 8);
+build_append_int_noprefix(table_data, 0x, 8);
build_append_int_noprefix(table_data, 0, 4); /* Reserved */
/*
* FIXME: clarify comment:
@@ -618,7 +617,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
/* Non-Secure EL2 timer Flags */
build_append_int_noprefix(table_data, irqflags, 4);
/* CntReadBase Physical address */
-build_append_int_noprefix(table_data, 0, 8);
+build_append_int_noprefix(table_data, 0x, 8);
/* Platform Timer Count */
build_append_int_noprefix(table_data, 0, 4);
/* Platform Timer Offset */
--
2.37.3
Re: [External] : Re: [PATCH 3/3] tests/acpi: virt: update ACPI GTDT binaries
> On 21 Sep 2022, at 03:39, Ani Sinha wrote:
>
>
>
> On Tue, 20 Sep 2022, Miguel Luis wrote:
>
>> Step 6 & 7 of the bios-tables-test.c documented procedure.
>>
>> Differences between disassembled ASL files for GTDT:
>>
>>@@ -13,14 +13,14 @@
>> [000h 4]Signature : "GTDT"[Generic Timer
>> Description Table]
>> [004h 0004 4] Table Length : 0060
>> [008h 0008 1] Revision : 02
>>-[009h 0009 1] Checksum : 8C
>>+[009h 0009 1] Checksum : 9C
>> [00Ah 0010 6] Oem ID : "BOCHS "
>> [010h 0016 8] Oem Table ID : "BXPC"
>> [018h 0024 4] Oem Revision : 0001
>> [01Ch 0028 4] Asl Compiler ID : "BXPC"
>> [020h 0032 4]Asl Compiler Revision : 0001
>>
>>-[024h 0036 8]Counter Block Address :
>>+[024h 0036 8]Counter Block Address :
>> [02Ch 0044 4] Reserved :
>>
>> [030h 0048 4] Secure EL1 Interrupt : 001D
>>@@ -46,16 +46,16 @@
>> Trigger Mode : 0
>> Polarity : 0
>>Always On : 0
>>-[050h 0080 8] Counter Read Block Address :
>>+[050h 0080 8] Counter Read Block Address :
>>
>> [058h 0088 4] Platform Timer Count :
>> [05Ch 0092 4]Platform Timer Offset :
>>
>> Raw Table Data: Length 96 (0x60)
>>
>>-: 47 54 44 54 60 00 00 00 02 8C 42 4F 43 48 53 20 //
>> GTDT`.BOCHS
>>+: 47 54 44 54 60 00 00 00 02 9C 42 4F 43 48 53 20 //
>> GTDT`.BOCHS
>> 0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43 // BXPC
>> BXPC
>>-0020: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
>>
>>+0020: 01 00 00 00 FF FF FF FF FF FF FF FF 00 00 00 00 //
>>
>> 0030: 1D 00 00 00 00 00 00 00 1E 00 00 00 04 00 00 00 //
>>
>> 0040: 1B 00 00 00 00 00 00 00 1A 00 00 00 00 00 00 00 //
>>
>>-0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
>>
>>+0050: FF FF FF FF FF FF FF FF 00 00 00 00 00 00 00 00 //
>>
>>
>> Signed-off-by: Miguel Luis
>
> Acked-by: Ani Sinha
>
Thank you for the tags Ani. I’ve collected them and will spin v2 soon.
Thanks,
Miguel
>> ---
>> tests/data/acpi/virt/GTDT | Bin 96 -> 96 bytes
>> tests/data/acpi/virt/GTDT.memhp | Bin 96 -> 96 bytes
>> tests/data/acpi/virt/GTDT.numamem | Bin 96 -> 96 bytes
>> tests/qtest/bios-tables-test-allowed-diff.h | 3 ---
>> 4 files changed, 3 deletions(-)
>>
>> diff --git a/tests/data/acpi/virt/GTDT b/tests/data/acpi/virt/GTDT
>> index
>> 9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2
>> 100644
>> GIT binary patch
>> delta 45
>> kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*
>>
>> delta 45
>> jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8
>>
>> diff --git a/tests/data/acpi/virt/GTDT.memhp
>> b/tests/data/acpi/virt/GTDT.memhp
>> index
>> 9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2
>> 100644
>> GIT binary patch
>> delta 45
>> kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*
>>
>> delta 45
>> jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8
>>
>> diff --git a/tests/data/acpi/virt/GTDT.numamem
>> b/tests/data/acpi/virt/GTDT.numamem
>> index
>> 9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2
>> 100644
>> GIT binary patch
>> delta 45
>> kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*
>>
>> delta 45
>> jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8
>>
>> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h
>> b/tests/qtest/bios-tables-test-allowed-diff.h
>> index 957bd1b4f6..dfb8523c8b 100644
>> --- a/tests/qtest/bios-tables-test-allowed-diff.h
>> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
>> @@ -1,4 +1 @@
>> /* List of comma-separated changed AML files to ignore */
>> -"tests/data/acpi/virt/GTDT",
>> -"tests/data/acpi/virt/GTDT.memhp",
>> -"tests/data/acpi/virt/GTDT.numamem",
>> --
>> 2.36.0
>>
>>
Re: [External] : Re: [RFC PATCH 2/4] acpi: fadt: support revision 6.0 of the ACPI specification
Hi Ani,
> On 7 Oct 2022, at 04:25, Ani Sinha wrote:
>
>
>
> On Thu, 6 Oct 2022, Miguel Luis wrote:
>
>> Update the Fixed ACPI Description Table (FADT) to revision 6.0 of the ACPI
>> specification adding the field "Hypervisor Vendor Identity" that was missing.
>>
>> This field's description states the following: "64-bit identifier of
>> hypervisor
>> vendor. All bytes in this field are considered part of the vendor identity.
>> These identifiers are defined independently by the vendors themselves,
>> usually following the name of the hypervisor product. Version information
>> should NOT be included in this field - this shall simply denote the vendor's
>> name or identifier. Version information can be communicated through a
>> supplemental vendor-specific hypervisor API. Firmware implementers would
>> place zero bytes into this field, denoting that no hypervisor is present in
>> the actual firmware."
>>
>> Hereupon, what should a valid identifier of an Hypervisor Vendor ID be and
>> where should QEMU provide that information?
>>
>> On this RFC there's the suggestion of having this information in sync by the
>> current acceleration name. This also seems to imply that QEMU, which
>> generates
>> the FADT table, and the FADT consumer need to be in sync with the values of
>> this
>> field.
>>
>> Signed-off-by: Miguel Luis
>> ---
>> hw/acpi/aml-build.c | 14 +++---
>> hw/arm/virt-acpi-build.c | 10 +-
>> 2 files changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index e6bfac95c7..5258c4ac64 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -31,6 +31,7 @@
>> #include "hw/pci/pci_bus.h"
>> #include "hw/pci/pci_bridge.h"
>> #include "qemu/cutils.h"
>> +#include "qemu/accel.h"
>>
>> static GArray *build_alloc_array(void)
>> {
>> @@ -2070,7 +2071,7 @@ void build_pptt(GArray *table_data, BIOSLinker
>> *linker, MachineState *ms,
>> acpi_table_end(linker, );
>> }
>>
>> -/* build rev1/rev3/rev5.1 FADT */
>> +/* build rev1/rev3/rev5.1/rev6.0 FADT */
>> void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>> const char *oem_id, const char *oem_table_id)
>> {
>> @@ -2193,8 +2194,15 @@ void build_fadt(GArray *tbl, BIOSLinker *linker,
>> const AcpiFadtData *f,
>> /* SLEEP_STATUS_REG */
>> build_append_gas_from_struct(tbl, >sleep_sts);
>>
>> -/* TODO: extra fields need to be added to support revisions above rev5
>> */
>> -assert(f->rev == 5);
>> +if (f->rev <= 5) {
>
> <= does not make sense. It should be f->rev == 5.
> The previous code compares f->rev <= 4 since it needs to cover revisions
> 2, 3 and 4.
>
Indeed, that’s right. I will fix.
>> +goto done;
>> +}
>> +
>> +/* Hypervisor Vendor Identity */
>> +build_append_padded_str(tbl, current_accel_name(), 8, '\0');
>
> I do not think the vendor identity should change based on the accelerator.
> The accelerator QEMU uses should be hidden from the guest OS as far as
> possible. I would suggest a string like "QEMU" for vendor ID.
>
Thank you for the suggestion Ani. I will spin the next version with it.
Thanks,
Miguel
>> +
>> +/* TODO: extra fields need to be added to support revisions above rev6
>> */
>> +assert(f->rev == 6);
>>
>> done:
>> acpi_table_end(linker, );
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 9b3aee01bf..72bb6f61a5 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -809,13 +809,13 @@ build_madt(GArray *table_data, BIOSLinker *linker,
>> VirtMachineState *vms)
>> }
>>
>> /* FADT */
>> -static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
>> +static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker,
>> VirtMachineState *vms, unsigned dsdt_tbl_offset)
>> {
>> -/* ACPI v5.1 */
>> +/* ACPI v6.0 */
>> AcpiFadtData fadt = {
>> -.rev = 5,
>> -.minor_ver = 1,
>> +.rev = 6,
>> +.minor_ver = 0,
>> .flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
>> .xdsdt_tbl_offset = _tbl_offset,
>> };
>> @@ -945,7 +945,7 @@ void virt_acpi_build(VirtMachineState *vms,
>> AcpiBuildTables *tables)
>>
>> /* FADT MADT PPTT GTDT MCFG SPCR DBG2 pointed to by RSDT */
>> acpi_add_table(table_offsets, tables_blob);
>> -build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
>> +build_fadt_rev6(tables_blob, tables->linker, vms, dsdt);
>>
>> acpi_add_table(table_offsets, tables_blob);
>> build_madt(tables_blob, tables->linker, vms);
>> --
>> 2.37.3
>>
>>
[RFC PATCH 4/4] Step 6 & 7 of the bios-tables-test.c documented procedure.
Address :
-/ ACPI table terminates in the middle of a data structure! (dump table) */
+[10Ch 0268 8]Hypervisor ID : 00676374
-Raw Table Data: Length 268 (0x10C)
+Raw Table Data: Length 276 (0x114)
-: 46 41 43 50 0C 01 00 00 05 55 42 4F 43 48 53 20 // FACP.UBOCHS
+: 46 41 43 50 14 01 00 00 06 0F 42 4F 43 48 53 20 // FACP..BOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43 // BXPCBXPC
0020: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
@@ -185,7 +185,7 @@ Raw Table Data: Length 268 (0x10C)
0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0070: 00 00 10 00 00 00 00 00 00 00 00 00 00 00 00 00 //
-0080: 00 03 00 01 00 00 00 00 00 00 00 00 00 00 00 00 //
+0080: 00 03 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
0090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00A0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00B0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
@@ -193,4 +193,5 @@ Raw Table Data: Length 268 (0x10C)
00D0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00E0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
00F0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
-0100: 00 00 00 00 00 00 00 00 00 00 00 00 //
+0100: 00 00 00 00 00 00 00 00 00 00 00 00 74 63 67 00 // tcg.
+0110: 00 00 00 00 //
Signed-off-by: Miguel Luis
---
tests/data/acpi/virt/APIC | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.memhp | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.numamem | Bin 168 -> 172 bytes
tests/data/acpi/virt/FACP | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.memhp | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.numamem | Bin 268 -> 276 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 6 --
7 files changed, 6 deletions(-)
diff --git a/tests/data/acpi/virt/APIC b/tests/data/acpi/virt/APIC
index
023f15f12e74fb9a3a6d3d9dc994541016947d6a..179d274770a23209b949c90a929525e22368568b
100644
GIT binary patch
delta 26
hcmZ3%xQ3C-F~HM#4FdxMi~B?_YsP?yZeA06WB^*d2KE2|
delta 26
icmZ3(xPp<(F~HM#1p@;EbHGF{Yet`mZeA0oNB{s@&<6Sd
diff --git a/tests/data/acpi/virt/APIC.memhp b/tests/data/acpi/virt/APIC.memhp
index
023f15f12e74fb9a3a6d3d9dc994541016947d6a..179d274770a23209b949c90a929525e22368568b
100644
GIT binary patch
delta 26
hcmZ3%xQ3C-F~HM#4FdxMi~B?_YsP?yZeA06WB^*d2KE2|
delta 26
icmZ3(xPp<(F~HM#1p@;EbHGF{Yet`mZeA0oNB{s@&<6Sd
diff --git a/tests/data/acpi/virt/APIC.numamem
b/tests/data/acpi/virt/APIC.numamem
index
023f15f12e74fb9a3a6d3d9dc994541016947d6a..179d274770a23209b949c90a929525e22368568b
100644
GIT binary patch
delta 26
hcmZ3%xQ3C-F~HM#4FdxMi~B?_YsP?yZeA06WB^*d2KE2|
delta 26
icmZ3(xPp<(F~HM#1p@;EbHGF{Yet`mZeA0oNB{s@&<6Sd
diff --git a/tests/data/acpi/virt/FACP b/tests/data/acpi/virt/FACP
index
1f764220f8533c427168e80ccf298604826a00b4..bd514078decb368815482e5d1db2faf5371fc48e
100644
GIT binary patch
delta 33
mcmeBSn!?28=I9(C!pOkD#y^p(a^j?_i3a=}CCTXwAOHY?_6Iru
delta 26
hcmbQj)WgK(=I9*2!^ptE8ak1yl96%Z#OjF#yZ}u&1~C8t
diff --git a/tests/data/acpi/virt/FACP.memhp b/tests/data/acpi/virt/FACP.memhp
index
1f764220f8533c427168e80ccf298604826a00b4..bd514078decb368815482e5d1db2faf5371fc48e
100644
GIT binary patch
delta 33
mcmeBSn!?28=I9(C!pOkD#y^p(a^j?_i3a=}CCTXwAOHY?_6Iru
delta 26
hcmbQj)WgK(=I9*2!^ptE8ak1yl96%Z#OjF#yZ}u&1~C8t
diff --git a/tests/data/acpi/virt/FACP.numamem
b/tests/data/acpi/virt/FACP.numamem
index
1f764220f8533c427168e80ccf298604826a00b4..bd514078decb368815482e5d1db2faf5371fc48e
100644
GIT binary patch
delta 33
mcmeBSn!?28=I9(C!pOkD#y^p(a^j?_i3a=}CCTXwAOHY?_6Iru
delta 26
hcmbQj)WgK(=I9*2!^ptE8ak1yl96%Z#OjF#yZ}u&1~C8t
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h
b/tests/qtest/bios-tables-test-allowed-diff.h
index 8dc50f7a8a..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,7 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/FACP",
-"tests/data/acpi/virt/FACP.numamem",
-"tests/data/acpi/virt/FACP.memhp",
-"tests/data/acpi/virt/APIC",
-"tests/data/acpi/virt/APIC.memhp",
-"tests/data/acpi/virt/APIC.numamem",
--
2.37.3
[RFC PATCH 3/4] acpi: arm/virt: madt: bump to revision 4 accordingly to ACPI 6.0 Errata A
MADT has been updated with the GIC Structures from ACPI 6.0 Errata A
and so MADT revision and GICC Structure must be updated also.
Fixes: 37f33084ed2e ("acpi: arm/virt: madt: use build_append_int_noprefix() API
to compose MADT table")
Signed-off-by: Miguel Luis
---
hw/arm/virt-acpi-build.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 72bb6f61a5..2d21e3cec4 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -686,7 +686,7 @@ build_dbg2(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
};
/*
- * ACPI spec, Revision 5.1 Errata A
+ * ACPI spec, Revision 6.0 Errata A
* 5.2.12 Multiple APIC Description Table (MADT)
*/
static void build_append_gicr(GArray *table_data, uint64_t base, uint32_t size)
@@ -705,7 +705,7 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
int i;
VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
const MemMapEntry *memmap = vms->memmap;
-AcpiTable table = { .sig = "APIC", .rev = 3, .oem_id = vms->oem_id,
+AcpiTable table = { .sig = "APIC", .rev = 4, .oem_id = vms->oem_id,
.oem_table_id = vms->oem_table_id };
acpi_table_begin(, table_data);
@@ -740,7 +740,7 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
/* 5.2.12.14 GIC Structure */
build_append_int_noprefix(table_data, 0xB, 1); /* Type */
-build_append_int_noprefix(table_data, 76, 1); /* Length */
+build_append_int_noprefix(table_data, 80, 1); /* Length */
build_append_int_noprefix(table_data, 0, 2);/* Reserved */
build_append_int_noprefix(table_data, i, 4);/* GIC ID */
build_append_int_noprefix(table_data, i, 4);/* ACPI Processor UID
*/
@@ -760,6 +760,10 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
build_append_int_noprefix(table_data, 0, 8);/* GICR Base Address*/
/* MPIDR */
build_append_int_noprefix(table_data, armcpu->mp_affinity, 8);
+/* Processor Power Efficiency Class */
+build_append_int_noprefix(table_data, 0, 1);
+/* Reserved */
+build_append_int_noprefix(table_data, 0, 3);
}
if (vms->gic_version != VIRT_GIC_VERSION_2) {
@@ -771,12 +775,6 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
}
if (its_class_name() && !vmc->no_its) {
-/*
- * FIXME: Structure is from Revision 6.0 where 'GIC Structure'
- * has additional fields on top of implemented 5.1 Errata A,
- * to make it consistent with v6.0 we need to bump everything
- * to v6.0
- */
/*
* ACPI spec, Revision 6.0 Errata A
* (original 6.0 definition has invalid Length)
--
2.37.3
[RFC PATCH 0/4] ACPI MADT and FADT update according to the ACPI 6.0 spec
The MADT table structure has been updated in commit 37f33084ed2e
("acpi: arm/virt: madt: use build_append_int_noprefix() API to compose MADT
table")
to include the 5.2.12.18 GIC ITS Structure and so table's revision also needs to
be updated. MADT and the FADT tables from the same spec need to be in sync and
in
this case also the FADT needs to be updated.
Revision 6.0 of the ACPI FADT table introduces the field "Hypervisor Vendor
Identity" which is missing and must be included. Patch 2/4 includes a suggestion
for the value of this field by using the current acceleration name. This would
provide values like 'KVM' for example when KVM is used.
Ref: https://uefi.org/sites/default/files/resources/ACPI_6_0_Errata_A.PDF
Open to discussion, your comments, thoughts and suggestions are very welcome.
Thanks in advance.
Miguel
Miguel Luis (4):
tests/acpi: virt: allow acpi MADT and FADT changes
acpi: fadt: support revision 6.0 of the ACPI specification
acpi: arm/virt: madt: bump to revision 4 accordingly to ACPI 6.0
Errata A
Step 6 & 7 of the bios-tables-test.c documented procedure.
hw/acpi/aml-build.c | 14 +++---
hw/arm/virt-acpi-build.c | 26 --
tests/data/acpi/virt/APIC | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.memhp | Bin 168 -> 172 bytes
tests/data/acpi/virt/APIC.numamem | Bin 168 -> 172 bytes
tests/data/acpi/virt/FACP | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.memhp | Bin 268 -> 276 bytes
tests/data/acpi/virt/FACP.numamem | Bin 268 -> 276 bytes
8 files changed, 23 insertions(+), 17 deletions(-)
--
2.37.3
[RFC PATCH 2/4] acpi: fadt: support revision 6.0 of the ACPI specification
Update the Fixed ACPI Description Table (FADT) to revision 6.0 of the ACPI
specification adding the field "Hypervisor Vendor Identity" that was missing.
This field's description states the following: "64-bit identifier of hypervisor
vendor. All bytes in this field are considered part of the vendor identity.
These identifiers are defined independently by the vendors themselves,
usually following the name of the hypervisor product. Version information
should NOT be included in this field - this shall simply denote the vendor's
name or identifier. Version information can be communicated through a
supplemental vendor-specific hypervisor API. Firmware implementers would
place zero bytes into this field, denoting that no hypervisor is present in
the actual firmware."
Hereupon, what should a valid identifier of an Hypervisor Vendor ID be and
where should QEMU provide that information?
On this RFC there's the suggestion of having this information in sync by the
current acceleration name. This also seems to imply that QEMU, which generates
the FADT table, and the FADT consumer need to be in sync with the values of this
field.
Signed-off-by: Miguel Luis
---
hw/acpi/aml-build.c | 14 +++---
hw/arm/virt-acpi-build.c | 10 +-
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index e6bfac95c7..5258c4ac64 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -31,6 +31,7 @@
#include "hw/pci/pci_bus.h"
#include "hw/pci/pci_bridge.h"
#include "qemu/cutils.h"
+#include "qemu/accel.h"
static GArray *build_alloc_array(void)
{
@@ -2070,7 +2071,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker,
MachineState *ms,
acpi_table_end(linker, );
}
-/* build rev1/rev3/rev5.1 FADT */
+/* build rev1/rev3/rev5.1/rev6.0 FADT */
void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
const char *oem_id, const char *oem_table_id)
{
@@ -2193,8 +2194,15 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const
AcpiFadtData *f,
/* SLEEP_STATUS_REG */
build_append_gas_from_struct(tbl, >sleep_sts);
-/* TODO: extra fields need to be added to support revisions above rev5 */
-assert(f->rev == 5);
+if (f->rev <= 5) {
+goto done;
+}
+
+/* Hypervisor Vendor Identity */
+build_append_padded_str(tbl, current_accel_name(), 8, '\0');
+
+/* TODO: extra fields need to be added to support revisions above rev6 */
+assert(f->rev == 6);
done:
acpi_table_end(linker, );
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9b3aee01bf..72bb6f61a5 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -809,13 +809,13 @@ build_madt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
}
/* FADT */
-static void build_fadt_rev5(GArray *table_data, BIOSLinker *linker,
+static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms, unsigned dsdt_tbl_offset)
{
-/* ACPI v5.1 */
+/* ACPI v6.0 */
AcpiFadtData fadt = {
-.rev = 5,
-.minor_ver = 1,
+.rev = 6,
+.minor_ver = 0,
.flags = 1 << ACPI_FADT_F_HW_REDUCED_ACPI,
.xdsdt_tbl_offset = _tbl_offset,
};
@@ -945,7 +945,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables
*tables)
/* FADT MADT PPTT GTDT MCFG SPCR DBG2 pointed to by RSDT */
acpi_add_table(table_offsets, tables_blob);
-build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
+build_fadt_rev6(tables_blob, tables->linker, vms, dsdt);
acpi_add_table(table_offsets, tables_blob);
build_madt(tables_blob, tables->linker, vms);
--
2.37.3
[RFC PATCH 1/4] tests/acpi: virt: allow acpi MADT and FADT changes
Step 3 from bios-tables-test.c documented procedure. Signed-off-by: Miguel Luis --- tests/qtest/bios-tables-test-allowed-diff.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..8dc50f7a8a 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,7 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/FACP", +"tests/data/acpi/virt/FACP.numamem", +"tests/data/acpi/virt/FACP.memhp", +"tests/data/acpi/virt/APIC", +"tests/data/acpi/virt/APIC.memhp", +"tests/data/acpi/virt/APIC.numamem", -- 2.37.3
[PATCH 1/3] tests/acpi: virt: allow acpi GTDT changes
Step 3 from bios-tables-test.c documented procedure. Signed-off-by: Miguel Luis --- tests/qtest/bios-tables-test-allowed-diff.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..957bd1b4f6 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,4 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/GTDT", +"tests/data/acpi/virt/GTDT.memhp", +"tests/data/acpi/virt/GTDT.numamem", -- 2.36.0
[PATCH 3/3] tests/acpi: virt: update ACPI GTDT binaries
Step 6 & 7 of the bios-tables-test.c documented procedure.
Differences between disassembled ASL files for GTDT:
@@ -13,14 +13,14 @@
[000h 4]Signature : "GTDT"[Generic Timer
Description Table]
[004h 0004 4] Table Length : 0060
[008h 0008 1] Revision : 02
-[009h 0009 1] Checksum : 8C
+[009h 0009 1] Checksum : 9C
[00Ah 0010 6] Oem ID : "BOCHS "
[010h 0016 8] Oem Table ID : "BXPC"
[018h 0024 4] Oem Revision : 0001
[01Ch 0028 4] Asl Compiler ID : "BXPC"
[020h 0032 4]Asl Compiler Revision : 0001
-[024h 0036 8]Counter Block Address :
+[024h 0036 8]Counter Block Address :
[02Ch 0044 4] Reserved :
[030h 0048 4] Secure EL1 Interrupt : 001D
@@ -46,16 +46,16 @@
Trigger Mode : 0
Polarity : 0
Always On : 0
-[050h 0080 8] Counter Read Block Address :
+[050h 0080 8] Counter Read Block Address :
[058h 0088 4] Platform Timer Count :
[05Ch 0092 4]Platform Timer Offset :
Raw Table Data: Length 96 (0x60)
-: 47 54 44 54 60 00 00 00 02 8C 42 4F 43 48 53 20 //
GTDT`.BOCHS
+: 47 54 44 54 60 00 00 00 02 9C 42 4F 43 48 53 20 //
GTDT`.BOCHS
0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43 // BXPC
BXPC
-0020: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
+0020: 01 00 00 00 FF FF FF FF FF FF FF FF 00 00 00 00 //
0030: 1D 00 00 00 00 00 00 00 1E 00 00 00 04 00 00 00 //
0040: 1B 00 00 00 00 00 00 00 1A 00 00 00 00 00 00 00 //
-0050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 //
+0050: FF FF FF FF FF FF FF FF 00 00 00 00 00 00 00 00 //
Signed-off-by: Miguel Luis
---
tests/data/acpi/virt/GTDT | Bin 96 -> 96 bytes
tests/data/acpi/virt/GTDT.memhp | Bin 96 -> 96 bytes
tests/data/acpi/virt/GTDT.numamem | Bin 96 -> 96 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 3 ---
4 files changed, 3 deletions(-)
diff --git a/tests/data/acpi/virt/GTDT b/tests/data/acpi/virt/GTDT
index
9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2
100644
GIT binary patch
delta 45
kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*
delta 45
jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8
diff --git a/tests/data/acpi/virt/GTDT.memhp b/tests/data/acpi/virt/GTDT.memhp
index
9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2
100644
GIT binary patch
delta 45
kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*
delta 45
jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8
diff --git a/tests/data/acpi/virt/GTDT.numamem
b/tests/data/acpi/virt/GTDT.numamem
index
9408b71b59c0e0f2991c0053562280155b47bc0b..6f8cb9b8f30b55f4c93fe515982621e3db50feb2
100644
GIT binary patch
delta 45
kcmYdD;BpUf2}xjJU|^avkxPo>KNL*VQ4xT#fs$YV0LH=;ng9R*
delta 45
jcmYdD;BpUf2}xjJU|{N*$R))AWPrg$9Tfo>8%6^Foy!E8
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h
b/tests/qtest/bios-tables-test-allowed-diff.h
index 957bd1b4f6..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,4 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/virt/GTDT",
-"tests/data/acpi/virt/GTDT.memhp",
-"tests/data/acpi/virt/GTDT.numamem",
--
2.36.0
[PATCH 2/3] acpi: arm/virt: build_gtdt: fix invalid 64-bit physical addresses
Per the ACPI 6.5 specification, on the GTDT Table Structure, the Counter Control
Block Address and Counter Read Block Address fields of the GTDT table should be
set to 0x if not provided, rather than 0x0.
Fixes: 41041e57085 ("acpi: arm/virt: build_gtdt: use
acpi_table_begin()/acpi_table_end() instead of build_header()")
Signed-off-by: Miguel Luis
---
hw/arm/virt-acpi-build.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 9b3aee01bf..13c6e3e468 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -592,8 +592,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
acpi_table_begin(, table_data);
/* CntControlBase Physical Address */
-/* FIXME: invalid value, should be 0x if not impl. ? */
-build_append_int_noprefix(table_data, 0, 8);
+build_append_int_noprefix(table_data, 0x, 8);
build_append_int_noprefix(table_data, 0, 4); /* Reserved */
/*
* FIXME: clarify comment:
@@ -618,7 +617,7 @@ build_gtdt(GArray *table_data, BIOSLinker *linker,
VirtMachineState *vms)
/* Non-Secure EL2 timer Flags */
build_append_int_noprefix(table_data, irqflags, 4);
/* CntReadBase Physical address */
-build_append_int_noprefix(table_data, 0, 8);
+build_append_int_noprefix(table_data, 0x, 8);
/* Platform Timer Count */
build_append_int_noprefix(table_data, 0, 4);
/* Platform Timer Offset */
--
2.36.0
[PATCH 0/3] fix for two ACPI GTDT physical addresses
The ACPI GTDT table contains two invalid 64-bit physical addresses according to the ACPI spec. 6.5 [1]. Those are the Counter Control Base physical address and the Counter Read Base physical address. Those fields of the GTDT table should be set to 0x if not provided, rather than 0x0. [1]: https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html#gtdt-table-structure Miguel Luis (3): tests/acpi: virt: allow acpi GTDT changes acpi: arm/virt: build_gtdt: fix invalid 64-bit physical addresses tests/acpi: virt: update ACPI GTDT binaries hw/arm/virt-acpi-build.c | 5 ++--- tests/data/acpi/virt/GTDT | Bin 96 -> 96 bytes tests/data/acpi/virt/GTDT.memhp | Bin 96 -> 96 bytes tests/data/acpi/virt/GTDT.numamem | Bin 96 -> 96 bytes 4 files changed, 2 insertions(+), 3 deletions(-) -- 2.36.0
