Re: [PATCH 1/2] docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386
On 27/02/2023 21.25, Richard Henderson wrote: On 2/27/23 01:50, Daniel P. Berrangé wrote: On Mon, Feb 27, 2023 at 12:10:49PM +0100, Thomas Huth wrote: Hardly anybody still uses 32-bit x86 hosts today, so we should start deprecating them to finally have less test efforts. With regards to 32-bit KVM support in the x86 Linux kernel, the developers confirmed that they do not need a recent qemu-system-i386 binary here: https://lore.kernel.org/kvm/y%2ffkts5ajfy0h...@google.com/ Signed-off-by: Thomas Huth --- docs/about/deprecated.rst | 13 + 1 file changed, 13 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 15084f7bea..98517f5187 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -196,6 +196,19 @@ CI coverage support may bitrot away before the deprecation process completes. The little endian variants of MIPS (both 32 and 64 bit) are still a supported host architecture. +32-bit x86 hosts and ``qemu-system-i386`` (since 8.0) +' + +Testing 32-bit x86 host OS support takes a lot of precious time during the +QEMU contiguous integration tests, and considering that most OS vendors +stopped shipping 32-bit variants of their x86 OS distributions and most +x86 hardware from the past >10 years is capable of 64-bit, keeping the +32-bit support alive is an inadequate burden for the QEMU project. Thus +QEMU will soon drop the support for 32-bit x86 host systems and the +``qemu-system-i386`` binary. Use ``qemu-system-x86_64`` (which is a proper +superset of ``qemu-system-i386``) on a 64-bit host machine instead. I feel like we should have separate deprecation entries for the i686 host support, and for qemu-system-i386 emulator binary, as although they're related they are independant features with differing impact. Agreed. OK, fair, I'll rework my patch according to your suggestion, Daniel. 32-bit x86 hosts Support for 32-bit x86 host deployments is increasingly uncommon in mainstream Linux distributions given the widespread availability of 64-bit x86 hardware. The QEMU project no longer considers 32-bit x86 support to be an effective use of its limited resources, and thus intends to discontinue it. Current users of QEMU on 32-bit x86 hosts should either continue using existing releases of QEMU, with the caveat that they will no longer get security fixes, or migrate to a 64-bit platform which remains capable of running 32-bit guests if needed. Ack. ``qemu-system-i386`` binary removal ''' The ``qemu-system-x86_64`` binary can be used to run 32-bit guests by selecting a 32-bit CPU model, including KVM support on x86_64 hosts. Once support for the 32-bit x86 host platform is discontinued, the ``qemu-system-i386`` binary will be redundant. Missing "kvm" in this last sentence? It is otherwise untrue for tcg. I assume that Daniel only thought of 32-bit x86 hosts here, but indeed, it's untrue for non-x86 32-bit hosts. So this really should refer to KVM on 32-bit x86 hosts instead. I'll rephrase it in v2. Thomas
Re: [PATCH 1/2] docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386
On 27/02/2023 21.12, Michael S. Tsirkin wrote: On Mon, Feb 27, 2023 at 11:50:07AM +, Daniel P. Berrangé wrote: I feel like we should have separate deprecation entries for the i686 host support, and for qemu-system-i386 emulator binary, as although they're related they are independant features with differing impact. eg removing qemu-system-i386 affects all host architectures, not merely 32-bit x86 host, so I think we can explain the impact more clearly if we separate them. Removing qemu-system-i386 seems ok to me - I think qemu-system-x86_64 is a superset. Removing support for building on 32 bit systems seems like a pity - it's one of a small number of ways to run 64 bit binaries on 32 bit systems, and the maintainance overhead is quite small. Note: We're talking about 32-bit *x86* hosts here. Do you really think that someone is still using QEMU usermode emulation to run 64-bit binaries on a 32-bit x86 host?? ... If so, I'd be very surprised! In fact, keeping this support around forces correct use of posix APIs such as e.g. PRIx64 which makes the code base more future-proof. If you're concerned about PRIx64 and friends: We still continue to do compile testing with 32-bit MIPS cross-compilers and Windows 32-bit cross-compilers for now. The only thing we'd lose is the 32-bit "make check" run in the CI. Thomas
[PATCH v2 2/2] hw/riscv: Move the dtb load bits outside of create_fdt()
Move the dtb load bits outside of create_fdt(), and put it explicitly
in sifive_u_machine_init() and virt_machine_init(). With such change
create_fdt() does exactly what its function name tells us.
Suggested-by: Daniel Henrique Barboza
Signed-off-by: Bin Meng
---
Changes in v2:
- new patch: Move the dtb load bits outside of create_fdt()
include/hw/riscv/sifive_u.h | 1 +
hw/riscv/sifive_u.c | 31 +++
hw/riscv/virt.c | 29 ++---
3 files changed, 30 insertions(+), 31 deletions(-)
diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 65af306963..0696f85942 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -68,6 +68,7 @@ typedef struct SiFiveUState {
/*< public >*/
SiFiveUSoCState soc;
+int fdt_size;
bool start_in_flash;
uint32_t msel;
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 76db5ed3dd..35a335b8d0 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -99,7 +99,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
MachineState *ms = MACHINE(s);
uint64_t mem_size = ms->ram_size;
void *fdt;
-int cpu, fdt_size;
+int cpu;
uint32_t *cells;
char *nodename;
uint32_t plic_phandle, prci_phandle, gpio_phandle, phandle = 1;
@@ -112,19 +112,10 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
"sifive,plic-1.0.0", "riscv,plic0"
};
-if (ms->dtb) {
-fdt = ms->fdt = load_device_tree(ms->dtb, &fdt_size);
-if (!fdt) {
-error_report("load_device_tree() failed");
-exit(1);
-}
-return;
-} else {
-fdt = ms->fdt = create_device_tree(&fdt_size);
-if (!fdt) {
-error_report("create_device_tree() failed");
-exit(1);
-}
+fdt = ms->fdt = create_device_tree(&s->fdt_size);
+if (!fdt) {
+error_report("create_device_tree() failed");
+exit(1);
}
qemu_fdt_setprop_string(fdt, "/", "model", "SiFive HiFive Unleashed A00");
@@ -561,8 +552,16 @@ static void sifive_u_machine_init(MachineState *machine)
qdev_connect_gpio_out(DEVICE(&(s->soc.gpio)), 10,
qemu_allocate_irq(sifive_u_machine_reset, NULL, 0));
-/* create device tree */
-create_fdt(s, memmap, riscv_is_32bit(&s->soc.u_cpus));
+/* load/create device tree */
+if (machine->dtb) {
+machine->fdt = load_device_tree(machine->dtb, &s->fdt_size);
+if (!machine->fdt) {
+error_report("load_device_tree() failed");
+exit(1);
+}
+} else {
+create_fdt(s, memmap, riscv_is_32bit(&s->soc.u_cpus));
+}
if (s->start_in_flash) {
/*
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 0c7b4a1e46..53ed2e8369 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1008,19 +1008,10 @@ static void create_fdt(RISCVVirtState *s, const
MemMapEntry *memmap)
uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
uint8_t rng_seed[32];
-if (ms->dtb) {
-ms->fdt = load_device_tree(ms->dtb, &s->fdt_size);
-if (!ms->fdt) {
-error_report("load_device_tree() failed");
-exit(1);
-}
-return;
-} else {
-ms->fdt = create_device_tree(&s->fdt_size);
-if (!ms->fdt) {
-error_report("create_device_tree() failed");
-exit(1);
-}
+ms->fdt = create_device_tree(&s->fdt_size);
+if (!ms->fdt) {
+error_report("create_device_tree() failed");
+exit(1);
}
qemu_fdt_setprop_string(ms->fdt, "/", "model", "riscv-virtio,qemu");
@@ -1505,8 +1496,16 @@ static void virt_machine_init(MachineState *machine)
}
virt_flash_map(s, system_memory);
-/* create device tree */
-create_fdt(s, memmap);
+/* load/create device tree */
+if (machine->dtb) {
+machine->fdt = load_device_tree(machine->dtb, &s->fdt_size);
+if (!machine->fdt) {
+error_report("load_device_tree() failed");
+exit(1);
+}
+} else {
+create_fdt(s, memmap);
+}
s->machine_done.notify = virt_machine_done;
qemu_add_machine_init_done_notifier(&s->machine_done);
--
2.25.1
[PATCH v2 1/2] hw/riscv: Skip re-generating DT nodes for a given DTB
Launch qemu-system-riscv64 with a given dtb for 'sifive_u' and 'virt'
machines, QEMU complains:
qemu_fdt_add_subnode: Failed to create subnode /soc: FDT_ERR_EXISTS
The whole DT generation logic should be skipped when a given DTB is
present.
Fixes: b1f19f238cae ("hw/riscv: write bootargs 'chosen' FDT after
riscv_load_kernel()")
Signed-off-by: Bin Meng
Reviewed-by: Daniel Henrique Barboza
---
(no changes since v1)
hw/riscv/sifive_u.c | 1 +
hw/riscv/virt.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index ad3bb35b34..76db5ed3dd 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -118,6 +118,7 @@ static void create_fdt(SiFiveUState *s, const MemMapEntry
*memmap,
error_report("load_device_tree() failed");
exit(1);
}
+return;
} else {
fdt = ms->fdt = create_device_tree(&fdt_size);
if (!fdt) {
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 86c4adc0c9..0c7b4a1e46 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -1014,6 +1014,7 @@ static void create_fdt(RISCVVirtState *s, const
MemMapEntry *memmap)
error_report("load_device_tree() failed");
exit(1);
}
+return;
} else {
ms->fdt = create_device_tree(&s->fdt_size);
if (!ms->fdt) {
--
2.25.1
Re: [RFC PATCH 10/43] target/loongarch: Implement vaddw/vsubw
Hi, Richard
在 2023/2/25 上午7:01, Richard Henderson 写道:
On 2/23/23 21:24, gaosong wrote:
{
.fniv = gen_vaddwev_s,
.fno = gen_helper_vaddwev_w_h,
.opt_opc = vecop_list,
.vece = MO_32
},
{
.fniv = gen_vaddwev_s,
.fno = gen_helper_vaddwev_d_w,
.opt_opc = vecop_list,
.vece = MO_64
},
Oh, these two can also include .fni4 and .fni8 integer versions,
respectively, for hosts without the proper vector support.
OK,
but fno is not for hosts without the proper vector support?
Thanks.
Song Gao
Re: [PATCH 1/2] docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386
On 27/02/2023 23.32, Philippe Mathieu-Daudé wrote: On 27/2/23 21:12, Michael S. Tsirkin wrote: On Mon, Feb 27, 2023 at 11:50:07AM +, Daniel P. Berrangé wrote: I feel like we should have separate deprecation entries for the i686 host support, and for qemu-system-i386 emulator binary, as although they're related they are independant features with differing impact. eg removing qemu-system-i386 affects all host architectures, not merely 32-bit x86 host, so I think we can explain the impact more clearly if we separate them. Removing qemu-system-i386 seems ok to me - I think qemu-system-x86_64 is a superset. Doesn't qemu-system-i386 start the CPU in a different mode that qemu-system-x86_64? Last time we discussed it, we mention adding -32 and -64 CLI flags to maintain compat, and IIRC this flag would add boot code to switch the CPU in 32-b. But then maybe I misunderstood. Thomas said, "CPUs must start in the same mode they start in HW". No, I think you misunderstood something here. x86 CPUs always start in 16-bit mode, as far as I know, and the firmware / OS then has to switch to 32-bit or 64-bit mode as desired. Thomas
Re: [PATCH 0/2] Deprecate support for 32-bit x86 and arm hosts
On 27/02/2023 19.38, Daniel P. Berrangé wrote: On Mon, Feb 27, 2023 at 12:10:48PM +0100, Thomas Huth wrote: We're struggling quite badly with our CI minutes on the shared gitlab runners, so we urgently need to think of ways to cut down our supported build and target environments. qemu-system-i386 and qemu-system-arm are not really required anymore, since nobody uses KVM on the corresponding systems for production anymore, and the -x86_64 and -arch64 variants are a proper superset of those binaries. So it's time to deprecate them and the corresponding 32-bit host environments now. This is a follow-up patch series from the previous discussion here: https://lore.kernel.org/qemu-devel/20230130114428.1297295-1-th...@redhat.com/ where people still mentioned that there is still interest in certain support for 32-bit host hardware. But as far as I could see, there is no real need for 32-bit host support for system emulation on x86 and arm anymore, so it should be fine if we drop these host environments now (these are also the two architectures that contribute the most to the long test times in our CI, so we would benefit a lot by dropping those). Your description here is a little ambiguous about what's being proposed. When you say dropping 32-bit host support do you mean just for the system emulator binaries, or for QEMU entirely ? Just for system emulation. Some people said that user emulation still might be useful for some 32-bit environments. And when the deprecation period is passed, are you proposing to actively prevent 32-bit builds, or merely stopping CI testing and leave 32-bit builds still working if people want them ? CI is the main pain point, so that's the most important thing. So whether we throw a warning or a hard error while configuring the build, I don't care too much. Thomas
Re: [PATCH V4 5/8] hw/riscv/virt: virt-acpi-build.c: Add RINTC in MADT
On Mon, Feb 27, 2023 at 04:41:21PM +0100, Igor Mammedov wrote:
> On Fri, 24 Feb 2023 19:56:58 +0530
> Sunil V L wrote:
>
> > Hi Igor,
> >
> > On Fri, Feb 24, 2023 at 01:53:43PM +0100, Igor Mammedov wrote:
> > > On Fri, 24 Feb 2023 14:06:58 +0530
> > > Sunil V L wrote:
> > >
> > > > Add Multiple APIC Description Table (MADT) with the
> > > > RINTC structure for each cpu.
> > > >
> > > > Signed-off-by: Sunil V L
> > > > Acked-by: Alistair Francis
> > > > Reviewed-by: Andrew Jones
> > > > ---
> > > > hw/riscv/virt-acpi-build.c | 44 ++
> > > > 1 file changed, 44 insertions(+)
> > > >
> > > > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
> > > > index 3a5e2e6d53..8b85b34c55 100644
> > > > --- a/hw/riscv/virt-acpi-build.c
> > > > +++ b/hw/riscv/virt-acpi-build.c
> > > > @@ -32,6 +32,7 @@
> > > > #include "sysemu/reset.h"
> > > > #include "migration/vmstate.h"
> > > > #include "hw/riscv/virt.h"
> > > > +#include "hw/riscv/numa.h"
> > > >
> > > > #define ACPI_BUILD_TABLE_SIZE 0x2
> > > >
> > > > @@ -132,6 +133,46 @@ static void build_dsdt(GArray *table_data,
> > > > free_aml_allocator();
> > > > }
> > > >
> > > > +/* MADT */
> > >
> > > see build_srat() how this comment must look like
> > >
> > Currently, even though ECRs are approved, the ACPI spec is not released
> > for these MADT structures. I can add the spec version for the generic
> > MADT but not for the RINTC. Same issue with a new table RHCT.
> > What is the recommendation in such case?
>
> ther must be some draft variant of spec or a ticket where it was approved
> or a reference impl. somewhere.
>
Sure, I can add the ticket ID. Thanks!
> >
> > > > +static void build_madt(GArray *table_data,
> > > > + BIOSLinker *linker,
> > > > + RISCVVirtState *s)
> > > > +{
> > > > +MachineState *ms = MACHINE(s);
> > > > +int socket;
> > > > +uint16_t base_hartid = 0;
> > > > +uint32_t cpu_id = 0;
> > > > +
> > > > +AcpiTable table = { .sig = "APIC", .rev = 6, .oem_id = s->oem_id,
> > > > +.oem_table_id = s->oem_table_id };
> > > > +
> > > > +acpi_table_begin(&table, table_data);
> > > > +/* Local Interrupt Controller Address */
> > > > +build_append_int_noprefix(table_data, 0, 4);
> > > > +build_append_int_noprefix(table_data, 0, 4); /* MADT Flags */
> > > > +
> > > > +/* RISC-V Local INTC structures per HART */
> > > > +for (socket = 0; socket < riscv_socket_count(ms); socket++) {
> > > > +base_hartid = riscv_socket_first_hartid(ms, socket);
> > > > +
> > > > +for (int i = 0; i < s->soc[socket].num_harts; i++) {
> > > > +build_append_int_noprefix(table_data, 0x18, 1);/* Type
> > > > */
> > > > +build_append_int_noprefix(table_data, 20, 1); /*
> > > > Length */
> > > > +build_append_int_noprefix(table_data, 1, 1); /*
> > > > Version */
> > > > +build_append_int_noprefix(table_data, 0, 1); /*
> > > > Reserved */
> > > > +build_append_int_noprefix(table_data, 1, 4); /*
> > > > Flags*/
> > > > +build_append_int_noprefix(table_data,
> > > > + (base_hartid + i), 8); /* Hart
> > > > ID */
> > > > +
> > > > +/* ACPI Processor UID */
> > > > +build_append_int_noprefix(table_data, cpu_id, 4);
> > >
> > > cpu_id here seems to be unrelated to one in DSDT.
> > > Could you explain how socket/hartid and cpu_id are related to each other?
> > >
> > cpu_id should match the _UID. I needed two loops here to get the
> > base_hartid of the socket. hart_id is the unique ID for each hart
> > similar to MPIDR / APIC ID. I understand your point. Let me make DSDT
> > also created using two loops so that both match.
>
> Why not reuse possible CPUs to describe topology there and then
> use ids from it to build ACPI tables instead of inventing your own
> cpu topo all over the place?
>
> PS: look for possible_cpus and how it's used (virt-arm already uses it
> although partially). And I'd like to avoid adding new ad-hoc ways
> to describe CPU topology is current possible_cpu ca do the job.
Okay, sure. Let me take a look at possible_cpus and use in cpu topology.
Thanks!
RE: [External] : Re: [PATCH 1/1] modules: load modules from /var/run/qemu/ directory firstly
Hi ,
Can I know when this patch will be integrated to upstream?
Thank you,
Siddhi Katage
-Original Message-
From: Philippe Mathieu-Daudé
Sent: Wednesday, January 25, 2023 5:52 AM
To: Siddhi Katage ; qemu-devel@nongnu.org
Cc: Joe Jin ; Dongli Zhang ;
christian.ehrha...@canonical.com; berra...@redhat.com; pbonz...@redhat.com
Subject: [External] : Re: [PATCH 1/1] modules: load modules from
/var/run/qemu/ directory firstly
On 24/1/23 19:39, Siddhi Katage wrote:
> From: Siddhi Katage
>
> An old running QEMU will try to load modules with new build-id first,
> this will fail as expected, then QEMU will fallback to load the old
> modules that
You corrected the comma/space typo :)
> matches its build-id from /var/run/qemu/ directory.
> Make /var/run/qemu/ directory as first search path to load modules.
>
> Fixes: bd83c861c0 ("modules: load modules from versioned /var/run
> dir")
> Signed-off-by: Siddhi Katage
> ---
> util/module.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v4 10/10] hw/cxl/mailbox: Use new UUID network order define for cel_uuid
On Mon, Feb 06, 2023 at 05:28:16PM +, Jonathan Cameron wrote:
> From: Ira Weiny
>
> The cel_uuid was programatically generated previously because there was
> no static initializer for network order UUIDs.
>
> Use the new network order initializer for cel_uuid. Adjust
> cxl_initialize_mailbox() because it can't fail now.
>
> Update specification reference.
>
> Reviewed-by: Philippe Mathieu-Daudé
> Reviewed-by: Gregory Price
> Tested-by: Gregory Price
> Signed-off-by: Ira Weiny
> Signed-off-by: Jonathan Cameron
>
Reviewed-by: Fan Ni
> ---
> v2:
> Make it const (Philippe)
> ---
> hw/cxl/cxl-device-utils.c | 2 +-
> hw/cxl/cxl-mailbox-utils.c | 13 ++---
> include/hw/cxl/cxl_device.h | 2 +-
> 3 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> index 83ce7a8270..4c5e88aaf5 100644
> --- a/hw/cxl/cxl-device-utils.c
> +++ b/hw/cxl/cxl-device-utils.c
> @@ -267,5 +267,5 @@ void cxl_device_register_init_common(CXLDeviceState
> *cxl_dstate)
> cxl_device_cap_init(cxl_dstate, MEMORY_DEVICE, 0x4000);
> memdev_reg_init_common(cxl_dstate);
>
> -assert(cxl_initialize_mailbox(cxl_dstate) == 0);
> +cxl_initialize_mailbox(cxl_dstate);
> }
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 3f67b665f5..206e04a4b8 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -193,7 +193,11 @@ static ret_code cmd_timestamp_set(struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> -static QemuUUID cel_uuid;
> +/* CXL 3.0 8.2.9.5.2.1 Command Effects Log (CEL) */
> +static const QemuUUID cel_uuid = {
> +.data = UUID(0x0da9c0b5, 0xbf41, 0x4b78, 0x8f, 0x79,
> + 0x96, 0xb1, 0x62, 0x3b, 0x3f, 0x17)
> +};
>
> /* 8.2.9.4.1 */
> static ret_code cmd_logs_get_supported(struct cxl_cmd *cmd,
> @@ -458,11 +462,8 @@ void cxl_process_mailbox(CXLDeviceState *cxl_dstate)
> DOORBELL, 0);
> }
>
> -int cxl_initialize_mailbox(CXLDeviceState *cxl_dstate)
> +void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate)
> {
> -/* CXL 2.0: Table 169 Get Supported Logs Log Entry */
> -const char *cel_uuidstr = "0da9c0b5-bf41-4b78-8f79-96b1623b3f17";
> -
> for (int set = 0; set < 256; set++) {
> for (int cmd = 0; cmd < 256; cmd++) {
> if (cxl_cmd_set[set][cmd].handler) {
> @@ -476,6 +477,4 @@ int cxl_initialize_mailbox(CXLDeviceState *cxl_dstate)
> }
> }
> }
> -
> -return qemu_uuid_parse(cel_uuidstr, &cel_uuid);
> }
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index 250adf18b2..7e5ad65c1d 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -170,7 +170,7 @@ CXL_DEVICE_CAPABILITY_HEADER_REGISTER(MEMORY_DEVICE,
>CXL_DEVICE_CAP_HDR1_OFFSET +
>CXL_DEVICE_CAP_REG_SIZE * 2)
>
> -int cxl_initialize_mailbox(CXLDeviceState *cxl_dstate);
> +void cxl_initialize_mailbox(CXLDeviceState *cxl_dstate);
> void cxl_process_mailbox(CXLDeviceState *cxl_dstate);
>
> #define cxl_device_cap_init(dstate, reg, cap_id) \
> --
> 2.37.2
>
>
Re: [PATCH v4 09/10] qemu/uuid: Add UUID static initializer
On Mon, Feb 06, 2023 at 05:28:15PM +, Jonathan Cameron wrote:
> From: Ira Weiny
>
> UUID's are defined as network byte order fields. No static initializer
> was available for UUID's in their standard big endian format.
>
> Define a big endian initializer for UUIDs.
>
> Reviewed-by: Gregory Price
> Tested-by: Gregory Price
> Signed-off-by: Ira Weiny
> Signed-off-by: Jonathan Cameron
Reviewed-by: Fan Ni
> ---
> include/qemu/uuid.h | 12
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/qemu/uuid.h b/include/qemu/uuid.h
> index 9925febfa5..dc40ee1fc9 100644
> --- a/include/qemu/uuid.h
> +++ b/include/qemu/uuid.h
> @@ -61,6 +61,18 @@ typedef struct {
> (clock_seq_hi_and_reserved), (clock_seq_low), (node0), (node1), (node2),\
> (node3), (node4), (node5) }
>
> +/* Normal (network byte order) UUID */
> +#define UUID(time_low, time_mid, time_hi_and_version,\
> + clock_seq_hi_and_reserved, clock_seq_low, node0, node1, node2, \
> + node3, node4, node5) \
> + { ((time_low) >> 24) & 0xff, ((time_low) >> 16) & 0xff,\
> +((time_low) >> 8) & 0xff, (time_low) & 0xff, \
> +((time_mid) >> 8) & 0xff, (time_mid) & 0xff, \
> +((time_hi_and_version) >> 8) & 0xff, (time_hi_and_version) & 0xff, \
> +(clock_seq_hi_and_reserved), (clock_seq_low),\
> +(node0), (node1), (node2), (node3), (node4), (node5) \
> + }
> +
> #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-" \
> "%02hhx%02hhx-%02hhx%02hhx-" \
> "%02hhx%02hhx-" \
> --
> 2.37.2
>
>
Re: [PATCH v4 08/10] qemu/bswap: Add const_le64()
On Mon, Feb 06, 2023 at 05:28:14PM +, Jonathan Cameron wrote:
> From: Ira Weiny
>
> Gcc requires constant versions of cpu_to_le* calls.
>
> Add a 64 bit version.
>
> Reviewed-by: Peter Maydell
> Reviewed-by: Philippe Mathieu-Daudé
> Reviewed-by: Gregory Price
> Tested-by: Gregory Price
> Signed-off-by: Ira Weiny
> Signed-off-by: Jonathan Cameron
>
Reviewed-by: Fan Ni
> ---
> v2: Update comment (Philippe)
> ---
> include/qemu/bswap.h | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h
> index 3cbe52246b..eb3bcf2520 100644
> --- a/include/qemu/bswap.h
> +++ b/include/qemu/bswap.h
> @@ -129,11 +129,20 @@ CPU_CONVERT(le, 32, uint32_t)
> CPU_CONVERT(le, 64, uint64_t)
>
> /*
> - * Same as cpu_to_le{16,32}, except that gcc will figure the result is
> + * Same as cpu_to_le{16,32,64}, except that gcc will figure the result is
> * a compile-time constant if you pass in a constant. So this can be
> * used to initialize static variables.
> */
> #if HOST_BIG_ENDIAN
> +# define const_le64(_x) \
> +_x) & 0x00ffU) << 56) | \
> + (((_x) & 0xff00U) << 40) | \
> + (((_x) & 0x00ffU) << 24) | \
> + (((_x) & 0xff00U) << 8) | \
> + (((_x) & 0x00ffU) >> 8) | \
> + (((_x) & 0xff00U) >> 24) | \
> + (((_x) & 0x00ffU) >> 40) | \
> + (((_x) & 0xff00U) >> 56))
> # define const_le32(_x) \
> _x) & 0x00ffU) << 24) | \
> (((_x) & 0xff00U) << 8) | \
> @@ -143,6 +152,7 @@ CPU_CONVERT(le, 64, uint64_t)
> _x) & 0x00ff) << 8) |\
> (((_x) & 0xff00) >> 8))
> #else
> +# define const_le64(_x) (_x)
> # define const_le32(_x) (_x)
> # define const_le16(_x) (_x)
> #endif
> --
> 2.37.2
>
>
Re: [PATCH v4 07/10] tests: acpi: Update q35/DSDT.cxl for removed duplicate UID
On Mon, Feb 06, 2023 at 05:28:13PM +, Jonathan Cameron wrote: > Dropping the ID effects this table in trivial fashion. > > Reviewed-by: Gregory Price > Tested-by: Gregory Price > Signed-off-by: Jonathan Cameron > --- Reviewed-by: Fan Ni > tests/data/acpi/q35/DSDT.cxl| Bin 9578 -> 9564 bytes > tests/qtest/bios-tables-test-allowed-diff.h | 1 - > 2 files changed, 1 deletion(-) > > diff --git a/tests/data/acpi/q35/DSDT.cxl b/tests/data/acpi/q35/DSDT.cxl > index > 3d18b9672d124a0cf11a79e92c396a1b883d0589..4586b9a18b24acd946cd32c7e3e3a70891a246d2 > 100644 > GIT binary patch > delta 65 > zcmaFmb;pa#CD IB*D!F0I~xVRsaA1 > > delta 79 > zcmccP^~#IOCDSUKwt0m6-Tor}*e5CzZ*UWUScYLp@!%?rjc`U&A SyId%Wytq76o(Cw;!v+8Y7Z@l2 > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h > b/tests/qtest/bios-tables-test-allowed-diff.h > index 9ce0f596cc..dfb8523c8b 100644 > --- a/tests/qtest/bios-tables-test-allowed-diff.h > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > @@ -1,2 +1 @@ > /* List of comma-separated changed AML files to ignore */ > -"tests/data/acpi/q35/DSDT.cxl", > -- > 2.37.2 > >
Re: [PATCH v4 06/10] hw/i386/acpi: Drop duplicate _UID entry for CXL root bridge
On Mon, Feb 06, 2023 at 05:28:12PM +, Jonathan Cameron wrote:
> Noticed as this prevents iASL disasembling the DSDT table.
>
> Reviewed-by: Ira Weiny
> Reviewed-by: Gregory Price
> Tested-by: Gregory Price
> Signed-off-by: Jonathan Cameron
> ---
Reviewed-by: Fan Ni
> hw/i386/acpi-build.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 145389aa58..4840d11799 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1514,7 +1514,6 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> aml_append(pkg, aml_eisaid("PNP0A03"));
> aml_append(dev, aml_name_decl("_CID", pkg));
> aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
> -aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
> build_cxl_osc_method(dev);
> } else if (pci_bus_is_express(bus)) {
> aml_append(dev, aml_name_decl("_HID",
> aml_eisaid("PNP0A08")));
> --
> 2.37.2
>
>
Re: [PATCH v4 05/10] tests/acpi: Allow update of q35/DSDT.cxl
On Mon, Feb 06, 2023 at 05:28:11PM +, Jonathan Cameron wrote: > Next patch will drop duplicate _UID entry so allow update. > > Reviewed-by: Gregory Price > Tested-by: Gregory Price > Signed-off-by: Jonathan Cameron Reviewed-by: Fan Ni > --- > tests/qtest/bios-tables-test-allowed-diff.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tests/qtest/bios-tables-test-allowed-diff.h > b/tests/qtest/bios-tables-test-allowed-diff.h > index dfb8523c8b..9ce0f596cc 100644 > --- a/tests/qtest/bios-tables-test-allowed-diff.h > +++ b/tests/qtest/bios-tables-test-allowed-diff.h > @@ -1 +1,2 @@ > /* List of comma-separated changed AML files to ignore */ > +"tests/data/acpi/q35/DSDT.cxl", > -- > 2.37.2 > >
Re: [PATCH v4 04/10] hw/cxl: Add CXL_CAPACITY_MULTIPLIER definition
On Mon, Feb 06, 2023 at 05:28:10PM +, Jonathan Cameron wrote:
> From: Gregory Price
>
> Remove usage of magic numbers when accessing capacity fields and replace
> with CXL_CAPACITY_MULTIPLIER, matching the kernel definition.
>
> Signed-off-by: Gregory Price
> Reviewed-by: Davidlohr Bueso
> Signed-off-by: Jonathan Cameron
>
Reviewed-by: Fan Ni
> ---
> v2:
> Change to 256 * MiB and include qemu/units.h (Philippe Mathieu-Daudé)
> ---
> hw/cxl/cxl-mailbox-utils.c | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index bc1bb18844..3f67b665f5 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -12,8 +12,11 @@
> #include "hw/pci/pci.h"
> #include "qemu/cutils.h"
> #include "qemu/log.h"
> +#include "qemu/units.h"
> #include "qemu/uuid.h"
>
> +#define CXL_CAPACITY_MULTIPLIER (256 * MiB)
> +
> /*
> * How to add a new command, example. The command set FOO, with cmd BAR.
> * 1. Add the command set and cmd to the enum.
> @@ -138,7 +141,7 @@ static ret_code cmd_firmware_update_get_info(struct
> cxl_cmd *cmd,
> } QEMU_PACKED *fw_info;
> QEMU_BUILD_BUG_ON(sizeof(*fw_info) != 0x50);
>
> -if (cxl_dstate->pmem_size < (256 << 20)) {
> +if (cxl_dstate->pmem_size < CXL_CAPACITY_MULTIPLIER) {
> return CXL_MBOX_INTERNAL_ERROR;
> }
>
> @@ -283,7 +286,7 @@ static ret_code cmd_identify_memory_device(struct cxl_cmd
> *cmd,
> CXLType3Class *cvc = CXL_TYPE3_GET_CLASS(ct3d);
> uint64_t size = cxl_dstate->pmem_size;
>
> -if (!QEMU_IS_ALIGNED(size, 256 << 20)) {
> +if (!QEMU_IS_ALIGNED(size, CXL_CAPACITY_MULTIPLIER)) {
> return CXL_MBOX_INTERNAL_ERROR;
> }
>
> @@ -293,8 +296,8 @@ static ret_code cmd_identify_memory_device(struct cxl_cmd
> *cmd,
> /* PMEM only */
> snprintf(id->fw_revision, 0x10, "BWFW VERSION %02d", 0);
>
> -id->total_capacity = size / (256 << 20);
> -id->persistent_capacity = size / (256 << 20);
> +id->total_capacity = size / CXL_CAPACITY_MULTIPLIER;
> +id->persistent_capacity = size / CXL_CAPACITY_MULTIPLIER;
> id->lsa_size = cvc->get_lsa_size(ct3d);
>
> *len = sizeof(*id);
> @@ -314,14 +317,14 @@ static ret_code cmd_ccls_get_partition_info(struct
> cxl_cmd *cmd,
> QEMU_BUILD_BUG_ON(sizeof(*part_info) != 0x20);
> uint64_t size = cxl_dstate->pmem_size;
>
> -if (!QEMU_IS_ALIGNED(size, 256 << 20)) {
> +if (!QEMU_IS_ALIGNED(size, CXL_CAPACITY_MULTIPLIER)) {
> return CXL_MBOX_INTERNAL_ERROR;
> }
>
> /* PMEM only */
> part_info->active_vmem = 0;
> part_info->next_vmem = 0;
> -part_info->active_pmem = size / (256 << 20);
> +part_info->active_pmem = size / CXL_CAPACITY_MULTIPLIER;
> part_info->next_pmem = 0;
>
> *len = sizeof(*part_info);
> --
> 2.37.2
>
>
Re: [PATCH v4 03/10] hw/cxl: set cxl-type3 device type to PCI_CLASS_MEMORY_CXL
On Mon, Feb 06, 2023 at 05:28:09PM +, Jonathan Cameron wrote:
> From: Gregory Price
>
> Current code sets to STORAGE_EXPRESS and then overrides it.
>
> Reviewed-by: Davidlohr Bueso
> Reviewed-by: Ira Weiny
> Signed-off-by: Gregory Price
> Signed-off-by: Jonathan Cameron
> ---
Reviewed-by: Fan Ni
> hw/mem/cxl_type3.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 252822bd82..217a5e639b 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -408,7 +408,6 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
> }
>
> pci_config_set_prog_interface(pci_conf, 0x10);
> -pci_config_set_class(pci_conf, PCI_CLASS_MEMORY_CXL);
>
> pcie_endpoint_cap_init(pci_dev, 0x80);
> if (ct3d->sn != UI64_NULL) {
> @@ -627,7 +626,7 @@ static void ct3_class_init(ObjectClass *oc, void *data)
>
> pc->realize = ct3_realize;
> pc->exit = ct3_exit;
> -pc->class_id = PCI_CLASS_STORAGE_EXPRESS;
> +pc->class_id = PCI_CLASS_MEMORY_CXL;
> pc->vendor_id = PCI_VENDOR_ID_INTEL;
> pc->device_id = 0xd93; /* LVF for now */
> pc->revision = 1;
> --
> 2.37.2
>
>
Re: [PATCH v4 01/10] hw/mem/cxl_type3: Improve error handling in realize()
On Mon, Feb 06, 2023 at 05:28:07PM +, Jonathan Cameron wrote:
> msix_init_exclusive_bar() can fail, so if it does cleanup the address space.
>
> Reviewed-by: Ira Weiny
> Reviewed-by: Gregory Price
> Tested-by: Gregory Price
> Signed-off-by: Jonathan Cameron
> ---
Reviewed-by: Fan Ni
> hw/mem/cxl_type3.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index dae4fd89ca..252822bd82 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -401,7 +401,7 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
> MemoryRegion *mr = ®s->component_registers;
> uint8_t *pci_conf = pci_dev->config;
> unsigned short msix_num = 1;
> -int i;
> +int i, rc;
>
> if (!cxl_setup_memory(ct3d, errp)) {
> return;
> @@ -438,7 +438,10 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
> &ct3d->cxl_dstate.device_registers);
>
> /* MSI(-X) Initailization */
> -msix_init_exclusive_bar(pci_dev, msix_num, 4, NULL);
> +rc = msix_init_exclusive_bar(pci_dev, msix_num, 4, NULL);
> +if (rc) {
> +goto err_address_space_free;
> +}
> for (i = 0; i < msix_num; i++) {
> msix_vector_use(pci_dev, i);
> }
> @@ -450,6 +453,11 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
> cxl_cstate->cdat.free_cdat_table = ct3_free_cdat_table;
> cxl_cstate->cdat.private = ct3d;
> cxl_doe_cdat_init(cxl_cstate, errp);
> +return;
> +
> +err_address_space_free:
> +address_space_destroy(&ct3d->hostmem_as);
> +return;
> }
>
> static void ct3_exit(PCIDevice *pci_dev)
> --
> 2.37.2
>
>
Re: [PATCH v4 02/10] hw/pci-bridge/cxl_downstream: Fix type naming mismatch
On Mon, Feb 06, 2023 at 05:28:08PM +, Jonathan Cameron wrote:
> Fix capitalization difference between struct name and typedef.
>
> Tested-by: Philippe Mathieu-Daudé
> Reviewed-by: Philippe Mathieu-Daudé
> Reviewed-by: Ira Weiny
> Reviewed-by: Gregory Price
> Tested-by: Gregory Price
> Signed-off-by: Jonathan Cameron
> ---
Reviewed-by: Fan Ni
> hw/pci-bridge/cxl_downstream.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci-bridge/cxl_downstream.c b/hw/pci-bridge/cxl_downstream.c
> index 3d4e6b59cd..54f507318f 100644
> --- a/hw/pci-bridge/cxl_downstream.c
> +++ b/hw/pci-bridge/cxl_downstream.c
> @@ -15,7 +15,7 @@
> #include "hw/pci/pcie_port.h"
> #include "qapi/error.h"
>
> -typedef struct CXLDownStreamPort {
> +typedef struct CXLDownstreamPort {
> /*< private >*/
> PCIESlot parent_obj;
>
> --
> 2.37.2
>
>
Re: [RFC PATCH 10/43] target/loongarch: Implement vaddw/vsubw
在 2023/2/28 上午2:40, Richard Henderson 写道:
On 2/27/23 02:55, gaosong wrote:
在 2023/2/25 上午3:24, Richard Henderson 写道:
{
.fniv = gen_vaddwev_s,
.fno = gen_helper_vaddwev_q_d,
.opt_opc = vecop_list,
.vece = MO_128
},
There are no 128-bit vector operations; you'll need to do this one
differently.
Presumably just load the two 64-bit elements, sign-extend into
128-bits, add with tcg_gen_add2_i64, and store the two 64-bit
elements as output. But that won't fit into the tcg_gen_gvec_3
interface.
'sign-extend into 128-bits,' Could you give a example?
Well, for vadd, as the example we have been using:
tcg_gen_ld_i64(lo1, cpu_env, offsetof(vector_reg[A].lo));
tcg_gen_ld_i64(lo2, cpu_env, offsetof(vector_reg[B].lo));
tcg_gen_sari_i64(hi1, lo1, 63);
tcg_gen_sari_i64(hi2, lo2, 63);
tcg_gen_add2_i64(lo1, hi1, lo1, hi1, lo2, hi2);
tcg_gen_st_i64(lo1, cpu_env, offsetof(vector_reg[R].lo));
tcg_gen_st_i64(hi1, cpu_env, offsetof(vector_reg[R].hi));
The middle two sari operations replicate the sign bit across the
entire high word, so the pair of variables constitute a sign-extended
128-bit value.
Thank you .
This is a way to translate:
static trans_vaddwev_q_d( DisasContext *ctx, arg_vvv *a)
{
...
tcg_gen_ld_i64(lo1, cpu_env, offsetof(vector_reg[A].lo));
tcg_gen_ld_i64(lo2, cpu_env, offsetof(vector_reg[B].lo));
tcg_gen_sari_i64(hi1, lo1, 63);
tcg_gen_sari_i64(hi2, lo2, 63);
tcg_gen_add2_i64(lo1, hi1, lo1, hi1, lo2, hi2);
tcg_gen_st_i64(lo1, cpu_env, offsetof(vector_reg[R].lo));
tcg_gen_st_i64(hi1, cpu_env, offsetof(vector_reg[R].hi));
...
}
I see a example at target/ppc/translate/vmx-impl.c.inc
static bool do_vx_vprtyb(DisasContext *ctx, arg_VX_tb *a,
unsigned vece)
{
...
{
.fno = gen_helper_VPRTYBQ,
.vece = MO_128
},
tcg_gen_gvec_2(avr_full_offset(a->vrt),
avr_full_offset(a->vrb),
16, 16, &op[vece - MO_32]);
return true;
}
TRANS(VPRTYBQ, do_vx_vprtyb, MO_128)
...
do_vx_vprtyb fit the fno into the tcg_gen_gvec_2.
I am not sure this example is right.
Ah, well. When .fno is the only callback, the implementation is
entirely out-of-line, and the .vece member is not used. I see that is
confusing.
and This is another way to translate:
...
{
.fno = gen_helper_vaddwev_q_d,
.vece = MO_128
},
...
void HELPER(vaddwev_q_d)(void *vd, void *vj, void *vk, uint32_t v)
{
VReg *Vd = (VReg *)vd;
VReg *Vj = (VReg *)vj;
VReg *Vk = (VReg *)vk;
Vd->Q(0) = int128_add((Int128)Vj->D(0), (Int128)Vk->D(0));
}
These ways are can be chosen?
Thanks.
Song Gao
[PATCH] virtio-balloon: optimize the virtio-balloon on the ARM platform.
> > Optimize the virtio-balloon feature on the ARM platform by adding a
> > variable to keep track of the current hot-plugged pc-dimm size,
> > instead of traversing the virtual machine's memory modules to count
> > the current RAM size during the balloon inflation or deflation
> > process. This variable can be updated only when plugging or unplugging
> > the device, which will result in an increase of approximately 60%
> > efficiency of balloon process on the ARM platform.
> >
> > We tested the total amount of time required for the balloon inflation
> process on ARM:
> > inflate the balloon to 64GB of a 128GB guest under stress.
> > Before: 102 seconds
> > After: 42 seconds
> >
> > Signed-off-by: Qi Xi
> > Signed-off-by: Ming Yang yangmin...@huawei.com
> > ---
> > hw/mem/pc-dimm.c | 2 ++
> > hw/virtio/virtio-balloon.c | 33 +
> > include/hw/boards.h| 1 +
> > 3 files changed, 8 insertions(+), 28 deletions(-)
> >
> > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c index
> > 50ef83215c..192fc7922c 100644
> > --- a/hw/mem/pc-dimm.c
> > +++ b/hw/mem/pc-dimm.c
> > @@ -81,6 +81,7 @@ void pc_dimm_plug(PCDIMMDevice *dimm,
> MachineState
> > *machine)
> >
> > memory_device_plug(MEMORY_DEVICE(dimm), machine);
> > vmstate_register_ram(vmstate_mr, DEVICE(dimm));
> > +machine->device_memory->dimm_size += vmstate_mr->size;
> > }
> >
> > void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine)
> @@
> > -90,6 +91,7 @@ void pc_dimm_unplug(PCDIMMDevice *dimm,
> MachineState
> > *machine)
> >
> > memory_device_unplug(MEMORY_DEVICE(dimm), machine);
> > vmstate_unregister_ram(vmstate_mr, DEVICE(dimm));
> > +machine->device_memory->dimm_size -= vmstate_mr->size;
> > }
>
> Ahh, missed that my previous comment was not addressed: we only want to
> track "real" DIMMs, not NVDIMMs.
>
> --
> Thanks,
>
> David / dhildenb
Optimize the virtio-balloon feature on the ARM platform by adding
a variable to keep track of the current hot-plugged pc-dimm size,
instead of traversing the virtual machine's memory modules to count
the current RAM size during the balloon inflation or deflation
process. This variable can be updated only when plugging or unplugging
the device, which will result in an increase of approximately 60%
efficiency of balloon process on the ARM platform.
We tested the total amount of time required for the balloon inflation process
on ARM:
inflate the balloon to 64GB of a 128GB guest under stress.
Before: 102 seconds
After: 42 seconds
Signed-off-by: Qi Xi
Signed-off-by: Ming Yang yangmin...@huawei.com
---
hw/mem/pc-dimm.c | 8
hw/virtio/virtio-balloon.c | 33 +
include/hw/boards.h| 1 +
3 files changed, 14 insertions(+), 28 deletions(-)
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 50ef83215c..2107615016 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -81,6 +81,10 @@ void pc_dimm_plug(PCDIMMDevice *dimm, MachineState *machine)
memory_device_plug(MEMORY_DEVICE(dimm), machine);
vmstate_register_ram(vmstate_mr, DEVICE(dimm));
+bool is_nvdimm = object_dynamic_cast(OBJECT(dimm), TYPE_NVDIMM);
+if (!is_nvdimm) {
+machine->device_memory->dimm_size += vmstate_mr->size;
+}
}
void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState *machine)
@@ -90,6 +94,10 @@ void pc_dimm_unplug(PCDIMMDevice *dimm, MachineState
*machine)
memory_device_unplug(MEMORY_DEVICE(dimm), machine);
vmstate_unregister_ram(vmstate_mr, DEVICE(dimm));
+bool is_nvdimm = object_dynamic_cast(OBJECT(dimm), TYPE_NVDIMM);
+if (!is_nvdimm) {
+machine->device_memory->dimm_size -= vmstate_mr->size;
+}
}
static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 746f07c4d2..80bbb59132 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -729,37 +729,14 @@ static void virtio_balloon_get_config(VirtIODevice *vdev,
uint8_t *config_data)
memcpy(config_data, &config, virtio_balloon_config_size(dev));
}
-static int build_dimm_list(Object *obj, void *opaque)
-{
-GSList **list = opaque;
-
-if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
-DeviceState *dev = DEVICE(obj);
-if (dev->realized) { /* only realized DIMMs matter */
-*list = g_slist_prepend(*list, dev);
-}
-}
-
-object_child_foreach(obj, build_dimm_list, opaque);
-return 0;
-}
-
static ram_addr_t get_current_ram_size(void)
{
-GSList *list = NULL, *item;
-ram_addr_t size = current_machine->ram_size;
-
-build_dimm_list(qdev_get_machine(), &list);
-for (item = list; item; item = g_slist_next(item)) {
-Object *obj = OBJECT(item->data);
-if (!strcmp(object_get_typename(obj), TYPE_PC_DIMM)) {
-size += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
-
Re: [PATCH 3/3] qga: test: Add tests for `merge-output` flag
Hi, On Mon, Feb 27, 2023, at 1:40 AM, Marc-André Lureau wrote: > Hi > > On Fri, Feb 24, 2023 at 8:31 AM Daniel Xu wrote: >> >> This commit adds a test to ensure `merge-output` functions as expected. >> We also add a negative test to ensure we haven't regressed previous >> functionality. >> >> Signed-off-by: Daniel Xu > > Please check your patch with ASAN, you have use after-free introduced > by this change: > ==664972==ERROR: AddressSanitizer: heap-use-after-free on address > 0x621000135028 at pc 0x55e617a38b39 bp 0x7fff7fe85390 sp > 0x7fff7fe85388 > READ of size 8 at 0x621000135028 thread T0 > #0 0x55e617a38b38 in qdict_find ../qobject/qdict.c:96 > #1 0x55e617a39bea in qdict_get ../qobject/qdict.c:164 > #2 0x55e617a39bea in qdict_get_int ../qobject/qdict.c:209 > #3 0x55e6179e2519 in test_qga_guest_exec ../tests/unit/test-qga.c:807 > #4 0x7fbaa499dc7d in g_test_run_suite_internal > (/lib64/libglib-2.0.so.0+0x84c7d) > #5 0x7fbaa499d9e4 in g_test_run_suite_internal > (/lib64/libglib-2.0.so.0+0x849e4) > #6 0x7fbaa499e181 in g_test_run_suite (/lib64/libglib-2.0.so.0+0x85181) > #7 0x7fbaa49966ec in g_test_run (/lib64/libglib-2.0.so.0+0x7d6ec) > #8 0x55e6179da0ac in main ../tests/unit/test-qga.c:1083 > #9 0x7fbaa384a50f in __libc_start_call_main (/lib64/libc.so.6+0x2750f) > #10 0x7fbaa384a5c8 in __libc_start_main@GLIBC_2.2.5 > (/lib64/libc.so.6+0x275c8) > #11 0x55e6179daf44 in _start > (/home/elmarco/src/qemu/build/tests/unit/test-qga+0x1bbf44) Ack. Will fix. [...] Thanks, Daniel
Re: [PATCH 2/3] qga: Add optional `merge-output` flag to guest-exec qapi
Hi,
On Mon, Feb 27, 2023, at 1:22 AM, Marc-André Lureau wrote:
> Hi
>
> On Fri, Feb 24, 2023 at 8:31 AM Daniel Xu wrote:
>>
>> Currently, the captured output (via `capture-output`) is segregated into
>> separate GuestExecStatus fields (`out-data` and `err-data`). This means
>> that downstream consumers have no way to reassemble the captured data
>> back into the original stream.
>>
>> This is relevant for chatty and semi-interactive (ie. read only) CLI
>> tools. Such tools may deliberately interleave stdout and stderr for
>> visual effect. If segregated, the output becomes harder to visually
>> understand.
>>
>> This commit adds a new optional flag to the guest-exec qapi to merge the
>> output streams such that consumers can have a pristine view of the
>> original command output.
>>
>> Signed-off-by: Daniel Xu
>> ---
>> qga/commands.c | 13 -
>> qga/qapi-schema.json | 6 +-
>> 2 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/qga/commands.c b/qga/commands.c
>> index 360077364e..14b970e768 100644
>> --- a/qga/commands.c
>> +++ b/qga/commands.c
>> @@ -274,6 +274,15 @@ static void guest_exec_child_watch(GPid pid, gint
>> status, gpointer data)
>> /** Reset ignored signals back to default. */
>> static void guest_exec_task_setup(gpointer data)
>> {
>> +bool has_merge = *(bool *)data;
>> +
>> +if (has_merge) {
>> +if (dup2(STDOUT_FILENO, STDERR_FILENO) != 0) {
>> +slog("dup2() failed to merge stderr into stdout: %s",
>> + strerror(errno));
>> +}
>> +}
>
> https://docs.gtk.org/glib/callback.SpawnChildSetupFunc.html
>
> "On Windows, the function is called in the parent. Its usefulness on
> Windows is thus questionable. In many cases executing the child setup
> function in the parent can have ill effects, and you should be very
> careful when porting software to Windows that uses child setup
> functions."
>
> It looks like this would be bad.
Ah that's a good catch. I'm not very familiar with windows APIs so
unfortunately I don't have any good ideas here.
Best I can tell g_spawn_async_with_pipes_and_fds() work with it's
source_fds and target_fds mapping. But it looks like that came in
glib 2.68 so we cannot use it yet.
How about limiting this merge-output flag to linux/unix systems
for now? Could document this in the qapi doc string.
>
>> +
>> #if !defined(G_OS_WIN32)
>> struct sigaction sigact;
>>
>> @@ -385,6 +394,7 @@ GuestExec *qmp_guest_exec(const char *path,
>> bool has_env, strList *env,
>> const char *input_data,
>> bool has_capture_output, bool capture_output,
>> + bool has_merge_output, bool merge_output,
>> Error **errp)
>> {
>> GPid pid;
>> @@ -398,6 +408,7 @@ GuestExec *qmp_guest_exec(const char *path,
>> GIOChannel *in_ch, *out_ch, *err_ch;
>> GSpawnFlags flags;
>> bool has_output = (has_capture_output && capture_output);
>> +bool has_merge = (has_merge_output && merge_output);
>> g_autofree uint8_t *input = NULL;
>> size_t ninput = 0;
>>
>> @@ -421,7 +432,7 @@ GuestExec *qmp_guest_exec(const char *path,
>> }
>>
>> ret = g_spawn_async_with_pipes(NULL, argv, envp, flags,
>> -guest_exec_task_setup, NULL, &pid, input_data ? &in_fd : NULL,
>> +guest_exec_task_setup, &has_merge, &pid, input_data ? &in_fd :
>> NULL,
>> has_output ? &out_fd : NULL, has_output ? &err_fd : NULL,
>> &gerr);
>> if (!ret) {
>> error_setg(errp, QERR_QGA_COMMAND_FAILED, gerr->message);
>> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
>> index 796434ed34..4192fcc5a4 100644
>> --- a/qga/qapi-schema.json
>> +++ b/qga/qapi-schema.json
>> @@ -1211,6 +1211,9 @@
>> # @input-data: data to be passed to process stdin (base64 encoded)
>> # @capture-output: bool flag to enable capture of
>> # stdout/stderr of running process. defaults to false.
>> +# @merge-output: bool flag to merge stdout/stderr of running process
>> +#into stdout. only effective if used with @capture-output.
>> +#defaults to false.
>
> Add (since: 8.0)
Ack.
[...]
Thanks,
Daniel
Re: [PATCH 1/3] qga: test: Use absolute path to test data
Hi Marc-André, Thanks for reviewing the series. On Mon, Feb 27, 2023, at 1:16 AM, Marc-André Lureau wrote: > Hi > > On Fri, Feb 24, 2023 at 8:31 AM Daniel Xu wrote: >> >> It looks like qga's working directory is in a tempdir. So the relative >> path that the test case gives qga through the QGA_OS_RELEASE= >> env variable does not resolve correctly. >> >> Fix by doing a poor man's path canonicalization of the test data file. >> >> Note we cannot use g_canonicalize_filename() b/c that helper was only >> introduced in glib 2.58 and the current GLIB_VERSION_MAX_ALLOWED is >> pinned to 2.56. >> >> Signed-off-by: Daniel Xu > > This breaks "meson test test-qga" for me. How do you run the tests? Ah, thanks for the hint. I was running the qga tests in build/ with: $ ./tests/unit/test-qga Using meson to drive the tests fixed it for me. I will drop this patch. [...] Thanks, Daniel
Re: [PATCH v1] target/loongarch: Implement Chip Configuraiton Version Register(0x0000)
在 2023/2/28 上午3:37, Richard Henderson 写道:
On 2/26/23 21:10, Song Gao wrote:
According to the 3A5000 manual 4.1 implement Chip Configuration
Version Register(0x). The manual does not state that 0x0018 is
reserved for the vendor name and 0x0028 is reserved for the chip name.
Signed-off-by: Song Gao
---
target/loongarch/cpu.c | 2 ++
target/loongarch/cpu.h | 3 +++
2 files changed, 5 insertions(+)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 290ab4d526..d1c803c9d6 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -545,6 +545,8 @@ static void loongarch_qemu_write(void *opaque,
hwaddr addr,
static uint64_t loongarch_qemu_read(void *opaque, hwaddr addr,
unsigned size)
{
switch (addr) {
+ case VERSION_REG:
+ return 0x11ULL;
This one is back in manual v1.03.
I can't find manual 4.1?
Oh, Chapter 4 Section 1. I will correct it.
+#define VENDOR_RESERVED_REG 0x18
+#define CPUNAME_RESERVED_REG 0x28
Since these are unused, perhaps omit them?
OK.
Thanks.
Song Gao
Either way,
Reviewed-by: Richard Henderson
r~
[PATCH 1/3] target/arm: Avoid splitting Zregs across lines in dump
Allow the line length to extend to 548 columns. While annoyingly wide,
it's still less confusing than the continuations we print. Also, the
default VL used by Linux (and max for A64FX) uses only 140 columns.
Signed-off-by: Richard Henderson
---
target/arm/cpu.c | 36 ++--
1 file changed, 14 insertions(+), 22 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5182ed0c91..f1f454e7a0 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -953,7 +953,7 @@ static void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
int flags)
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
uint32_t psr = pstate_read(env);
-int i;
+int i, j;
int el = arm_current_el(env);
const char *ns_status;
bool sve;
@@ -1012,7 +1012,7 @@ static void aarch64_cpu_dump_state(CPUState *cs, FILE *f,
int flags)
}
if (sve) {
-int j, zcr_len = sve_vqm1_for_el(env, el);
+int zcr_len = sve_vqm1_for_el(env, el);
for (i = 0; i <= FFR_PRED_NUM; i++) {
bool eol;
@@ -1052,32 +1052,24 @@ static void aarch64_cpu_dump_state(CPUState *cs, FILE
*f, int flags)
}
}
-for (i = 0; i < 32; i++) {
-if (zcr_len == 0) {
+if (zcr_len == 0) {
+/*
+ * With vl=16, there are only 37 columns per register,
+ * so output two registers per line.
+ */
+for (i = 0; i < 32; i++) {
qemu_fprintf(f, "Z%02d=%016" PRIx64 ":%016" PRIx64 "%s",
i, env->vfp.zregs[i].d[1],
env->vfp.zregs[i].d[0], i & 1 ? "\n" : " ");
-} else if (zcr_len == 1) {
-qemu_fprintf(f, "Z%02d=%016" PRIx64 ":%016" PRIx64
- ":%016" PRIx64 ":%016" PRIx64 "\n",
- i, env->vfp.zregs[i].d[3], env->vfp.zregs[i].d[2],
- env->vfp.zregs[i].d[1], env->vfp.zregs[i].d[0]);
-} else {
+}
+} else {
+for (i = 0; i < 32; i++) {
+qemu_fprintf(f, "Z%02d=", i);
for (j = zcr_len; j >= 0; j--) {
-bool odd = (zcr_len - j) % 2 != 0;
-if (j == zcr_len) {
-qemu_fprintf(f, "Z%02d[%x-%x]=", i, j, j - 1);
-} else if (!odd) {
-if (j > 0) {
-qemu_fprintf(f, " [%x-%x]=", j, j - 1);
-} else {
-qemu_fprintf(f, " [%x]=", j);
-}
-}
qemu_fprintf(f, "%016" PRIx64 ":%016" PRIx64 "%s",
env->vfp.zregs[i].d[j * 2 + 1],
- env->vfp.zregs[i].d[j * 2],
- odd || j == 0 ? "\n" : ":");
+ env->vfp.zregs[i].d[j * 2 + 0],
+ j ? ":" : "\n");
}
}
}
--
2.34.1
[PATCH 2/3] target/arm: Dump ZA[] when active
Always print each matrix row whole, one per line, so that we
get the entire matrix in the proper shape.
Signed-off-by: Richard Henderson
---
target/arm/cpu.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index f1f454e7a0..0e54e19b05 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1080,6 +1080,24 @@ static void aarch64_cpu_dump_state(CPUState *cs, FILE
*f, int flags)
i, q[1], q[0], (i & 1 ? "\n" : " "));
}
}
+
+if (cpu_isar_feature(aa64_sme, cpu) &&
+FIELD_EX64(env->svcr, SVCR, ZA) &&
+sme_exception_el(env, el) == 0) {
+int zcr_len = sve_vqm1_for_el_sm(env, el, true);
+int svl = (zcr_len + 1) * 16;
+int svl_lg10 = svl < 100 ? 2 : 3;
+
+for (i = 0; i < svl; i++) {
+qemu_fprintf(f, "ZA[%0*d]=", svl_lg10, i);
+for (j = zcr_len; j >= 0; --j) {
+qemu_fprintf(f, "%016" PRIx64 ":%016" PRIx64 "%c",
+ env->zarray[i].d[2 * j + 1],
+ env->zarray[i].d[2 * j],
+ j ? ':' : '\n');
+}
+}
+}
}
#else
--
2.34.1
[PATCH 0/3] target/arm: dump and gdbstub support for ZA[]
Based-on: 20230227213329.793795-1-richard.hender...@linaro.org
("[PATCH v3 00/14] target/arm: gdbstub cleanups and additions")
Support SME by the same method as currently used by SVE. While I'm
sure proper gdb support for dynamically sizing SME will require changes,
this is good enough to examine contents of the ZA matrix.
r~
Richard Henderson (3):
target/arm: Avoid splitting Zregs across lines in dump
target/arm: Dump ZA[] when active
target/arm: Support reading ZA[] from gdbstub
target/arm/cpu.h | 1 +
target/arm/internals.h | 3 ++
target/arm/cpu.c | 54 +++---
target/arm/gdbstub.c | 8
target/arm/gdbstub64.c | 88 ++
5 files changed, 132 insertions(+), 22 deletions(-)
--
2.34.1
[PATCH 3/3] target/arm: Support reading ZA[] from gdbstub
Mirror the existing support for SVE.
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 1 +
target/arm/internals.h | 3 ++
target/arm/gdbstub.c | 8
target/arm/gdbstub64.c | 88 ++
4 files changed, 100 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 6e97a256fb..9971280577 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -869,6 +869,7 @@ struct ArchCPU {
DynamicGDBXMLInfo dyn_sysreg_xml;
DynamicGDBXMLInfo dyn_svereg_xml;
+DynamicGDBXMLInfo dyn_zareg_xml;
DynamicGDBXMLInfo dyn_m_systemreg_xml;
DynamicGDBXMLInfo dyn_m_secextreg_xml;
diff --git a/target/arm/internals.h b/target/arm/internals.h
index a03748aa10..a27e5b3f28 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1343,12 +1343,15 @@ static inline uint64_t pmu_counter_mask(CPUARMState
*env)
#ifdef TARGET_AARCH64
int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
+int arm_gen_dynamic_zareg_xml(CPUState *cpu, int base_reg);
int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
int aarch64_gdb_set_fpu_reg(CPUARMState *env, uint8_t *buf, int reg);
int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg);
int aarch64_gdb_set_pauth_reg(CPUARMState *env, uint8_t *buf, int reg);
+int aarch64_gdb_get_za_reg(CPUARMState *env, GByteArray *buf, int reg);
+int aarch64_gdb_set_za_reg(CPUARMState *env, uint8_t *buf, int reg);
void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 3f799f5d05..60a9ade732 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -488,6 +488,8 @@ const char *arm_gdb_get_dynamic_xml(CPUState *cs, const
char *xmlname)
return cpu->dyn_sysreg_xml.desc;
} else if (strcmp(xmlname, "sve-registers.xml") == 0) {
return cpu->dyn_svereg_xml.desc;
+} else if (strcmp(xmlname, "za-registers.xml") == 0) {
+return cpu->dyn_zareg_xml.desc;
} else if (strcmp(xmlname, "arm-m-system.xml") == 0) {
return cpu->dyn_m_systemreg_xml.desc;
#ifndef CONFIG_USER_ONLY
@@ -524,6 +526,12 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
aarch64_gdb_set_pauth_reg,
4, "aarch64-pauth.xml", 0);
}
+if (cpu_isar_feature(aa64_sme, cpu)) {
+int nreg = arm_gen_dynamic_zareg_xml(cs, cs->gdb_num_regs);
+gdb_register_coprocessor(cs, aarch64_gdb_get_za_reg,
+ aarch64_gdb_set_za_reg, nreg,
+ "za-registers.xml", 0);
+}
#endif
} else {
if (arm_feature(env, ARM_FEATURE_NEON)) {
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 3bee892fb7..9210d12c4e 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -244,6 +244,61 @@ int aarch64_gdb_set_pauth_reg(CPUARMState *env, uint8_t
*buf, int reg)
return 0;
}
+static int max_svq(ARMCPU *cpu)
+{
+return 32 - clz32(cpu->sme_vq.map);
+}
+
+int aarch64_gdb_get_za_reg(CPUARMState *env, GByteArray *buf, int reg)
+{
+ARMCPU *cpu = env_archcpu(env);
+int max_vq = max_svq(cpu);
+int cur_vq = EX_TBFLAG_A64(env->hflags, SVL) + 1;
+int i;
+
+if (reg >= max_vq * 16) {
+return 0;
+}
+
+/* If ZA is unset, or reg out of range, the contents are zero. */
+if (FIELD_EX64(env->svcr, SVCR, ZA) && reg < cur_vq * 16) {
+for (i = 0; i < cur_vq; i++) {
+gdb_get_reg128(buf, env->zarray[reg].d[i * 2 + 1],
+ env->zarray[reg].d[i * 2]);
+}
+} else {
+cur_vq = 0;
+}
+
+for (i = cur_vq; i < max_vq; i++) {
+gdb_get_reg128(buf, 0, 0);
+}
+
+return max_vq * 16;
+}
+
+int aarch64_gdb_set_za_reg(CPUARMState *env, uint8_t *buf, int reg)
+{
+ARMCPU *cpu = env_archcpu(env);
+uint64_t *p = (uint64_t *) buf;
+int max_vq = max_svq(cpu);
+int cur_vq = EX_TBFLAG_A64(env->hflags, SVL) + 1;
+int i;
+
+if (reg >= max_vq * 16) {
+return 0;
+}
+
+/* If ZA is unset, or reg out of range, the contents are zero. */
+if (FIELD_EX64(env->svcr, SVCR, ZA) && reg < cur_vq * 16) {
+for (i = 0; i < cur_vq; i++) {
+env->zarray[reg].d[i * 2 + 1] = *p++;
+env->zarray[reg].d[i * 2 + 0] = *p++;
+}
+}
+return max_vq * 16;
+}
+
static void output_vector_union_type(GString *s, int reg_width,
const char *name)
{
@@ -376,3 +431,36 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int
orig_base_reg)
info->num = base_reg - orig_bas
Re: [PATCH v2 1/2] docs/system/loongarch: update loongson3.rst and rename it to virt.rst
On 2/26/23 17:59, Song Gao wrote: +Note: build release bios need set --buildtarget=RELEASE, Note: To build the release version of the bios, set... Otherwise, Reviewed-by: Richard Henderson r~
Re: [PATCH v2 2/2] loongarch: Add smbios command line option.
On 2/26/23 17:59, Song Gao wrote:
LoongArch has enabled CONFIG_SMBIOS, but didn't enable CLI '-smbios'.
Fixes: 3efa6fa1e629 ("hw/loongarch: Add smbios support")
Acked-by: Michael S. Tsirkin
Reviewed-by: Markus Armbruster
Signed-off-by: Song Gao
Message-Id: <20230208094133.2945979-2-gaos...@loongson.cn>
Reviewed-by: Richard Henderson
r~
---
qemu-options.hx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index beeb4475ba..d42f60fb91 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2585,7 +2585,7 @@ DEF("smbios", HAS_ARG, QEMU_OPTION_smbios,
"specify SMBIOS type 17 fields\n"
"-smbios
type=41[,designation=str][,kind=str][,instance=%d][,pcidev=str]\n"
"specify SMBIOS type 41 fields\n",
-QEMU_ARCH_I386 | QEMU_ARCH_ARM)
+QEMU_ARCH_I386 | QEMU_ARCH_ARM | QEMU_ARCH_LOONGARCH)
SRST
``-smbios file=binary``
Load SMBIOS entry from binary file.
RE: [PATCH v5 12/14] Hexagon (target/hexagon) Remove gen_log_predicated_reg_write[_pair]
> -Original Message-
> From: Anton Johansson
> Sent: Friday, February 24, 2023 6:06 AM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: richard.hender...@linaro.org; phi...@linaro.org; a...@rev.ng; Brian Cain
> ; Matheus Bernardino (QUIC)
>
> Subject: Re: [PATCH v5 12/14] Hexagon (target/hexagon) Remove
> gen_log_predicated_reg_write[_pair]
>
> On 1/31/23 23:56, Taylor Simpson wrote:
> > static void gen_log_reg_write_pair(int rnum, TCGv_i64 val)
> > {
> > const target_ulong reg_mask_low = reg_immut_masks[rnum]; @@
> > -167,6 +120,7 @@ static void gen_log_reg_write_pair(int rnum, TCGv_i64
> val)
> > }
> >
> > tcg_temp_free(val32);
> > +tcg_temp_free_i64(val);
> > }
> >
> > void gen_log_pred_write(DisasContext *ctx, int pnum, TCGv val) @@
> > -306,12 +260,14 @@ static inline void
> gen_write_ctrl_reg_pair(DisasContext *ctx, int reg_num,
> > TCGv_i64 val)
> > {
> > if (reg_num == HEX_REG_P3_0_ALIASED) {
> > +TCGv result = get_result_gpr(ctx, reg_num + 1);
> > TCGv val32 = tcg_temp_new();
> > tcg_gen_extrl_i64_i32(val32, val);
> > gen_write_p3_0(ctx, val32);
> > tcg_gen_extrh_i64_i32(val32, val);
> > -gen_log_reg_write(reg_num + 1, val32);
> > +tcg_gen_mov_tl(result, val32);
> > tcg_temp_free(val32);
> > +tcg_temp_free_i64(val);
> > } else {
> > gen_log_reg_write_pair(reg_num, val);
> > if (reg_num == HEX_REG_QEMU_PKT_CNT) {
>
> Hiding the free of a TCGv argument scares me a bit, it's bound to cause
> annoying bugs down the line, I feel.
>
> Any reason we can't keep the frees in the same scope as allocation?
The only ones that need to be free'd are the pairs. Those are created by
get_result_gpr_pair, so I'm using the gen_log_reg_write_pair function as the
central place to free them.
Also, Richard Henderson is working on a patch series that will eliminate the
need for all free's 😊
>
> Otherwise, the patch looks good,
>
> Reviewed-by: Anton Johansson
Re: [PATCH for-8.0 v4 00/21] target/arm: Implement FEAT_RME
Gah! I didn't mean to tag this one with for-8.0. r~
Re: [PATCH v3 17/27] target/riscv: Replace `tb_pc()` with `tb->pc`
On 2/27/23 13:05, Palmer Dabbelt wrote:
On Mon, 27 Feb 2023 05:51:52 PST (-0800), a...@rev.ng wrote:
Signed-off-by: Anton Johansson
Reviewed-by: Philippe Mathieu-Daudé
---
target/riscv/cpu.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 93b52b826c..9eb748a283 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -34,6 +34,7 @@
#include "fpu/softfloat-helpers.h"
#include "sysemu/kvm.h"
#include "kvm_riscv.h"
+#include "tcg/tcg.h"
/* RISC-V CPU definitions */
@@ -533,10 +534,12 @@ static void riscv_cpu_synchronize_from_tb(CPUState *cs,
CPURISCVState *env = &cpu->env;
RISCVMXL xl = FIELD_EX32(tb->flags, TB_FLAGS, XL);
+ tcg_debug_assert(!(cs->tcg_cflags & CF_PCREL));
+
if (xl == MXL_RV32) {
- env->pc = (int32_t)tb_pc(tb);
+ env->pc = (int32_t) tb->pc;
} else {
- env->pc = tb_pc(tb);
+ env->pc = tb->pc;
}
}
Reviewed-by: Palmer Dabbelt
Acked-by: Palmer Dabbelt
Thanks! I'm going to assume you want these to stay together, but LMK if you were looking
to aim this at the RISC-V tree.
I've queued to tcg-next, so they'll stay together.
I've now added your r-b.
r~
Re: [PATCH v3 17/27] target/riscv: Replace `tb_pc()` with `tb->pc`
On Mon, 27 Feb 2023 05:51:52 PST (-0800), a...@rev.ng wrote:
Signed-off-by: Anton Johansson
Reviewed-by: Philippe Mathieu-Daudé
---
target/riscv/cpu.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 93b52b826c..9eb748a283 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -34,6 +34,7 @@
#include "fpu/softfloat-helpers.h"
#include "sysemu/kvm.h"
#include "kvm_riscv.h"
+#include "tcg/tcg.h"
/* RISC-V CPU definitions */
@@ -533,10 +534,12 @@ static void riscv_cpu_synchronize_from_tb(CPUState *cs,
CPURISCVState *env = &cpu->env;
RISCVMXL xl = FIELD_EX32(tb->flags, TB_FLAGS, XL);
+tcg_debug_assert(!(cs->tcg_cflags & CF_PCREL));
+
if (xl == MXL_RV32) {
-env->pc = (int32_t)tb_pc(tb);
+env->pc = (int32_t) tb->pc;
} else {
-env->pc = tb_pc(tb);
+env->pc = tb->pc;
}
}
Reviewed-by: Palmer Dabbelt
Acked-by: Palmer Dabbelt
Thanks! I'm going to assume you want these to stay together, but LMK if
you were looking to aim this at the RISC-V tree.
[PATCH for-8.0 v4 14/21] target/arm: Use get_phys_addr_with_struct in S1_ptw_translate
Do not provide a fast-path for physical addresses,
as those will need to be validated for GPC.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 35 ++-
1 file changed, 14 insertions(+), 21 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 2eae7a69cb..0177dea0cf 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -232,29 +232,22 @@ static bool S1_ptw_translate(CPUARMState *env,
S1Translate *ptw,
* From gdbstub, do not use softmmu so that we don't modify the
* state of the cpu at all, including softmmu tlb contents.
*/
-if (regime_is_stage2(s2_mmu_idx)) {
-S1Translate s2ptw = {
-.in_mmu_idx = s2_mmu_idx,
-.in_ptw_idx = arm_space_to_phys(space),
-.in_space = space,
-.in_secure = is_secure,
-.in_debug = true,
-};
-GetPhysAddrResult s2 = { };
+S1Translate s2ptw = {
+.in_mmu_idx = s2_mmu_idx,
+.in_ptw_idx = arm_space_to_phys(space),
+.in_space = space,
+.in_secure = is_secure,
+.in_debug = true,
+};
+GetPhysAddrResult s2 = { };
-if (get_phys_addr_lpae(env, &s2ptw, addr, MMU_DATA_LOAD,
- false, &s2, fi)) {
-goto fail;
-}
-ptw->out_phys = s2.f.phys_addr;
-pte_attrs = s2.cacheattrs.attrs;
-pte_secure = s2.f.attrs.secure;
-} else {
-/* Regime is physical. */
-ptw->out_phys = addr;
-pte_attrs = 0;
-pte_secure = is_secure;
+if (get_phys_addr_with_struct(env, &s2ptw, addr,
+ MMU_DATA_LOAD, &s2, fi)) {
+goto fail;
}
+ptw->out_phys = s2.f.phys_addr;
+pte_attrs = s2.cacheattrs.attrs;
+pte_secure = s2.f.attrs.secure;
ptw->out_host = NULL;
ptw->out_rw = false;
} else {
--
2.34.1
[PATCH for-8.0 v4 20/21] NOTFORMERGE target/arm: Enable RME for -cpu max
Add a cpu property to set GPCCR_EL3.L0GPTSZ, for testing
various possible configurations.
Signed-off-by: Richard Henderson
---
target/arm/cpu64.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 4066950da1..70c173ee3d 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -672,6 +672,40 @@ void arm_cpu_lpa2_finalize(ARMCPU *cpu, Error **errp)
cpu->isar.id_aa64mmfr0 = t;
}
+static void cpu_max_set_l0gptsz(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+uint32_t value;
+
+if (!visit_type_uint32(v, name, &value, errp)) {
+return;
+}
+
+/* Encode the value for the GPCCR_EL3 field. */
+switch (value) {
+case 30:
+case 34:
+case 36:
+case 39:
+cpu->reset_l0gptsz = value - 30;
+break;
+default:
+error_setg(errp, "invalid value for l0gptsz");
+error_append_hint(errp, "valid values are 30, 34, 36, 39\n");
+break;
+}
+}
+
+static void cpu_max_get_l0gptsz(Object *obj, Visitor *v, const char *name,
+void *opaque, Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(obj);
+uint32_t value = cpu->reset_l0gptsz + 30;
+
+visit_type_uint32(v, name, &value, errp);
+}
+
static void aarch64_a57_initfn(Object *obj)
{
ARMCPU *cpu = ARM_CPU(obj);
@@ -1200,6 +1234,7 @@ static void aarch64_max_initfn(Object *obj)
t = FIELD_DP64(t, ID_AA64PFR0, SVE, 1);
t = FIELD_DP64(t, ID_AA64PFR0, SEL2, 1); /* FEAT_SEL2 */
t = FIELD_DP64(t, ID_AA64PFR0, DIT, 1); /* FEAT_DIT */
+t = FIELD_DP64(t, ID_AA64PFR0, RME, 1); /* FEAT_RME */
t = FIELD_DP64(t, ID_AA64PFR0, CSV2, 2); /* FEAT_CSV2_2 */
t = FIELD_DP64(t, ID_AA64PFR0, CSV3, 1); /* FEAT_CSV3 */
cpu->isar.id_aa64pfr0 = t;
@@ -1301,6 +1336,8 @@ static void aarch64_max_initfn(Object *obj)
object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_max_vq,
cpu_max_set_sve_max_vq, NULL, NULL);
qdev_property_add_static(DEVICE(obj), &arm_cpu_lpa2_property);
+object_property_add(obj, "l0gptsz", "uint32", cpu_max_get_l0gptsz,
+cpu_max_set_l0gptsz, NULL, NULL);
}
static const ARMCPUInfo aarch64_cpus[] = {
--
2.34.1
[PATCH for-8.0 v4 17/21] target/arm: Add GPC syndrome
The function takes the fields as filled in by
the Arm ARM pseudocode for TakeGPCException.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/syndrome.h | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/arm/syndrome.h b/target/arm/syndrome.h
index d27d1bc31f..62254d0e51 100644
--- a/target/arm/syndrome.h
+++ b/target/arm/syndrome.h
@@ -50,6 +50,7 @@ enum arm_exception_class {
EC_SVEACCESSTRAP = 0x19,
EC_ERETTRAP = 0x1a,
EC_SMETRAP= 0x1d,
+EC_GPC= 0x1e,
EC_INSNABORT = 0x20,
EC_INSNABORT_SAME_EL = 0x21,
EC_PCALIGNMENT= 0x22,
@@ -247,6 +248,15 @@ static inline uint32_t syn_bxjtrap(int cv, int cond, int
rm)
(cv << 24) | (cond << 20) | rm;
}
+static inline uint32_t syn_gpc(int s2ptw, int ind, int gpcsc,
+ int cm, int s1ptw, int wnr, int fsc)
+{
+/* TODO: FEAT_NV2 adds VNCR */
+return (EC_GPC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (s2ptw << 21)
+| (ind << 20) | (gpcsc << 14) | (cm << 8) | (s1ptw << 7)
+| (wnr << 6) | fsc;
+}
+
static inline uint32_t syn_insn_abort(int same_el, int ea, int s1ptw, int fsc)
{
return (EC_INSNABORT << ARM_EL_EC_SHIFT) | (same_el << ARM_EL_EC_SHIFT)
--
2.34.1
[PATCH for-8.0 v4 03/21] target/arm: SCR_EL3.NS may be RES1
With RME, SEL2 must also be present to support secure state.
The NS bit is RES1 if SEL2 is not present.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/helper.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 66c578c360..20e28d5e09 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1856,6 +1856,9 @@ static void scr_write(CPUARMState *env, const
ARMCPRegInfo *ri, uint64_t value)
}
if (cpu_isar_feature(aa64_sel2, cpu)) {
valid_mask |= SCR_EEL2;
+} else if (cpu_isar_feature(aa64_rme, cpu)) {
+/* With RME and without SEL2, NS is RES1 (R_GSWWH, I_DJJQJ). */
+value |= SCR_NS;
}
if (cpu_isar_feature(aa64_mte, cpu)) {
valid_mask |= SCR_ATA;
--
2.34.1
[PATCH for-8.0 v4 16/21] target/arm: Use get_phys_addr_with_struct for stage2
This fixes a bug in which we failed to initialize
the result attributes properly after the memset.
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index eb3f37495c..95424bca4c 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -37,10 +37,6 @@ typedef struct S1Translate {
void *out_host;
} S1Translate;
-static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
- uint64_t address, MMUAccessType access_type,
- GetPhysAddrResult *result, ARMMMUFaultInfo *fi);
-
static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
target_ulong address,
MMUAccessType access_type,
@@ -2863,12 +2859,7 @@ static bool get_phys_addr_twostage(CPUARMState *env,
S1Translate *ptw,
cacheattrs1 = result->cacheattrs;
memset(result, 0, sizeof(*result));
-if (arm_feature(env, ARM_FEATURE_PMSA)) {
-ret = get_phys_addr_pmsav8(env, ipa, access_type,
- ptw->in_mmu_idx, is_secure, result, fi);
-} else {
-ret = get_phys_addr_lpae(env, ptw, ipa, access_type, result, fi);
-}
+ret = get_phys_addr_with_struct(env, ptw, ipa, access_type, result, fi);
fi->s2addr = ipa;
/* Combine the S1 and S2 perms. */
--
2.34.1
[PATCH for-8.0 v4 10/21] target/arm: Pipe ARMSecuritySpace through ptw.c
Add input and output space members to S1Translate.
Set and adjust them in S1_ptw_translate, and the
various points at which we drop secure state.
Initialize the space in get_phys_addr; for now
leave get_phys_addr_with_secure considering only
secure vs non-secure spaces.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 98 ++--
1 file changed, 78 insertions(+), 20 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 1a51add39c..75f276b520 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -19,11 +19,13 @@
typedef struct S1Translate {
ARMMMUIdx in_mmu_idx;
ARMMMUIdx in_ptw_idx;
+ARMSecuritySpace in_space;
bool in_secure;
bool in_debug;
bool out_secure;
bool out_rw;
bool out_be;
+ARMSecuritySpace out_space;
hwaddr out_virt;
hwaddr out_phys;
void *out_host;
@@ -216,6 +218,7 @@ static bool S2_attrs_are_device(uint64_t hcr, uint8_t attrs)
static bool S1_ptw_translate(CPUARMState *env, S1Translate *ptw,
hwaddr addr, ARMMMUFaultInfo *fi)
{
+ARMSecuritySpace space = ptw->in_space;
bool is_secure = ptw->in_secure;
ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
ARMMMUIdx s2_mmu_idx = ptw->in_ptw_idx;
@@ -232,7 +235,8 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate
*ptw,
if (regime_is_stage2(s2_mmu_idx)) {
S1Translate s2ptw = {
.in_mmu_idx = s2_mmu_idx,
-.in_ptw_idx = is_secure ? ARMMMUIdx_Phys_S : ARMMMUIdx_Phys_NS,
+.in_ptw_idx = arm_space_to_phys(space),
+.in_space = space,
.in_secure = is_secure,
.in_debug = true,
};
@@ -294,10 +298,17 @@ static bool S1_ptw_translate(CPUARMState *env,
S1Translate *ptw,
}
/* Check if page table walk is to secure or non-secure PA space. */
-ptw->out_secure = (is_secure
- && !(pte_secure
+if (is_secure) {
+bool out_secure = !(pte_secure
? env->cp15.vstcr_el2 & VSTCR_SW
-: env->cp15.vtcr_el2 & VTCR_NSW));
+: env->cp15.vtcr_el2 & VTCR_NSW);
+if (!out_secure) {
+is_secure = false;
+space = ARMSS_NonSecure;
+}
+}
+ptw->out_secure = is_secure;
+ptw->out_space = space;
ptw->out_be = regime_translation_big_endian(env, mmu_idx);
return true;
@@ -328,7 +339,10 @@ static uint32_t arm_ldl_ptw(CPUARMState *env, S1Translate
*ptw,
}
} else {
/* Page tables are in MMIO. */
-MemTxAttrs attrs = { .secure = ptw->out_secure };
+MemTxAttrs attrs = {
+.secure = ptw->out_secure,
+.space = ptw->out_space,
+};
AddressSpace *as = arm_addressspace(cs, attrs);
MemTxResult result = MEMTX_OK;
@@ -371,7 +385,10 @@ static uint64_t arm_ldq_ptw(CPUARMState *env, S1Translate
*ptw,
#endif
} else {
/* Page tables are in MMIO. */
-MemTxAttrs attrs = { .secure = ptw->out_secure };
+MemTxAttrs attrs = {
+.secure = ptw->out_secure,
+.space = ptw->out_space,
+};
AddressSpace *as = arm_addressspace(cs, attrs);
MemTxResult result = MEMTX_OK;
@@ -877,6 +894,7 @@ static bool get_phys_addr_v6(CPUARMState *env, S1Translate
*ptw,
* regime, because the attribute will already be non-secure.
*/
result->f.attrs.secure = false;
+result->f.attrs.space = ARMSS_NonSecure;
}
result->f.phys_addr = phys_addr;
return false;
@@ -1581,6 +1599,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
* regime, because the attribute will already be non-secure.
*/
result->f.attrs.secure = false;
+result->f.attrs.space = ARMSS_NonSecure;
}
/* When in aarch64 mode, and BTI is enabled, remember GP in the TLB. */
@@ -2365,6 +2384,7 @@ static bool get_phys_addr_pmsav8(CPUARMState *env,
uint32_t address,
*/
if (sattrs.ns) {
result->f.attrs.secure = false;
+result->f.attrs.space = ARMSS_NonSecure;
} else if (!secure) {
/*
* NS access to S memory must fault.
@@ -2714,6 +2734,7 @@ static bool get_phys_addr_twostage(CPUARMState *env,
S1Translate *ptw,
bool is_secure = ptw->in_secure;
bool ret, ipa_secure, s2walk_secure;
ARMCacheAttrs cacheattrs1;
+ARMSecuritySpace ipa_space, s2walk_space;
bool is_el0;
uint64_t hcr;
@@ -2726,20 +2747,24 @@ static bool get_phys_addr_twostage(CPUARMState *env,
S1Translate *ptw,
ipa = result->f.phys_addr;
ipa_secure = result->f.attrs.secure;
+ipa_space = result->f.attrs.space;
if (is_secure) {
/* Select TCR based on the NS bit fr
[PATCH for-8.0 v4 19/21] target/arm: Implement the granule protection check
Place the check at the end of get_phys_addr_with_struct,
so that we check all physical results.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 249 +++
1 file changed, 232 insertions(+), 17 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 95424bca4c..f3a3a74f74 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -37,11 +37,17 @@ typedef struct S1Translate {
void *out_host;
} S1Translate;
-static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
- target_ulong address,
- MMUAccessType access_type,
- GetPhysAddrResult *result,
- ARMMMUFaultInfo *fi);
+static bool get_phys_addr_nogpc(CPUARMState *env, S1Translate *ptw,
+target_ulong address,
+MMUAccessType access_type,
+GetPhysAddrResult *result,
+ARMMMUFaultInfo *fi);
+
+static bool get_phys_addr_gpc(CPUARMState *env, S1Translate *ptw,
+ target_ulong address,
+ MMUAccessType access_type,
+ GetPhysAddrResult *result,
+ ARMMMUFaultInfo *fi);
/* This mapping is common between ID_AA64MMFR0.PARANGE and TCR_ELx.{I}PS. */
static const uint8_t pamax_map[] = {
@@ -197,6 +203,197 @@ static bool regime_translation_disabled(CPUARMState *env,
ARMMMUIdx mmu_idx,
return (regime_sctlr(env, mmu_idx) & SCTLR_M) == 0;
}
+static bool granule_protection_check(CPUARMState *env, uint64_t paddress,
+ ARMSecuritySpace pspace,
+ ARMMMUFaultInfo *fi)
+{
+MemTxAttrs attrs = {
+.secure = true,
+.space = ARMSS_Root,
+};
+ARMCPU *cpu = env_archcpu(env);
+uint64_t gpccr = env->cp15.gpccr_el3;
+unsigned pps, pgs, l0gptsz, level = 0;
+uint64_t tableaddr, pps_mask, align, entry, index;
+AddressSpace *as;
+MemTxResult result;
+int gpi;
+
+if (!FIELD_EX64(gpccr, GPCCR, GPC)) {
+return true;
+}
+
+/*
+ * GPC Priority 1 (R_GMGRR):
+ * R_JWCSM: If the configuration of GPCCR_EL3 is invalid,
+ * the access fails as GPT walk fault at level 0.
+ */
+
+/*
+ * Configuration of PPS to a value exceeding the implemented
+ * physical address size is invalid.
+ */
+pps = FIELD_EX64(gpccr, GPCCR, PPS);
+if (pps > FIELD_EX64(cpu->isar.id_aa64mmfr0, ID_AA64MMFR0, PARANGE)) {
+goto fault_walk;
+}
+pps = pamax_map[pps];
+pps_mask = MAKE_64BIT_MASK(0, pps);
+
+switch (FIELD_EX64(gpccr, GPCCR, SH)) {
+case 0b10: /* outer shareable */
+break;
+case 0b00: /* non-shareable */
+case 0b11: /* inner shareable */
+/* Inner and Outer non-cacheable requires Outer shareable. */
+if (FIELD_EX64(gpccr, GPCCR, ORGN) == 0 &&
+FIELD_EX64(gpccr, GPCCR, IRGN) == 0) {
+goto fault_walk;
+}
+break;
+default: /* reserved */
+goto fault_walk;
+}
+
+switch (FIELD_EX64(gpccr, GPCCR, PGS)) {
+case 0b00: /* 4KB */
+pgs = 12;
+break;
+case 0b01: /* 64KB */
+pgs = 16;
+break;
+case 0b10: /* 16KB */
+pgs = 14;
+break;
+default: /* reserved */
+goto fault_walk;
+}
+
+/* Note this field is read-only and fixed at reset. */
+l0gptsz = 30 + FIELD_EX64(gpccr, GPCCR, L0GPTSZ);
+
+/*
+ * GPC Priority 2: Secure, Realm or Root address exceeds PPS.
+ * R_CPDSB: A NonSecure physical address input exceeding PPS
+ * does not experience any fault.
+ */
+if (paddress & ~pps_mask) {
+if (pspace == ARMSS_NonSecure) {
+return true;
+}
+goto fault_size;
+}
+
+/* GPC Priority 3: the base address of GPTBR_EL3 exceeds PPS. */
+tableaddr = env->cp15.gptbr_el3 << 12;
+if (tableaddr & ~pps_mask) {
+goto fault_size;
+}
+
+/*
+ * BADDR is aligned per a function of PPS and L0GPTSZ.
+ * These bits of GPTBR_EL3 are RES0, but are not a configuration error,
+ * unlike the RES0 bits of the GPT entries (R_XNKFZ).
+ */
+align = MAX(pps - l0gptsz + 3, 12);
+align = MAKE_64BIT_MASK(0, align);
+tableaddr &= ~align;
+
+as = arm_addressspace(env_cpu(env), attrs);
+
+/* Level 0 lookup. */
+index = extract64(paddress, l0gptsz, pps - l0gptsz);
+tableaddr += index * 8;
+entry = address_space_ldq_le(as, tableaddr, attrs, &result);
+if (result != MEMTX_OK) {
+goto fault_eabt;
+}
+
+switch (extract32(entry, 0, 4)) {
+case 1: /* block descriptor */
+if (entry >> 8) {
+goto fault_walk; /* RES0 bits not
[PATCH for-8.0 v4 08/21] target/arm: Introduce ARMMMUIdx_Phys_{Realm, Root}
With FEAT_RME, there are four physical address spaces.
For now, just define the symbols, and mention them in
the same spots as the other Phys indexes in ptw.c.
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/cpu-param.h | 2 +-
target/arm/cpu.h | 23 +--
target/arm/ptw.c | 10 --
3 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/target/arm/cpu-param.h b/target/arm/cpu-param.h
index 53cac9c89b..8dfd7a0bb6 100644
--- a/target/arm/cpu-param.h
+++ b/target/arm/cpu-param.h
@@ -47,6 +47,6 @@
bool guarded;
#endif
-#define NB_MMU_MODES 12
+#define NB_MMU_MODES 14
#endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index c5fc475cf8..05fd6e61aa 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2865,8 +2865,10 @@ typedef enum ARMMMUIdx {
ARMMMUIdx_Stage2= 9 | ARM_MMU_IDX_A,
/* TLBs with 1-1 mapping to the physical address spaces. */
-ARMMMUIdx_Phys_S= 10 | ARM_MMU_IDX_A,
-ARMMMUIdx_Phys_NS = 11 | ARM_MMU_IDX_A,
+ARMMMUIdx_Phys_S = 10 | ARM_MMU_IDX_A,
+ARMMMUIdx_Phys_NS= 11 | ARM_MMU_IDX_A,
+ARMMMUIdx_Phys_Root = 12 | ARM_MMU_IDX_A,
+ARMMMUIdx_Phys_Realm = 13 | ARM_MMU_IDX_A,
/*
* These are not allocated TLBs and are used only for AT system
@@ -2930,6 +2932,23 @@ typedef enum ARMASIdx {
ARMASIdx_TagS = 3,
} ARMASIdx;
+static inline ARMMMUIdx arm_space_to_phys(ARMSecuritySpace space)
+{
+/* Assert the relative order of the physical mmu indexes. */
+QEMU_BUILD_BUG_ON(ARMSS_Secure != 0);
+QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_NS != ARMMMUIdx_Phys_S + ARMSS_NonSecure);
+QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_Root != ARMMMUIdx_Phys_S + ARMSS_Root);
+QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_Realm != ARMMMUIdx_Phys_S + ARMSS_Realm);
+
+return ARMMMUIdx_Phys_S + space;
+}
+
+static inline ARMSecuritySpace arm_phys_to_space(ARMMMUIdx idx)
+{
+assert(idx >= ARMMMUIdx_Phys_S && idx <= ARMMMUIdx_Phys_Realm);
+return idx - ARMMMUIdx_Phys_S;
+}
+
static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu)
{
/* If all the CLIDR.Ctypem bits are 0 there are no caches, and
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 5aa58c200c..0788c342b8 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -182,8 +182,10 @@ static bool regime_translation_disabled(CPUARMState *env,
ARMMMUIdx mmu_idx,
case ARMMMUIdx_E3:
break;
-case ARMMMUIdx_Phys_NS:
case ARMMMUIdx_Phys_S:
+case ARMMMUIdx_Phys_NS:
+case ARMMMUIdx_Phys_Root:
+case ARMMMUIdx_Phys_Realm:
/* No translation for physical address spaces. */
return true;
@@ -2636,8 +2638,10 @@ static bool get_phys_addr_disabled(CPUARMState *env,
target_ulong address,
switch (mmu_idx) {
case ARMMMUIdx_Stage2:
case ARMMMUIdx_Stage2_S:
-case ARMMMUIdx_Phys_NS:
case ARMMMUIdx_Phys_S:
+case ARMMMUIdx_Phys_NS:
+case ARMMMUIdx_Phys_Root:
+case ARMMMUIdx_Phys_Realm:
break;
default:
@@ -2834,6 +2838,8 @@ static bool get_phys_addr_with_struct(CPUARMState *env,
S1Translate *ptw,
switch (mmu_idx) {
case ARMMMUIdx_Phys_S:
case ARMMMUIdx_Phys_NS:
+case ARMMMUIdx_Phys_Root:
+case ARMMMUIdx_Phys_Realm:
/* Checking Phys early avoids special casing later vs regime_el. */
return get_phys_addr_disabled(env, address, access_type, mmu_idx,
is_secure, result, fi);
--
2.34.1
[PATCH for-8.0 v4 18/21] target/arm: Implement GPC exceptions
Handle GPC Fault types in arm_deliver_fault, reporting as
either a GPC exception at EL3, or falling through to insn
or data aborts at various exception levels.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/cpu.h| 1 +
target/arm/internals.h | 27 +++
target/arm/helper.c | 5 ++
target/arm/tcg/tlb_helper.c | 96 +++--
4 files changed, 126 insertions(+), 3 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 05fd6e61aa..b189efadf8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -57,6 +57,7 @@
#define EXCP_UNALIGNED 22 /* v7M UNALIGNED UsageFault */
#define EXCP_DIVBYZERO 23 /* v7M DIVBYZERO UsageFault */
#define EXCP_VSERR 24
+#define EXCP_GPC25 /* v9 Granule Protection Check Fault */
/* NB: add new EXCP_ defines to the array in arm_log_exception() too */
#define ARMV7M_EXCP_RESET 1
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 680c574717..80aa6d0ede 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -352,14 +352,27 @@ typedef enum ARMFaultType {
ARMFault_ICacheMaint,
ARMFault_QEMU_NSCExec, /* v8M: NS executing in S&NSC memory */
ARMFault_QEMU_SFault, /* v8M: SecureFault INVTRAN, INVEP or AUVIOL */
+ARMFault_GPCFOnWalk,
+ARMFault_GPCFOnOutput,
} ARMFaultType;
+typedef enum ARMGPCF {
+GPCF_None,
+GPCF_AddressSize,
+GPCF_Walk,
+GPCF_EABT,
+GPCF_Fail,
+} ARMGPCF;
+
/**
* ARMMMUFaultInfo: Information describing an ARM MMU Fault
* @type: Type of fault
+ * @gpcf: Subtype of ARMFault_GPCFOn{Walk,Output}.
* @level: Table walk level (for translation, access flag and permission
faults)
* @domain: Domain of the fault address (for non-LPAE CPUs only)
* @s2addr: Address that caused a fault at stage 2
+ * @paddr: physical address that caused a fault for gpc
+ * @paddr_space: physical address space that caused a fault for gpc
* @stage2: True if we faulted at stage 2
* @s1ptw: True if we faulted at stage 2 while doing a stage 1 page-table walk
* @s1ns: True if we faulted on a non-secure IPA while in secure state
@@ -368,7 +381,10 @@ typedef enum ARMFaultType {
typedef struct ARMMMUFaultInfo ARMMMUFaultInfo;
struct ARMMMUFaultInfo {
ARMFaultType type;
+ARMGPCF gpcf;
target_ulong s2addr;
+target_ulong paddr;
+ARMSecuritySpace paddr_space;
int level;
int domain;
bool stage2;
@@ -542,6 +558,17 @@ static inline uint32_t arm_fi_to_lfsc(ARMMMUFaultInfo *fi)
case ARMFault_Exclusive:
fsc = 0x35;
break;
+case ARMFault_GPCFOnWalk:
+assert(fi->level >= -1 && fi->level <= 3);
+if (fi->level < 0) {
+fsc = 0b100011;
+} else {
+fsc = 0b100100 | fi->level;
+}
+break;
+case ARMFault_GPCFOnOutput:
+fsc = 0b101000;
+break;
default:
/* Other faults can't occur in a context that requires a
* long-format status code.
diff --git a/target/arm/helper.c b/target/arm/helper.c
index af71caa7c7..de164c31c3 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10214,6 +10214,7 @@ void arm_log_exception(CPUState *cs)
[EXCP_UNALIGNED] = "v7M UNALIGNED UsageFault",
[EXCP_DIVBYZERO] = "v7M DIVBYZERO UsageFault",
[EXCP_VSERR] = "Virtual SERR",
+[EXCP_GPC] = "Granule Protection Check",
};
if (idx >= 0 && idx < ARRAY_SIZE(excnames)) {
@@ -10945,6 +10946,10 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
}
switch (cs->exception_index) {
+case EXCP_GPC:
+qemu_log_mask(CPU_LOG_INT, "...with MFAR 0x%" PRIx64 "\n",
+ env->cp15.mfar_el3);
+/* fall through */
case EXCP_PREFETCH_ABORT:
case EXCP_DATA_ABORT:
/*
diff --git a/target/arm/tcg/tlb_helper.c b/target/arm/tcg/tlb_helper.c
index 31eb77f7df..b1ad20e68a 100644
--- a/target/arm/tcg/tlb_helper.c
+++ b/target/arm/tcg/tlb_helper.c
@@ -91,17 +91,106 @@ static uint32_t compute_fsr_fsc(CPUARMState *env,
ARMMMUFaultInfo *fi,
return fsr;
}
+static bool report_as_gpc_exception(ARMCPU *cpu, int current_el,
+ARMMMUFaultInfo *fi)
+{
+bool ret;
+
+switch (fi->gpcf) {
+case GPCF_None:
+return false;
+case GPCF_AddressSize:
+case GPCF_Walk:
+case GPCF_EABT:
+/* R_PYTGX: GPT faults are reported as GPC. */
+ret = true;
+break;
+case GPCF_Fail:
+/*
+ * R_BLYPM: A GPF at EL3 is reported as insn or data abort.
+ * R_VBZMW, R_LXHQR: A GPF at EL[0-2] is reported as a GPC
+ * if SCR_EL3.GPF is set, otherwise an insn or data abort.
+ */
+ret = (cpu->env.cp15.scr_el3 & SCR_GPF) && current_el != 3;
+break;
+default:
+g_assert_not_reached();
+}
+
+assert(cpu_isar_feat
[PATCH for-8.0 v4 13/21] target/arm: Handle no-execute for Realm and Root regimes
While Root and Realm may read and write data from other spaces,
neither may execute from other pa spaces.
This happens for Stage1 EL3, EL2, EL2&0, and Stage2 EL1&0.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 52 ++--
1 file changed, 46 insertions(+), 6 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 887c91ed13..2eae7a69cb 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -911,7 +911,7 @@ do_fault:
* @xn: XN (execute-never) bits
* @s1_is_el0: true if this is S2 of an S1+2 walk for EL0
*/
-static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
+static int get_S2prot_noexecute(int s2ap)
{
int prot = 0;
@@ -921,6 +921,12 @@ static int get_S2prot(CPUARMState *env, int s2ap, int xn,
bool s1_is_el0)
if (s2ap & 2) {
prot |= PAGE_WRITE;
}
+return prot;
+}
+
+static int get_S2prot(CPUARMState *env, int s2ap, int xn, bool s1_is_el0)
+{
+int prot = get_S2prot_noexecute(s2ap);
if (cpu_isar_feature(any_tts2uxn, env_archcpu(env))) {
switch (xn) {
@@ -986,9 +992,39 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx,
bool is_aa64,
}
}
-if (out_pa == ARMSS_NonSecure && in_pa == ARMSS_Secure &&
-(env->cp15.scr_el3 & SCR_SIF)) {
-return prot_rw;
+if (in_pa != out_pa) {
+switch (in_pa) {
+case ARMSS_Root:
+/*
+ * R_ZWRVD: permission fault for insn fetched from non-Root,
+ * I_WWBFB: SIF has no effect in EL3.
+ */
+return prot_rw;
+case ARMSS_Realm:
+/*
+ * R_PKTDS: permission fault for insn fetched from non-Realm,
+ * for Realm EL2 or EL2&0. The corresponding fault for EL1&0
+ * happens during any stage2 translation.
+ */
+switch (mmu_idx) {
+case ARMMMUIdx_E2:
+case ARMMMUIdx_E20_0:
+case ARMMMUIdx_E20_2:
+case ARMMMUIdx_E20_2_PAN:
+return prot_rw;
+default:
+break;
+}
+break;
+case ARMSS_Secure:
+if (env->cp15.scr_el3 & SCR_SIF) {
+return prot_rw;
+}
+break;
+default:
+/* Input NonSecure must have output NonSecure. */
+g_assert_not_reached();
+}
}
/* TODO have_wxn should be replaced with
@@ -1565,12 +1601,16 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
/*
* R_GYNXY: For stage2 in Realm security state, bit 55 is NS.
* The bit remains ignored for other security states.
+ * R_YMCSL: Executing an insn fetched from non-Realm causes
+ * a stage2 permission fault.
*/
if (out_space == ARMSS_Realm && extract64(attrs, 55, 1)) {
out_space = ARMSS_NonSecure;
+result->f.prot = get_S2prot_noexecute(ap);
+} else {
+xn = extract64(attrs, 53, 2);
+result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
}
-xn = extract64(attrs, 53, 2);
-result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
} else {
int nse, ns = extract32(attrs, 5, 1);
switch (out_space) {
--
2.34.1
[PATCH for-8.0 v4 11/21] target/arm: NSTable is RES0 for the RME EL3 regime
Test in_space instead of in_secure so that we don't switch
out of Root space. Handle the output space change immediately,
rather than try and combine the NSTable and NS bits later.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 27 ++-
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 75f276b520..0c07e5e24f 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1242,7 +1242,6 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
{
ARMCPU *cpu = env_archcpu(env);
ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
-bool is_secure = ptw->in_secure;
int32_t level;
ARMVAParameters param;
uint64_t ttbr;
@@ -1258,7 +1257,6 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
uint64_t descaddrmask;
bool aarch64 = arm_el_is_aa64(env, el);
uint64_t descriptor, new_descriptor;
-bool nstable;
/* TODO: This code does not support shareability levels. */
if (aarch64) {
@@ -1419,20 +1417,19 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
descaddrmask = MAKE_64BIT_MASK(0, 40);
}
descaddrmask &= ~indexmask_grainsize;
-
-/*
- * Secure accesses start with the page table in secure memory and
- * can be downgraded to non-secure at any step. Non-secure accesses
- * remain non-secure. We implement this by just ORing in the NSTable/NS
- * bits at each step.
- */
-tableattrs = is_secure ? 0 : (1 << 4);
+tableattrs = 0;
next_level:
descaddr |= (address >> (stride * (4 - level))) & indexmask;
descaddr &= ~7ULL;
-nstable = extract32(tableattrs, 4, 1);
-if (nstable && ptw->in_secure) {
+
+/*
+ * Process the NSTable bit from the previous level. This changes
+ * the table address space and the output space from Secure to
+ * NonSecure. With RME, the EL3 translation regime does not change
+ * from Root to NonSecure.
+ */
+if (extract32(tableattrs, 4, 1) && ptw->in_space == ARMSS_Secure) {
/*
* Stage2_S -> Stage2 or Phys_S -> Phys_NS
* Assert the relative order of the secure/non-secure indexes.
@@ -1441,7 +1438,11 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2_S + 1 != ARMMMUIdx_Stage2);
ptw->in_ptw_idx += 1;
ptw->in_secure = false;
+ptw->in_space = ARMSS_NonSecure;
+result->f.attrs.secure = false;
+result->f.attrs.space = ARMSS_NonSecure;
}
+
if (!S1_ptw_translate(env, ptw, descaddr, fi)) {
goto do_fault;
}
@@ -1544,7 +1545,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
*/
attrs = new_descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(50,
14));
if (!regime_is_stage2(mmu_idx)) {
-attrs |= nstable << 5; /* NS */
+attrs |= !ptw->in_secure << 5; /* NS */
if (!param.hpd) {
attrs |= extract64(tableattrs, 0, 2) << 53; /* XN, PXN */
/*
--
2.34.1
[PATCH for-8.0 v4 09/21] target/arm: Remove __attribute__((nonnull)) from ptw.c
This was added in 7e98e21c098 as part of a reorg in which
one of the argument had been legally NULL, and this caught
actual instances. Now that the reorg is complete, this
serves little purpose.
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 0788c342b8..1a51add39c 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -32,15 +32,13 @@ typedef struct S1Translate {
static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
uint64_t address,
MMUAccessType access_type, bool s1_is_el0,
- GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
-__attribute__((nonnull));
+ GetPhysAddrResult *result, ARMMMUFaultInfo *fi);
static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
target_ulong address,
MMUAccessType access_type,
GetPhysAddrResult *result,
- ARMMMUFaultInfo *fi)
-__attribute__((nonnull));
+ ARMMMUFaultInfo *fi);
/* This mapping is common between ID_AA64MMFR0.PARANGE and TCR_ELx.{I}PS. */
static const uint8_t pamax_map[] = {
--
2.34.1
[PATCH for-8.0 v4 12/21] target/arm: Handle Block and Page bits for security space
With Realm security state, bit 55 of a block or page descriptor during
the stage2 walk becomes the NS bit; during the stage1 walk the bit 5
NS bit is RES0. With Root security state, bit 11 of the block or page
descriptor during the stage1 walk becomes the NSE bit.
Rather than collecting an NS bit and applying it later, compute the
output pa space from the input pa space and unconditionally assign.
This means that we no longer need to adjust the output space earlier
for the NSTable bit.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 91 ++--
1 file changed, 73 insertions(+), 18 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 0c07e5e24f..887c91ed13 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -958,12 +958,14 @@ static int get_S2prot(CPUARMState *env, int s2ap, int xn,
bool s1_is_el0)
* @mmu_idx: MMU index indicating required translation regime
* @is_aa64: TRUE if AArch64
* @ap: The 2-bit simple AP (AP[2:1])
- * @ns: NS (non-secure) bit
* @xn: XN (execute-never) bit
* @pxn: PXN (privileged execute-never) bit
+ * @in_pa: The original input pa space
+ * @out_pa: The output pa space, modified by NSTable, NS, and NSE
*/
static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
- int ap, int ns, int xn, int pxn)
+ int ap, int xn, int pxn,
+ ARMSecuritySpace in_pa, ARMSecuritySpace out_pa)
{
bool is_user = regime_is_user(env, mmu_idx);
int prot_rw, user_rw;
@@ -984,7 +986,8 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx,
bool is_aa64,
}
}
-if (ns && arm_is_secure(env) && (env->cp15.scr_el3 & SCR_SIF)) {
+if (out_pa == ARMSS_NonSecure && in_pa == ARMSS_Secure &&
+(env->cp15.scr_el3 & SCR_SIF)) {
return prot_rw;
}
@@ -1252,11 +1255,12 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
int32_t stride;
int addrsize, inputsize, outputsize;
uint64_t tcr = regime_tcr(env, mmu_idx);
-int ap, ns, xn, pxn;
+int ap, xn, pxn;
uint32_t el = regime_el(env, mmu_idx);
uint64_t descaddrmask;
bool aarch64 = arm_el_is_aa64(env, el);
uint64_t descriptor, new_descriptor;
+ARMSecuritySpace out_space;
/* TODO: This code does not support shareability levels. */
if (aarch64) {
@@ -1439,8 +1443,6 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
ptw->in_ptw_idx += 1;
ptw->in_secure = false;
ptw->in_space = ARMSS_NonSecure;
-result->f.attrs.secure = false;
-result->f.attrs.space = ARMSS_NonSecure;
}
if (!S1_ptw_translate(env, ptw, descaddr, fi)) {
@@ -1558,15 +1560,75 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
}
ap = extract32(attrs, 6, 2);
+out_space = ptw->in_space;
if (regime_is_stage2(mmu_idx)) {
-ns = mmu_idx == ARMMMUIdx_Stage2;
+/*
+ * R_GYNXY: For stage2 in Realm security state, bit 55 is NS.
+ * The bit remains ignored for other security states.
+ */
+if (out_space == ARMSS_Realm && extract64(attrs, 55, 1)) {
+out_space = ARMSS_NonSecure;
+}
xn = extract64(attrs, 53, 2);
result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
} else {
-ns = extract32(attrs, 5, 1);
+int nse, ns = extract32(attrs, 5, 1);
+switch (out_space) {
+case ARMSS_Root:
+/*
+ * R_GVZML: Bit 11 becomes the NSE field in the EL3 regime.
+ * R_XTYPW: NSE and NS together select the output pa space.
+ */
+nse = extract32(attrs, 11, 1);
+out_space = (nse << 1) | ns;
+if (out_space == ARMSS_Secure &&
+!cpu_isar_feature(aa64_sel2, cpu)) {
+out_space = ARMSS_NonSecure;
+}
+break;
+case ARMSS_Secure:
+if (ns) {
+out_space = ARMSS_NonSecure;
+}
+break;
+case ARMSS_Realm:
+switch (mmu_idx) {
+case ARMMMUIdx_Stage1_E0:
+case ARMMMUIdx_Stage1_E1:
+case ARMMMUIdx_Stage1_E1_PAN:
+/* I_CZPRF: For Realm EL1&0 stage1, NS bit is RES0. */
+break;
+case ARMMMUIdx_E2:
+case ARMMMUIdx_E20_0:
+case ARMMMUIdx_E20_2:
+case ARMMMUIdx_E20_2_PAN:
+/*
+ * R_LYKFZ, R_WGRZN: For Realm EL2 and EL2&1,
+ * NS changes the output to non-secure space.
+ */
+if (ns) {
+out_space = ARMSS_NonSecure;
+}
+break;
+default:
+g_assert_not_reached();
+}
+break;
+case ARMSS_
[PATCH for-8.0 v4 21/21] NOTFORMERGE hw/arm/virt: Add some memory for Realm Management Monitor
This is arbitrary, but used by the Huawei TF-A test code.
Signed-off-by: Richard Henderson
---
include/hw/arm/virt.h | 2 ++
hw/arm/virt.c | 43 +++
2 files changed, 45 insertions(+)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index e1ddbea96b..5c0c8a67e4 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -86,6 +86,7 @@ enum {
VIRT_ACPI_GED,
VIRT_NVDIMM_ACPI,
VIRT_PVTIME,
+VIRT_RMM_MEM,
VIRT_LOWMEMMAP_LAST,
};
@@ -159,6 +160,7 @@ struct VirtMachineState {
bool virt;
bool ras;
bool mte;
+bool rmm;
bool dtb_randomness;
OnOffAuto acpi;
VirtGICType gic_version;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ac626b3bef..067f16cd77 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -159,6 +159,7 @@ static const MemMapEntry base_memmap[] = {
/* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
[VIRT_PLATFORM_BUS] = { 0x0c00, 0x0200 },
[VIRT_SECURE_MEM] = { 0x0e00, 0x0100 },
+[VIRT_RMM_MEM] ={ 0x0f00, 0x0010 },
[VIRT_PCIE_MMIO] = { 0x1000, 0x2eff },
[VIRT_PCIE_PIO] = { 0x3eff, 0x0001 },
[VIRT_PCIE_ECAM] = { 0x3f00, 0x0100 },
@@ -1602,6 +1603,25 @@ static void create_secure_ram(VirtMachineState *vms,
g_free(nodename);
}
+static void create_rmm_ram(VirtMachineState *vms,
+ MemoryRegion *sysmem,
+ MemoryRegion *tag_sysmem)
+{
+MemoryRegion *rmm_ram = g_new(MemoryRegion, 1);
+hwaddr base = vms->memmap[VIRT_RMM_MEM].base;
+hwaddr size = vms->memmap[VIRT_RMM_MEM].size;
+
+memory_region_init_ram(rmm_ram, NULL, "virt.rmm-ram", size,
+ &error_fatal);
+memory_region_add_subregion(sysmem, base, rmm_ram);
+
+/* do not fill in fdt to hide rmm from normal world guest */
+
+if (tag_sysmem) {
+create_tag_ram(tag_sysmem, base, size, "mach-virt.rmm-tag");
+}
+}
+
static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
{
const VirtMachineState *board = container_of(binfo, VirtMachineState,
@@ -2283,6 +2303,10 @@ static void machvirt_init(MachineState *machine)
machine->ram_size, "mach-virt.tag");
}
+if (vms->rmm) {
+create_rmm_ram(vms, sysmem, tag_sysmem);
+}
+
vms->highmem_ecam &= (!firmware_loaded || aarch64);
create_rtc(vms);
@@ -2562,6 +2586,20 @@ static void virt_set_mte(Object *obj, bool value, Error
**errp)
vms->mte = value;
}
+static bool virt_get_rmm(Object *obj, Error **errp)
+{
+VirtMachineState *vms = VIRT_MACHINE(obj);
+
+return vms->rmm;
+}
+
+static void virt_set_rmm(Object *obj, bool value, Error **errp)
+{
+VirtMachineState *vms = VIRT_MACHINE(obj);
+
+vms->rmm = value;
+}
+
static char *virt_get_gic_version(Object *obj, Error **errp)
{
VirtMachineState *vms = VIRT_MACHINE(obj);
@@ -3115,6 +3153,11 @@ static void virt_machine_class_init(ObjectClass *oc,
void *data)
"guest CPU which implements the ARM "
"Memory Tagging Extension");
+object_class_property_add_bool(oc, "rmm", virt_get_rmm, virt_set_rmm);
+object_class_property_set_description(oc, "rmm",
+ "Set on/off to enable/disable ram "
+ "for the Realm Management Monitor");
+
object_class_property_add_bool(oc, "its", virt_get_its,
virt_set_its);
object_class_property_set_description(oc, "its",
--
2.34.1
[PATCH for-8.0 v4 07/21] target/arm: Adjust the order of Phys and Stage2 ARMMMUIdx
It will be helpful to have ARMMMUIdx_Phys_* to be in the same
relative order as ARMSecuritySpace enumerators. This requires
the adjustment to the nstable check. While there, check for being
in secure state rather than rely on clearing the low bit making
no change to non-secure state.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 12 ++--
target/arm/ptw.c | 12 +---
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 203a3e0046..c5fc475cf8 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2855,18 +2855,18 @@ typedef enum ARMMMUIdx {
ARMMMUIdx_E2= 6 | ARM_MMU_IDX_A,
ARMMMUIdx_E3= 7 | ARM_MMU_IDX_A,
-/* TLBs with 1-1 mapping to the physical address spaces. */
-ARMMMUIdx_Phys_NS = 8 | ARM_MMU_IDX_A,
-ARMMMUIdx_Phys_S= 9 | ARM_MMU_IDX_A,
-
/*
* Used for second stage of an S12 page table walk, or for descriptor
* loads during first stage of an S1 page table walk. Note that both
* are in use simultaneously for SecureEL2: the security state for
* the S2 ptw is selected by the NS bit from the S1 ptw.
*/
-ARMMMUIdx_Stage2= 10 | ARM_MMU_IDX_A,
-ARMMMUIdx_Stage2_S = 11 | ARM_MMU_IDX_A,
+ARMMMUIdx_Stage2_S = 8 | ARM_MMU_IDX_A,
+ARMMMUIdx_Stage2= 9 | ARM_MMU_IDX_A,
+
+/* TLBs with 1-1 mapping to the physical address spaces. */
+ARMMMUIdx_Phys_S= 10 | ARM_MMU_IDX_A,
+ARMMMUIdx_Phys_NS = 11 | ARM_MMU_IDX_A,
/*
* These are not allocated TLBs and are used only for AT system
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 89cc7e2aff..5aa58c200c 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1414,16 +1414,14 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
descaddr |= (address >> (stride * (4 - level))) & indexmask;
descaddr &= ~7ULL;
nstable = extract32(tableattrs, 4, 1);
-if (nstable) {
+if (nstable && ptw->in_secure) {
/*
* Stage2_S -> Stage2 or Phys_S -> Phys_NS
- * Assert that the non-secure idx are even, and relative order.
+ * Assert the relative order of the secure/non-secure indexes.
*/
-QEMU_BUILD_BUG_ON((ARMMMUIdx_Phys_NS & 1) != 0);
-QEMU_BUILD_BUG_ON((ARMMMUIdx_Stage2 & 1) != 0);
-QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_NS + 1 != ARMMMUIdx_Phys_S);
-QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2 + 1 != ARMMMUIdx_Stage2_S);
-ptw->in_ptw_idx &= ~1;
+QEMU_BUILD_BUG_ON(ARMMMUIdx_Phys_S + 1 != ARMMMUIdx_Phys_NS);
+QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2_S + 1 != ARMMMUIdx_Stage2);
+ptw->in_ptw_idx += 1;
ptw->in_secure = false;
}
if (!S1_ptw_translate(env, ptw, descaddr, fi)) {
--
2.34.1
[PATCH for-8.0 v4 15/21] target/arm: Move s1_is_el0 into S1Translate
Instead of passing this to get_phys_addr_lpae, stash it
in the S1Translate structure.
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 27 ---
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 0177dea0cf..eb3f37495c 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -22,6 +22,12 @@ typedef struct S1Translate {
ARMSecuritySpace in_space;
bool in_secure;
bool in_debug;
+/*
+ * If this is stage 2 of a stage 1+2 page table walk, then this must
+ * be true if stage 1 is an EL0 access; otherwise this is ignored.
+ * Stage 2 is indicated by in_mmu_idx set to ARMMMUIdx_Stage2{,_S}.
+ */
+bool in_s1_is_el0;
bool out_secure;
bool out_rw;
bool out_be;
@@ -32,8 +38,7 @@ typedef struct S1Translate {
} S1Translate;
static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
- uint64_t address,
- MMUAccessType access_type, bool s1_is_el0,
+ uint64_t address, MMUAccessType access_type,
GetPhysAddrResult *result, ARMMMUFaultInfo *fi);
static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
@@ -1259,17 +1264,12 @@ static int check_s2_mmu_setup(ARMCPU *cpu, bool
is_aa64, uint64_t tcr,
* @ptw: Current and next stage parameters for the walk.
* @address: virtual address to get physical address for
* @access_type: MMU_DATA_LOAD, MMU_DATA_STORE or MMU_INST_FETCH
- * @s1_is_el0: if @ptw->in_mmu_idx is ARMMMUIdx_Stage2
- * (so this is a stage 2 page table walk),
- * must be true if this is stage 2 of a stage 1+2
- * walk for an EL0 access. If @mmu_idx is anything else,
- * @s1_is_el0 is ignored.
* @result: set on translation success,
* @fi: set to fault info if the translation fails
*/
static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
uint64_t address,
- MMUAccessType access_type, bool s1_is_el0,
+ MMUAccessType access_type,
GetPhysAddrResult *result, ARMMMUFaultInfo *fi)
{
ARMCPU *cpu = env_archcpu(env);
@@ -1602,7 +1602,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
result->f.prot = get_S2prot_noexecute(ap);
} else {
xn = extract64(attrs, 53, 2);
-result->f.prot = get_S2prot(env, ap, xn, s1_is_el0);
+result->f.prot = get_S2prot(env, ap, xn, ptw->in_s1_is_el0);
}
} else {
int nse, ns = extract32(attrs, 5, 1);
@@ -2824,7 +2824,6 @@ static bool get_phys_addr_twostage(CPUARMState *env,
S1Translate *ptw,
bool ret, ipa_secure, s2walk_secure;
ARMCacheAttrs cacheattrs1;
ARMSecuritySpace ipa_space, s2walk_space;
-bool is_el0;
uint64_t hcr;
ret = get_phys_addr_with_struct(env, ptw, address, access_type, result,
fi);
@@ -2849,7 +2848,7 @@ static bool get_phys_addr_twostage(CPUARMState *env,
S1Translate *ptw,
s2walk_space = ipa_space;
}
-is_el0 = ptw->in_mmu_idx == ARMMMUIdx_Stage1_E0;
+ptw->in_s1_is_el0 = ptw->in_mmu_idx == ARMMMUIdx_Stage1_E0;
ptw->in_mmu_idx = s2walk_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
ptw->in_ptw_idx = arm_space_to_phys(s2walk_space);
ptw->in_secure = s2walk_secure;
@@ -2868,8 +2867,7 @@ static bool get_phys_addr_twostage(CPUARMState *env,
S1Translate *ptw,
ret = get_phys_addr_pmsav8(env, ipa, access_type,
ptw->in_mmu_idx, is_secure, result, fi);
} else {
-ret = get_phys_addr_lpae(env, ptw, ipa, access_type,
- is_el0, result, fi);
+ret = get_phys_addr_lpae(env, ptw, ipa, access_type, result, fi);
}
fi->s2addr = ipa;
@@ -3045,8 +3043,7 @@ static bool get_phys_addr_with_struct(CPUARMState *env,
S1Translate *ptw,
}
if (regime_using_lpae_format(env, mmu_idx)) {
-return get_phys_addr_lpae(env, ptw, address, access_type, false,
- result, fi);
+return get_phys_addr_lpae(env, ptw, address, access_type, result, fi);
} else if (arm_feature(env, ARM_FEATURE_V7) ||
regime_sctlr(env, mmu_idx) & SCTLR_XP) {
return get_phys_addr_v6(env, ptw, address, access_type, result, fi);
--
2.34.1
[PATCH for-8.0 v4 06/21] include/exec/memattrs: Add two bits of space to MemTxAttrs
We will need 2 bits to represent ARMSecurityState.
Do not attempt to replace or widen secure, even though it
logically overlaps the new field -- there are uses within
e.g. hw/block/pflash_cfi01.c, which don't know anything
specific about ARM.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
include/exec/memattrs.h | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/exec/memattrs.h b/include/exec/memattrs.h
index 9fb98bc1ef..d04170aa27 100644
--- a/include/exec/memattrs.h
+++ b/include/exec/memattrs.h
@@ -29,10 +29,17 @@ typedef struct MemTxAttrs {
* "didn't specify" if necessary.
*/
unsigned int unspecified:1;
-/* ARM/AMBA: TrustZone Secure access
+/*
+ * ARM/AMBA: TrustZone Secure access
* x86: System Management Mode access
*/
unsigned int secure:1;
+/*
+ * ARM: ArmSecuritySpace. This partially overlaps secure, but it is
+ * easier to have both fields to assist code that does not understand
+ * ARMv9 RME, or no specific knowledge of ARM at all (e.g. pflash).
+ */
+unsigned int space:2;
/* Memory access is usermode (unprivileged) */
unsigned int user:1;
/*
--
2.34.1
[PATCH for-8.0 v4 01/21] target/arm: Add isar_feature_aa64_rme
Add the missing field for ID_AA64PFR0, and the predicate.
Disable it if EL3 is forced off by the board or command-line.
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 6 ++
target/arm/cpu.c | 4
2 files changed, 10 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index cb4e405f04..b046f96e4e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2190,6 +2190,7 @@ FIELD(ID_AA64PFR0, SEL2, 36, 4)
FIELD(ID_AA64PFR0, MPAM, 40, 4)
FIELD(ID_AA64PFR0, AMU, 44, 4)
FIELD(ID_AA64PFR0, DIT, 48, 4)
+FIELD(ID_AA64PFR0, RME, 52, 4)
FIELD(ID_AA64PFR0, CSV2, 56, 4)
FIELD(ID_AA64PFR0, CSV3, 60, 4)
@@ -3808,6 +3809,11 @@ static inline bool isar_feature_aa64_sel2(const
ARMISARegisters *id)
return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, SEL2) != 0;
}
+static inline bool isar_feature_aa64_rme(const ARMISARegisters *id)
+{
+return FIELD_EX64(id->id_aa64pfr0, ID_AA64PFR0, RME) != 0;
+}
+
static inline bool isar_feature_aa64_vh(const ARMISARegisters *id)
{
return FIELD_EX64(id->id_aa64mmfr1, ID_AA64MMFR1, VH) != 0;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 0b333a749f..5092450e5b 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1950,6 +1950,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
cpu->isar.id_dfr0 = FIELD_DP32(cpu->isar.id_dfr0, ID_DFR0, COPSDBG, 0);
cpu->isar.id_aa64pfr0 = FIELD_DP64(cpu->isar.id_aa64pfr0,
ID_AA64PFR0, EL3, 0);
+
+/* Disable the realm management extension, which requires EL3. */
+cpu->isar.id_aa64pfr0 = FIELD_DP64(cpu->isar.id_aa64pfr0,
+ ID_AA64PFR0, RME, 0);
}
if (!cpu->has_el2) {
--
2.34.1
[PATCH for-8.0 v4 04/21] target/arm: Add RME cpregs
This includes GPCCR, GPTBR, MFAR, the TLB flush insns PAALL, PAALLOS,
RPALOS, RPAOS, and the cache flush insns CIPAPA and CIGDPAPA.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/cpu.h| 19 +++
target/arm/helper.c | 83 +
2 files changed, 102 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 230241cf93..8d18d98350 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -541,6 +541,11 @@ typedef struct CPUArchState {
uint64_t fgt_read[2]; /* HFGRTR, HDFGRTR */
uint64_t fgt_write[2]; /* HFGWTR, HDFGWTR */
uint64_t fgt_exec[1]; /* HFGITR */
+
+/* RME registers */
+uint64_t gpccr_el3;
+uint64_t gptbr_el3;
+uint64_t mfar_el3;
} cp15;
struct {
@@ -1043,6 +1048,7 @@ struct ArchCPU {
uint64_t reset_cbar;
uint32_t reset_auxcr;
bool reset_hivecs;
+uint8_t reset_l0gptsz;
/*
* Intermediate values used during property parsing.
@@ -2336,6 +2342,19 @@ FIELD(MVFR1, SIMDFMAC, 28, 4)
FIELD(MVFR2, SIMDMISC, 0, 4)
FIELD(MVFR2, FPMISC, 4, 4)
+FIELD(GPCCR, PPS, 0, 3)
+FIELD(GPCCR, IRGN, 8, 2)
+FIELD(GPCCR, ORGN, 10, 2)
+FIELD(GPCCR, SH, 12, 2)
+FIELD(GPCCR, PGS, 14, 2)
+FIELD(GPCCR, GPC, 16, 1)
+FIELD(GPCCR, GPCP, 17, 1)
+FIELD(GPCCR, L0GPTSZ, 20, 4)
+
+FIELD(MFAR, FPA, 12, 40)
+FIELD(MFAR, NSE, 62, 1)
+FIELD(MFAR, NS, 63, 1)
+
QEMU_BUILD_BUG_ON(ARRAY_SIZE(((ARMCPU *)0)->ccsidr) <=
R_V7M_CSSELR_INDEX_MASK);
/* If adding a feature bit which corresponds to a Linux ELF
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 20e28d5e09..6e9dcb17c4 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6911,6 +6911,83 @@ static const ARMCPRegInfo sme_reginfo[] = {
.access = PL2_RW, .accessfn = access_esm,
.type = ARM_CP_CONST, .resetvalue = 0 },
};
+
+static void tlbi_aa64_paall_write(CPUARMState *env, const ARMCPRegInfo *ri,
+ uint64_t value)
+{
+CPUState *cs = env_cpu(env);
+
+tlb_flush(cs);
+}
+
+static void gpccr_write(CPUARMState *env, const ARMCPRegInfo *ri,
+uint64_t value)
+{
+/* L0GPTSZ is RO; other bits not mentioned are RES0. */
+uint64_t rw_mask = R_GPCCR_PPS_MASK | R_GPCCR_IRGN_MASK |
+R_GPCCR_ORGN_MASK | R_GPCCR_SH_MASK | R_GPCCR_PGS_MASK |
+R_GPCCR_GPC_MASK | R_GPCCR_GPCP_MASK;
+
+env->cp15.gpccr_el3 = (value & rw_mask) | (env->cp15.gpccr_el3 & ~rw_mask);
+}
+
+static void gpccr_reset(CPUARMState *env, const ARMCPRegInfo *ri)
+{
+env->cp15.gpccr_el3 = FIELD_DP64(0, GPCCR, L0GPTSZ,
+ env_archcpu(env)->reset_l0gptsz);
+}
+
+static void tlbi_aa64_paallos_write(CPUARMState *env, const ARMCPRegInfo *ri,
+uint64_t value)
+{
+CPUState *cs = env_cpu(env);
+
+tlb_flush_all_cpus_synced(cs);
+}
+
+static const ARMCPRegInfo rme_reginfo[] = {
+{ .name = "GPCCR_EL3", .state = ARM_CP_STATE_AA64,
+ .opc0 = 3, .opc1 = 6, .crn = 2, .crm = 1, .opc2 = 6,
+ .access = PL3_RW, .writefn = gpccr_write, .resetfn = gpccr_reset,
+ .fieldoffset = offsetof(CPUARMState, cp15.gpccr_el3) },
+{ .name = "GPTBR_EL3", .state = ARM_CP_STATE_AA64,
+ .opc0 = 3, .opc1 = 6, .crn = 2, .crm = 1, .opc2 = 4,
+ .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.gptbr_el3) },
+{ .name = "MFAR_EL3", .state = ARM_CP_STATE_AA64,
+ .opc0 = 3, .opc1 = 6, .crn = 6, .crm = 0, .opc2 = 5,
+ .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.mfar_el3) },
+{ .name = "TLBI_PAALL", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 7, .opc2 = 4,
+ .access = PL3_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_paall_write },
+{ .name = "TLBI_PAALLOS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 1, .opc2 = 4,
+ .access = PL3_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_paallos_write },
+/*
+ * QEMU does not have a way to invalidate by physical address, thus
+ * invalidating a range of physical addresses is accomplished by
+ * flushing all tlb entries in the outer sharable domain,
+ * just like PAALLOS.
+ */
+{ .name = "TLBI_RPALOS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 4, .opc2 = 7,
+ .access = PL3_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_paallos_write },
+{ .name = "TLBI_RPAOS", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 6, .crn = 8, .crm = 4, .opc2 = 3,
+ .access = PL3_W, .type = ARM_CP_NO_RAW,
+ .writefn = tlbi_aa64_paallos_write },
+{ .name = "DC_CIPAPA", .state = ARM_CP_STATE_AA64,
+ .opc0 = 1, .opc1 = 6, .crn = 7, .crm = 14, .opc2 = 1,
+ .access = PL3_W, .type = ARM_CP_NOP },
+};
+
+static const ARMCPRegInfo rme_mte_reginfo[] = {
+{ .name = "DC_CIGDPAPA", .state = ARM_CP_STATE_AA64,
+
[PATCH for-8.0 v4 05/21] target/arm: Introduce ARMSecuritySpace
Introduce both the enumeration and functions to retrieve
the current state, and state outside of EL3.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/cpu.h| 89 ++---
target/arm/helper.c | 60 ++
2 files changed, 127 insertions(+), 22 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 8d18d98350..203a3e0046 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2409,25 +2409,53 @@ static inline int arm_feature(CPUARMState *env, int
feature)
void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp);
-#if !defined(CONFIG_USER_ONLY)
/*
+ * ARM v9 security states.
+ * The ordering of the enumeration corresponds to the low 2 bits
+ * of the GPI value, and (except for Root) the concat of NSE:NS.
+ */
+
+typedef enum ARMSecuritySpace {
+ARMSS_Secure = 0,
+ARMSS_NonSecure = 1,
+ARMSS_Root = 2,
+ARMSS_Realm = 3,
+} ARMSecuritySpace;
+
+/* Return true if @space is secure, in the pre-v9 sense. */
+static inline bool arm_space_is_secure(ARMSecuritySpace space)
+{
+return space == ARMSS_Secure || space == ARMSS_Root;
+}
+
+/* Return the ARMSecuritySpace for @secure, assuming !RME or EL[0-2]. */
+static inline ARMSecuritySpace arm_secure_to_space(bool secure)
+{
+return secure ? ARMSS_Secure : ARMSS_NonSecure;
+}
+
+#if !defined(CONFIG_USER_ONLY)
+/**
+ * arm_security_space_below_el3:
+ * @env: cpu context
+ *
+ * Return the security space of exception levels below EL3, following
+ * an exception return to those levels. Unlike arm_security_space,
+ * this doesn't care about the current EL.
+ */
+ARMSecuritySpace arm_security_space_below_el3(CPUARMState *env);
+
+/**
+ * arm_is_secure_below_el3:
+ * @env: cpu context
+ *
* Return true if exception levels below EL3 are in secure state,
- * or would be following an exception return to that level.
- * Unlike arm_is_secure() (which is always a question about the
- * _current_ state of the CPU) this doesn't care about the current
- * EL or mode.
+ * or would be following an exception return to those levels.
*/
static inline bool arm_is_secure_below_el3(CPUARMState *env)
{
-assert(!arm_feature(env, ARM_FEATURE_M));
-if (arm_feature(env, ARM_FEATURE_EL3)) {
-return !(env->cp15.scr_el3 & SCR_NS);
-} else {
-/* If EL3 is not supported then the secure state is implementation
- * defined, in which case QEMU defaults to non-secure.
- */
-return false;
-}
+ARMSecuritySpace ss = arm_security_space_below_el3(env);
+return ss == ARMSS_Secure;
}
/* Return true if the CPU is AArch64 EL3 or AArch32 Mon */
@@ -2447,16 +2475,23 @@ static inline bool arm_is_el3_or_mon(CPUARMState *env)
return false;
}
-/* Return true if the processor is in secure state */
+/**
+ * arm_security_space:
+ * @env: cpu context
+ *
+ * Return the current security space of the cpu.
+ */
+ARMSecuritySpace arm_security_space(CPUARMState *env);
+
+/**
+ * arm_is_secure:
+ * @env: cpu context
+ *
+ * Return true if the processor is in secure state.
+ */
static inline bool arm_is_secure(CPUARMState *env)
{
-if (arm_feature(env, ARM_FEATURE_M)) {
-return env->v7m.secure;
-}
-if (arm_is_el3_or_mon(env)) {
-return true;
-}
-return arm_is_secure_below_el3(env);
+return arm_space_is_secure(arm_security_space(env));
}
/*
@@ -2475,11 +2510,21 @@ static inline bool arm_is_el2_enabled(CPUARMState *env)
}
#else
+static inline ARMSecuritySpace arm_security_space_below_el3(CPUARMState *env)
+{
+return ARMSS_NonSecure;
+}
+
static inline bool arm_is_secure_below_el3(CPUARMState *env)
{
return false;
}
+static inline ARMSecuritySpace arm_security_space(CPUARMState *env)
+{
+return ARMSS_NonSecure;
+}
+
static inline bool arm_is_secure(CPUARMState *env)
{
return false;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 6e9dcb17c4..af71caa7c7 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -12155,3 +12155,63 @@ void aarch64_sve_change_el(CPUARMState *env, int
old_el,
}
}
#endif
+
+#ifndef CONFIG_USER_ONLY
+ARMSecuritySpace arm_security_space(CPUARMState *env)
+{
+if (arm_feature(env, ARM_FEATURE_M)) {
+return arm_secure_to_space(env->v7m.secure);
+}
+
+/*
+ * If EL3 is not supported then the secure state is implementation
+ * defined, in which case QEMU defaults to non-secure.
+ */
+if (!arm_feature(env, ARM_FEATURE_EL3)) {
+return ARMSS_NonSecure;
+}
+
+/* Check for AArch64 EL3 or AArch32 Mon. */
+if (is_a64(env)) {
+if (extract32(env->pstate, 2, 2) == 3) {
+if (cpu_isar_feature(aa64_rme, env_archcpu(env))) {
+return ARMSS_Root;
+} else {
+return ARMSS_Secure;
+}
+}
+} else {
+if ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_MON) {
[PATCH for-8.0 v4 00/21] target/arm: Implement FEAT_RME
Based-on: 20230227225832.816605-1-richard.hender...@linaro.org
("[PATCH for-8.0 v4 0/4] target/arm: pre-FEAT_RME fixes")
This is based on mainline, without any extra ARMv9-A dependencies
which are still under development. This is good enough to pass
all of the tests within
https://github.com/Huawei/Huawei_CCA_QEMU
Changes for v4:
* Rebase, all except the NOTFORMERGE patches are reviewed.
r~
Richard Henderson (21):
target/arm: Add isar_feature_aa64_rme
target/arm: Update SCR and HCR for RME
target/arm: SCR_EL3.NS may be RES1
target/arm: Add RME cpregs
target/arm: Introduce ARMSecuritySpace
include/exec/memattrs: Add two bits of space to MemTxAttrs
target/arm: Adjust the order of Phys and Stage2 ARMMMUIdx
target/arm: Introduce ARMMMUIdx_Phys_{Realm,Root}
target/arm: Remove __attribute__((nonnull)) from ptw.c
target/arm: Pipe ARMSecuritySpace through ptw.c
target/arm: NSTable is RES0 for the RME EL3 regime
target/arm: Handle Block and Page bits for security space
target/arm: Handle no-execute for Realm and Root regimes
target/arm: Use get_phys_addr_with_struct in S1_ptw_translate
target/arm: Move s1_is_el0 into S1Translate
target/arm: Use get_phys_addr_with_struct for stage2
target/arm: Add GPC syndrome
target/arm: Implement GPC exceptions
target/arm: Implement the granule protection check
NOTFORMERGE target/arm: Enable RME for -cpu max
NOTFORMERGE hw/arm/virt: Add some memory for Realm Management Monitor
include/exec/memattrs.h | 9 +-
include/hw/arm/virt.h | 2 +
target/arm/cpu-param.h | 2 +-
target/arm/cpu.h| 151 --
target/arm/internals.h | 27 ++
target/arm/syndrome.h | 10 +
hw/arm/virt.c | 43 +++
target/arm/cpu.c| 4 +
target/arm/cpu64.c | 37 +++
target/arm/helper.c | 161 +-
target/arm/ptw.c| 580 +---
target/arm/tcg/tlb_helper.c | 96 +-
12 files changed, 971 insertions(+), 151 deletions(-)
--
2.34.1
[PATCH for-8.0 v4 02/21] target/arm: Update SCR and HCR for RME
Define the missing SCR and HCR bits, allow SCR_NSE and {SCR,HCR}_GPF
to be set, and invalidate TLBs when NSE changes.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/cpu.h| 5 +++--
target/arm/helper.c | 10 --
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b046f96e4e..230241cf93 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1650,7 +1650,7 @@ static inline void xpsr_write(CPUARMState *env, uint32_t
val, uint32_t mask)
#define HCR_TERR (1ULL << 36)
#define HCR_TEA (1ULL << 37)
#define HCR_MIOCNCE (1ULL << 38)
-/* RES0 bit 39 */
+#define HCR_TME (1ULL << 39)
#define HCR_APK (1ULL << 40)
#define HCR_API (1ULL << 41)
#define HCR_NV(1ULL << 42)
@@ -1659,7 +1659,7 @@ static inline void xpsr_write(CPUARMState *env, uint32_t
val, uint32_t mask)
#define HCR_NV2 (1ULL << 45)
#define HCR_FWB (1ULL << 46)
#define HCR_FIEN (1ULL << 47)
-/* RES0 bit 48 */
+#define HCR_GPF (1ULL << 48)
#define HCR_TID4 (1ULL << 49)
#define HCR_TICAB (1ULL << 50)
#define HCR_AMVOFFEN (1ULL << 51)
@@ -1724,6 +1724,7 @@ static inline void xpsr_write(CPUARMState *env, uint32_t
val, uint32_t mask)
#define SCR_TRNDR (1ULL << 40)
#define SCR_ENTP2 (1ULL << 41)
#define SCR_GPF (1ULL << 48)
+#define SCR_NSE (1ULL << 62)
#define HSTR_TTEE (1 << 16)
#define HSTR_TJDBX (1 << 17)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index bd12efd392..66c578c360 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -1875,6 +1875,9 @@ static void scr_write(CPUARMState *env, const
ARMCPRegInfo *ri, uint64_t value)
if (cpu_isar_feature(aa64_fgt, cpu)) {
valid_mask |= SCR_FGTEN;
}
+if (cpu_isar_feature(aa64_rme, cpu)) {
+valid_mask |= SCR_NSE | SCR_GPF;
+}
} else {
valid_mask &= ~(SCR_RW | SCR_ST);
if (cpu_isar_feature(aa32_ras, cpu)) {
@@ -1904,10 +1907,10 @@ static void scr_write(CPUARMState *env, const
ARMCPRegInfo *ri, uint64_t value)
env->cp15.scr_el3 = value;
/*
- * If SCR_EL3.NS changes, i.e. arm_is_secure_below_el3, then
+ * If SCR_EL3.{NS,NSE} changes, i.e. change of security state,
* we must invalidate all TLBs below EL3.
*/
-if (changed & SCR_NS) {
+if (changed & (SCR_NS | SCR_NSE)) {
tlb_flush_by_mmuidx(env_cpu(env), (ARMMMUIdxBit_E10_0 |
ARMMMUIdxBit_E20_0 |
ARMMMUIdxBit_E10_1 |
@@ -5655,6 +5658,9 @@ static void do_hcr_write(CPUARMState *env, uint64_t
value, uint64_t valid_mask)
if (cpu_isar_feature(aa64_fwb, cpu)) {
valid_mask |= HCR_FWB;
}
+if (cpu_isar_feature(aa64_rme, cpu)) {
+valid_mask |= HCR_GPF;
+}
}
if (cpu_isar_feature(any_evt, cpu)) {
--
2.34.1
[PATCH for-8.0 v4 0/4] target/arm: pre-FEAT_RME fixes
These were part of the v3 RME patch set, but are bug fixes that should not be deferred until all of RME is ready. r~ Richard Henderson (4): target/arm: Handle m-profile in arm_is_secure target/arm: Stub arm_hcr_el2_eff for m-profile target/arm: Diagnose incorrect usage of arm_is_secure subroutines target/arm: Rewrite check_s2_mmu_setup target/arm/cpu.h| 8 +- target/arm/helper.c | 3 + target/arm/ptw.c| 173 +--- 3 files changed, 107 insertions(+), 77 deletions(-) -- 2.34.1
[PATCH for-8.0 v4 3/4] target/arm: Diagnose incorrect usage of arm_is_secure subroutines
In several places we use arm_is_secure_below_el3 and
arm_is_el3_or_mon separately from arm_is_secure.
These functions make no sense for m-profile, and
would indicate prior incorrect feature testing.
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 7a2f804aeb..cb4e405f04 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2389,7 +2389,8 @@ static inline int arm_feature(CPUARMState *env, int
feature)
void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp);
#if !defined(CONFIG_USER_ONLY)
-/* Return true if exception levels below EL3 are in secure state,
+/*
+ * Return true if exception levels below EL3 are in secure state,
* or would be following an exception return to that level.
* Unlike arm_is_secure() (which is always a question about the
* _current_ state of the CPU) this doesn't care about the current
@@ -2397,6 +2398,7 @@ void arm_cpu_finalize_features(ARMCPU *cpu, Error **errp);
*/
static inline bool arm_is_secure_below_el3(CPUARMState *env)
{
+assert(!arm_feature(env, ARM_FEATURE_M));
if (arm_feature(env, ARM_FEATURE_EL3)) {
return !(env->cp15.scr_el3 & SCR_NS);
} else {
@@ -2410,6 +2412,7 @@ static inline bool arm_is_secure_below_el3(CPUARMState
*env)
/* Return true if the CPU is AArch64 EL3 or AArch32 Mon */
static inline bool arm_is_el3_or_mon(CPUARMState *env)
{
+assert(!arm_feature(env, ARM_FEATURE_M));
if (arm_feature(env, ARM_FEATURE_EL3)) {
if (is_a64(env) && extract32(env->pstate, 2, 2) == 3) {
/* CPU currently in AArch64 state and EL3 */
--
2.34.1
[PATCH for-8.0 v4 1/4] target/arm: Handle m-profile in arm_is_secure
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1421
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 12b1082537..7a2f804aeb 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2426,6 +2426,9 @@ static inline bool arm_is_el3_or_mon(CPUARMState *env)
/* Return true if the processor is in secure state */
static inline bool arm_is_secure(CPUARMState *env)
{
+if (arm_feature(env, ARM_FEATURE_M)) {
+return env->v7m.secure;
+}
if (arm_is_el3_or_mon(env)) {
return true;
}
--
2.34.1
[PATCH for-8.0 v4 2/4] target/arm: Stub arm_hcr_el2_eff for m-profile
M-profile doesn't have HCR_EL2. While we could test features
before each call, zero is a generally safe return value to
disable the code in the caller. This test is required to
avoid an assert in arm_is_secure_below_el3.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/helper.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 14af7ba095..bd12efd392 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5788,6 +5788,9 @@ uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, bool
secure)
uint64_t arm_hcr_el2_eff(CPUARMState *env)
{
+if (arm_feature(env, ARM_FEATURE_M)) {
+return 0;
+}
return arm_hcr_el2_eff_secstate(env, arm_is_secure_below_el3(env));
}
--
2.34.1
[PATCH for-8.0 v4 4/4] target/arm: Rewrite check_s2_mmu_setup
Integrate neighboring code from get_phys_addr_lpae which computed
starting level, as it is easier to validate when doing both at the
same time. Mirror the checks at the start of AArch{64,32}.S2Walk,
especially S2InvalidSL and S2InconsistentSL.
This reverts 49ba115bb74, which was incorrect -- there is nothing
in the ARM pseudocode that depends on TxSZ, i.e. outputsize; the
pseudocode is consistent in referencing PAMax.
Fixes: 49ba115bb74 ("target/arm: Pass outputsize down to check_s2_mmu_setup")
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/ptw.c | 173 ++-
1 file changed, 97 insertions(+), 76 deletions(-)
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index be0cc6bc15..89cc7e2aff 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1081,70 +1081,119 @@ static ARMVAParameters aa32_va_parameters(CPUARMState
*env, uint32_t va,
* check_s2_mmu_setup
* @cpu:ARMCPU
* @is_aa64:True if the translation regime is in AArch64 state
- * @startlevel: Suggested starting level
- * @inputsize: Bitsize of IPAs
+ * @tcr:VTCR_EL2 or VSTCR_EL2
+ * @ds: Effective value of TCR.DS.
+ * @iasize: Bitsize of IPAs
* @stride: Page-table stride (See the ARM ARM)
*
- * Returns true if the suggested S2 translation parameters are OK and
- * false otherwise.
+ * Decode the starting level of the S2 lookup, returning INT_MIN if
+ * the configuration is invalid.
*/
-static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level,
- int inputsize, int stride, int outputsize)
+static int check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, uint64_t tcr,
+ bool ds, int iasize, int stride)
{
-const int grainsize = stride + 3;
-int startsizecheck;
-
-/*
- * Negative levels are usually not allowed...
- * Except for FEAT_LPA2, 4k page table, 52-bit address space, which
- * begins with level -1. Note that previous feature tests will have
- * eliminated this combination if it is not enabled.
- */
-if (level < (inputsize == 52 && stride == 9 ? -1 : 0)) {
-return false;
-}
-
-startsizecheck = inputsize - ((3 - level) * stride + grainsize);
-if (startsizecheck < 1 || startsizecheck > stride + 4) {
-return false;
-}
+int sl0, sl2, startlevel, granulebits, levels;
+int s1_min_iasize, s1_max_iasize;
+sl0 = extract32(tcr, 6, 2);
if (is_aa64) {
+/*
+ * AArch64.S2InvalidTxSZ: While we checked tsz_oob near the top of
+ * get_phys_addr_lpae, that used aa64_va_parameters which apply
+ * to aarch64. If Stage1 is aarch32, the min_txsz is larger.
+ * See AArch64.S2MinTxSZ, where min_tsz is 24, translated to
+ * inputsize is 64 - 24 = 40.
+ */
+if (iasize < 40 && !arm_el_is_aa64(&cpu->env, 1)) {
+goto fail;
+}
+
+/*
+ * AArch64.S2InvalidSL: Interpretation of SL depends on the page size,
+ * so interleave AArch64.S2StartLevel.
+ */
switch (stride) {
-case 13: /* 64KB Pages. */
-if (level == 0 || (level == 1 && outputsize <= 42)) {
-return false;
+case 9: /* 4KB */
+/* SL2 is RES0 unless DS=1 & 4KB granule. */
+sl2 = extract64(tcr, 33, 1);
+if (ds && sl2) {
+if (sl0 != 0) {
+goto fail;
+}
+startlevel = -1;
+} else {
+startlevel = 2 - sl0;
+switch (sl0) {
+case 2:
+if (arm_pamax(cpu) < 44) {
+goto fail;
+}
+break;
+case 3:
+if (!cpu_isar_feature(aa64_st, cpu)) {
+goto fail;
+}
+startlevel = 3;
+break;
+}
}
break;
-case 11: /* 16KB Pages. */
-if (level == 0 || (level == 1 && outputsize <= 40)) {
-return false;
+case 11: /* 16KB */
+switch (sl0) {
+case 2:
+if (arm_pamax(cpu) < 42) {
+goto fail;
+}
+break;
+case 3:
+if (!ds) {
+goto fail;
+}
+break;
}
+startlevel = 3 - sl0;
break;
-case 9: /* 4KB Pages. */
-if (level == 0 && outputsize <= 42) {
-return false;
+case 13: /* 64KB */
+switch (sl0) {
+case 2:
+if (arm_pamax(cpu) < 44) {
+goto fail;
+}
+break;
+case 3:
+goto fail;
}
+startlevel =
RE: [PATCH 19/70] target/hexagon/idef-parser: Use gen_constant for gen_extend_tcg_width_op
> -Original Message-
> From: Richard Henderson
> Sent: Monday, February 27, 2023 3:01 PM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org; qemu-...@nongnu.org; qemu-
> ri...@nongnu.org; qemu-s3...@nongnu.org; jcmvb...@gmail.com;
> kbast...@mail.uni-paderborn.de; ys...@users.sourceforge.jp;
> gaos...@loongson.cn; jiaxun.y...@flygoat.com; a...@rev.ng;
> mrol...@gmail.com; edgar.igles...@gmail.com
> Subject: Re: [PATCH 19/70] target/hexagon/idef-parser: Use gen_constant
> for gen_extend_tcg_width_op
>
> On 2/27/23 11:55, Taylor Simpson wrote:
> >> -HexValue mask = gen_tmp_value(c, locp, mask_str,
> >> - dst_width, UNSIGNED);
> >> +HexValue mask = gen_constant(c, locp, "-1", dst_width,
> >> + UNSIGNED);
> >> OUT(c, locp, "tcg_gen_shr_i", &dst_width, "(",
> >> -&mask, ", ", &mask, ", ", &shift, ");\n");
> >> +&res, ", ", &mask, ", ", &shift, ");\n");
> >> OUT(c, locp, "tcg_gen_and_i", &dst_width, "(",
> >> -&res, ", ", value, ", ", &mask, ");\n");
> >> +&res, ", ", &res, ", ", value, ");\n");
> >
> > What's the advantage of putting the result of the tcg_gen_shr into res
> instead of mask? Is there something in TCG code generation that takes
> advantage?
>
> With this patch, mask is read-only, so a write to it is illegal.
I see.
Reviewed-by: Taylor Simpson
Re: [PATCH] tests/acceptance/virtio-gpu.py: update fedora URL
On 27/2/23 15:52, Erico Nunes wrote: The URL listed previously is no longer valid and that caused the test to skip. Signed-off-by: Erico Nunes --- tests/avocado/virtio-gpu.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/avocado/virtio-gpu.py b/tests/avocado/virtio-gpu.py index 2a249a3a2c..e3b58fe799 100644 --- a/tests/avocado/virtio-gpu.py +++ b/tests/avocado/virtio-gpu.py @@ -36,13 +36,13 @@ class VirtioGPUx86(QemuSystemTest): KERNEL_COMMAND_LINE = "printk.time=0 console=ttyS0 rdinit=/bin/bash" KERNEL_URL = ( -"https://archives.fedoraproject.org/pub/fedora"; +"https://archives.fedoraproject.org/pub/archive/fedora"; "/linux/releases/33/Everything/x86_64/os/images" "/pxeboot/vmlinuz" ) KERNEL_HASH = '1433cfe3f2ffaa44de4ecfb57ec25dc2399cdecf' INITRD_URL = ( -"https://archives.fedoraproject.org/pub/fedora"; +"https://archives.fedoraproject.org/pub/archive/fedora"; "/linux/releases/33/Everything/x86_64/os/images" "/pxeboot/initrd.img" ) Another (unrelated) occurence in docs/system/device-url-syntax.rst.inc: |qemu_system_x86| --drive media=cdrom,file.driver=http,file.url=http://archives.fedoraproject.org/pub/fedora/linux/releases/20/Live/x86_64/Fedora-Live-Desktop-x86_64-20-1.iso,readonly Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v14 15/60] i386/xen: add pc_machine_kvm_type to initialize XEN_EMULATE mode
On 27/2/23 15:28, David Woodhouse wrote:
From: David Woodhouse
The xen_overlay device (and later similar devices for event channels and
grant tables) need to be instantiated. Do this from a kvm_type method on
the PC machine derivatives, since KVM is only way to support Xen emulation
for now.
Signed-off-by: David Woodhouse
Reviewed-by: Paul Durrant
---
hw/i386/pc.c | 11 +++
include/hw/i386/pc.h | 3 +++
2 files changed, 14 insertions(+)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index a7a2ededf9..9eb572b64b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -89,6 +89,7 @@
#include "hw/virtio/virtio-iommu.h"
#include "hw/virtio/virtio-pmem-pci.h"
#include "hw/virtio/virtio-mem-pci.h"
+#include "hw/i386/kvm/xen_overlay.h"
#include "hw/mem/memory-device.h"
#include "sysemu/replay.h"
#include "target/i386/cpu.h"
@@ -1843,6 +1844,16 @@ static void pc_machine_initfn(Object *obj)
cxl_machine_init(obj, &pcms->cxl_devices_state);
}
+int pc_machine_kvm_type(MachineState *machine, const char *kvm_type)
+{
+#ifdef CONFIG_XEN_EMU
+if (xen_mode == XEN_EMULATE) {
+xen_overlay_create();
+}
+#endif
+return 0;
+}
+
static void pc_machine_reset(MachineState *machine, ShutdownCause reason)
{
CPUState *cs;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 66e3d059ef..740497a961 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -291,12 +291,15 @@ extern const size_t pc_compat_1_5_len;
extern GlobalProperty pc_compat_1_4[];
extern const size_t pc_compat_1_4_len;
+extern int pc_machine_kvm_type(MachineState *machine, const char *vm_type);
No 'extern' qualifier for function prototype please.
#define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data)
\
{ \
MachineClass *mc = MACHINE_CLASS(oc); \
optsfn(mc); \
mc->init = initfn; \
+mc->kvm_type = pc_machine_kvm_type; \
} \
static const TypeInfo pc_machine_type_##suffix = { \
.name = namestr TYPE_MACHINE_SUFFIX, \
Re: [PATCH 1/2] docs/about: Deprecate 32-bit x86 hosts and qemu-system-i386
On 27/2/23 21:12, Michael S. Tsirkin wrote: On Mon, Feb 27, 2023 at 11:50:07AM +, Daniel P. Berrangé wrote: I feel like we should have separate deprecation entries for the i686 host support, and for qemu-system-i386 emulator binary, as although they're related they are independant features with differing impact. eg removing qemu-system-i386 affects all host architectures, not merely 32-bit x86 host, so I think we can explain the impact more clearly if we separate them. Removing qemu-system-i386 seems ok to me - I think qemu-system-x86_64 is a superset. Doesn't qemu-system-i386 start the CPU in a different mode that qemu-system-x86_64? Last time we discussed it, we mention adding -32 and -64 CLI flags to maintain compat, and IIRC this flag would add boot code to switch the CPU in 32-b. But then maybe I misunderstood. Thomas said, "CPUs must start in the same mode they start in HW". Removing support for building on 32 bit systems seems like a pity - it's one of a small number of ways to run 64 bit binaries on 32 bit systems, and the maintainance overhead is quite small. In fact, keeping this support around forces correct use of posix APIs such as e.g. PRIx64 which makes the code base more future-proof.
Re: [PATCH v2 06.5/18] hw/ide/piix: Allow using PIIX3-IDE as standalone PCI function
On 20/2/23 10:52, Philippe Mathieu-Daudé wrote:
On 20/2/23 10:10, Gerd Hoffmann wrote:
On Mon, Feb 20, 2023 at 09:00:44AM +0100, Philippe Mathieu-Daudé wrote:
In order to allow Frankenstein uses such plugging a PIIX3
IDE function on a ICH9 chipset (which already exposes AHCI
ports...) as:
$ qemu-system-x86_64 -M q35 -device piix3-ide
add a kludge to automatically wires the IDE IRQs on an ISA
bus exposed by a PCI-to-ISA bridge (usually function #0).
Restrict this kludge to the PIIX3.
Well. On physical hardware you have a config switch in the bios
setup which turns off sata and enables ide instead (i.e. the
chipset implements both and can be configured to expose the one
or the other).
If we want support ide for q35 we should IMHO do something simliar
instead of making piix-ide user-pluggable. We already have -machine
q35,sata={on,off}. We could extend that somehow, by adding
ide={on,off}, or by using storage={sata,ide,off} instead.
This has been discussed now and then in the past and the usual
conclusion was that there is little reason to implement that given
that you can just use the 'pc' machine type. For guests that old
that they can't handle sata storage this is usually the better fit
anyway ...
I think we might not using the same words, but agree on the goal :)
Since this has been discussed in the past, I suppose some users
want this config available. Why are they using this convoluted
config? Could it be due to lack of good documentation?
Do we really need a storage={sata,ide,off} flag? I don't see its
value. Help cloud users to have a sane config?
(old) boards exist with both IDE/SATA and we might want to emulate
some of them, but IMO such boards should be well modeled (Either
in C or later in declarative language) but not automagically created
from CLI.
IOW:
- using PIIX on Q35 (or any machine exposing a PCI bus) is
acceptable, but the chipset should be instantiated as a whole.
So to be clear I'm not against "-M q35 -device piix3", we should
keep that working.
- we shouldn't try to maintain such Frankenstein corner cases which
force us to add complex hacks, hard to remove, which limit us
in implementing new features and cost us a lot of maintenance /
testing time.
So I propose we deprecate this config so we can keep going forward.
(More generally I'd like to not allow instantiating part of chipset).
So there is a design problem here.
- ICH expose ISA bridge (fn0) with ISA IRQs.
- internal ICH SATA fn wires the IRQs 14/15
if we plug a cripple piix3-ide function which lookup for fn0 (ISA
bridge) to wire its IRQs 14/15, we end up having 2 devices able
to raise/lower IRQs while in the BQL iothread.
IOW, one device raise an IRQ, while the other lower the same IRQ...
thus the 1st device IRQ is acked from HW and missed from guest SW.
Daniel tested such config (2 drives used concurrently, one on IDE
and one on SATA) and reported "lost interrupt" from dmesg.
I haven't investigated using a SplitIrq object, but this doesn't
match real hw.
If user want to suffer from poor quality, we should find a different
(valid) config to add IDE drives to Q35 machine. Or maybe it is
a documentation problem, and we should redirect the users to the
best config.
Ref about similar problems:
https://lore.kernel.org/qemu-devel/b8d457d1-40b1-adb5-a2ac-98070f62a...@eik.bme.hu/
https://lore.kernel.org/qemu-devel/y%2fsu8eb2nio0i...@redhat.com/
PD: For the remote-pci it is different because it is based on
PCIe which serializes MSIX IRQs, so no way to overwrite one.
Re: [PATCH v4 0/7] Pegasos2 fixes and audio output support
On 27/2/23 18:47, BALATON Zoltan wrote: On Mon, 27 Feb 2023, Bernhard Beschow wrote: Unfortunately my patches had changes merged in. This now makes it hard to show what really changed (spoiler: nothing that affects behavior). As you probably noticed in the "resend" version of this iteration I split off a patch introducing the priq properties. It belongs to the sub series of the Pegasos2 IRQ fixes which appear unnecessary to me, so I don't want to show up in `git blame` as the author of any of these changes. I attributed it to you because this was really your change which I'm not even sure is legal. Let's avoid such complications by keeping our series separate. Let's cool down a bit. Philippe took some of the sm501 patches in his giant pull request (and a lot of your patches too) now so I'll wait until that lands and hope to get some review for the remaining patches too. Once that pull req is merged I'll rebase the remaining patches and resubmit the series also adding changes for reasonable review comments I get by then. I'm sorry it took me so long, I was expecting these patches to be picked up by other maintainers but everybody is very busy. I know you'll need to rebase and it might be painful. But I did that believing it is the best I could give to the project with my current capacity. Also I don't want to overlap (too much) into other (sub)maintained areas. If you are stuck with a rebase, I can try to help. We'll get to the end of PIIX, if this isn't this release, it will be the next one. I've been waiting for these cleanups since 4 years already, before Hervé Poussineau pushed toward that direction during 3 years. We always hit problems due to PC world inheritance which, as a production system, can not regress. Also I don't want to compete with you guys, I'd really love to work together as team, but I have other responsibilities and sometime I can't spend the time I'd like here. What blocks the PIIX changes is the "q35/ich9/-device piix3" broken config. I'll explain elsewhere why it is broken. The problem is I don't know how to suggest alternative. Bernhard sometimes it is easier to share visions in a 30min call, rather than on a such thread. If you want I'm open for one.
Re: [PATCH] tests: Ensure TAP version is printed before other messages
On 27/2/23 18:40, Richard W.M. Jones wrote: These two tests were failing with this error: stderr: TAP parsing error: version number must be on the first line [...] Unknown TAP version. The first line MUST be `TAP version `. Assuming version 12. This can be fixed by ensuring we always call g_test_init first in the body of main. Thanks: Daniel Berrange, for diagnosing the problem Signed-off-by: Richard W.M. Jones --- tests/qtest/fuzz-lsi53c895a-test.c | 4 ++-- tests/qtest/rtl8139-test.c | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) Tested-by: Philippe Mathieu-Daudé Thanks for tackling this!
Re: [PATCH 19/70] target/hexagon/idef-parser: Use gen_constant for gen_extend_tcg_width_op
On 2/27/23 11:55, Taylor Simpson wrote:
-HexValue mask = gen_tmp_value(c, locp, mask_str,
- dst_width, UNSIGNED);
+HexValue mask = gen_constant(c, locp, "-1", dst_width,
+ UNSIGNED);
OUT(c, locp, "tcg_gen_shr_i", &dst_width, "(",
-&mask, ", ", &mask, ", ", &shift, ");\n");
+&res, ", ", &mask, ", ", &shift, ");\n");
OUT(c, locp, "tcg_gen_and_i", &dst_width, "(",
-&res, ", ", value, ", ", &mask, ");\n");
+&res, ", ", &res, ", ", value, ");\n");
What's the advantage of putting the result of the tcg_gen_shr into res instead
of mask? Is there something in TCG code generation that takes advantage?
With this patch, mask is read-only, so a write to it is illegal.
r~
[PATCH v5] audio/pwaudio.c: Add Pipewire audio backend for QEMU
This commit adds a new audiodev backend to allow QEMU to use Pipewire as
both an audio sink and source. This backend is available on most systems
Add Pipewire entry points for QEMU Pipewire audio backend
Add wrappers for QEMU Pipewire audio backend in qpw_pcm_ops()
qpw_write function returns the current state of the stream to pwaudio
and Writes some data to the server for playback streams using pipewire
spa_ringbuffer implementation.
qpw_read function returns the current state of the stream to pwaudio and
reads some data from the server for capture streams using pipewire
spa_ringbuffer implementation. These functions qpw_write and qpw_read
are called during playback and capture.
Added some functions that convert pw audio formats to QEMU audio format
and vice versa which would be needed in the pipewire audio sink and
source functions qpw_init_in() & qpw_init_out().
These methods that implement playback and recording will create streams
for playback and capture that will start processing and will result in
the on_process callbacks to be called.
Built a connection to the Pipewire sound system server in the
qpw_audio_init() method.
Signed-off-by: Dorinda Bassey
---
v5:
silence output to the console, use pw debug log
use SPDX identifier
fix typo
change version release
audio/audio.c | 3 +
audio/audio_template.h| 4 +
audio/meson.build | 1 +
audio/pwaudio.c | 811 ++
meson.build | 8 +
meson_options.txt | 4 +-
qapi/audio.json | 45 ++
qemu-options.hx | 17 +
scripts/meson-buildoptions.sh | 8 +-
9 files changed, 898 insertions(+), 3 deletions(-)
create mode 100644 audio/pwaudio.c
diff --git a/audio/audio.c b/audio/audio.c
index 4290309d18..aa55e41ad8 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2069,6 +2069,9 @@ void audio_create_pdos(Audiodev *dev)
#ifdef CONFIG_AUDIO_PA
CASE(PA, pa, Pa);
#endif
+#ifdef CONFIG_AUDIO_PIPEWIRE
+CASE(PIPEWIRE, pipewire, Pipewire);
+#endif
#ifdef CONFIG_AUDIO_SDL
CASE(SDL, sdl, Sdl);
#endif
diff --git a/audio/audio_template.h b/audio/audio_template.h
index 42b4712acb..0f02afb921 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -355,6 +355,10 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_,
TYPE)(Audiodev *dev)
case AUDIODEV_DRIVER_PA:
return qapi_AudiodevPaPerDirectionOptions_base(dev->u.pa.TYPE);
#endif
+#ifdef CONFIG_AUDIO_PIPEWIRE
+case AUDIODEV_DRIVER_PIPEWIRE:
+return
qapi_AudiodevPipewirePerDirectionOptions_base(dev->u.pipewire.TYPE);
+#endif
#ifdef CONFIG_AUDIO_SDL
case AUDIODEV_DRIVER_SDL:
return qapi_AudiodevSdlPerDirectionOptions_base(dev->u.sdl.TYPE);
diff --git a/audio/meson.build b/audio/meson.build
index 074ba9..65a49c1a10 100644
--- a/audio/meson.build
+++ b/audio/meson.build
@@ -19,6 +19,7 @@ foreach m : [
['sdl', sdl, files('sdlaudio.c')],
['jack', jack, files('jackaudio.c')],
['sndio', sndio, files('sndioaudio.c')],
+ ['pipewire', pipewire, files('pwaudio.c')],
['spice', spice, files('spiceaudio.c')]
]
if m[1].found()
diff --git a/audio/pwaudio.c b/audio/pwaudio.c
new file mode 100644
index 00..7448f0abd0
--- /dev/null
+++ b/audio/pwaudio.c
@@ -0,0 +1,811 @@
+/*
+ * QEMU Pipewire audio driver
+ *
+ * Copyright (c) 2023 Red Hat Inc.
+ *
+ * Author: Dorinda Bassey
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "audio.h"
+#include
+#include
+#include
+#include
+
+#include
+
+#define AUDIO_CAP "pipewire"
+#define RINGBUFFER_SIZE(1u << 22)
+#define RINGBUFFER_MASK(RINGBUFFER_SIZE - 1)
+#define BUFFER_SAMPLES512
+
+#include "audio_int.h"
+
+enum {
+MODE_SINK,
+MODE_SOURCE
+};
+
+typedef struct pwaudio {
+Audiodev *dev;
+struct pw_thread_loop *thread_loop;
+struct pw_context *context;
+
+struct pw_core *core;
+struct spa_hook core_listener;
+int seq;
+} pwaudio;
+
+typedef struct PWVoice {
+pwaudio *g;
+bool enabled;
+struct pw_stream *stream;
+struct spa_hook stream_listener;
+struct spa_audio_info_raw info;
+uint32_t frame_size;
+struct spa_ringbuffer ring;
+uint8_t buffer[RINGBUFFER_SIZE];
+
+uint32_t mode;
+struct pw_properties *props;
+} PWVoice;
+
+typedef struct PWVoiceOut {
+HWVoiceOut hw;
+PWVoice v;
+} PWVoiceOut;
+
+typedef struct PWVoiceIn {
+HWVoiceIn hw;
+PWVoice v;
+} PWVoiceIn;
+
+static void
+stream_destroy(void *data)
+{
+PWVoice *v = (PWVoice *) data;
+spa_hook_remove(&v->stream_listener);
+v->stream = NULL;
+}
+
+/* output data processing function to read stuffs from the buffer */
+static void
+playback_on_process(void *data)
+{
+PWVoice *v = (PWVoice *) data;
+void *p;
+struct pw_buffer *b;
+struct spa_buffer *buf;
+uint32_t n_frames, req, index, n_b
Re: [PATCH 1/2] hw/vfio/migration: Remove unused 'exec/ram_addr.h' header
Hi Alex, On 27/2/23 17:34, Alex Williamson wrote: On Mon, 27 Feb 2023 16:04:16 + Peter Maydell wrote: On Mon, 27 Feb 2023 at 15:46, Alex Williamson wrote: On Mon, 27 Feb 2023 11:32:57 +0100 Philippe Mathieu-Daudé wrote: Signed-off-by: Philippe Mathieu-Daudé Empty commit logs are a pet peeve of mine, there must be some sort of motivation for the change, something that changed to make this possible, or perhaps why this was never necessary. Thanks, I generally agree, but "this file doesn't actually need to include this header" seems straightforward enough that the commit subject says everything you'd want to say about it. The underlying reason is going to be one of: * this used to be needed, but somewhere along the line the thing the file needs moved to a different header * this used to be needed, but the code in the file changed and no longer uses the thing the header was providing * this was never needed, and the include was just cut-n-pasted from a different file when the file was originally written Tracking down which of those is the case for every single "file is including unnecessary headers" cleanup seems like a lot of work trawling through git histories and doesn't really provide any interesting information. OTOH, not providing any justification shows a lack of due diligence. Even a justification to the extent of "This passes gitlab CI across all architectures after removing the include" or copying from the cover letter to explain that this include is the only reason the file is built per target would be an improvement. I find that empty commit logs create a barrier to entry for participation in a project, there is always some motivation that can help provide a better commit that doesn't force an undue burden on the submitter. Thanks, You are totally right and I apologize for such low quality patch. If I want these cleanups to scale (being done by other) I certainly need to explain why they are useful. Eventually copy/pasting the same reasoning for class of change. OTOH I sometime feel these cleanups are more noise than anything to maintainers, and various end up queued via qemu-trivial@ (which isn't what I expect for trivial@). We have a bit of both, maintainers tired by poor descriptions / justifications, and contributors exhausted to get trivial patch - for their perspective - merged without too much doc. This patch you insist on is a good one-line example of a pattern we should start cleaning: being overzealous with header included pull so many stuff that we shoot on our foot and end in weird corner cases, such here making an object target-dependent when it isn't. target-dependent objects add build complexity, and delay us from the goal of single binary and heterogeneous emulation. Good English explanation for non-English native speaker is not easy as native speaker, but usually the community is opened and bare with that barrier. I'll reword to something work committing and referring to later, when looking at git history. Thanks for insisting in improving us! Phil.
RE: [PATCH 17/70] target/hexagon/idef-parser: Use gen_tmp for gen_pred_assign
> -Original Message-
> From: Richard Henderson
> Sent: Sunday, February 26, 2023 10:42 PM
> To: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org; qemu-...@nongnu.org; qemu-
> ri...@nongnu.org; qemu-s3...@nongnu.org; jcmvb...@gmail.com;
> kbast...@mail.uni-paderborn.de; ys...@users.sourceforge.jp;
> gaos...@loongson.cn; jiaxun.y...@flygoat.com; Taylor Simpson
> ; a...@rev.ng; mrol...@gmail.com;
> edgar.igles...@gmail.com
> Subject: [PATCH 17/70] target/hexagon/idef-parser: Use gen_tmp for
> gen_pred_assign
>
> The allocation is immediately followed by tcg_gen_mov_i32, so the initial
> assignment of zero is discarded.
>
> Signed-off-by: Richard Henderson
> ---
> target/hexagon/idef-parser/parser-helpers.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/hexagon/idef-parser/parser-helpers.c
> b/target/hexagon/idef-parser/parser-helpers.c
> index be979dac86..760e499149 100644
> --- a/target/hexagon/idef-parser/parser-helpers.c
> +++ b/target/hexagon/idef-parser/parser-helpers.c
> @@ -1743,7 +1743,7 @@ void gen_pred_assign(Context *c, YYLTYPE *locp,
> HexValue *left_pred,
> "Predicate assign not allowed in ternary!");
> /* Extract predicate TCGv */
> if (is_direct) {
> -*left_pred = gen_tmp_value(c, locp, "0", 32, UNSIGNED);
> +*left_pred = gen_tmp(c, locp, 32, UNSIGNED);
> }
> /* Extract first 8 bits, and store new predicate value */
> OUT(c, locp, "tcg_gen_mov_i32(", left_pred, ", ", &r, ");\n");
Let's combine this OUT statement with the next one
-OUT(c, locp, "tcg_gen_mov_i32(", left_pred, ", ", &r, ");\n");
-OUT(c, locp, "tcg_gen_andi_i32(", left_pred, ", ", left_pred,
+OUT(c, locp, "tcg_gen_andi_i32(", left_pred, ", ", &r,
", 0xff);\n");
Otherwise,
Reviewed-by: Taylor Simpson
RE: [PATCH 16/70] target/hexagon/idef-parser: Use gen_tmp for LPCFG
> -Original Message- > From: Richard Henderson > Sent: Sunday, February 26, 2023 10:42 PM > To: qemu-devel@nongnu.org > Cc: qemu-...@nongnu.org; qemu-...@nongnu.org; qemu- > ri...@nongnu.org; qemu-s3...@nongnu.org; jcmvb...@gmail.com; > kbast...@mail.uni-paderborn.de; ys...@users.sourceforge.jp; > gaos...@loongson.cn; jiaxun.y...@flygoat.com; Taylor Simpson > ; a...@rev.ng; mrol...@gmail.com; > edgar.igles...@gmail.com > Subject: [PATCH 16/70] target/hexagon/idef-parser: Use gen_tmp for LPCFG > > The GET_USR_FIELD macro initializes the output, so the initial assignment of > zero is discarded. This is the only use of get_tmp_value outside of parser- > helper.c, so make it static. > > Signed-off-by: Richard Henderson > --- > target/hexagon/idef-parser/parser-helpers.h | 6 -- > target/hexagon/idef-parser/parser-helpers.c | 2 +- > target/hexagon/idef-parser/idef-parser.y| 2 +- > 3 files changed, 2 insertions(+), 8 deletions(-) Reviewed-by: Taylor Simpson
RE: [PATCH 18/70] target/hexagon/idef-parser: Use gen_tmp for gen_rvalue_pred
> -Original Message- > From: Richard Henderson > Sent: Sunday, February 26, 2023 10:42 PM > To: qemu-devel@nongnu.org > Cc: qemu-...@nongnu.org; qemu-...@nongnu.org; qemu- > ri...@nongnu.org; qemu-s3...@nongnu.org; jcmvb...@gmail.com; > kbast...@mail.uni-paderborn.de; ys...@users.sourceforge.jp; > gaos...@loongson.cn; jiaxun.y...@flygoat.com; Taylor Simpson > ; a...@rev.ng; mrol...@gmail.com; > edgar.igles...@gmail.com > Subject: [PATCH 18/70] target/hexagon/idef-parser: Use gen_tmp for > gen_rvalue_pred > > The allocation is immediately followed by either tcg_gen_mov_i32 or > gen_read_preg (which contains tcg_gen_mov_i32), so the zero initialization > is immediately discarded. > > Signed-off-by: Richard Henderson > --- > target/hexagon/idef-parser/parser-helpers.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Taylor Simpson
RE: [PATCH 19/70] target/hexagon/idef-parser: Use gen_constant for gen_extend_tcg_width_op
> -Original Message-
> From: Richard Henderson
> Sent: Sunday, February 26, 2023 10:42 PM
> To: qemu-devel@nongnu.org
> Cc: qemu-...@nongnu.org; qemu-...@nongnu.org; qemu-
> ri...@nongnu.org; qemu-s3...@nongnu.org; jcmvb...@gmail.com;
> kbast...@mail.uni-paderborn.de; ys...@users.sourceforge.jp;
> gaos...@loongson.cn; jiaxun.y...@flygoat.com; Taylor Simpson
> ; a...@rev.ng; mrol...@gmail.com;
> edgar.igles...@gmail.com
> Subject: [PATCH 19/70] target/hexagon/idef-parser: Use gen_constant for
> gen_extend_tcg_width_op
>
> We already have a temporary, res, which we can use for the intermediate
> shift result. Simplify the constant to -1 instead of 0xf*f.
> This was the last use of gen_tmp_value, so remove it.
>
> Signed-off-by: Richard Henderson
> ---
> target/hexagon/idef-parser/parser-helpers.c | 30 +++--
> 1 file changed, 3 insertions(+), 27 deletions(-)
>
> diff --git a/target/hexagon/idef-parser/parser-helpers.c
> b/target/hexagon/idef-parser/parser-helpers.c
> index c0e6f2190c..e1a55412c8 100644
> --- a/target/hexagon/idef-parser/parser-helpers.c
> +++ b/target/hexagon/idef-parser/parser-helpers.c
> @@ -1120,15 +1100,11 @@ static
> HexValue gen_extend_tcg_width_op(Context *c,
> OUT(c, locp, "tcg_gen_subfi_i", &dst_width);
> OUT(c, locp, "(", &shift, ", ", &dst_width, ", ", &src_width_m, ");\n");
> if (signedness == UNSIGNED) {
> -const char *mask_str = (dst_width == 32)
> -? "0x"
> -: "0x";
> -HexValue mask = gen_tmp_value(c, locp, mask_str,
> - dst_width, UNSIGNED);
> +HexValue mask = gen_constant(c, locp, "-1", dst_width,
> + UNSIGNED);
> OUT(c, locp, "tcg_gen_shr_i", &dst_width, "(",
> -&mask, ", ", &mask, ", ", &shift, ");\n");
> +&res, ", ", &mask, ", ", &shift, ");\n");
> OUT(c, locp, "tcg_gen_and_i", &dst_width, "(",
> -&res, ", ", value, ", ", &mask, ");\n");
> +&res, ", ", &res, ", ", value, ");\n");
What's the advantage of putting the result of the tcg_gen_shr into res instead
of mask? Is there something in TCG code generation that takes advantage?
> } else {
> OUT(c, locp, "tcg_gen_shl_i", &dst_width, "(",
> &res, ", ", value, ", ", &shift, ");\n");
RE: [PATCH 15/70] target/hexagon: Use tcg_constant_* for gen_constant_from_imm
> -Original Message- > From: Richard Henderson > Sent: Sunday, February 26, 2023 10:42 PM > To: qemu-devel@nongnu.org > Cc: qemu-...@nongnu.org; qemu-...@nongnu.org; qemu- > ri...@nongnu.org; qemu-s3...@nongnu.org; jcmvb...@gmail.com; > kbast...@mail.uni-paderborn.de; ys...@users.sourceforge.jp; > gaos...@loongson.cn; jiaxun.y...@flygoat.com; Taylor Simpson > ; a...@rev.ng; mrol...@gmail.com; > edgar.igles...@gmail.com > Subject: [PATCH 15/70] target/hexagon: Use tcg_constant_* for > gen_constant_from_imm > > Rename from gen_tmp_value_from_imm to match gen_constant vs > gen_tmp. > > Signed-off-by: Richard Henderson > --- > target/hexagon/idef-parser/parser-helpers.c | 15 +++ > 1 file changed, 7 insertions(+), 8 deletions(-) Reviewed-by: Taylor Simpson
Re: [PATCH v2 0/2] hw/cxl: Passthrough HDM decoder emulation
On Mon, Feb 27, 2023 at 03:31:26PM +, Jonathan Cameron wrote: > v2: > - Rebase and pick up tags. > - State prereq patche sets more clearly. > > Mostly sending out again because some of the precursors have been updated > and to fix a typo in a tag given on v1. > > Until now, testing using CXL has relied up always using two root ports > below a host bridge, to work around a current assumption in the Linux > kernel support that, in the single root port case, the implementation will > use the allowed passthrough decoder implementation choice. If that choice > is made all accesses are routed from the host bridge to the single > root port that is present. Effectively we have a pass through decoder > (it is called that in the kernel driver). > > This patch series implements that functionality and makes it the default > See patch 2 for a discussion of why I think we can make this change > without backwards compatibility issues (basically if it didn't work before > who are we breaking by making it work?) > > Whilst this limitation has been known since the initial QEMU patch > postings / kernel CXL region support, Fan Ni ran into it recently reminding > me that we should solve it. > > Note that if you enable hdm_for_passthrough and use a configuration that > would otherwise get a passthrough decoder, the linux kernel will currently > fail to set it up correctly. That's a bug / missing feature in Linux > not an issue with the emulation. > > Based on series "[PATCH v4 00/10] hw/cxl: CXL emulation cleanups and minor > fixes for upstream" > Based on series "[PATCH v6 0/8] hw/cxl: RAS error emulation and injection" > > Based on: Message-Id: 20230206172816.8201-1-jonathan.came...@huawei.com > Based on: Message-Id: 20230227112751.6101-1-jonathan.came...@huawei.com > > Jonathan Cameron (2): > hw/pci: Add pcie_count_ds_port() and pcie_find_port_first() helpers > hw/pxb-cxl: Support passthrough HDM Decoders unless overridden > > hw/cxl/cxl-host.c | 31 > hw/pci-bridge/pci_expander_bridge.c | 44 + > hw/pci/pcie_port.c | 38 + > include/hw/cxl/cxl.h| 1 + > include/hw/cxl/cxl_component.h | 1 + > include/hw/pci/pci_bridge.h | 1 + > include/hw/pci/pcie_port.h | 2 ++ > 7 files changed, 101 insertions(+), 17 deletions(-) > > -- > 2.37.2 > > The code looks good for me. Applied the two patch series mentioned above before applying this patch series on ToT, it is clean. Tested to crete region, create namespace and convert to system RAM and use it with simple command (numactl --membind=1 htop), no issue related to the patch has been found. Also tried with the latest volatile patch (Message-ID: 20230227163157.6621-1-jonathan.came...@huawei.com), creating volatile region and converting to system RAM (automatically) and online and test it with the same command as for pmem above works fine. Fan
[PULL v2 000/125] Buildsys / QOM / QDev / UI patches for 2023-02-27
Hi Peter, I apologize for the size of this request, I was hoping various series would go via other tree, but everybody has been quite busy and the freezing windows is close, so I'm sending a huge mixed patchset here. Since v1: - Dropped "cputlb: Restrict SavedIOTLB to system emulation" patch The following changes since commit e1f9f73ba15e0356ce1aa3317d7bd294f587ab58: Merge tag 'pull-target-arm-20230227' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2023-02-27 14:46:00 +) are available in the Git repository at: https://github.com/philmd/qemu.git tags/buildsys-qom-qdev-ui-20230227 for you to fetch changes up to 23bdd0de97a18e34fe05126fbaf4de540e9eb7b2: ui/cocoa: user friendly characters for release mouse (2023-02-27 22:29:02 +0100) - buildsys - Various header cleaned up (removing pointless headers) - Mark various files/code user/system specific - Make various objects target-independent - Remove tswapN() calls from dump.o - Suggest g_assert_not_reached() instead of assert(0) - qdev / qom - Replace various container_of() by QOM cast macros - Declare some QOM macros using OBJECT_DECLARE_TYPE() - Embed OHCI QOM child in SM501 chipset - hw (ISA & IDE) - add some documentation, improve function names - un-inline, open-code few functions - have ISA API accessing IRQ/DMA prefer ISABus over ISADevice - Demote IDE subsystem maintenance to "Odd Fixes" - ui: Improve Ctrl+Alt hint on Darwin Cocoa BALATON Zoltan (8): hw/audio/ac97: Split off some definitions to a header hw/usb/ohci: Code style fix comments hw/usb/ohci: Code style fix white space errors hw/usb/ohci: Code style fix missing braces and extra parenthesis hw/usb/ohci: Move a function next to where it is used hw/usb/ohci: Add trace points for register access hw/display/sm501: Implement more 2D raster operations hw/display/sm501: Add fallbacks to pixman routines Bernhard Beschow (15): target/loongarch/cpu: Remove unused "sysbus.h" header hw/i386/ich9: Rename Q35_MASK to ICH9_MASK hw/isa/lpc_ich9: Unexport PIRQ functions hw/isa/lpc_ich9: Eliminate ICH9LPCState::isa_bus hw/i2c/smbus_ich9: Move ich9_smb_set_irq() in front of ich9_smbus_realize() hw/i2c/smbus_ich9: Inline ich9_smb_init() and remove it hw/i386/pc_q35: Allow for setting properties before realizing TYPE_ICH9_LPC_DEVICE hw/isa/lpc_ich9: Connect PM stuff to LPC internally hw/isa/lpc_ich9: Remove redundant ich9_lpc_reset() invocation hw/i386/ich9: Remove redundant GSI_NUM_PINS hw: Move ioapic*.h to intc/ hw/i386/ich9: Clean up includes hw: Move ich9.h to southbridge/ hw/ide/pci: Unexport bmdma_active_if() hw/ide/pci: Add PCIIDEState::isa_irq[] Christian Schoenebeck (1): ui/cocoa: user friendly characters for release mouse Fiona Ebner (1): hw/ide/ahci: Trace ncq write command as write instead of read John Snow (1): MAINTAINERS: Mark IDE and Floppy as "Odd Fixes" Mauro Matteo Cascella (1): hw/nubus/nubus-device: Fix memory leak in nubus_device_realize Philippe Mathieu-Daudé (97): cpu: Remove capstone meson dependency cpu: Move breakpoint helpers to common code gdbstub: Use vaddr type for generic insert/remove_breakpoint() API target/cpu: Restrict cpu_get_phys_page_debug() handlers to sysemu target/cpu: Restrict do_transaction_failed() handlers to sysemu target/i386: Remove NEED_CPU_H guard from target-specific headers target/i386/cpu: Remove dead helper_lock() declaration target/i386: Remove x86_cpu_dump_local_apic_state() dead stub target/hppa: Extract FPU helpers to fpu_helper.c target/hppa: Extract system helpers to sys_helper.c target/alpha: Remove obsolete STATUS document target/loongarch/cpu: Restrict "memory.h" header to sysemu target/ppc/internal: Restrict MMU declarations to sysemu target/ppc/kvm: Remove unused "sysbus.h" header target/riscv/cpu: Move Floating-Point fields closer target/sparc/sysemu: Remove pointless CONFIG_USER_ONLY guard target/xtensa/cpu: Include missing "memory.h" header target/tricore: Remove unused fields from CPUTriCoreState qom/object_interfaces: Fix QAPI headers included trace: Do not try to include QMP commands in user emulation binaries exec: Remove unused 'qemu/timer.h' timer tcg: Silent -Wmissing-field-initializers warning tcg/tcg-op-gvec: Remove unused "qemu/main-loop.h" header accel/tcg: Restrict 'qapi-commands-machine.h' to system emulation accel/xen: Remove dead code accel/kvm: Silent -Wmissing-field-initializers warning sysemu/kvm: Remove CONFIG_USER_ONLY guard replay: Extract core API to 'exec/replay-core.h' tests/unit: Restrict machine-smp.c test to system emulation softmmu: Silent -Wmissing-field-initializers w
[PATCH v3 12/14] target/arm: Export arm_v7m_mrs_control
From: David Reiss
Allow the function to be used outside of m_helper.c.
Rename with an "arm_" prefix.
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: David Reiss
[rth: Split out of a larger patch]
Signed-off-by: Richard Henderson
---
target/arm/internals.h| 3 +++
target/arm/tcg/m_helper.c | 6 +++---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 97271117a4..0f2c48ad4b 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1355,6 +1355,9 @@ void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_lpa2_finalize(ARMCPU *cpu, Error **errp);
#endif
+/* Read the CONTROL register as the MRS instruction would. */
+uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure);
+
#ifdef CONFIG_USER_ONLY
static inline void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu) { }
#else
diff --git a/target/arm/tcg/m_helper.c b/target/arm/tcg/m_helper.c
index f94e87e728..03be79e7bf 100644
--- a/target/arm/tcg/m_helper.c
+++ b/target/arm/tcg/m_helper.c
@@ -56,7 +56,7 @@ static uint32_t v7m_mrs_xpsr(CPUARMState *env, uint32_t reg,
unsigned el)
return xpsr_read(env) & mask;
}
-static uint32_t v7m_mrs_control(CPUARMState *env, uint32_t secure)
+uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure)
{
uint32_t value = env->v7m.control[secure];
@@ -93,7 +93,7 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
case 0 ... 7: /* xPSR sub-fields */
return v7m_mrs_xpsr(env, reg, 0);
case 20: /* CONTROL */
-return v7m_mrs_control(env, 0);
+return arm_v7m_mrs_control(env, 0);
default:
/* Unprivileged reads others as zero. */
return 0;
@@ -2465,7 +2465,7 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
case 0 ... 7: /* xPSR sub-fields */
return v7m_mrs_xpsr(env, reg, el);
case 20: /* CONTROL */
-return v7m_mrs_control(env, env->v7m.secure);
+return arm_v7m_mrs_control(env, env->v7m.secure);
case 0x94: /* CONTROL_NS */
/*
* We have to handle this here because unprivileged Secure code
--
2.34.1
[PATCH v3 02/14] target/arm: Unexport arm_gen_dynamic_sysreg_xml
This function is not used outside gdbstub.c.
Reviewed-by: Fabiano Rosas
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 1 -
target/arm/gdbstub.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 12b1082537..32ca6c9a0d 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1116,7 +1116,6 @@ int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t
*buf, int reg);
* Helpers to dynamically generates XML descriptions of the sysregs
* and SVE registers. Returns the number of registers in each set.
*/
-int arm_gen_dynamic_sysreg_xml(CPUState *cpu, int base_reg);
int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
/* Returns the dynamically generated XML for the gdb stub.
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index cf1c01e3cf..52581e9784 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -305,7 +305,7 @@ static void arm_register_sysreg_for_xml(gpointer key,
gpointer value,
}
}
-int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
+static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
GString *s = g_string_new(NULL);
--
2.34.1
[PATCH v3 00/14] target/arm: gdbstub cleanups and additions
This is my pauth enhancements from last year, the corresponding gdb
patches for which are nearing merge. If lore and patchew are to be
believed, I never posted this to the list, only pushed a branch so
that issue #1105 could see it.
Since the cleanups there conflict with the recent m-profile gdbstub
patch set, I set about to resolve those. In the process, I merged
the secure extension code with the sysregs, since they're simply
presenting different views of the same registers.
Changes for v3:
* Drop write paths; there's quite a lot to reorg in v7m_msr, and
it's not clear what should even happen on the exception paths.
Changes for v2:
* Incorporate feedback for pauth.
* Rewrite m-profile systemreg and secext xml.
- Since these are Known to gdb, do not merge the two xml.
- Upstream gdb only knows about {M,P}SP, but David's extension
to include the other registers makes sense and Luis confirms
that it's ok to have extra registers in the two features.
- Continue to share code between the two xml, but separate
out the mapping from gdbstub regno to internal enumeration.
r~
David Reiss (2):
target/arm: Export arm_v7m_mrs_control
target/arm: Export arm_v7m_get_sp_ptr
Richard Henderson (12):
target/arm: Normalize aarch64 gdbstub get/set function names
target/arm: Unexport arm_gen_dynamic_sysreg_xml
target/arm: Move arm_gen_dynamic_svereg_xml to gdbstub64.c
target/arm: Split out output_vector_union_type
target/arm: Simplify register counting in arm_gen_dynamic_svereg_xml
target/arm: Hoist pred_width in arm_gen_dynamic_svereg_xml
target/arm: Fix svep width in arm_gen_dynamic_svereg_xml
target/arm: Add name argument to output_vector_union_type
target/arm: Simplify iteration over bit widths
target/arm: Create pauth_ptr_mask
target/arm: Implement gdbstub pauth extension
target/arm: Implement gdbstub m-profile systemreg and secext
configs/targets/aarch64-linux-user.mak| 2 +-
configs/targets/aarch64-softmmu.mak | 2 +-
configs/targets/aarch64_be-linux-user.mak | 2 +-
target/arm/cpu.h | 9 +-
target/arm/internals.h| 34 ++-
target/arm/gdbstub.c | 278 +-
target/arm/gdbstub64.c| 175 +-
target/arm/tcg/m_helper.c | 90 ---
target/arm/tcg/pauth_helper.c | 26 +-
gdb-xml/aarch64-pauth.xml | 15 ++
10 files changed, 458 insertions(+), 175 deletions(-)
create mode 100644 gdb-xml/aarch64-pauth.xml
--
2.34.1
[PATCH v3 11/14] target/arm: Implement gdbstub pauth extension
The extension is primarily defined by the Linux kernel NT_ARM_PAC_MASK
ptrace register set.
The original gdb feature consists of two masks, data and code, which are
used to mask out the authentication code within a pointer. Following
discussion with Luis Machado, add two more masks in order to support
pointers within the high half of the address space (i.e. TTBR1 vs TTBR0).
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1105
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
configs/targets/aarch64-linux-user.mak| 2 +-
configs/targets/aarch64-softmmu.mak | 2 +-
configs/targets/aarch64_be-linux-user.mak | 2 +-
target/arm/internals.h| 2 ++
target/arm/gdbstub.c | 5
target/arm/gdbstub64.c| 34 +++
gdb-xml/aarch64-pauth.xml | 15 ++
7 files changed, 59 insertions(+), 3 deletions(-)
create mode 100644 gdb-xml/aarch64-pauth.xml
diff --git a/configs/targets/aarch64-linux-user.mak
b/configs/targets/aarch64-linux-user.mak
index db552f1839..ba8bc5fe3f 100644
--- a/configs/targets/aarch64-linux-user.mak
+++ b/configs/targets/aarch64-linux-user.mak
@@ -1,6 +1,6 @@
TARGET_ARCH=aarch64
TARGET_BASE_ARCH=arm
-TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
+TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
gdb-xml/aarch64-pauth.xml
TARGET_HAS_BFLT=y
CONFIG_SEMIHOSTING=y
CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/configs/targets/aarch64-softmmu.mak
b/configs/targets/aarch64-softmmu.mak
index d489e6da83..b4338e9568 100644
--- a/configs/targets/aarch64-softmmu.mak
+++ b/configs/targets/aarch64-softmmu.mak
@@ -1,5 +1,5 @@
TARGET_ARCH=aarch64
TARGET_BASE_ARCH=arm
TARGET_SUPPORTS_MTTCG=y
-TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml
gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
gdb-xml/arm-m-profile-mve.xml
+TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml
gdb-xml/arm-vfp-sysregs.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
gdb-xml/arm-m-profile-mve.xml gdb-xml/aarch64-pauth.xml
TARGET_NEED_FDT=y
diff --git a/configs/targets/aarch64_be-linux-user.mak
b/configs/targets/aarch64_be-linux-user.mak
index dc78044fb1..acb5620cdb 100644
--- a/configs/targets/aarch64_be-linux-user.mak
+++ b/configs/targets/aarch64_be-linux-user.mak
@@ -1,7 +1,7 @@
TARGET_ARCH=aarch64
TARGET_BASE_ARCH=arm
TARGET_BIG_ENDIAN=y
-TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
+TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
gdb-xml/aarch64-pauth.xml
TARGET_HAS_BFLT=y
CONFIG_SEMIHOSTING=y
CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/target/arm/internals.h b/target/arm/internals.h
index c02dd59743..97271117a4 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1347,6 +1347,8 @@ int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray
*buf, int reg);
int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
int aarch64_gdb_set_fpu_reg(CPUARMState *env, uint8_t *buf, int reg);
+int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg);
+int aarch64_gdb_set_pauth_reg(CPUARMState *env, uint8_t *buf, int reg);
void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index bf8aff7824..062c8d447a 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -355,6 +355,11 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
aarch64_gdb_set_fpu_reg,
34, "aarch64-fpu.xml", 0);
}
+if (isar_feature_aa64_pauth(&cpu->isar)) {
+gdb_register_coprocessor(cs, aarch64_gdb_get_pauth_reg,
+ aarch64_gdb_set_pauth_reg,
+ 4, "aarch64-pauth.xml", 0);
+}
#endif
} else {
if (arm_feature(env, ARM_FEATURE_NEON)) {
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 3d9e9e97c8..3bee892fb7 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -210,6 +210,40 @@ int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t
*buf, int reg)
return 0;
}
+int aarch64_gdb_get_pauth_reg(CPUARMState *env, GByteArray *buf, int reg)
+{
+switch (reg) {
+case 0: /* pauth_dmask */
+case 1: /* pauth_cmask */
+case 2: /* pauth_dmask_high */
+case 3: /* pauth_cmask_high */
+/*
+ * Note that older versions of this feature only contained
+ * pauth_{d,c}mask, for use with Linux user proces
Re: Adopting abandoned patch?
It looks like the author didn't include a "Signed off" in their patch draft and it doesn't look like Debian qemu-kvm maintainers ever merged it. Does this change the patch adoption process? Thanks, -Dinah On Mon, Feb 27, 2023 at 4:23 PM Dinah B wrote: > Thanks, here's the original patch: > https://bugs.debian.org/cgi-bin/bugreport.cgi?att=2;bug=621529;filename=multiboot2.patch;msg=15 > > On Mon, Feb 27, 2023 at 4:59 AM Alex Bennée > wrote: > >> >> Dinah B writes: >> >> > Hi, >> > >> > I'm looking to get more involved in contributing to QEMU. I noticed >> that there are some issues in the tracker >> > where a sample patch has been contributed but never got merged, like a >> proposal to add multiboot2 support: >> > https://gitlab.com/qemu-project/qemu/-/issues/389 >> >> I couldn't see a patch attached to the bug report. Is it elsewhere? >> >> > >> > Is another dev allowed to "adopt" the patch as-is, with proper >> attribution to the original dev and drive it to >> > completion/merging (there are some features missing)? Or is "starting >> from scratch" required for legal >> > reasons? >> >> It's certainly possible to pick up a patch from someone else and take it >> forward. Aside from addressing any review comments I think the minimum >> requirement is the authors original Signed-off-by is intact which >> asserts they could contribute code to the project. >> >> -- >> Alex Bennée >> Virtualisation Tech Lead @ Linaro >> >
[PATCH v3 04/14] target/arm: Split out output_vector_union_type
Create a subroutine for creating the union of unions
of the various type sizes that a vector may contain.
Reviewed-by: Fabiano Rosas
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/gdbstub64.c | 83 +++---
1 file changed, 45 insertions(+), 38 deletions(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 59fb5465d5..811833d8de 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -210,44 +210,39 @@ int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t
*buf, int reg)
return 0;
}
-struct TypeSize {
-const char *gdb_type;
-short size;
-char sz, suffix;
-};
-
-static const struct TypeSize vec_lanes[] = {
-/* quads */
-{ "uint128", 128, 'q', 'u' },
-{ "int128", 128, 'q', 's' },
-/* 64 bit */
-{ "ieee_double", 64, 'd', 'f' },
-{ "uint64", 64, 'd', 'u' },
-{ "int64", 64, 'd', 's' },
-/* 32 bit */
-{ "ieee_single", 32, 's', 'f' },
-{ "uint32", 32, 's', 'u' },
-{ "int32", 32, 's', 's' },
-/* 16 bit */
-{ "ieee_half", 16, 'h', 'f' },
-{ "uint16", 16, 'h', 'u' },
-{ "int16", 16, 'h', 's' },
-/* bytes */
-{ "uint8", 8, 'b', 'u' },
-{ "int8", 8, 'b', 's' },
-};
-
-int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
+static void output_vector_union_type(GString *s, int reg_width)
{
-ARMCPU *cpu = ARM_CPU(cs);
-GString *s = g_string_new(NULL);
-DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
+struct TypeSize {
+const char *gdb_type;
+short size;
+char sz, suffix;
+};
+
+static const struct TypeSize vec_lanes[] = {
+/* quads */
+{ "uint128", 128, 'q', 'u' },
+{ "int128", 128, 'q', 's' },
+/* 64 bit */
+{ "ieee_double", 64, 'd', 'f' },
+{ "uint64", 64, 'd', 'u' },
+{ "int64", 64, 'd', 's' },
+/* 32 bit */
+{ "ieee_single", 32, 's', 'f' },
+{ "uint32", 32, 's', 'u' },
+{ "int32", 32, 's', 's' },
+/* 16 bit */
+{ "ieee_half", 16, 'h', 'f' },
+{ "uint16", 16, 'h', 'u' },
+{ "int16", 16, 'h', 's' },
+/* bytes */
+{ "uint8", 8, 'b', 'u' },
+{ "int8", 8, 'b', 's' },
+};
+
+static const char suf[] = { 'q', 'd', 's', 'h', 'b' };
+
g_autoptr(GString) ts = g_string_new("");
-int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
-info->num = 0;
-g_string_printf(s, "");
-g_string_append_printf(s, "");
-g_string_append_printf(s, "");
+int i, j, bits;
/* First define types and totals in a whole VL */
for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
@@ -263,7 +258,6 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
* 8 bits.
*/
for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
-const char suf[] = { 'q', 'd', 's', 'h', 'b' };
g_string_append_printf(s, "", suf[i]);
for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
if (vec_lanes[j].size == bits) {
@@ -277,11 +271,24 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
/* And now the final union of unions */
g_string_append(s, "");
for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
-const char suf[] = { 'q', 'd', 's', 'h', 'b' };
g_string_append_printf(s, "",
suf[i], suf[i]);
}
g_string_append(s, "");
+}
+
+int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+GString *s = g_string_new(NULL);
+DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
+int i, reg_width = (cpu->sve_max_vq * 128);
+info->num = 0;
+g_string_printf(s, "");
+g_string_append_printf(s, "");
+g_string_append_printf(s, "");
+
+output_vector_union_type(s, reg_width);
/* Finally the sve prefix type */
g_string_append_printf(s,
--
2.34.1
[PATCH v3 10/14] target/arm: Create pauth_ptr_mask
Keep the logic for pauth within pauth_helper.c, and expose
a helper function for use with the gdbstub pac extension.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/internals.h| 10 ++
target/arm/tcg/pauth_helper.c | 26 ++
2 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 725244d72d..c02dd59743 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1366,6 +1366,16 @@ int exception_target_el(CPUARMState *env);
bool arm_singlestep_active(CPUARMState *env);
bool arm_generate_debug_exceptions(CPUARMState *env);
+/**
+ * pauth_ptr_mask:
+ * @env: cpu context
+ * @ptr: selects between TTBR0 and TTBR1
+ * @data: selects between TBI and TBID
+ *
+ * Return a mask of the bits of @ptr that contain the authentication code.
+ */
+uint64_t pauth_ptr_mask(CPUARMState *env, uint64_t ptr, bool data);
+
/* Add the cpreg definitions for debug related system registers */
void define_debug_regs(ARMCPU *cpu);
diff --git a/target/arm/tcg/pauth_helper.c b/target/arm/tcg/pauth_helper.c
index d0483bf051..20f347332d 100644
--- a/target/arm/tcg/pauth_helper.c
+++ b/target/arm/tcg/pauth_helper.c
@@ -339,14 +339,32 @@ static uint64_t pauth_addpac(CPUARMState *env, uint64_t
ptr, uint64_t modifier,
return pac | ext | ptr;
}
-static uint64_t pauth_original_ptr(uint64_t ptr, ARMVAParameters param)
+static uint64_t pauth_ptr_mask_internal(ARMVAParameters param)
{
-/* Note that bit 55 is used whether or not the regime has 2 ranges. */
-uint64_t extfield = sextract64(ptr, 55, 1);
int bot_pac_bit = 64 - param.tsz;
int top_pac_bit = 64 - 8 * param.tbi;
-return deposit64(ptr, bot_pac_bit, top_pac_bit - bot_pac_bit, extfield);
+return MAKE_64BIT_MASK(bot_pac_bit, top_pac_bit - bot_pac_bit);
+}
+
+static uint64_t pauth_original_ptr(uint64_t ptr, ARMVAParameters param)
+{
+uint64_t mask = pauth_ptr_mask_internal(param);
+
+/* Note that bit 55 is used whether or not the regime has 2 ranges. */
+if (extract64(ptr, 55, 1)) {
+return ptr | mask;
+} else {
+return ptr & ~mask;
+}
+}
+
+uint64_t pauth_ptr_mask(CPUARMState *env, uint64_t ptr, bool data)
+{
+ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
+ARMVAParameters param = aa64_va_parameters(env, ptr, mmu_idx, data);
+
+return pauth_ptr_mask_internal(param);
}
static uint64_t pauth_auth(CPUARMState *env, uint64_t ptr, uint64_t modifier,
--
2.34.1
[PATCH v3 08/14] target/arm: Add name argument to output_vector_union_type
This will make the function usable between SVE and SME.
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
target/arm/gdbstub64.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index d0e1305f6f..36166bf81e 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -210,7 +210,8 @@ int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf,
int reg)
return 0;
}
-static void output_vector_union_type(GString *s, int reg_width)
+static void output_vector_union_type(GString *s, int reg_width,
+ const char *name)
{
struct TypeSize {
const char *gdb_type;
@@ -240,39 +241,38 @@ static void output_vector_union_type(GString *s, int
reg_width)
};
static const char suf[] = { 'q', 'd', 's', 'h', 'b' };
-
-g_autoptr(GString) ts = g_string_new("");
int i, j, bits;
/* First define types and totals in a whole VL */
for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
-int count = reg_width / vec_lanes[i].size;
-g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
g_string_append_printf(s,
- "",
- ts->str, vec_lanes[i].gdb_type, count);
+ "",
+ name, vec_lanes[i].sz, vec_lanes[i].suffix,
+ vec_lanes[i].gdb_type, reg_width /
vec_lanes[i].size);
}
+
/*
* Now define a union for each size group containing unsigned and
* signed and potentially float versions of each size from 128 to
* 8 bits.
*/
for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
-g_string_append_printf(s, "", suf[i]);
+g_string_append_printf(s, "", name, suf[i]);
for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
if (vec_lanes[j].size == bits) {
-g_string_append_printf(s, "",
- vec_lanes[j].suffix,
+g_string_append_printf(s, "",
+ vec_lanes[j].suffix, name,
vec_lanes[j].sz, vec_lanes[j].suffix);
}
}
g_string_append(s, "");
}
+
/* And now the final union of unions */
-g_string_append(s, "");
+g_string_append_printf(s, "", name);
for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
-g_string_append_printf(s, "",
- suf[i], suf[i]);
+g_string_append_printf(s, "",
+ suf[i], name, suf[i]);
}
g_string_append(s, "");
}
@@ -292,7 +292,7 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int
orig_base_reg)
g_string_append_printf(s, "");
/* Create the vector union type. */
-output_vector_union_type(s, reg_width);
+output_vector_union_type(s, reg_width, "svev");
/* Create the predicate vector type. */
g_string_append_printf(s,
--
2.34.1
[PATCH v3 01/14] target/arm: Normalize aarch64 gdbstub get/set function names
Make the form of the function names between fp and sve the same:
- arm_gdb_*_svereg -> aarch64_gdb_*_sve_reg.
- aarch64_fpu_gdb_*_reg -> aarch64_gdb_*_fpu_reg.
Reviewed-by: Fabiano Rosas
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
target/arm/internals.h | 8
target/arm/gdbstub.c | 9 +
target/arm/gdbstub64.c | 8
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 680c574717..fa21393ebc 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1342,10 +1342,10 @@ static inline uint64_t pmu_counter_mask(CPUARMState
*env)
}
#ifdef TARGET_AARCH64
-int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg);
-int arm_gdb_set_svereg(CPUARMState *env, uint8_t *buf, int reg);
-int aarch64_fpu_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg);
-int aarch64_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg);
+int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
+int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
+int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
+int aarch64_gdb_set_fpu_reg(CPUARMState *env, uint8_t *buf, int reg);
void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 2f806512d0..cf1c01e3cf 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -466,12 +466,13 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
*/
#ifdef TARGET_AARCH64
if (isar_feature_aa64_sve(&cpu->isar)) {
-gdb_register_coprocessor(cs, arm_gdb_get_svereg,
arm_gdb_set_svereg,
- arm_gen_dynamic_svereg_xml(cs,
cs->gdb_num_regs),
+int nreg = arm_gen_dynamic_svereg_xml(cs, cs->gdb_num_regs);
+gdb_register_coprocessor(cs, aarch64_gdb_get_sve_reg,
+ aarch64_gdb_set_sve_reg, nreg,
"sve-registers.xml", 0);
} else {
-gdb_register_coprocessor(cs, aarch64_fpu_gdb_get_reg,
- aarch64_fpu_gdb_set_reg,
+gdb_register_coprocessor(cs, aarch64_gdb_get_fpu_reg,
+ aarch64_gdb_set_fpu_reg,
34, "aarch64-fpu.xml", 0);
}
#endif
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 07a6746944..c598cb0375 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -72,7 +72,7 @@ int aarch64_cpu_gdb_write_register(CPUState *cs, uint8_t
*mem_buf, int n)
return 0;
}
-int aarch64_fpu_gdb_get_reg(CPUARMState *env, GByteArray *buf, int reg)
+int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg)
{
switch (reg) {
case 0 ... 31:
@@ -92,7 +92,7 @@ int aarch64_fpu_gdb_get_reg(CPUARMState *env, GByteArray
*buf, int reg)
}
}
-int aarch64_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf, int reg)
+int aarch64_gdb_set_fpu_reg(CPUARMState *env, uint8_t *buf, int reg)
{
switch (reg) {
case 0 ... 31:
@@ -116,7 +116,7 @@ int aarch64_fpu_gdb_set_reg(CPUARMState *env, uint8_t *buf,
int reg)
}
}
-int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf, int reg)
+int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg)
{
ARMCPU *cpu = env_archcpu(env);
@@ -164,7 +164,7 @@ int arm_gdb_get_svereg(CPUARMState *env, GByteArray *buf,
int reg)
return 0;
}
-int arm_gdb_set_svereg(CPUARMState *env, uint8_t *buf, int reg)
+int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg)
{
ARMCPU *cpu = env_archcpu(env);
--
2.34.1
[PATCH v3 09/14] target/arm: Simplify iteration over bit widths
Order suf[] by the log8 of the width.
Use ARRAY_SIZE instead of hard-coding 128.
This changes the order of the union definitions,
but retains the order of the union-of-union members.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/gdbstub64.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 36166bf81e..3d9e9e97c8 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -240,8 +240,8 @@ static void output_vector_union_type(GString *s, int
reg_width,
{ "int8", 8, 'b', 's' },
};
-static const char suf[] = { 'q', 'd', 's', 'h', 'b' };
-int i, j, bits;
+static const char suf[] = { 'b', 'h', 's', 'd', 'q' };
+int i, j;
/* First define types and totals in a whole VL */
for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
@@ -256,7 +256,9 @@ static void output_vector_union_type(GString *s, int
reg_width,
* signed and potentially float versions of each size from 128 to
* 8 bits.
*/
-for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+for (i = 0; i < ARRAY_SIZE(suf); i++) {
+int bits = 8 << i;
+
g_string_append_printf(s, "", name, suf[i]);
for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
if (vec_lanes[j].size == bits) {
@@ -270,7 +272,7 @@ static void output_vector_union_type(GString *s, int
reg_width,
/* And now the final union of unions */
g_string_append_printf(s, "", name);
-for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
+for (i = ARRAY_SIZE(suf) - 1; i >= 0; i--) {
g_string_append_printf(s, "",
suf[i], name, suf[i]);
}
--
2.34.1
[PATCH v3 03/14] target/arm: Move arm_gen_dynamic_svereg_xml to gdbstub64.c
The function is only used for aarch64, so move it to the
file that has the other aarch64 gdbstub stuff. Move the
declaration to internals.h.
Reviewed-by: Fabiano Rosas
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 6 ---
target/arm/internals.h | 1 +
target/arm/gdbstub.c | 120 -
target/arm/gdbstub64.c | 118
4 files changed, 119 insertions(+), 126 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 32ca6c9a0d..059fe62eaa 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1112,12 +1112,6 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cpu,
vaddr addr,
int arm_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
int arm_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
-/*
- * Helpers to dynamically generates XML descriptions of the sysregs
- * and SVE registers. Returns the number of registers in each set.
- */
-int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
-
/* Returns the dynamically generated XML for the gdb stub.
* Returns a pointer to the XML contents for the specified XML file or NULL
* if the XML name doesn't match the predefined one.
diff --git a/target/arm/internals.h b/target/arm/internals.h
index fa21393ebc..725244d72d 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1342,6 +1342,7 @@ static inline uint64_t pmu_counter_mask(CPUARMState *env)
}
#ifdef TARGET_AARCH64
+int arm_gen_dynamic_svereg_xml(CPUState *cpu, int base_reg);
int aarch64_gdb_get_sve_reg(CPUARMState *env, GByteArray *buf, int reg);
int aarch64_gdb_set_sve_reg(CPUARMState *env, uint8_t *buf, int reg);
int aarch64_gdb_get_fpu_reg(CPUARMState *env, GByteArray *buf, int reg);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 52581e9784..bf8aff7824 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -322,126 +322,6 @@ static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int
base_reg)
return cpu->dyn_sysreg_xml.num;
}
-struct TypeSize {
-const char *gdb_type;
-int size;
-const char sz, suffix;
-};
-
-static const struct TypeSize vec_lanes[] = {
-/* quads */
-{ "uint128", 128, 'q', 'u' },
-{ "int128", 128, 'q', 's' },
-/* 64 bit */
-{ "ieee_double", 64, 'd', 'f' },
-{ "uint64", 64, 'd', 'u' },
-{ "int64", 64, 'd', 's' },
-/* 32 bit */
-{ "ieee_single", 32, 's', 'f' },
-{ "uint32", 32, 's', 'u' },
-{ "int32", 32, 's', 's' },
-/* 16 bit */
-{ "ieee_half", 16, 'h', 'f' },
-{ "uint16", 16, 'h', 'u' },
-{ "int16", 16, 'h', 's' },
-/* bytes */
-{ "uint8", 8, 'b', 'u' },
-{ "int8", 8, 'b', 's' },
-};
-
-
-int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
-{
-ARMCPU *cpu = ARM_CPU(cs);
-GString *s = g_string_new(NULL);
-DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
-g_autoptr(GString) ts = g_string_new("");
-int i, j, bits, reg_width = (cpu->sve_max_vq * 128);
-info->num = 0;
-g_string_printf(s, "");
-g_string_append_printf(s, "");
-g_string_append_printf(s, "");
-
-/* First define types and totals in a whole VL */
-for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
-int count = reg_width / vec_lanes[i].size;
-g_string_printf(ts, "svev%c%c", vec_lanes[i].sz, vec_lanes[i].suffix);
-g_string_append_printf(s,
- "",
- ts->str, vec_lanes[i].gdb_type, count);
-}
-/*
- * Now define a union for each size group containing unsigned and
- * signed and potentially float versions of each size from 128 to
- * 8 bits.
- */
-for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
-const char suf[] = { 'q', 'd', 's', 'h', 'b' };
-g_string_append_printf(s, "", suf[i]);
-for (j = 0; j < ARRAY_SIZE(vec_lanes); j++) {
-if (vec_lanes[j].size == bits) {
-g_string_append_printf(s, "",
- vec_lanes[j].suffix,
- vec_lanes[j].sz, vec_lanes[j].suffix);
-}
-}
-g_string_append(s, "");
-}
-/* And now the final union of unions */
-g_string_append(s, "");
-for (bits = 128, i = 0; bits >= 8; bits /= 2, i++) {
-const char suf[] = { 'q', 'd', 's', 'h', 'b' };
-g_string_append_printf(s, "",
- suf[i], suf[i]);
-}
-g_string_append(s, "");
-
-/* Finally the sve prefix type */
-g_string_append_printf(s,
- "",
- reg_width / 8);
-
-/* Then define each register in parts for each vq */
-for (i = 0; i < 32; i++) {
-g_string_append_printf(s,
- "",
- i, reg_width, base_reg++);
-info->num++;
-}
-/* fpscr &
[PATCH v3 07/14] target/arm: Fix svep width in arm_gen_dynamic_svereg_xml
Define svep based on the size of the predicates,
not the primary vector registers.
Reviewed-by: Peter Maydell
Signed-off-by: Richard Henderson
---
target/arm/gdbstub64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 895e19f084..d0e1305f6f 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -297,7 +297,7 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int
orig_base_reg)
/* Create the predicate vector type. */
g_string_append_printf(s,
"",
- reg_width / 8);
+ pred_width / 8);
/* Define the vector registers. */
for (i = 0; i < 32; i++) {
--
2.34.1
[PATCH v3 06/14] target/arm: Hoist pred_width in arm_gen_dynamic_svereg_xml
Reviewed-by: Fabiano Rosas Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/arm/gdbstub64.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c index 070ba20d99..895e19f084 100644 --- a/target/arm/gdbstub64.c +++ b/target/arm/gdbstub64.c @@ -283,6 +283,7 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg) GString *s = g_string_new(NULL); DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml; int reg_width = cpu->sve_max_vq * 128; +int pred_width = cpu->sve_max_vq * 16; int base_reg = orig_base_reg; int i; @@ -319,13 +320,13 @@ int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg) g_string_append_printf(s, "", - i, cpu->sve_max_vq * 16, base_reg++); + i, pred_width, base_reg++); } g_string_append_printf(s, "", - cpu->sve_max_vq * 16, base_reg++); + pred_width, base_reg++); /* Define the vector length pseudo-register. */ g_string_append_printf(s, -- 2.34.1
[PATCH v3 05/14] target/arm: Simplify register counting in arm_gen_dynamic_svereg_xml
Rather than increment base_reg and num, compute num from the change
to base_reg at the end. Clean up some nearby comments.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
target/arm/gdbstub64.c | 27 ---
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
index 811833d8de..070ba20d99 100644
--- a/target/arm/gdbstub64.c
+++ b/target/arm/gdbstub64.c
@@ -277,32 +277,35 @@ static void output_vector_union_type(GString *s, int
reg_width)
g_string_append(s, "");
}
-int arm_gen_dynamic_svereg_xml(CPUState *cs, int base_reg)
+int arm_gen_dynamic_svereg_xml(CPUState *cs, int orig_base_reg)
{
ARMCPU *cpu = ARM_CPU(cs);
GString *s = g_string_new(NULL);
DynamicGDBXMLInfo *info = &cpu->dyn_svereg_xml;
-int i, reg_width = (cpu->sve_max_vq * 128);
-info->num = 0;
+int reg_width = cpu->sve_max_vq * 128;
+int base_reg = orig_base_reg;
+int i;
+
g_string_printf(s, "");
g_string_append_printf(s, "");
g_string_append_printf(s, "");
+/* Create the vector union type. */
output_vector_union_type(s, reg_width);
-/* Finally the sve prefix type */
+/* Create the predicate vector type. */
g_string_append_printf(s,
"",
reg_width / 8);
-/* Then define each register in parts for each vq */
+/* Define the vector registers. */
for (i = 0; i < 32; i++) {
g_string_append_printf(s,
"",
i, reg_width, base_reg++);
-info->num++;
}
+
/* fpscr & status registers */
g_string_append_printf(s, "", base_reg++);
-info->num += 2;
+/* Define the predicate registers. */
for (i = 0; i < 16; i++) {
g_string_append_printf(s,
"",
i, cpu->sve_max_vq * 16, base_reg++);
-info->num++;
}
g_string_append_printf(s,
"",
cpu->sve_max_vq * 16, base_reg++);
+
+/* Define the vector length pseudo-register. */
g_string_append_printf(s,
"",
base_reg++);
-info->num += 2;
-g_string_append_printf(s, "");
-info->desc = g_string_free(s, false);
+g_string_append_printf(s, "");
+
+info->desc = g_string_free(s, false);
+info->num = base_reg - orig_base_reg;
return info->num;
}
--
2.34.1
[PATCH v3 13/14] target/arm: Export arm_v7m_get_sp_ptr
From: David Reiss
Allow the function to be used outside of m_helper.c.
Move to be outside of ifndef CONFIG_USER_ONLY block.
Rename from get_v7m_sp_ptr.
Reviewed-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: David Reiss
[rth: Split out of a larger patch]
Signed-off-by: Richard Henderson
---
target/arm/internals.h| 10 +
target/arm/tcg/m_helper.c | 84 +++
2 files changed, 51 insertions(+), 43 deletions(-)
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 0f2c48ad4b..a03748aa10 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1358,6 +1358,16 @@ void arm_cpu_lpa2_finalize(ARMCPU *cpu, Error **errp);
/* Read the CONTROL register as the MRS instruction would. */
uint32_t arm_v7m_mrs_control(CPUARMState *env, uint32_t secure);
+/*
+ * Return a pointer to the location where we currently store the
+ * stack pointer for the requested security state and thread mode.
+ * This pointer will become invalid if the CPU state is updated
+ * such that the stack pointers are switched around (eg changing
+ * the SPSEL control bit).
+ */
+uint32_t *arm_v7m_get_sp_ptr(CPUARMState *env, bool secure,
+ bool threadmode, bool spsel);
+
#ifdef CONFIG_USER_ONLY
static inline void define_cortex_a72_a57_a53_cp_reginfo(ARMCPU *cpu) { }
#else
diff --git a/target/arm/tcg/m_helper.c b/target/arm/tcg/m_helper.c
index 03be79e7bf..081fc3f5f7 100644
--- a/target/arm/tcg/m_helper.c
+++ b/target/arm/tcg/m_helper.c
@@ -650,42 +650,6 @@ void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest)
arm_rebuild_hflags(env);
}
-static uint32_t *get_v7m_sp_ptr(CPUARMState *env, bool secure, bool threadmode,
-bool spsel)
-{
-/*
- * Return a pointer to the location where we currently store the
- * stack pointer for the requested security state and thread mode.
- * This pointer will become invalid if the CPU state is updated
- * such that the stack pointers are switched around (eg changing
- * the SPSEL control bit).
- * Compare the v8M ARM ARM pseudocode LookUpSP_with_security_mode().
- * Unlike that pseudocode, we require the caller to pass us in the
- * SPSEL control bit value; this is because we also use this
- * function in handling of pushing of the callee-saves registers
- * part of the v8M stack frame (pseudocode PushCalleeStack()),
- * and in the tailchain codepath the SPSEL bit comes from the exception
- * return magic LR value from the previous exception. The pseudocode
- * opencodes the stack-selection in PushCalleeStack(), but we prefer
- * to make this utility function generic enough to do the job.
- */
-bool want_psp = threadmode && spsel;
-
-if (secure == env->v7m.secure) {
-if (want_psp == v7m_using_psp(env)) {
-return &env->regs[13];
-} else {
-return &env->v7m.other_sp;
-}
-} else {
-if (want_psp) {
-return &env->v7m.other_ss_psp;
-} else {
-return &env->v7m.other_ss_msp;
-}
-}
-}
-
static bool arm_v7m_load_vector(ARMCPU *cpu, int exc, bool targets_secure,
uint32_t *pvec)
{
@@ -810,8 +774,8 @@ static bool v7m_push_callee_stack(ARMCPU *cpu, uint32_t lr,
bool dotailchain,
!mode;
mmu_idx = arm_v7m_mmu_idx_for_secstate_and_priv(env, M_REG_S, priv);
-frame_sp_p = get_v7m_sp_ptr(env, M_REG_S, mode,
-lr & R_V7M_EXCRET_SPSEL_MASK);
+frame_sp_p = arm_v7m_get_sp_ptr(env, M_REG_S, mode,
+lr & R_V7M_EXCRET_SPSEL_MASK);
want_psp = mode && (lr & R_V7M_EXCRET_SPSEL_MASK);
if (want_psp) {
limit = env->v7m.psplim[M_REG_S];
@@ -1656,10 +1620,8 @@ static void do_v7m_exception_exit(ARMCPU *cpu)
* use 'frame_sp_p' after we do something that makes it invalid.
*/
bool spsel = env->v7m.control[return_to_secure] &
R_V7M_CONTROL_SPSEL_MASK;
-uint32_t *frame_sp_p = get_v7m_sp_ptr(env,
- return_to_secure,
- !return_to_handler,
- spsel);
+uint32_t *frame_sp_p = arm_v7m_get_sp_ptr(env, return_to_secure,
+ !return_to_handler, spsel);
uint32_t frameptr = *frame_sp_p;
bool pop_ok = true;
ARMMMUIdx mmu_idx;
@@ -1965,7 +1927,7 @@ static bool do_v7m_function_return(ARMCPU *cpu)
threadmode = !arm_v7m_is_handler_mode(env);
spsel = env->v7m.control[M_REG_S] & R_V7M_CONTROL_SPSEL_MASK;
-frame_sp_p = get_v7m_sp_ptr(env, true, threadmode, spsel);
+frame_sp_p = arm_v7m_get_sp_ptr(env, true, threadmode, spsel);
frameptr = *frame_sp_p;
/*
@@
[PATCH v3 14/14] target/arm: Implement gdbstub m-profile systemreg and secext
The upstream gdb xml only implements {MSP,PSP}{,_NS,S}, but
go ahead and implement the other system registers as well.
Since there is significant overlap between the two, implement
them with common code. The only exception is the systemreg
view of CONTROL, which merges the banked bits as per MRS.
Signed-off-by: David Reiss
[rth: Substatial rewrite using enumerator and shared code.]
Signed-off-by: Richard Henderson
---
target/arm/cpu.h | 2 +
target/arm/gdbstub.c | 178 +++
2 files changed, 180 insertions(+)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 059fe62eaa..6e97a256fb 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -869,6 +869,8 @@ struct ArchCPU {
DynamicGDBXMLInfo dyn_sysreg_xml;
DynamicGDBXMLInfo dyn_svereg_xml;
+DynamicGDBXMLInfo dyn_m_systemreg_xml;
+DynamicGDBXMLInfo dyn_m_secextreg_xml;
/* Timers used by the generic (architected) timer */
QEMUTimer *gt_timer[NUM_GTIMERS];
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index 062c8d447a..3f799f5d05 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -322,6 +322,164 @@ static int arm_gen_dynamic_sysreg_xml(CPUState *cs, int
base_reg)
return cpu->dyn_sysreg_xml.num;
}
+typedef enum {
+M_SYSREG_MSP,
+M_SYSREG_PSP,
+M_SYSREG_PRIMASK,
+M_SYSREG_CONTROL,
+M_SYSREG_BASEPRI,
+M_SYSREG_FAULTMASK,
+M_SYSREG_MSPLIM,
+M_SYSREG_PSPLIM,
+} MProfileSysreg;
+
+static const struct {
+const char *name;
+int feature;
+} m_sysreg_def[] = {
+[M_SYSREG_MSP] = { "msp", ARM_FEATURE_M },
+[M_SYSREG_PSP] = { "psp", ARM_FEATURE_M },
+[M_SYSREG_PRIMASK] = { "primask", ARM_FEATURE_M },
+[M_SYSREG_CONTROL] = { "control", ARM_FEATURE_M },
+[M_SYSREG_BASEPRI] = { "basepri", ARM_FEATURE_M_MAIN },
+[M_SYSREG_FAULTMASK] = { "faultmask", ARM_FEATURE_M_MAIN },
+[M_SYSREG_MSPLIM] = { "msplim", ARM_FEATURE_V8 },
+[M_SYSREG_PSPLIM] = { "psplim", ARM_FEATURE_V8 },
+};
+
+static uint32_t *m_sysreg_ptr(CPUARMState *env, MProfileSysreg reg, bool sec)
+{
+uint32_t *ptr;
+
+switch (reg) {
+case M_SYSREG_MSP:
+ptr = arm_v7m_get_sp_ptr(env, sec, false, true);
+break;
+case M_SYSREG_PSP:
+ptr = arm_v7m_get_sp_ptr(env, sec, true, true);
+break;
+case M_SYSREG_MSPLIM:
+ptr = &env->v7m.msplim[sec];
+break;
+case M_SYSREG_PSPLIM:
+ptr = &env->v7m.psplim[sec];
+break;
+case M_SYSREG_PRIMASK:
+ptr = &env->v7m.primask[sec];
+break;
+case M_SYSREG_BASEPRI:
+ptr = &env->v7m.basepri[sec];
+break;
+case M_SYSREG_FAULTMASK:
+ptr = &env->v7m.faultmask[sec];
+break;
+case M_SYSREG_CONTROL:
+ptr = &env->v7m.control[sec];
+break;
+default:
+return NULL;
+}
+return arm_feature(env, m_sysreg_def[reg].feature) ? ptr : NULL;
+}
+
+static int m_sysreg_get(CPUARMState *env, GByteArray *buf,
+MProfileSysreg reg, bool secure)
+{
+uint32_t *ptr = m_sysreg_ptr(env, reg, secure);
+
+if (ptr == NULL) {
+return 0;
+}
+return gdb_get_reg32(buf, *ptr);
+}
+
+static int arm_gdb_get_m_systemreg(CPUARMState *env, GByteArray *buf, int reg)
+{
+/*
+ * Here, we emulate MRS instruction, where CONTROL has a mix of
+ * banked and non-banked bits.
+ */
+if (reg == M_SYSREG_CONTROL) {
+return gdb_get_reg32(buf, arm_v7m_mrs_control(env, env->v7m.secure));
+}
+return m_sysreg_get(env, buf, reg, env->v7m.secure);
+}
+
+static int arm_gdb_set_m_systemreg(CPUARMState *env, uint8_t *buf, int reg)
+{
+return 0; /* TODO */
+}
+
+static int arm_gen_dynamic_m_systemreg_xml(CPUState *cs, int orig_base_reg)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = &cpu->env;
+GString *s = g_string_new(NULL);
+int base_reg = orig_base_reg;
+int i;
+
+g_string_printf(s, "");
+g_string_append_printf(s, "");
+g_string_append_printf(s, "\n");
+
+for (i = 0; i < ARRAY_SIZE(m_sysreg_def); i++) {
+if (arm_feature(env, m_sysreg_def[i].feature)) {
+g_string_append_printf(s,
+"\n",
+m_sysreg_def[i].name, base_reg++);
+}
+}
+
+g_string_append_printf(s, "");
+cpu->dyn_m_systemreg_xml.desc = g_string_free(s, false);
+cpu->dyn_m_systemreg_xml.num = base_reg - orig_base_reg;
+
+return cpu->dyn_m_systemreg_xml.num;
+}
+
+#ifndef CONFIG_USER_ONLY
+/*
+ * For user-only, we see the non-secure registers via m_systemreg above.
+ * For secext, encode the non-secure view as even and secure view as odd.
+ */
+static int arm_gdb_get_m_secextreg(CPUARMState *env, GByteArray *buf, int reg)
+{
+return m_sysreg_get(env, buf, reg >> 1, reg & 1);
+}
+
+static int arm_gdb_set_m_secextreg(CPUARMState *env, uint8_t *buf, int reg)
+{
+return 0; /* TODO */
+}
+
+static int arm_gen_dynamic_m_sece
Re: [RESEND PULL 000/126] Buildsys / QOM / QDev / UI patches for 2023-02-27
On 27/2/23 19:22, Peter Maydell wrote:
On Mon, 27 Feb 2023 at 14:08, Philippe Mathieu-Daudé wrote:
- buildsys
- Various header cleaned up (removing pointless headers)
- Mark various files/code user/system specific
- Make various objects target-independent
- Remove tswapN() calls from dump.o
- Suggest g_assert_not_reached() instead of assert(0)
- qdev / qom
- Replace various container_of() by QOM cast macros
- Declare some QOM macros using OBJECT_DECLARE_TYPE()
- Embed OHCI QOM child in SM501 chipset
- hw (ISA & IDE)
- add some documentation, improve function names
- un-inline, open-code few functions
- have ISA API accessing IRQ/DMA prefer ISABus over ISADevice
- Demote IDE subsystem maintenance to "Odd Fixes"
- ui: Improve Ctrl+Alt hint on Darwin Cocoa
Seems to have broken the TCG plugins somehow?
https://gitlab.com/pm215/qemu/-/jobs/3841174348
TEST munmap-pthread on aarch64
**
ERROR:../plugins/core.c:221:qemu_plugin_vcpu_init_hook: assertion
failed: (success)
Similarly in
https://gitlab.com/pm215/qemu/-/jobs/3841174344
https://gitlab.com/pm215/qemu/-/jobs/3841174341
Sorry since this wasn't reliable, I first thought this was
pre-existing / unrelated to this patchset. But...
012add0d52621e34632ea24361a0c751bc2f5ad1 is the first bad commit
commit 012add0d52621e34632ea24361a0c751bc2f5ad1
Author: Philippe Mathieu-Daudé
Date: Tue Dec 6 16:06:00 2022 +0100
cputlb: Restrict SavedIOTLB to system emulation
Commit 2f3a57ee47 ("cputlb: ensure we save the IOTLB data in
case of reset") added the SavedIOTLB structure -- which is
system emulation specific -- in the generic CPUState structure.
For some reason this was hard to bisect. The usual "make check-tcg"
rule wasn't coherent along bisection. Eventually I ended using
"ninja clean && ninja qemu-user && make check-tcg" which worked.
Could a .so rule be incomplete, so while qemu-user binary is rebuilt,
the plugin sharedlibs aren't?
Anyhow, short term I'll simply drop this commit from the pullreq.
Re: Adopting abandoned patch?
Thanks, here's the original patch: https://bugs.debian.org/cgi-bin/bugreport.cgi?att=2;bug=621529;filename=multiboot2.patch;msg=15 On Mon, Feb 27, 2023 at 4:59 AM Alex Bennée wrote: > > Dinah B writes: > > > Hi, > > > > I'm looking to get more involved in contributing to QEMU. I noticed that > there are some issues in the tracker > > where a sample patch has been contributed but never got merged, like a > proposal to add multiboot2 support: > > https://gitlab.com/qemu-project/qemu/-/issues/389 > > I couldn't see a patch attached to the bug report. Is it elsewhere? > > > > > Is another dev allowed to "adopt" the patch as-is, with proper > attribution to the original dev and drive it to > > completion/merging (there are some features missing)? Or is "starting > from scratch" required for legal > > reasons? > > It's certainly possible to pick up a patch from someone else and take it > forward. Aside from addressing any review comments I think the minimum > requirement is the authors original Signed-off-by is intact which > asserts they could contribute code to the project. > > -- > Alex Bennée > Virtualisation Tech Lead @ Linaro >
Re: [RFC PATCH v3 1/1] gitlab: Use plain docker in container-template.yml
Fabiano Rosas writes:
> Alex Bennée writes:
>
>> Fabiano Rosas writes:
>>
>>> Our dockerfiles no longer reference layers from other qemu images so
>>> we can now use 'docker build' on them.
>>>
>>> Also reinstate the caching that was disabled due to bad interactions
>>> with certain runners. See commit 6ddc3dc7a8 ("tests/docker: don't use
>>> BUILDKIT in GitLab either"). We now believe those issues to be fixed.
>>>
>>> The COMMON_TAG needed to be fixed for the caching to work. The
>>> docker.py script was not using the variable, but constructing the
>>> correct URL directly.
>>>
>>> Signed-off-by: Fabiano Rosas
>>> ---
>>> .gitlab-ci.d/container-template.yml | 9 -
>>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/.gitlab-ci.d/container-template.yml
>>> b/.gitlab-ci.d/container-template.yml
>>> index c434b9c8f3..519b8a9482 100644
>>> --- a/.gitlab-ci.d/container-template.yml
>>> +++ b/.gitlab-ci.d/container-template.yml
>>> @@ -6,17 +6,16 @@
>>> - docker:dind
>>>before_script:
>>> - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
>>> -- export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/$NAME:latest"
>>> +- export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest"
>>> - apk add python3
>>> - docker info
>>> - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p
>>> "$CI_REGISTRY_PASSWORD"
>>>script:
>>> - echo "TAG:$TAG"
>>> - echo "COMMON_TAG:$COMMON_TAG"
>>> -- ./tests/docker/docker.py --engine docker build
>>> - -t "qemu/$NAME" -f "tests/docker/dockerfiles/$NAME.docker"
>>> - -r $CI_REGISTRY/qemu-project/qemu
>>> -- docker tag "qemu/$NAME" "$TAG"
>>> +- docker build --tag "$TAG" --cache-from "$TAG" --cache-from
>>> "$COMMON_TAG"
>>> + --build-arg BUILDKIT_INLINE_CACHE=1
>>> + -f "tests/docker/dockerfiles/$NAME.docker" "."
>>
>> I wonder why this doesn't injest a bunch of context. If I run:
>>
>> docker build --cache-from
>> registry.gitlab.com/stsquad/qemu/qemu/debian-alpha-cross --build-arg
>> BUILDKIT_INLINE_CACHE=1 --build-arg USER=alex --build-arg UID=1000 -t qemu
>> /debian-alpha-cross -f
>> "/home/alex/lsrc/qemu.git/tests/docker/dockerfiles/debian-alpha-cross.docker"
>> .
>>
>> it attempts to bring my entire build directory in as build context. This
>> is why we use the - < docker form in the Makefile.
>>
>
> I only see that without DOCKER_BUILDKIT=1. With the variable set it does
> like in the CI jobs. I presume it is being set automatically by gitlab,
> but we could add it to the script to be explicit.
>
>>> - docker push "$TAG"
>>>after_script:
>>> - docker logout
>>
>> So what I don't understand is if I do:
>>
>> docker pull registry.gitlab.com/stsquad/qemu/qemu/debian-alpha-cross
>> docker build --cache-from
>> registry.gitlab.com/stsquad/qemu/qemu/debian-alpha-cross --build-arg
>> BUILDKIT_INLINE_CACHE=1 -t qemu/debian-alpha-cross - <
>>
>> /home/alex/lsrc/qemu.git/tests/docker/dockerfiles/debian-alpha-cross.docker
>>
>> I still see pretty much a full rebuild of the image.
>
> I don't use docker and podman does not support caching. I have
> fresh-installed docker today and indeed it seems to not use the cache at
> every build. We're missing something.
>
> Sometimes it works:
>
Oops, sorry about the long lines, here it is again:
===
$ docker system prune -a -f
Deleted build cache objects:
xzbxmzaib3a8s0ufetop5ikhi
0ce9qln4ipd2vgf9xw9to0fdb
se4hq3rce3lad20t9sqqnubob
Total reclaimed space: 5.845kB
$ docker images -a
REPOSITORY TAG IMAGE ID CREATED SIZE
$ DOCKER_BUILDKIT=1 docker build --tag
registry.gitlab.com/farosas/qemu/qemu/debian-amd64:latest --cache-from
registry.gitlab.com/farosas/qemu/qemu/debian-amd64:latest --build-arg
BUILDKIT_INLINE_CACHE=1 -f debian-amd64.docker .
[+] Building 57.8s (12/12) FINISHED
=> [internal] load build definition from debian-amd64.docker
=> => transferring dockerfile: 5.90kB
=> [internal] load .dockerignore
=> => transferring context: 2B
=> [internal] load metadata for docker.io/library/debian:11-slim
=> importing cache manifest from
registry.gitlab.com/farosas/qemu/qemu/debian-amd64:latest
=> [1/6] FROM
docker.io/library/debian:11-slim@sha256:8eaee63a5ea83744e62d5bf88e7d472d7f19b5feda3bfc6a2304cc074f269269
=> CACHED [2/6] RUN export DEBIAN_FRONTEND=noninteractive && apt-get
update && apt-get install -y eatmydata
&& eatmydata apt-get dist-upgrade -y && eatmydata apt-get i
=> CACHED [3/6] RUN DEBIAN_FRONTEND=noninteractive eatmydata apt install -y
--no-install-recommends cscope
global linux-headers-amd64
=> CACHED [4/6] RUN git clone https://github.com/luigirizzo/netmap.git
/usr/src/netmap
=> CACHED [5/6] RUN cd /usr/src/netmap && git checkout v11.3
=> CACHED [6/6] RUN cd /usr/src/netmap/LINUX && ./configure --no-drivers
--no-apps --kernel-dir=$(ls -d
/usr/src/linux-headers-*-amd64) && make install
=> => pulling
sha256:bb263680
Re: [RFC PATCH v3 1/1] gitlab: Use plain docker in container-template.yml
Alex Bennée writes:
> Fabiano Rosas writes:
>
>> Our dockerfiles no longer reference layers from other qemu images so
>> we can now use 'docker build' on them.
>>
>> Also reinstate the caching that was disabled due to bad interactions
>> with certain runners. See commit 6ddc3dc7a8 ("tests/docker: don't use
>> BUILDKIT in GitLab either"). We now believe those issues to be fixed.
>>
>> The COMMON_TAG needed to be fixed for the caching to work. The
>> docker.py script was not using the variable, but constructing the
>> correct URL directly.
>>
>> Signed-off-by: Fabiano Rosas
>> ---
>> .gitlab-ci.d/container-template.yml | 9 -
>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/.gitlab-ci.d/container-template.yml
>> b/.gitlab-ci.d/container-template.yml
>> index c434b9c8f3..519b8a9482 100644
>> --- a/.gitlab-ci.d/container-template.yml
>> +++ b/.gitlab-ci.d/container-template.yml
>> @@ -6,17 +6,16 @@
>> - docker:dind
>>before_script:
>> - export TAG="$CI_REGISTRY_IMAGE/qemu/$NAME:latest"
>> -- export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/$NAME:latest"
>> +- export COMMON_TAG="$CI_REGISTRY/qemu-project/qemu/qemu/$NAME:latest"
>> - apk add python3
>> - docker info
>> - docker login $CI_REGISTRY -u "$CI_REGISTRY_USER" -p
>> "$CI_REGISTRY_PASSWORD"
>>script:
>> - echo "TAG:$TAG"
>> - echo "COMMON_TAG:$COMMON_TAG"
>> -- ./tests/docker/docker.py --engine docker build
>> - -t "qemu/$NAME" -f "tests/docker/dockerfiles/$NAME.docker"
>> - -r $CI_REGISTRY/qemu-project/qemu
>> -- docker tag "qemu/$NAME" "$TAG"
>> +- docker build --tag "$TAG" --cache-from "$TAG" --cache-from
>> "$COMMON_TAG"
>> + --build-arg BUILDKIT_INLINE_CACHE=1
>> + -f "tests/docker/dockerfiles/$NAME.docker" "."
>
> I wonder why this doesn't injest a bunch of context. If I run:
>
> docker build --cache-from
> registry.gitlab.com/stsquad/qemu/qemu/debian-alpha-cross --build-arg
> BUILDKIT_INLINE_CACHE=1 --build-arg USER=alex --build-arg UID=1000 -t qemu
> /debian-alpha-cross -f
> "/home/alex/lsrc/qemu.git/tests/docker/dockerfiles/debian-alpha-cross.docker"
> .
>
> it attempts to bring my entire build directory in as build context. This
> is why we use the - < docker form in the Makefile.
>
I only see that without DOCKER_BUILDKIT=1. With the variable set it does
like in the CI jobs. I presume it is being set automatically by gitlab,
but we could add it to the script to be explicit.
>> - docker push "$TAG"
>>after_script:
>> - docker logout
>
> So what I don't understand is if I do:
>
> docker pull registry.gitlab.com/stsquad/qemu/qemu/debian-alpha-cross
> docker build --cache-from
> registry.gitlab.com/stsquad/qemu/qemu/debian-alpha-cross --build-arg
> BUILDKIT_INLINE_CACHE=1 -t qemu/debian-alpha-cross - <
>
> /home/alex/lsrc/qemu.git/tests/docker/dockerfiles/debian-alpha-cross.docker
>
> I still see pretty much a full rebuild of the image.
I don't use docker and podman does not support caching. I have
fresh-installed docker today and indeed it seems to not use the cache at
every build. We're missing something.
Sometimes it works:
===
$ docker system prune -a -f
Deleted build cache objects:
xzbxmzaib3a8s0ufetop5ikhi
0ce9qln4ipd2vgf9xw9to0fdb
se4hq3rce3lad20t9sqqnubob
Total reclaimed space: 5.845kB
$ docker images -a
REPOSITORY TAG IMAGE ID CREATED SIZE
$ DOCKER_BUILDKIT=1 docker build --tag
registry.gitlab.com/farosas/qemu/qemu/debian-amd64:latest --cache-from
registry.gitlab.com/farosas/qemu/qemu/debian-amd64:latest --build-arg
BUILDKIT_INLINE_CACHE=1 -f debian-amd64.docker .
[+] Building 57.8s (12/12) FINISHED
=> [internal] load build definition from debian-amd64.docker
0.2s
=> => transferring dockerfile: 5.90kB
0.0s
=> [internal] load .dockerignore
0.3s
=> => transferring context: 2B
[PATCH 3/3] block: protect BlockBackend->queued_requests with a lock
The CoQueue API offers thread-safety via the lock argument that
qemu_co_queue_wait() and qemu_co_enter_next() take. BlockBackend
currently does not make use of the lock argument. This means that
multiple threads submitting I/O requests can corrupt the CoQueue's
QSIMPLEQ.
Add a QemuMutex and pass it to CoQueue APIs so that the queue is
protected. While we're at it, also assert that the queue is empty when
the BlockBackend is deleted.
Signed-off-by: Stefan Hajnoczi
---
block/block-backend.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index ec7eafd7fb..c4dd16f4da 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -81,6 +81,7 @@ struct BlockBackend {
QLIST_HEAD(, BlockBackendAioNotifier) aio_notifiers;
int quiesce_counter; /* atomic: written under BQL, read by other threads */
+QemuMutex queued_requests_lock; /* protects queued_requests */
CoQueue queued_requests;
bool disable_request_queuing; /* atomic */
@@ -368,6 +369,7 @@ BlockBackend *blk_new(AioContext *ctx, uint64_t perm,
uint64_t shared_perm)
block_acct_init(&blk->stats);
+qemu_mutex_init(&blk->queued_requests_lock);
qemu_co_queue_init(&blk->queued_requests);
notifier_list_init(&blk->remove_bs_notifiers);
notifier_list_init(&blk->insert_bs_notifiers);
@@ -485,6 +487,8 @@ static void blk_delete(BlockBackend *blk)
assert(QLIST_EMPTY(&blk->remove_bs_notifiers.notifiers));
assert(QLIST_EMPTY(&blk->insert_bs_notifiers.notifiers));
assert(QLIST_EMPTY(&blk->aio_notifiers));
+assert(qemu_co_queue_empty(&blk->queued_requests));
+qemu_mutex_destroy(&blk->queued_requests_lock);
QTAILQ_REMOVE(&block_backends, blk, link);
drive_info_del(blk->legacy_dinfo);
block_acct_cleanup(&blk->stats);
@@ -1273,9 +1277,16 @@ static void coroutine_fn
blk_wait_while_drained(BlockBackend *blk)
if (qatomic_read(&blk->quiesce_counter) &&
!qatomic_read(&blk->disable_request_queuing)) {
+/*
+ * Take lock before decrementing in flight counter so main loop thread
+ * waits for us to enqueue ourselves before it can leave the drained
+ * section.
+ */
+qemu_mutex_lock(&blk->queued_requests_lock);
blk_dec_in_flight(blk);
-qemu_co_queue_wait(&blk->queued_requests, NULL);
+qemu_co_queue_wait(&blk->queued_requests, &blk->queued_requests_lock);
blk_inc_in_flight(blk);
+qemu_mutex_unlock(&blk->queued_requests_lock);
}
}
@@ -2611,9 +2622,12 @@ static void blk_root_drained_end(BdrvChild *child)
if (blk->dev_ops && blk->dev_ops->drained_end) {
blk->dev_ops->drained_end(blk->dev_opaque);
}
-while (qemu_co_enter_next(&blk->queued_requests, NULL)) {
+qemu_mutex_lock(&blk->queued_requests_lock);
+while (qemu_co_enter_next(&blk->queued_requests,
+ &blk->queued_requests_lock)) {
/* Resume all queued requests */
}
+qemu_mutex_unlock(&blk->queued_requests_lock);
}
}
--
2.39.2
