Re: [PATCH v1 01/12] s390x/tcg: wrap address for RRBE
On 05/08/2021 17.27, David Hildenbrand wrote:
Let's wrap the address just like for SSKE and ISKE.
Signed-off-by: David Hildenbrand
---
target/s390x/tcg/mem_helper.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 21a4de4067..e0befd0f03 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -2223,11 +2223,12 @@ void HELPER(sske)(CPUS390XState *env, uint64_t r1,
uint64_t r2)
uint32_t HELPER(rrbe)(CPUS390XState *env, uint64_t r2)
{
MachineState *ms = MACHINE(qdev_get_machine());
+uint64_t addr = wrap_address(env, r2);
static S390SKeysState *ss;
static S390SKeysClass *skeyclass;
uint8_t re, key;
-if (r2 > ms->ram_size) {
+if (addr > ms->ram_size) {
return 0;
}
@@ -2236,14 +2237,14 @@ uint32_t HELPER(rrbe)(CPUS390XState *env, uint64_t r2)
skeyclass = S390_SKEYS_GET_CLASS(ss);
}
-if (skeyclass->get_skeys(ss, r2 / TARGET_PAGE_SIZE, 1, )) {
+if (skeyclass->get_skeys(ss, addr / TARGET_PAGE_SIZE, 1, )) {
return 0;
}
re = key & (SK_R | SK_C);
key &= ~SK_R;
-if (skeyclass->set_skeys(ss, r2 / TARGET_PAGE_SIZE, 1, )) {
+if (skeyclass->set_skeys(ss, addr / TARGET_PAGE_SIZE, 1, )) {
return 0;
}
That's certainly the right thing to do!
Reviewed-by: Thomas Huth
Re: [PATCH v1] s390x/ioinst: Fix wrong MSCH alignment check on little endian
On 05/08/2021 16.37, David Hildenbrand wrote:
schib->pmcw.chars is 32bit, not 16bit. This fixes the kvm-unit-tests
"css" test, which fails with:
FAIL: Channel Subsystem: measurement block format1: Unaligned MB origin:
Program interrupt: expected(21) == received(0)
Because we end up not injecting an operand program exception.
Fixes: a54b8ac340c2 ("css: SCHIB measurement block origin must be aligned")
Cc: Halil Pasic
Cc: Cornelia Huck
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: Thomas Huth
Cc: Pierre Morel
Cc: qemu-s3...@nongnu.org
Signed-off-by: David Hildenbrand
---
target/s390x/ioinst.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index 4eb0a7a9f8..bdae5090bc 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -123,7 +123,7 @@ static int ioinst_schib_valid(SCHIB *schib)
}
/* for MB format 1 bits 26-31 of word 11 must be 0 */
/* MBA uses words 10 and 11, it means align on 2**6 */
-if ((be16_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
+if ((be32_to_cpu(schib->pmcw.chars) & PMCW_CHARS_MASK_MBFC) &&
(be64_to_cpu(schib->mba) & 0x03fUL)) {
return 0;
}
Reviewed-by: Thomas Huth
Re: [PATCH v8] tests/tcg/s390x: Test SIGILL and SIGSEGV handling
On 05/08/2021 00.51, Ilya Leoshkevich wrote: Verify that s390x-specific uc_mcontext.psw.addr is reported correctly and that signal handling interacts properly with debugging. Signed-off-by: Ilya Leoshkevich --- v7: https://lists.nongnu.org/archive/html/qemu-devel/2021-08/msg00463.html v7 -> v8: Another rebase needed due to the conflict with Jonathan's 50e36dd61652. Thanks for respinning this patch! I now gave it a try, and it seems to work, but the output looks a little funny: SKIPPED signals on s390x because BROKEN awaiting sigframe clean-ups and vdso support TESTtest-mmap (default) on s390x TESTtestthread on s390x TESTthreadcount on s390x TESThello-s390x on s390x TESTcsst on s390x TESTipm on s390x TESTexrl-trt on s390x TESTexrl-trtr on s390x TESTpack on s390x TESTmvo on s390x TESTmvc on s390x TESTtrap on s390x TESTsignals-s390x on s390x i.e. it first says "SKIPPED signals", but later still executes the test. Could that be fixed somehow? Thomas
Re: [PATCH v1] s390x/tcg: fix and optimize SPX (SET PREFIX)
On 05/08/2021 14.59, David Hildenbrand wrote:
We not only invalidate the translation of the range 0x0-0x2000, we also
invalidate the translation of the new prefix range and the translation
of the old prefix range -- because real2abs would return different
results for all of these ranges when changing the prefix location.
This fixes the kvm-unit-tests "edat" test that just hangs before this
patch because we end up clearing the new prefix area instead of the old
prefix area.
While at it, let's not do anything in case the prefix doesn't change.
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Cornelia Huck
Cc: Thomas Huth
Cc: Claudio Imbrenda
Cc: qemu-s3...@nongnu.org
Signed-off-by: David Hildenbrand
---
target/s390x/tcg/misc_helper.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index 33e6999e15..aab9c47747 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -151,13 +151,26 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1,
uint32_t r3, uint32_t num)
/* Set Prefix */
void HELPER(spx)(CPUS390XState *env, uint64_t a1)
{
+const uint32_t prefix = a1 & 0x7fffe000;
+const uint32_t old_prefix = env->psa;
CPUState *cs = env_cpu(env);
-uint32_t prefix = a1 & 0x7fffe000;
+
+if (prefix == old_prefix) {
+return;
+}
env->psa = prefix;
HELPER_LOG("prefix: %#x\n", prefix);
tlb_flush_page(cs, 0);
tlb_flush_page(cs, TARGET_PAGE_SIZE);
+if (prefix != 0) {
+tlb_flush_page(cs, prefix);
+tlb_flush_page(cs, prefix + TARGET_PAGE_SIZE);
+}
+if (old_prefix != 0) {
+tlb_flush_page(cs, old_prefix);
+tlb_flush_page(cs, old_prefix + TARGET_PAGE_SIZE);
+}
}
Sounds reasonable.
Reviewed-by: Thomas Huth
Re: [PATCH-for-6.1 v2] softmmu/physmem: fix wrong assertion in qemu_ram_alloc_internal()
> When adding RAM_NORESERVE, we forgot to remove the old assertion when
> adding the updated one, most probably when reworking the patches or
> rebasing. We can easily crash QEMU by adding
> -object memory-backend-ram,id=mem0,size=500G,reserve=off
> to the QEMU cmdline:
> qemu-system-x86_64: ../softmmu/physmem.c:2146: qemu_ram_alloc_internal:
> Assertion `(ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC))
> == 0' failed.
>
> Fix it by removing the old assertion.
>
> Fixes: 8dbe22c6868b ("memory: Introduce RAM_NORESERVE and wire it up in
> qemu_ram_mmap()")
> Reviewed-by: Peter Xu
> Reviewed-by: Philippe Mathieu-Daudé
> Tested-by: Philippe Mathieu-Daudé
> Cc: Paolo Bonzini
> Cc: Peter Xu
> Cc: Philippe Mathieu-Daudé
> Signed-off-by: David Hildenbrand
> ---
>
> v1 -> v2:
> - Added rbs
> - Tagged for 6.1 inclusion
>
> ---
> softmmu/physmem.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index 3c1912a1a0..2e18947598 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -2143,7 +2143,6 @@ RAMBlock *qemu_ram_alloc_internal(ram_addr_t size,
> ram_addr_t max_size,
> RAMBlock *new_block;
> Error *local_err = NULL;
>
> -assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC)) == 0);
> assert((ram_flags & ~(RAM_SHARED | RAM_RESIZEABLE | RAM_PREALLOC |
>RAM_NORESERVE)) == 0);
> assert(!host ^ (ram_flags & RAM_PREALLOC));
> --
> 2.31.1
Reviewed-by: Pankaj Gupta
>
>
Re: [PATCH for-6.2 v4 02/14] machine: Uniformly use maxcpus to calculate the omitted parameters
> We are currently using maxcpus to calculate the omitted sockets
> but using cpus to calculate the omitted cores/threads. This makes
> cmdlines like:
> -smp cpus=8,maxcpus=16
> -smp cpus=8,cores=4,maxcpus=16
> -smp cpus=8,threads=2,maxcpus=16
> work fine but the ones like:
> -smp cpus=8,sockets=2,maxcpus=16
> -smp cpus=8,sockets=2,cores=4,maxcpus=16
> -smp cpus=8,sockets=2,threads=2,maxcpus=16
> break the sanity check.
>
> Since we require for a valid config that the product of "sockets * cores
> * threads" should equal to the maxcpus, we should uniformly use maxcpus
> to calculate their omitted values.
>
> Also the if-branch of "cpus == 0 || sockets == 0" was split into two
> branches of "cpus == 0" and "sockets == 0" so that we can clearly read
> that we are parsing the configuration with a preference on cpus over
> sockets over cores over threads.
>
> Note: change in this patch won't affect any existing working cmdlines
> but improves consistency and allows more incomplete configs to be valid.
>
> Reviewed-by: Andrew Jones
> Signed-off-by: Yanan Wang
> ---
> hw/core/machine.c | 30 +++---
> hw/i386/pc.c | 30 +++---
> 2 files changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 696d9e8e47..69979c93dd 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -755,24 +755,26 @@ static void smp_parse(MachineState *ms,
> SMPConfiguration *config, Error **errp)
> }
>
> /* compute missing values, prefer sockets over cores over threads */
> -if (cpus == 0 || sockets == 0) {
> +maxcpus = maxcpus > 0 ? maxcpus : cpus;
> +
> +if (cpus == 0) {
> +sockets = sockets > 0 ? sockets : 1;
> cores = cores > 0 ? cores : 1;
> threads = threads > 0 ? threads : 1;
> -if (cpus == 0) {
> -sockets = sockets > 0 ? sockets : 1;
> -cpus = cores * threads * sockets;
> -} else {
> -maxcpus = maxcpus > 0 ? maxcpus : cpus;
> -sockets = maxcpus / (cores * threads);
> -}
> +cpus = sockets * cores * threads;
> +maxcpus = maxcpus > 0 ? maxcpus : cpus;
> +} else if (sockets == 0) {
> +cores = cores > 0 ? cores : 1;
> +threads = threads > 0 ? threads : 1;
> +sockets = maxcpus / (cores * threads);
> } else if (cores == 0) {
> threads = threads > 0 ? threads : 1;
> -cores = cpus / (sockets * threads);
> -cores = cores > 0 ? cores : 1;
> +cores = maxcpus / (sockets * threads);
> } else if (threads == 0) {
> -threads = cpus / (cores * sockets);
> -threads = threads > 0 ? threads : 1;
> -} else if (sockets * cores * threads < cpus) {
> +threads = maxcpus / (sockets * cores);
> +}
> +
> +if (sockets * cores * threads < cpus) {
> error_setg(errp, "cpu topology: "
> "sockets (%u) * cores (%u) * threads (%u) < "
> "smp_cpus (%u)",
> @@ -780,8 +782,6 @@ static void smp_parse(MachineState *ms, SMPConfiguration
> *config, Error **errp)
> return;
> }
>
> -maxcpus = maxcpus > 0 ? maxcpus : cpus;
> -
> if (maxcpus < cpus) {
> error_setg(errp, "maxcpus must be equal to or greater than smp");
> return;
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index acd31af452..a9ff9ef52c 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -725,24 +725,26 @@ static void pc_smp_parse(MachineState *ms,
> SMPConfiguration *config, Error **err
> dies = dies > 0 ? dies : 1;
>
> /* compute missing values, prefer sockets over cores over threads */
> -if (cpus == 0 || sockets == 0) {
> +maxcpus = maxcpus > 0 ? maxcpus : cpus;
> +
> +if (cpus == 0) {
> +sockets = sockets > 0 ? sockets : 1;
> cores = cores > 0 ? cores : 1;
> threads = threads > 0 ? threads : 1;
> -if (cpus == 0) {
> -sockets = sockets > 0 ? sockets : 1;
> -cpus = cores * threads * dies * sockets;
> -} else {
> -maxcpus = maxcpus > 0 ? maxcpus : cpus;
> -sockets = maxcpus / (dies * cores * threads);
> -}
> +cpus = sockets * dies * cores * threads;
> +maxcpus = maxcpus > 0 ? maxcpus : cpus;
> +} else if (sockets == 0) {
> +cores = cores > 0 ? cores : 1;
> +threads = threads > 0 ? threads : 1;
> +sockets = maxcpus / (dies * cores * threads);
> } else if (cores == 0) {
> threads = threads > 0 ? threads : 1;
> -cores = cpus / (sockets * dies * threads);
> -cores = cores > 0 ? cores : 1;
> +cores = maxcpus / (sockets * dies * threads);
> } else if (threads == 0) {
> -threads = cpus / (cores * dies * sockets);
> -threads = threads > 0 ? threads : 1;
> -} else if (sockets * dies * cores * threads < cpus) {
> +threads = maxcpus /
Re: [PATCH for-6.2 v4 04/14] machine: Improve the error reporting of smp parsing
> We have two requirements for a valid SMP configuration:
> the product of "sockets * cores * threads" must represent all the
> possible cpus, i.e., max_cpus, and then must include the initially
> present cpus, i.e., smp_cpus.
>
> So we only need to ensure 1) "sockets * cores * threads == maxcpus"
> at first and then ensure 2) "maxcpus >= cpus". With a reasonable
> order of the sanity check, we can simplify the error reporting code.
> When reporting an error message we also report the exact value of
> each topology member to make users easily see what's going on.
>
> Reviewed-by: Andrew Jones
> Signed-off-by: Yanan Wang
> ---
> hw/core/machine.c | 22 +-
> hw/i386/pc.c | 24 ++--
> 2 files changed, 19 insertions(+), 27 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 958e6e7107..e879163c3b 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -777,25 +777,21 @@ static void smp_parse(MachineState *ms,
> SMPConfiguration *config, Error **errp)
> maxcpus = maxcpus > 0 ? maxcpus : sockets * cores * threads;
> cpus = cpus > 0 ? cpus : maxcpus;
>
> -if (sockets * cores * threads < cpus) {
> -error_setg(errp, "cpu topology: "
> - "sockets (%u) * cores (%u) * threads (%u) < "
> - "smp_cpus (%u)",
> - sockets, cores, threads, cpus);
> +if (sockets * cores * threads != maxcpus) {
> +error_setg(errp, "Invalid CPU topology: "
> + "product of the hierarchy must match maxcpus: "
> + "sockets (%u) * cores (%u) * threads (%u) "
> + "!= maxcpus (%u)",
> + sockets, cores, threads, maxcpus);
> return;
> }
>
> if (maxcpus < cpus) {
> -error_setg(errp, "maxcpus must be equal to or greater than smp");
> -return;
> -}
> -
> -if (sockets * cores * threads != maxcpus) {
> error_setg(errp, "Invalid CPU topology: "
> + "maxcpus must be equal to or greater than smp: "
> "sockets (%u) * cores (%u) * threads (%u) "
> - "!= maxcpus (%u)",
> - sockets, cores, threads,
> - maxcpus);
> + "== maxcpus (%u) < smp_cpus (%u)",
> + sockets, cores, threads, maxcpus, cpus);
> return;
> }
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 9ad7ae5254..fcf6905219 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -747,25 +747,21 @@ static void pc_smp_parse(MachineState *ms,
> SMPConfiguration *config, Error **err
> maxcpus = maxcpus > 0 ? maxcpus : sockets * dies * cores * threads;
> cpus = cpus > 0 ? cpus : maxcpus;
>
> -if (sockets * dies * cores * threads < cpus) {
> -error_setg(errp, "cpu topology: "
> - "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
> - "smp_cpus (%u)",
> - sockets, dies, cores, threads, cpus);
> +if (sockets * dies * cores * threads != maxcpus) {
> +error_setg(errp, "Invalid CPU topology: "
> + "product of the hierarchy must match maxcpus: "
> + "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
> + "!= maxcpus (%u)",
> + sockets, dies, cores, threads, maxcpus);
> return;
> }
>
> if (maxcpus < cpus) {
> -error_setg(errp, "maxcpus must be equal to or greater than smp");
> -return;
> -}
> -
> -if (sockets * dies * cores * threads != maxcpus) {
> -error_setg(errp, "Invalid CPU topology deprecated: "
> +error_setg(errp, "Invalid CPU topology: "
> + "maxcpus must be equal to or greater than smp: "
> "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
> - "!= maxcpus (%u)",
> - sockets, dies, cores, threads,
> - maxcpus);
> + "== maxcpus (%u) < smp_cpus (%u)",
> + sockets, dies, cores, threads, maxcpus, cpus);
> return;
> }
>
Reviewed-by: Pankaj Gupta
Re: [PATCH for-6.2 v4 12/14] machine: Put all sanity-check in the generic SMP parser
> Put both sanity-check of the input SMP configuration and sanity-check
> of the output SMP configuration uniformly in the generic parser. Then
> machine_set_smp() will become cleaner, also all the invalid scenarios
> can be tested only by calling the parser.
>
> Signed-off-by: Yanan Wang
> ---
> hw/core/machine.c | 63 +++
> 1 file changed, 31 insertions(+), 32 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 443ae5aa1b..3dd6c6f81e 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -811,6 +811,20 @@ static void smp_parse(MachineState *ms, SMPConfiguration
> *config, Error **errp)
> unsigned threads = config->has_threads ? config->threads : 0;
> unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
>
> +/*
> + * A specified topology parameter must be greater than zero,
> + * explicit configuration like "cpus=0" is not allowed.
> + */
> +if ((config->has_cpus && config->cpus == 0) ||
> +(config->has_sockets && config->sockets == 0) ||
> +(config->has_dies && config->dies == 0) ||
> +(config->has_cores && config->cores == 0) ||
> +(config->has_threads && config->threads == 0) ||
> +(config->has_maxcpus && config->maxcpus == 0)) {
> +error_setg(errp, "CPU topology parameters must be greater than
> zero");
> +return;
> +}
> +
> /*
> * If not supported by the machine, a topology parameter must be
> * omitted or specified equal to 1.
> @@ -889,6 +903,22 @@ static void smp_parse(MachineState *ms, SMPConfiguration
> *config, Error **errp)
> topo_msg, maxcpus, cpus);
> return;
> }
> +
> +if (ms->smp.cpus < mc->min_cpus) {
> +error_setg(errp, "Invalid SMP CPUs %d. The min CPUs "
> + "supported by machine '%s' is %d",
> + ms->smp.cpus,
> + mc->name, mc->min_cpus);
> +return;
> +}
> +
> +if (ms->smp.max_cpus > mc->max_cpus) {
> +error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
> + "supported by machine '%s' is %d",
> + ms->smp.max_cpus,
> + mc->name, mc->max_cpus);
> +return;
> +}
> }
>
> static void machine_get_smp(Object *obj, Visitor *v, const char *name,
> @@ -911,7 +941,6 @@ static void machine_get_smp(Object *obj, Visitor *v,
> const char *name,
> static void machine_set_smp(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> -MachineClass *mc = MACHINE_GET_CLASS(obj);
> MachineState *ms = MACHINE(obj);
> SMPConfiguration *config;
> ERRP_GUARD();
> @@ -920,40 +949,10 @@ static void machine_set_smp(Object *obj, Visitor *v,
> const char *name,
> return;
> }
>
> -/*
> - * The CPU topology parameters must be specified greater than zero or
> - * just omitted, explicit configuration like "cpus=0" is not allowed.
> - */
> -if ((config->has_cpus && config->cpus == 0) ||
> -(config->has_sockets && config->sockets == 0) ||
> -(config->has_dies && config->dies == 0) ||
> -(config->has_cores && config->cores == 0) ||
> -(config->has_threads && config->threads == 0) ||
> -(config->has_maxcpus && config->maxcpus == 0)) {
> -error_setg(errp, "CPU topology parameters must be greater than
> zero");
> -goto out_free;
> -}
> -
> smp_parse(ms, config, errp);
> if (errp) {
> -goto out_free;
> +qapi_free_SMPConfiguration(config);
> }
> -
> -/* sanity-check smp_cpus and max_cpus against mc */
> -if (ms->smp.cpus < mc->min_cpus) {
> -error_setg(errp, "Invalid SMP CPUs %d. The min CPUs "
> - "supported by machine '%s' is %d",
> - ms->smp.cpus,
> - mc->name, mc->min_cpus);
> -} else if (ms->smp.max_cpus > mc->max_cpus) {
> -error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
> - "supported by machine '%s' is %d",
> - ms->smp.max_cpus,
> - mc->name, mc->max_cpus);
> -}
> -
> -out_free:
> -qapi_free_SMPConfiguration(config);
> }
>
> static void machine_class_init(ObjectClass *oc, void *data)
Looks good.
Reviewed-by: Pankaj Gupta
Re: [PATCH for-6.2 v4 06/14] machine: Prefer cores over sockets in smp parsing since 6.2
> In the real SMP hardware topology world, it's much more likely that
> we have high cores-per-socket counts and few sockets totally. While
> the current preference of sockets over cores in smp parsing results
> in a virtual cpu topology with low cores-per-sockets counts and a
> large number of sockets, which is just contrary to the real world.
>
> Given that it is better to make the virtual cpu topology be more
> reflective of the real world and also for the sake of compatibility,
> we start to prefer cores over sockets over threads in smp parsing
> since machine type 6.2 for different arches.
>
> In this patch, a boolean "smp_prefer_sockets" is added, and we only
> enable the old preference on older machines and enable the new one
> since type 6.2 for all arches by using the machine compat mechanism.
>
> Reviewed-by: Andrew Jones
> Acked-by: Cornelia Huck
> Acked-by: David Gibson
> Suggested-by: Daniel P. Berrange
> Signed-off-by: Yanan Wang
> ---
> hw/arm/virt.c | 1 +
> hw/core/machine.c | 36 ++--
> hw/i386/pc.c | 36 ++--
> hw/i386/pc_piix.c | 1 +
> hw/i386/pc_q35.c | 1 +
> hw/ppc/spapr.c | 1 +
> hw/s390x/s390-virtio-ccw.c | 1 +
> include/hw/boards.h| 1 +
> qemu-options.hx| 3 ++-
> 9 files changed, 60 insertions(+), 21 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 01165f7f53..7babea40dc 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2797,6 +2797,7 @@ static void virt_machine_6_1_options(MachineClass *mc)
> {
> virt_machine_6_2_options(mc);
> compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
> +mc->smp_prefer_sockets = true;
> }
> DEFINE_VIRT_MACHINE(6, 1)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 458d9736e3..a8173a0f45 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -746,6 +746,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
>
> static void smp_parse(MachineState *ms, SMPConfiguration *config, Error
> **errp)
> {
> +MachineClass *mc = MACHINE_GET_CLASS(ms);
> unsigned cpus= config->has_cpus ? config->cpus : 0;
> unsigned sockets = config->has_sockets ? config->sockets : 0;
> unsigned cores = config->has_cores ? config->cores : 0;
> @@ -757,7 +758,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration
> *config, Error **errp)
> return;
> }
>
> -/* compute missing values, prefer sockets over cores over threads */
> +/* compute missing values based on the provided ones */
> if (cpus == 0 && maxcpus == 0) {
> sockets = sockets > 0 ? sockets : 1;
> cores = cores > 0 ? cores : 1;
> @@ -765,15 +766,30 @@ static void smp_parse(MachineState *ms,
> SMPConfiguration *config, Error **errp)
> } else {
> maxcpus = maxcpus > 0 ? maxcpus : cpus;
>
> -if (sockets == 0) {
> -cores = cores > 0 ? cores : 1;
> -threads = threads > 0 ? threads : 1;
> -sockets = maxcpus / (cores * threads);
> -} else if (cores == 0) {
> -threads = threads > 0 ? threads : 1;
> -cores = maxcpus / (sockets * threads);
> -} else if (threads == 0) {
> -threads = maxcpus / (sockets * cores);
> +if (mc->smp_prefer_sockets) {
> +/* prefer sockets over cores over threads before 6.2 */
> +if (sockets == 0) {
> +cores = cores > 0 ? cores : 1;
> +threads = threads > 0 ? threads : 1;
> +sockets = maxcpus / (cores * threads);
> +} else if (cores == 0) {
> +threads = threads > 0 ? threads : 1;
> +cores = maxcpus / (sockets * threads);
> +} else if (threads == 0) {
> +threads = maxcpus / (sockets * cores);
> +}
> +} else {
> +/* prefer cores over sockets over threads since 6.2 */
> +if (cores == 0) {
> +sockets = sockets > 0 ? sockets : 1;
> +threads = threads > 0 ? threads : 1;
> +cores = maxcpus / (sockets * threads);
> +} else if (sockets == 0) {
> +threads = threads > 0 ? threads : 1;
> +sockets = maxcpus / (cores * threads);
> +} else if (threads == 0) {
> +threads = maxcpus / (sockets * cores);
> +}
I feel this code is repeated at multiple places. Also, (threads == 0) case
at the end is common for all the cases, we can move it out of if-else?
> }
> }
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index afd8b9c283..5d7c3efc43 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -717,6 +717,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int
> level)
> */
> static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error
Re: [PATCH for-6.2 3/6] s390-sclp-events-bus: Set instance_size
On 05/08/2021 21.34, Eduardo Habkost wrote:
We have a SCLPEventsBus struct defined, but the struct is not
used at the TypeInfo definition. This works today but will break
silently if anybody adds a new field to SCLPEventsBus.
Set instance_size properly to avoid problems in the future.
Signed-off-by: Eduardo Habkost
---
Cc: Cornelia Huck
Cc: Thomas Huth
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index ed92ce510d9..0a65e16cdd9 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -332,6 +332,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB
*sccb)
static const TypeInfo sclp_events_bus_info = {
.name = TYPE_SCLP_EVENTS_BUS,
.parent = TYPE_BUS,
+.instance_size = sizeof(SCLPEventsBus),
};
static void command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
Reviewed-by: Thomas Huth
Re: [PATCH for-6.2 5/6] s390x: event-facility: Use SCLP_EVENT_BUS macro
On 05/08/2021 21.34, Eduardo Habkost wrote:
Use the SCLP_EVENT_BUS macro instead of manually calling
OBJECT_CHECK.
Signed-off-by: Eduardo Habkost
---
Cc: Cornelia Huck
Cc: Thomas Huth
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 9f7883d6e20..bc706bd19b4 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -532,7 +532,7 @@ BusState *sclp_get_event_facility_bus(void)
SCLPEventsBus *sbus;
busobj = object_resolve_path_type("", TYPE_SCLP_EVENTS_BUS, NULL);
-sbus = OBJECT_CHECK(SCLPEventsBus, busobj, TYPE_SCLP_EVENTS_BUS);
+sbus = SCLP_EVENT_BUS(busobj);
if (!sbus) {
return NULL;
}
I think this could be merged with the previous patch. Anyway:
Reviewed-by: Thomas Huth
Re: [PATCH for-6.2 v2] s390-sclp-events-bus: Use OBJECT_DECLARE_SIMPLE_TYPE
On 06/08/2021 04.46, Eduardo Habkost wrote:
We have a SCLPEventsBus struct type defined, but no QOM type
checkers are declared for the type.
Use OBJECT_DECLARE_SIMPLE_TYPE to declare the struct type and
have a SCLP_EVENT_BUS typecast wrapper defined.
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
* v1 was previously submitted as part of series:
[PATCH for-6.2 0/6] qom: Fix broken OBJECT_CHECK usage
at
https://lore.kernel.org/qemu-devel/20210805193431.307761-5-ehabk...@redhat.com
* Fix typo (s/SCLP_EVENT_BUS/SCLP_EVENTS_BUS/)
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index ed92ce510d9..4bfd0b194b4 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -328,6 +328,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB
*sccb)
/* qemu object creation and initialization functions */
#define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"
+OBJECT_DECLARE_SIMPLE_TYPE(SCLPEventsBus, SCLP_EVENTS_BUS)
static const TypeInfo sclp_events_bus_info = {
.name = TYPE_SCLP_EVENTS_BUS,
Reviewed-by: Thomas Huth
Re: [RFC PATCH v4 3/4] Adding Andes AX25 and A25 CPU model
On Fri, Aug 6, 2021 at 2:00 AM Ruinland Chuan-Tzu Tsai
wrote:
>
> From: Ruinalnd ChuanTzu Tsai
>
> Adding Andes AX25 and A25 CPU model into cpu.h and cpu.c without
The latest RISC-V core from Andes is AX45 and A45. Should we just
support the latest one?
> enhanced features (yet).
>
> Signed-off-by: Dylan Jhong
> ---
> target/riscv/cpu.c | 16
> target/riscv/cpu.h | 2 ++
> 2 files changed, 18 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 3a638b5..9eb1e3a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -182,6 +182,13 @@ static void rv64_base_cpu_init(Object *obj)
> set_misa(env, RV64);
> }
>
> +static void ax25_cpu_init(Object *obj)
nits: for name consistency, should be rv64_andes_ax25_cpu_init()
> +{
> +CPURISCVState *env = _CPU(obj)->env;
> +set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> +set_priv_version(env, PRIV_VERSION_1_10_0);
> +}
> +
> static void rv64_sifive_u_cpu_init(Object *obj)
> {
> CPURISCVState *env = _CPU(obj)->env;
> @@ -235,6 +242,13 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
> set_resetvec(env, DEFAULT_RSTVEC);
> qdev_prop_set_bit(DEVICE(obj), "mmu", false);
> }
> +
> +static void a25_cpu_init(Object *obj)
nits: rv32_andes_a25_cpu_init()
> +{
> +CPURISCVState *env = _CPU(obj)->env;
> +set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
> +set_priv_version(env, PRIV_VERSION_1_10_0);
> +}
> #endif
>
> static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
> @@ -726,8 +740,10 @@ static const TypeInfo riscv_cpu_type_infos[] = {
> DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31, rv32_sifive_e_cpu_init),
> DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34, rv32_imafcu_nommu_cpu_init),
> DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, rv32_sifive_u_cpu_init),
> +DEFINE_CPU(TYPE_RISCV_CPU_A25, a25_cpu_init),
> #elif defined(TARGET_RISCV64)
> DEFINE_CPU(TYPE_RISCV_CPU_BASE64, rv64_base_cpu_init),
> +DEFINE_CPU(TYPE_RISCV_CPU_AX25, ax25_cpu_init),
> DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, rv64_sifive_e_cpu_init),
> DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, rv64_sifive_u_cpu_init),
> #endif
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 52df9bb..bd79d63 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -37,6 +37,8 @@
> #define TYPE_RISCV_CPU_ANY RISCV_CPU_TYPE_NAME("any")
> #define TYPE_RISCV_CPU_BASE32 RISCV_CPU_TYPE_NAME("rv32")
> #define TYPE_RISCV_CPU_BASE64 RISCV_CPU_TYPE_NAME("rv64")
> +#define TYPE_RISCV_CPU_A25 RISCV_CPU_TYPE_NAME("andes-a25")
> +#define TYPE_RISCV_CPU_AX25 RISCV_CPU_TYPE_NAME("andes-ax25")
> #define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex")
> #define TYPE_RISCV_CPU_SIFIVE_E31 RISCV_CPU_TYPE_NAME("sifive-e31")
> #define TYPE_RISCV_CPU_SIFIVE_E34 RISCV_CPU_TYPE_NAME("sifive-e34")
Regards,
Bin
Re: [RFC PATCH v4 2/4] Adding basic custom/vendor CSR handling mechanism
+Alistair
On Fri, Aug 6, 2021 at 1:58 AM Ruinland Chuan-Tzu Tsai
wrote:
>
> From: Ruinalnd ChuanTzu Tsai
>
> For now we add a custom CSR handling mechanism to handle non-standard CSR read
> or write.
>
> The write_stub() and read_zero() are provided for quick placeholder usage if
> such CSRs' behavior are expected to fail-over in its user code.
>
> Signed-off-by: Dylan Jhong
> ---
> target/riscv/cpu.c | 23 ++
> target/riscv/cpu.h | 31 -
> target/riscv/cpu_bits.h| 4 ++
> target/riscv/csr.c | 83 --
> target/riscv/custom_cpu_bits.h | 8
> 5 files changed, 134 insertions(+), 15 deletions(-)
> create mode 100644 target/riscv/custom_cpu_bits.h
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 7401325..3a638b5 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -144,6 +144,29 @@ static void set_resetvec(CPURISCVState *env,
> target_ulong resetvec)
> #endif
> }
>
> +#if defined(CONFIG_RISCV_CUSTOM)
> +static void setup_custom_csr(CPURISCVState *env,
> + riscv_custom_csr_operations csr_map_struct[]
> + ) {
{ should be put to the next line, per QEMU coding convention. Please
fix this globally in this series.
> +
> +env->custom_csr_map = g_hash_table_new_full(g_direct_hash, \
> +g_direct_equal, \
> +NULL, NULL);
Is it possible to juse use g_hash_table_new() directly, with both 2
parameters being NULL, given you don't provide the destroy hooks?
like:
env->custom_csr_map = g_hash_table_new(NULL, NULL);
> +
> +
> +int i;
nits: please move this to the beginning of this function.
> +for (i = 0; i < MAX_CUSTOM_CSR_NUM; i++) {
> +if (csr_map_struct[i].csrno != 0) {
> +g_hash_table_insert(env->custom_csr_map,
> +GINT_TO_POINTER(csr_map_struct[i].csrno),
> +_map_struct[i].csr_opset);
> +} else {
> +break;
break? I think we should continue the loop.
> +}
> +}
> +}
> +#endif
> +
> static void riscv_any_cpu_init(Object *obj)
> {
> CPURISCVState *env = _CPU(obj)->env;
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0edb282..52df9bb 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -239,6 +239,16 @@ struct CPURISCVState {
>
> /* Fields from here on are preserved across CPU reset. */
> QEMUTimer *timer; /* Internal timer */
> +
> +/*
> + * The reason why we have an opset map for custom CSRs and a seperated
> + * storage map is that we might have heterogeneous architecture, in which
> + * different harts have different custom CSRs.
> + * Custom CSR opset map
> + */
> +GHashTable *custom_csr_map;
> +/* Custom CSR val holder */
> +void *custom_csr_val;
> };
>
> OBJECT_DECLARE_TYPE(RISCVCPU, RISCVCPUClass,
> @@ -485,17 +495,36 @@ typedef struct {
> riscv_csr_op_fn op;
> } riscv_csr_operations;
>
> +typedef struct {
> +int csrno;
> +riscv_csr_operations csr_opset;
> +} riscv_custom_csr_operations;
} should be put in the beginning without space. Please fix this
globally in this series.
> +
> +/*
> + * The reason why we have an abstraction here is that : We could have CSR
> + * number M on hart A is an alias of CSR number N on hart B. So we make a
> + * CSR number to value address map.
> + */
> +typedef struct {
> +int csrno;
> +target_ulong val;
> +} riscv_custom_csr_vals;
> +
It looks this struct is not used by any patch in this series?
> /* CSR function table constants */
> enum {
> -CSR_TABLE_SIZE = 0x1000
> +CSR_TABLE_SIZE = 0x1000,
> +MAX_CUSTOM_CSR_NUM = 100
To be consistent, name this to be: CUSTOM_CSR_TABLE_SIZE
> };
>
> /* CSR function table */
> +extern int andes_custom_csr_size;
> +extern riscv_custom_csr_operations
> andes_custom_csr_table[MAX_CUSTOM_CSR_NUM];
The above 2 should not be in this patch.
> extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
>
> void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
> void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
>
> +
This is unnecessary.
> void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
>
> #endif /* RISCV_CPU_H */
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index caf4599..de77242 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -593,3 +593,7 @@
> #define MIE_SSIE (1 << IRQ_S_SOFT)
> #define MIE_USIE (1 << IRQ_U_SOFT)
> #endif
> +
> +#if defined(CONFIG_RISCV_CUSTOM)
> +#include "custom_cpu_bits.h"
> +#endif
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index fd2e636..1c4dc94 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -137,7 +137,8 @@ static int
[PATCH v2] hw/i386/acpi-build: Get NUMA information from struct NumaState
We can get NUMA information completely from MachineState::numa_state.
Remove PCMachineState::numa_nodes and PCMachineState::node_mem,
since they are just copied from MachineState::numa_state.
Signed-off-by: Jingqi Liu
---
hw/i386/acpi-build.c | 12 +++-
hw/i386/pc.c | 9 -
include/hw/i386/pc.h | 4
3 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 17836149fe..e3c9ad011e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1902,6 +1902,8 @@ build_srat(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
X86MachineState *x86ms = X86_MACHINE(machine);
const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
PCMachineState *pcms = PC_MACHINE(machine);
+int nb_numa_nodes = machine->numa_state->num_nodes;
+NodeInfo *numa_info = machine->numa_state->nodes;
ram_addr_t hotplugabble_address_space_size =
object_property_get_int(OBJECT(pcms), PC_MACHINE_DEVMEM_REGION_SIZE,
NULL);
@@ -1945,9 +1947,9 @@ build_srat(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
next_base = 0;
numa_start = table_data->len;
-for (i = 1; i < pcms->numa_nodes + 1; ++i) {
+for (i = 1; i < nb_numa_nodes + 1; ++i) {
mem_base = next_base;
-mem_len = pcms->node_mem[i - 1];
+mem_len = numa_info[i - 1].node_mem;
next_base = mem_base + mem_len;
/* Cut out the 640K hole */
@@ -1995,7 +1997,7 @@ build_srat(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
}
slots = (table_data->len - numa_start) / sizeof *numamem;
-for (; slots < pcms->numa_nodes + 2; slots++) {
+for (; slots < nb_numa_nodes + 2; slots++) {
numamem = acpi_data_push(table_data, sizeof *numamem);
build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
}
@@ -2011,7 +2013,7 @@ build_srat(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
if (hotplugabble_address_space_size) {
numamem = acpi_data_push(table_data, sizeof *numamem);
build_srat_memory(numamem, machine->device_memory->base,
- hotplugabble_address_space_size, pcms->numa_nodes -
1,
+ hotplugabble_address_space_size, nb_numa_nodes - 1,
MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
}
@@ -2513,7 +2515,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState
*machine)
}
}
#endif
-if (pcms->numa_nodes) {
+if (machine->numa_state->num_nodes) {
acpi_add_table(table_offsets, tables_blob);
build_srat(tables_blob, tables->linker, machine);
if (machine->numa_state->have_numa_distance) {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c2b9d62a35..adbc348488 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -800,18 +800,9 @@ void pc_machine_done(Notifier *notifier, void *data)
void pc_guest_info_init(PCMachineState *pcms)
{
-int i;
-MachineState *ms = MACHINE(pcms);
X86MachineState *x86ms = X86_MACHINE(pcms);
x86ms->apic_xrupt_override = true;
-pcms->numa_nodes = ms->numa_state->num_nodes;
-pcms->node_mem = g_malloc0(pcms->numa_nodes *
-sizeof *pcms->node_mem);
-for (i = 0; i < ms->numa_state->num_nodes; i++) {
-pcms->node_mem[i] = ms->numa_state->nodes[i].node_mem;
-}
-
pcms->machine_done.notify = pc_machine_done;
qemu_add_machine_init_done_notifier(>machine_done);
}
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 88dffe7517..31b334e0a4 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -47,10 +47,6 @@ typedef struct PCMachineState {
bool default_bus_bypass_iommu;
uint64_t max_fw_size;
-/* NUMA information: */
-uint64_t numa_nodes;
-uint64_t *node_mem;
-
/* ACPI Memory hotplug IO base address */
hwaddr memhp_io_base;
} PCMachineState;
--
2.21.3
Re: [PATCH] hw/i386/acpi-build: Get NUMA information from struct NumaState
On 8/5/2021 5:26 PM, Igor Mammedov wrote:
On Tue, 3 Aug 2021 14:30:05 +0800
Jingqi Liu wrote:
The NUMA information in PCMachineState is copied from MachineState.
We get this information uniformly from struct NumaState in MachineState.
Is there a another reason behind this patch?
As cleanup it's not complete, why do you keep
PCMachineState::numa_nodes & co around?
I'd suggest to remove it completely and use data from
MachineState everywhere.
Thanks for your confirmation.
I think so too.
We can get NUMA information completely from MachineState::numa_state.
Actually, MachineState::numa_state is used everywhere except for
pc_guest_info_init(), which just copies NUMA information to
PCMachineState::numa_nodes and PCMachineState::node_mem.
I'll remove them completely.
Thanks,
Jingqi
Signed-off-by: Jingqi Liu
---
hw/i386/acpi-build.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 17836149fe..e3c9ad011e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1902,6 +1902,8 @@ build_srat(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
X86MachineState *x86ms = X86_MACHINE(machine);
const CPUArchIdList *apic_ids = mc->possible_cpu_arch_ids(machine);
PCMachineState *pcms = PC_MACHINE(machine);
+int nb_numa_nodes = machine->numa_state->num_nodes;
+NodeInfo *numa_info = machine->numa_state->nodes;
ram_addr_t hotplugabble_address_space_size =
object_property_get_int(OBJECT(pcms), PC_MACHINE_DEVMEM_REGION_SIZE,
NULL);
@@ -1945,9 +1947,9 @@ build_srat(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
next_base = 0;
numa_start = table_data->len;
-for (i = 1; i < pcms->numa_nodes + 1; ++i) {
+for (i = 1; i < nb_numa_nodes + 1; ++i) {
mem_base = next_base;
-mem_len = pcms->node_mem[i - 1];
+mem_len = numa_info[i - 1].node_mem;
next_base = mem_base + mem_len;
/* Cut out the 640K hole */
@@ -1995,7 +1997,7 @@ build_srat(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
}
slots = (table_data->len - numa_start) / sizeof *numamem;
-for (; slots < pcms->numa_nodes + 2; slots++) {
+for (; slots < nb_numa_nodes + 2; slots++) {
numamem = acpi_data_push(table_data, sizeof *numamem);
build_srat_memory(numamem, 0, 0, 0, MEM_AFFINITY_NOFLAGS);
}
@@ -2011,7 +2013,7 @@ build_srat(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
if (hotplugabble_address_space_size) {
numamem = acpi_data_push(table_data, sizeof *numamem);
build_srat_memory(numamem, machine->device_memory->base,
- hotplugabble_address_space_size, pcms->numa_nodes -
1,
+ hotplugabble_address_space_size, nb_numa_nodes - 1,
MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
}
@@ -2513,7 +2515,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
}
}
#endif
-if (pcms->numa_nodes) {
+if (machine->numa_state->num_nodes) {
acpi_add_table(table_offsets, tables_blob);
build_srat(tables_blob, tables->linker, machine);
if (machine->numa_state->have_numa_distance) {
Win32 and ACCEL macro/function
Hello, I'm looking for help dealing with a naming conflict when building QEMU for Windows hosts. The summary is: I'm trying to replace the ACCEL() macro in include/qemu/accel.h with an inline function, but the ACCEL name conflicts with a symbol provided by winuser.h: In file included from /builds/ehabkost/qemu/include/exec/memory.h:28, from /builds/ehabkost/qemu/hw/ppc/mac.h:30, from ../hw/pci-host/uninorth.c:27: /builds/ehabkost/qemu/include/qemu/accel.h:63:45: error: 'ACCEL' redeclared as different kind of symbol 63 | OBJECT_DECLARE_TYPE(AccelState, AccelClass, ACCEL) | ^ /builds/ehabkost/qemu/include/qom/object.h:178:5: note: in definition of macro 'DECLARE_INSTANCE_CHECKER' 178 | OBJ_NAME(const void *obj) \ | ^~~~ /builds/ehabkost/qemu/include/qom/object.h:240:5: note: in expansion of macro 'DECLARE_OBJ_CHECKERS' 240 | DECLARE_OBJ_CHECKERS(InstanceType, ClassType, \ | ^~~~ /builds/ehabkost/qemu/include/qemu/accel.h:63:1: note: in expansion of macro 'OBJECT_DECLARE_TYPE' 63 | OBJECT_DECLARE_TYPE(AccelState, AccelClass, ACCEL) | ^~~ In file included from /usr/x86_64-w64-mingw32/sys-root/mingw/include/windows.h:72, from /usr/x86_64-w64-mingw32/sys-root/mingw/include/winsock2.h:23, from /builds/ehabkost/qemu/include/sysemu/os-win32.h:29, from /builds/ehabkost/qemu/include/qemu/osdep.h:135, from ../hw/pci-host/uninorth.c:25: /usr/x86_64-w64-mingw32/sys-root/mingw/include/winuser.h:1757:5: note: previous declaration of 'ACCEL' was here 1757 | } ACCEL,*LPACCEL; | ^ [338/4278] Compiling C object libqemuutil.a.p/meson-generated_.._trace_trace-scsi.c.obj ninja: build stopped: subcommand failed. make: *** [Makefile:156: run-ninja] Error 1 (Full log at https://gitlab.com/ehabkost/qemu/-/jobs/1481978645) Does anybody more experienced with Win32 have a suggestion on how to deal with this? Do we really need to include winsock2.h / windows.h / winuser.h from qemu/osdep.h? -- Eduardo
Re: [RFC PATCH 01/13] target/riscv: Add UXL to tb flags
On 2021/8/6 上午3:01, Richard Henderson wrote:
On 8/4/21 4:53 PM, LIU Zhiwei wrote:
For 32-bit applications run on 64-bit cpu, it may share some code
with other 64-bit applictions. Thus we should distinguish the translated
cache of the share code with a tb flag.
Signed-off-by: LIU Zhiwei
---
target/riscv/cpu.h | 15 +++
target/riscv/translate.c | 3 +++
2 files changed, 18 insertions(+)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index bf1c899c00..2b3ba21a78 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -394,9 +394,20 @@ FIELD(TB_FLAGS, SEW, 5, 3)
FIELD(TB_FLAGS, VILL, 8, 1)
/* Is a Hypervisor instruction load/store allowed? */
FIELD(TB_FLAGS, HLSX, 9, 1)
+FIELD(TB_FLAGS, UXL, 10, 2)
Are you intending to reserve space for RV128 here?
Otherwise this could be a single bit.
Yes.
Also, you probably don't want to name it "UXL", since it should
indicate the current operating XLEN, taking MXL, SXL and UXL into
account.
Hi Richard,
I don't have the ambitious to define a XLEN32 at least for this patch
set. I think it is much more difficult.
The only purpose of this patch set is that we can run 32-bit application
on a 64 bit Linux from qemu-system-riscv64.
So I didn't change the default behavior of every instruction except when
1. Current CPU is 64 bit CPU, i.s. TARGET_LONG_BITS is 64.
2. Current privileged is U-mode.
3. UXL is 1.
I know that Alistair has done a lot to support 32-bit CPU on
qemu-system-riscv64. But We are doing different things,
and it maybe a little confusing.
I still do not find a good to combine them. In my opinion, some code in
this patch set can be reused for SXL32.
If you have any advice, please let me know.
Best Regards,
Zhiwei
Perhaps just name the field XLEN32, and make it a single bit?
+static inline bool riscv_cpu_is_uxl32(CPURISCVState *env)
+{
+#ifndef CONFIG_USER_ONLY
+ return (get_field(env->mstatus, MSTATUS64_UXL) == 1) &&
+ !riscv_cpu_is_32bit(env) &&
+ (env->priv == PRV_U);
+#endif
+ return false;
+}
Again, naming could be better?
It seems trivial to handle all of the fields here. Perhaps
static inline bool riscv_cpu_is_xlen32(env)
{
#if defined(TARGET_RISCV32)
return true;
#elif defined(CONFIG_USER_ONLY)
return false;
#else
/* When emulating a 32-bit-only cpu, use RV32. */
if (riscv_cpu_is_32bit(env)) {
return true;
}
/*
* If MXL has been reduced to RV32, MSTATUSH doesn't have UXL/SXL,
* therefore, XLEN cannot be widened back to RV64 for lower privs.
*/
if (get_field(env->misa, MISA64_MXL) == 1) {
return true;
}
switch (env->priv) {
case PRV_M:
return false;
case PRV_U:
return get_field(env->mstatus, MSTATUS64_UXL) == 1;
default: /* PRV_S & PRV_H */
return get_field(env->mstatus, MSTATUS64_SXL) == 1;
}
#endif
}
@@ -451,6 +462,10 @@ static inline void
cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
flags = FIELD_DP32(flags, TB_FLAGS, HLSX, 1);
}
}
+ if (riscv_cpu_is_uxl32(env)) {
+ flags = FIELD_DP32(flags, TB_FLAGS, UXL,
+ get_field(env->mstatus, MSTATUS64_UXL));
flags = FIELD_DP32(flags, TB_FLAGS, XLEN32,
riscv_cpu_is_xlen32(env));
r~
[PATCH for-6.2 v2] s390-sclp-events-bus: Use OBJECT_DECLARE_SIMPLE_TYPE
We have a SCLPEventsBus struct type defined, but no QOM type
checkers are declared for the type.
Use OBJECT_DECLARE_SIMPLE_TYPE to declare the struct type and
have a SCLP_EVENT_BUS typecast wrapper defined.
Signed-off-by: Eduardo Habkost
---
Changes v1 -> v2:
* v1 was previously submitted as part of series:
[PATCH for-6.2 0/6] qom: Fix broken OBJECT_CHECK usage
at
https://lore.kernel.org/qemu-devel/20210805193431.307761-5-ehabk...@redhat.com
* Fix typo (s/SCLP_EVENT_BUS/SCLP_EVENTS_BUS/)
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index ed92ce510d9..4bfd0b194b4 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -328,6 +328,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB
*sccb)
/* qemu object creation and initialization functions */
#define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"
+OBJECT_DECLARE_SIMPLE_TYPE(SCLPEventsBus, SCLP_EVENTS_BUS)
static const TypeInfo sclp_events_bus_info = {
.name = TYPE_SCLP_EVENTS_BUS,
--
2.31.1
Re: [PATCH for-6.2 4/6] s390-sclp-events-bus: Use OBJECT_DECLARE_SIMPLE_TYPE
On Thu, Aug 05, 2021 at 03:34:29PM -0400, Eduardo Habkost wrote: > We have a SCLPEventsBus struct type defined, but no QOM type > checkers are declared for the type. > > Use OBJECT_DECLARE_SIMPLE_TYPE to declare the struct type and > have a SCLP_EVENT_BUS typecast wrapper defined. > > Signed-off-by: Eduardo Habkost > --- > Cc: Richard Henderson > Cc: David Hildenbrand > Cc: Cornelia Huck > Cc: Halil Pasic > Cc: Christian Borntraeger > Cc: Thomas Huth > Cc: qemu-s3...@nongnu.org > Cc: qemu-devel@nongnu.org > --- > hw/s390x/event-facility.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c > index 0a65e16cdd9..9f7883d6e20 100644 > --- a/hw/s390x/event-facility.c > +++ b/hw/s390x/event-facility.c > @@ -328,6 +328,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB > *sccb) > /* qemu object creation and initialization functions */ > > #define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus" > +OBJECT_DECLARE_SIMPLE_TYPE(SCLPEventsBus, SCLP_EVENT_BUS) Oops, a typo (should be SCLP_EVENTS_BUS instead). I will submit v2 later as a standalone patch. -- Eduardo
Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR
On Fri, Aug 6, 2021 at 10:39 AM Bin Meng wrote: > > On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai > wrote: > > > > From: Ruinland ChuanTzu Tsai > > > > Adding option `riscv_custom` to configure script, meson.build and > > meson_options.txt so as to toggle custom CSR and will-be-upstreamed custom > > instructions handling logic. > > > > Signed-off-by: Dylan Jhong > > --- > > configure | 6 ++ > > meson.build | 2 ++ > > meson_options.txt | 2 ++ > > 3 files changed, 10 insertions(+) > > > > This sounds like unnecessary to bring such a config option to the meson level. > > I believe a Kconfig option should just be fine. +Alistair
Re: [RFC PATCH v4 1/4] Add options to config/meson files for custom CSR
On Fri, Aug 6, 2021 at 1:57 AM Ruinland Chuan-Tzu Tsai wrote: > > From: Ruinland ChuanTzu Tsai > > Adding option `riscv_custom` to configure script, meson.build and > meson_options.txt so as to toggle custom CSR and will-be-upstreamed custom > instructions handling logic. > > Signed-off-by: Dylan Jhong > --- > configure | 6 ++ > meson.build | 2 ++ > meson_options.txt | 2 ++ > 3 files changed, 10 insertions(+) > This sounds like unnecessary to bring such a config option to the meson level. I believe a Kconfig option should just be fine. Regards, Bin
[PATCH for-6.2] sbsa-ref: Rename SBSA_GWDT enum value
The SBSA_GWDT enum value conflicts with the SBSA_GWDT() QOM type
checking helper, preventing us from using a OBJECT_DEFINE* or
DEFINE_INSTANCE_CHECKER macro for the SBSA_GWDT() wrapper.
If I understand the SBSA 6.0 specification correctly, the signal
being connected to IRQ 16 is the WS0 output signal from the
Generic Watchdog. Rename the enum value to SBSA_GWDT_WS0 to be
more explicit and avoid the name conflict.
Signed-off-by: Eduardo Habkost
---
Cc: Radoslaw Biernacki
Cc: Peter Maydell
Cc: Leif Lindholm
Cc: Shashi Mallela
Cc: qemu-...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/arm/sbsa-ref.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index c1629df6031..509c5f09b4e 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -65,7 +65,7 @@ enum {
SBSA_GIC_DIST,
SBSA_GIC_REDIST,
SBSA_SECURE_EC,
-SBSA_GWDT,
+SBSA_GWDT_WS0,
SBSA_GWDT_REFRESH,
SBSA_GWDT_CONTROL,
SBSA_SMMU,
@@ -140,7 +140,7 @@ static const int sbsa_ref_irqmap[] = {
[SBSA_AHCI] = 10,
[SBSA_EHCI] = 11,
[SBSA_SMMU] = 12, /* ... to 15 */
-[SBSA_GWDT] = 16,
+[SBSA_GWDT_WS0] = 16,
};
static const char * const valid_cpus[] = {
@@ -481,7 +481,7 @@ static void create_wdt(const SBSAMachineState *sms)
hwaddr cbase = sbsa_ref_memmap[SBSA_GWDT_CONTROL].base;
DeviceState *dev = qdev_new(TYPE_WDT_SBSA);
SysBusDevice *s = SYS_BUS_DEVICE(dev);
-int irq = sbsa_ref_irqmap[SBSA_GWDT];
+int irq = sbsa_ref_irqmap[SBSA_GWDT_WS0];
sysbus_realize_and_unref(s, _fatal);
sysbus_mmio_map(s, 0, rbase);
--
2.31.1
Re: [PATCH v2 4/4] hw/riscv: virt: Add optional ACLINT support to virt machine
On Sat, Jul 24, 2021 at 8:27 PM Anup Patel wrote: > > We extend virt machine to emulate ACLINT devices only when "aclint=on" > parameter is passed along with machine name in QEMU command-line. > > Signed-off-by: Anup Patel > --- > hw/riscv/virt.c | 113 +++- > include/hw/riscv/virt.h | 2 + > 2 files changed, 114 insertions(+), 1 deletion(-) > Reviewed-by: Bin Meng Please add a "Machine-specific options" in the Virt documentation. See sifive_u example @ https://qemu.readthedocs.io/en/latest/system/riscv/sifive_u.html#machine-specific-options Regards, Bin
Re: [PATCH v2 3/4] hw/riscv: virt: Re-factor FDT generation
On Sat, Jul 24, 2021 at 8:24 PM Anup Patel wrote: > > We re-factor and break the FDT generation into smaller functions > so that it is easier to modify FDT generation for different > configurations of virt machine. > > Signed-off-by: Anup Patel > --- > hw/riscv/virt.c | 521 ++-- > 1 file changed, 324 insertions(+), 197 deletions(-) > Reviewed-by: Bin Meng
Re: [PATCH v2 2/4] hw/intc: Upgrade the SiFive CLINT implementation to RISC-V ACLINT
On Sat, Jul 24, 2021 at 8:27 PM Anup Patel wrote:
>
> The RISC-V ACLINT is more modular and backward compatible with
> original SiFive CLINT so instead of duplicating the original
> SiFive CLINT implementation we upgrade the current SiFive CLINT
> implementation to RISC-V ACLINT implementation.
>
> Signed-off-by: Anup Patel
> ---
> hw/intc/riscv_aclint.c | 351 ++---
> hw/riscv/microchip_pfsoc.c | 9 +-
> hw/riscv/shakti_c.c| 11 +-
> hw/riscv/sifive_e.c| 11 +-
> hw/riscv/sifive_u.c| 9 +-
> hw/riscv/spike.c | 14 +-
> hw/riscv/virt.c| 14 +-
> include/hw/intc/riscv_aclint.h | 54 +++--
> 8 files changed, 320 insertions(+), 153 deletions(-)
>
> diff --git a/hw/intc/riscv_aclint.c b/hw/intc/riscv_aclint.c
> index 0f940e332b..367f850b47 100644
> --- a/hw/intc/riscv_aclint.c
> +++ b/hw/intc/riscv_aclint.c
> @@ -1,8 +1,9 @@
> /*
> - * SiFive CLINT (Core Local Interruptor)
> + * RISC-V ACLINT (Advanced Core Local Interruptor)
We can put the ACLINT spec link here for referenence
> *
> * Copyright (c) 2016-2017 Sagar Karandikar, sag...@eecs.berkeley.edu
> * Copyright (c) 2017 SiFive, Inc.
> + * Copyright (c) 2021 Western Digital Corporation or its affiliates.
> *
> * This provides real-time clock, timer and interprocessor interrupts.
> *
> @@ -30,10 +31,10 @@
> #include "qemu/timer.h"
> #include "hw/irq.h"
>
> -typedef struct sifive_clint_callback {
> -SiFiveCLINTState *s;
> +typedef struct riscv_aclint_mtimer_callback {
> +RISCVAclintMTimerState *s;
> int num;
> -} sifive_clint_callback;
> +} riscv_aclint_mtimer_callback;
>
> static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
> {
> @@ -45,7 +46,8 @@ static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
> * Called when timecmp is written to update the QEMU timer or immediately
> * trigger timer interrupt if mtimecmp <= current timer value.
> */
> -static void sifive_clint_write_timecmp(SiFiveCLINTState *s, RISCVCPU *cpu,
> +static void riscv_aclint_mtimer_write_timecmp(RISCVAclintMTimerState *mtimer,
> + RISCVCPU *cpu,
nits: please align the argument to the (
> int hartid,
> uint64_t value,
> uint32_t timebase_freq)
> @@ -57,14 +59,16 @@ static void sifive_clint_write_timecmp(SiFiveCLINTState
> *s, RISCVCPU *cpu,
>
> cpu->env.timecmp = value;
> if (cpu->env.timecmp <= rtc_r) {
> -/* if we're setting an MTIMECMP value in the "past",
> - immediately raise the timer interrupt */
> -qemu_irq_raise(s->timer_irqs[hartid - s->hartid_base]);
> +/*
> + * If we're setting an MTIMECMP value in the "past",
> + * immediately raise the timer interrupt
> + */
> +qemu_irq_raise(mtimer->timer_irqs[hartid - mtimer->hartid_base]);
> return;
> }
>
> /* otherwise, set up the future timer interrupt */
> -qemu_irq_lower(s->timer_irqs[hartid - s->hartid_base]);
> +qemu_irq_lower(mtimer->timer_irqs[hartid - mtimer->hartid_base]);
> diff = cpu->env.timecmp - rtc_r;
> /* back to ns (note args switched in muldiv64) */
> next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> @@ -76,38 +80,27 @@ static void sifive_clint_write_timecmp(SiFiveCLINTState
> *s, RISCVCPU *cpu,
> * Callback used when the timer set using timer_mod expires.
> * Should raise the timer interrupt line
> */
> -static void sifive_clint_timer_cb(void *opaque)
> +static void riscv_aclint_mtimer_cb(void *opaque)
> {
> -sifive_clint_callback *state = opaque;
> +riscv_aclint_mtimer_callback *state = opaque;
>
> qemu_irq_raise(state->s->timer_irqs[state->num]);
> }
>
> -/* CPU wants to read rtc or timecmp register */
> -static uint64_t sifive_clint_read(void *opaque, hwaddr addr, unsigned size)
> +/* CPU read MTIMER register */
> +static uint64_t riscv_aclint_mtimer_read(void *opaque, hwaddr addr,
> +unsigned size)
> {
> -SiFiveCLINTState *clint = opaque;
> -if (addr >= clint->sip_base &&
> -addr < clint->sip_base + (clint->num_harts << 2)) {
> -size_t hartid = clint->hartid_base + ((addr - clint->sip_base) >> 2);
> -CPUState *cpu = qemu_get_cpu(hartid);
> -CPURISCVState *env = cpu ? cpu->env_ptr : NULL;
> -if (!env) {
> -error_report("clint: invalid timecmp hartid: %zu", hartid);
> -} else if ((addr & 0x3) == 0) {
> -return (env->mip & MIP_MSIP) > 0;
> -} else {
> -error_report("clint: invalid read: %08x", (uint32_t)addr);
> -return 0;
> -}
> -} else if (addr >= clint->timecmp_base &&
> -addr < clint->timecmp_base + (clint->num_harts << 3)) {
> -size_t hartid = clint->hartid_base +
> -((addr - clint->timecmp_base) >>
Re: [PATCH v2 1/4] hw/intc: Rename sifive_clint sources to riscv_aclint sources
On Sat, Jul 24, 2021 at 8:24 PM Anup Patel wrote:
>
> We will be upgrading SiFive CLINT implementation into RISC-V ACLINT
> implementation so let's first rename the sources.
>
> Signed-off-by: Anup Patel
> ---
> hw/intc/Kconfig| 2 +-
> hw/intc/meson.build| 2 +-
> hw/intc/{sifive_clint.c => riscv_aclint.c} | 2 +-
> hw/riscv/Kconfig | 12 ++--
> hw/riscv/microchip_pfsoc.c | 2 +-
> hw/riscv/shakti_c.c| 2 +-
> hw/riscv/sifive_e.c| 2 +-
> hw/riscv/sifive_u.c| 2 +-
> hw/riscv/spike.c | 2 +-
> hw/riscv/virt.c| 2 +-
> include/hw/intc/{sifive_clint.h => riscv_aclint.h} | 0
> 11 files changed, 15 insertions(+), 15 deletions(-)
> rename hw/intc/{sifive_clint.c => riscv_aclint.c} (99%)
> rename include/hw/intc/{sifive_clint.h => riscv_aclint.h} (100%)
>
Reviewed-by: Bin Meng
[PATCH v6 05/10] ACPI ERST: support for ACPI ERST feature
This implements a PCI device for ACPI ERST. This implements the
non-NVRAM "mode" of operation for ERST as it is supported by
Linux and Windows.
Signed-off-by: Eric DeVolder
---
hw/acpi/erst.c | 750 +++
hw/acpi/meson.build | 1 +
hw/acpi/trace-events | 15 ++
3 files changed, 766 insertions(+)
create mode 100644 hw/acpi/erst.c
diff --git a/hw/acpi/erst.c b/hw/acpi/erst.c
new file mode 100644
index 000..eb4ab34
--- /dev/null
+++ b/hw/acpi/erst.c
@@ -0,0 +1,750 @@
+/*
+ * ACPI Error Record Serialization Table, ERST, Implementation
+ *
+ * ACPI ERST introduced in ACPI 4.0, June 16, 2009.
+ * ACPI Platform Error Interfaces : Error Serialization
+ *
+ * Copyright (c) 2021 Oracle and/or its affiliates.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include
+#include
+#include
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/qdev-core.h"
+#include "exec/memory.h"
+#include "qom/object.h"
+#include "hw/pci/pci.h"
+#include "qom/object_interfaces.h"
+#include "qemu/error-report.h"
+#include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/acpi-defs.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/bios-linker-loader.h"
+#include "exec/address-spaces.h"
+#include "sysemu/hostmem.h"
+#include "hw/acpi/erst.h"
+#include "trace.h"
+
+/* ACPI 4.0: Table 17-16 Serialization Actions */
+#define ACTION_BEGIN_WRITE_OPERATION 0x0
+#define ACTION_BEGIN_READ_OPERATION 0x1
+#define ACTION_BEGIN_CLEAR_OPERATION 0x2
+#define ACTION_END_OPERATION 0x3
+#define ACTION_SET_RECORD_OFFSET 0x4
+#define ACTION_EXECUTE_OPERATION 0x5
+#define ACTION_CHECK_BUSY_STATUS 0x6
+#define ACTION_GET_COMMAND_STATUS0x7
+#define ACTION_GET_RECORD_IDENTIFIER 0x8
+#define ACTION_SET_RECORD_IDENTIFIER 0x9
+#define ACTION_GET_RECORD_COUNT 0xA
+#define ACTION_BEGIN_DUMMY_WRITE_OPERATION 0xB
+#define ACTION_RESERVED 0xC
+#define ACTION_GET_ERROR_LOG_ADDRESS_RANGE 0xD
+#define ACTION_GET_ERROR_LOG_ADDRESS_LENGTH 0xE
+#define ACTION_GET_ERROR_LOG_ADDRESS_RANGE_ATTRIBUTES 0xF
+#define ACTION_GET_EXECUTE_OPERATION_TIMINGS 0x10
+
+/* ACPI 4.0: Table 17-17 Command Status Definitions */
+#define STATUS_SUCCESS0x00
+#define STATUS_NOT_ENOUGH_SPACE 0x01
+#define STATUS_HARDWARE_NOT_AVAILABLE 0x02
+#define STATUS_FAILED 0x03
+#define STATUS_RECORD_STORE_EMPTY 0x04
+#define STATUS_RECORD_NOT_FOUND 0x05
+
+
+/* UEFI 2.1: Appendix N Common Platform Error Record */
+#define UEFI_CPER_RECORD_MIN_SIZE 128U
+#define UEFI_CPER_RECORD_LENGTH_OFFSET 20U
+#define UEFI_CPER_RECORD_ID_OFFSET 96U
+#define IS_UEFI_CPER_RECORD(ptr) \
+(((ptr)[0] == 'C') && \
+ ((ptr)[1] == 'P') && \
+ ((ptr)[2] == 'E') && \
+ ((ptr)[3] == 'R'))
+#define THE_UEFI_CPER_RECORD_ID(ptr) \
+(*(uint64_t *)(&(ptr)[UEFI_CPER_RECORD_ID_OFFSET]))
+
+/*
+ * This implementation is an ACTION (cmd) and VALUE (data)
+ * interface consisting of just two 64-bit registers.
+ */
+#define ERST_REG_SIZE (16UL)
+#define ERST_ACTION_OFFSET (0UL) /* action (cmd) */
+#define ERST_VALUE_OFFSET (8UL) /* argument/value (data) */
+
+/*
+ * ERST_RECORD_SIZE is the buffer size for exchanging ERST
+ * record contents. Thus, it defines the maximum record size.
+ * As this is mapped through a PCI BAR, it must be a power of
+ * two and larger than UEFI_CPER_RECORD_MIN_SIZE.
+ * The backing storage is divided into fixed size "slots",
+ * each ERST_RECORD_SIZE in length, and each "slot"
+ * storing a single record. No attempt at optimizing storage
+ * through compression, compaction, etc is attempted.
+ * NOTE that slot 0 is reserved for the backing storage header.
+ * Depending upon the size of the backing storage, additional
+ * slots will be part of the slot 0 header in order to account
+ * for a record_id for each available remaining slot.
+ */
+/* 8KiB records, not too small, not too big */
+#define ERST_RECORD_SIZE (8192UL)
+
+#define ACPI_ERST_MEMDEV_PROP "memdev"
+
+/*
+ * From the ACPI ERST spec sections:
+ * A record id of all 0s is used to indicate
+ * 'unspecified' record id.
+ * A record id of all 1s is used to indicate
+ * empty or end.
+ */
+#define ERST_UNSPECIFIED_RECORD_ID (0UL)
+#define ERST_EMPTY_END_RECORD_ID (~0UL)
+#define ERST_EXECUTE_OPERATION_MAGIC 0x9CUL
+#define ERST_IS_VALID_RECORD_ID(rid) \
+((rid != ERST_UNSPECIFIED_RECORD_ID) && \
+ (rid != ERST_EMPTY_END_RECORD_ID))
+
+typedef struct erst_storage_header_s {
+#define ERST_STORE_MAGIC 0x524F545354535245UL
+uint64_t magic;
+uint32_t record_size;
+uint32_t record_offset; /* offset to record storage beyond header */
+uint16_t version;
+uint16_t reserved;
+uint32_t record_count;
+uint64_t map[]; /* contains record_ids, and position indicates index */
+}
[PATCH v6 09/10] ACPI ERST: bios-tables-test testcase
This change implements the test suite checks for the ERST table.
Signed-off-by: Eric DeVolder
---
tests/qtest/bios-tables-test.c | 43 ++
1 file changed, 43 insertions(+)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 51d3a4e..6ee78ec 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1378,6 +1378,45 @@ static void test_acpi_piix4_tcg_acpi_hmat(void)
test_acpi_tcg_acpi_hmat(MACHINE_PC);
}
+static void test_acpi_erst(const char *machine)
+{
+test_data data;
+
+memset(, 0, sizeof(data));
+data.machine = machine;
+/*data.variant = ".acpierst";*/
+test_acpi_one(" -object memory-backend-file,id=erstnvram,"
+"mem-path=tests/acpi-erst.XX,size=0x1,share=on"
+" -device acpi-erst,memdev=erstnvram",
+ );
+free_test_data();
+}
+
+static void test_acpi_piix4_erst(void)
+{
+test_acpi_erst(MACHINE_PC);
+}
+
+static void test_acpi_q35_erst(void)
+{
+test_acpi_erst(MACHINE_Q35);
+}
+
+static void test_acpi_microvm_erst(void)
+{
+test_data data;
+
+test_acpi_microvm_prepare();
+data.variant = ".pcie";
+data.tcg_only = true; /* need constant host-phys-bits */
+test_acpi_one(" -machine microvm,acpi=on,ioapic2=off,rtc=off,pcie=on "
+"-object memory-backend-file,id=erstnvram,"
+"mem-path=tests/acpi-erst.XX,size=0x1,share=on "
+"-device acpi-erst,memdev=erstnvram",
+ );
+free_test_data();
+}
+
static void test_acpi_virt_tcg(void)
{
test_data data = {
@@ -1560,7 +1599,11 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/microvm/oem-fields",
test_acpi_oem_fields_microvm);
if (strcmp(arch, "x86_64") == 0) {
qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
+qtest_add_func("acpi/microvm/acpierst", test_acpi_microvm_erst);
}
+qtest_add_func("acpi/piix4/acpierst", test_acpi_piix4_erst);
+qtest_add_func("acpi/q35/acpierst", test_acpi_q35_erst);
+
} else if (strcmp(arch, "aarch64") == 0) {
qtest_add_func("acpi/virt", test_acpi_virt_tcg);
qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
--
1.8.3.1
[PATCH v6 08/10] ACPI ERST: qtest for ERST
This change provides a qtest that locates and then does a simple
interrogation of the ERST feature within the guest.
Signed-off-by: Eric DeVolder
---
tests/qtest/erst-test.c | 167
tests/qtest/meson.build | 2 +
2 files changed, 169 insertions(+)
create mode 100644 tests/qtest/erst-test.c
diff --git a/tests/qtest/erst-test.c b/tests/qtest/erst-test.c
new file mode 100644
index 000..370c119
--- /dev/null
+++ b/tests/qtest/erst-test.c
@@ -0,0 +1,167 @@
+/*
+ * QTest testcase for acpi-erst
+ *
+ * Copyright (c) 2021 Oracle
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include
+#include "libqos/libqos-pc.h"
+#include "libqos/libqtest.h"
+#include "qemu-common.h"
+
+static void save_fn(QPCIDevice *dev, int devfn, void *data)
+{
+QPCIDevice **pdev = (QPCIDevice **) data;
+
+*pdev = dev;
+}
+
+static QPCIDevice *get_device(QPCIBus *pcibus)
+{
+QPCIDevice *dev;
+
+dev = NULL;
+qpci_device_foreach(pcibus, 0x1b36, 0x0012, save_fn, );
+g_assert(dev != NULL);
+
+return dev;
+}
+
+typedef struct _ERSTState {
+QOSState *qs;
+QPCIBar reg_bar, mem_bar;
+uint64_t reg_barsize, mem_barsize;
+QPCIDevice *dev;
+} ERSTState;
+
+#define ACTION 0
+#define VALUE 8
+
+static const char *reg2str(unsigned reg)
+{
+switch (reg) {
+case 0:
+return "ACTION";
+case 8:
+return "VALUE";
+default:
+return NULL;
+}
+}
+
+static inline uint32_t in_reg32(ERSTState *s, unsigned reg)
+{
+const char *name = reg2str(reg);
+uint32_t res;
+
+res = qpci_io_readl(s->dev, s->reg_bar, reg);
+g_test_message("*%s -> %08x", name, res);
+
+return res;
+}
+
+static inline uint64_t in_reg64(ERSTState *s, unsigned reg)
+{
+const char *name = reg2str(reg);
+uint64_t res;
+
+res = qpci_io_readq(s->dev, s->reg_bar, reg);
+g_test_message("*%s -> %016llx", name, (unsigned long long)res);
+
+return res;
+}
+
+static inline void out_reg32(ERSTState *s, unsigned reg, uint32_t v)
+{
+const char *name = reg2str(reg);
+
+g_test_message("%08x -> *%s", v, name);
+qpci_io_writel(s->dev, s->reg_bar, reg, v);
+}
+
+static inline void out_reg64(ERSTState *s, unsigned reg, uint64_t v)
+{
+const char *name = reg2str(reg);
+
+g_test_message("%016llx -> *%s", (unsigned long long)v, name);
+qpci_io_writeq(s->dev, s->reg_bar, reg, v);
+}
+
+static void cleanup_vm(ERSTState *s)
+{
+g_free(s->dev);
+qtest_shutdown(s->qs);
+}
+
+static void setup_vm_cmd(ERSTState *s, const char *cmd)
+{
+const char *arch = qtest_get_arch();
+
+if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+s->qs = qtest_pc_boot(cmd);
+} else {
+g_printerr("erst-test tests are only available on x86\n");
+exit(EXIT_FAILURE);
+}
+s->dev = get_device(s->qs->pcibus);
+
+s->reg_bar = qpci_iomap(s->dev, 0, >reg_barsize);
+g_assert_cmpuint(s->reg_barsize, ==, 16);
+
+s->mem_bar = qpci_iomap(s->dev, 1, >mem_barsize);
+g_assert_cmpuint(s->mem_barsize, ==, 0x2000);
+
+qpci_device_enable(s->dev);
+}
+
+static void test_acpi_erst_basic(void)
+{
+ERSTState state;
+uint64_t log_address_range;
+uint64_t log_address_length;
+uint32_t log_address_attr;
+
+setup_vm_cmd(,
+"-object memory-backend-file,"
+"mem-path=acpi-erst.XX,"
+"size=64K,"
+"share=on,"
+"id=nvram "
+"-device acpi-erst,"
+"memdev=nvram");
+
+out_reg32(, ACTION, 0xD);
+log_address_range = in_reg64(, VALUE);
+out_reg32(, ACTION, 0xE);
+log_address_length = in_reg64(, VALUE);
+out_reg32(, ACTION, 0xF);
+log_address_attr = in_reg32(, VALUE);
+
+/* Check log_address_range is not 0, ~0 or base */
+g_assert_cmpuint(log_address_range, !=, 0ULL);
+g_assert_cmpuint(log_address_range, !=, ~0ULL);
+g_assert_cmpuint(log_address_range, !=, state.reg_bar.addr);
+g_assert_cmpuint(log_address_range, ==, state.mem_bar.addr);
+
+/* Check log_address_length is bar1_size */
+g_assert_cmpuint(log_address_length, ==, state.mem_barsize);
+
+/* Check log_address_attr is 0 */
+g_assert_cmpuint(log_address_attr, ==, 0);
+
+cleanup_vm();
+}
+
+int main(int argc, char **argv)
+{
+int ret;
+
+g_test_init(, , NULL);
+qtest_add_func("/acpi-erst/basic", test_acpi_erst_basic);
+ret = g_test_run();
+return ret;
+}
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index e22a079..b69834d 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -68,6 +68,7 @@ qtests_i386 = \
(config_all_devices.has_key('CONFIG_RTL8139_PCI') ? ['rtl8139-test'] : []) +
\
(config_all_devices.has_key('CONFIG_E1000E_PCI_EXPRESS') ?
['fuzz-e1000e-test'] : []) + \
[PATCH v6 06/10] ACPI ERST: build the ACPI ERST table
This builds the ACPI ERST table to inform OSPM how to communicate
with the acpi-erst device.
Signed-off-by: Eric DeVolder
---
hw/acpi/erst.c | 239 +
1 file changed, 239 insertions(+)
diff --git a/hw/acpi/erst.c b/hw/acpi/erst.c
index eb4ab34..ecf1533 100644
--- a/hw/acpi/erst.c
+++ b/hw/acpi/erst.c
@@ -601,6 +601,245 @@ static const MemoryRegionOps erst_reg_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
+
+/***/
+/***/
+
+/* ACPI 4.0: Table 17-19 Serialization Instructions */
+#define INST_READ_REGISTER 0x00
+#define INST_READ_REGISTER_VALUE 0x01
+#define INST_WRITE_REGISTER0x02
+#define INST_WRITE_REGISTER_VALUE 0x03
+#define INST_NOOP 0x04
+#define INST_LOAD_VAR1 0x05
+#define INST_LOAD_VAR2 0x06
+#define INST_STORE_VAR10x07
+#define INST_ADD 0x08
+#define INST_SUBTRACT 0x09
+#define INST_ADD_VALUE 0x0A
+#define INST_SUBTRACT_VALUE0x0B
+#define INST_STALL 0x0C
+#define INST_STALL_WHILE_TRUE 0x0D
+#define INST_SKIP_NEXT_INSTRUCTION_IF_TRUE 0x0E
+#define INST_GOTO 0x0F
+#define INST_SET_SRC_ADDRESS_BASE 0x10
+#define INST_SET_DST_ADDRESS_BASE 0x11
+#define INST_MOVE_DATA 0x12
+
+/* ACPI 4.0: 17.4.1.2 Serialization Instruction Entries */
+static void build_serialization_instruction_entry(GArray *table_data,
+uint8_t serialization_action,
+uint8_t instruction,
+uint8_t flags,
+uint8_t register_bit_width,
+uint64_t register_address,
+uint64_t value,
+uint64_t mask)
+{
+/* ACPI 4.0: Table 17-18 Serialization Instruction Entry */
+struct AcpiGenericAddress gas;
+
+/* Serialization Action */
+build_append_int_noprefix(table_data, serialization_action, 1);
+/* Instruction */
+build_append_int_noprefix(table_data, instruction , 1);
+/* Flags */
+build_append_int_noprefix(table_data, flags , 1);
+/* Reserved */
+build_append_int_noprefix(table_data, 0 , 1);
+/* Register Region */
+gas.space_id = AML_SYSTEM_MEMORY;
+gas.bit_width = register_bit_width;
+gas.bit_offset = 0;
+switch (register_bit_width) {
+case 8:
+gas.access_width = 1;
+break;
+case 16:
+gas.access_width = 2;
+break;
+case 32:
+gas.access_width = 3;
+break;
+case 64:
+gas.access_width = 4;
+break;
+default:
+gas.access_width = 0;
+break;
+}
+gas.address = register_address;
+build_append_gas_from_struct(table_data, );
+/* Value */
+build_append_int_noprefix(table_data, value , 8);
+/* Mask */
+build_append_int_noprefix(table_data, mask , 8);
+}
+
+/* ACPI 4.0: 17.4.1 Serialization Action Table */
+void build_erst(GArray *table_data, BIOSLinker *linker, Object *erst_dev,
+const char *oem_id, const char *oem_table_id)
+{
+GArray *table_instruction_data;
+unsigned action;
+unsigned erst_start = table_data->len;
+hwaddr bar0, bar1;
+
+bar0 = (hwaddr)pci_get_bar_addr(PCI_DEVICE(erst_dev), 0);
+trace_acpi_erst_pci_bar_0(bar0);
+bar1 = (hwaddr)pci_get_bar_addr(PCI_DEVICE(erst_dev), 1);
+trace_acpi_erst_pci_bar_1(bar1);
+
+#define MASK8 0x00FFUL
+#define MASK16 0xUL
+#define MASK32 0xUL
+#define MASK64 0xUL
+
+/*
+ * Serialization Action Table
+ * The serialization action table must be generated first
+ * so that its size can be known in order to populate the
+ * Instruction Entry Count field.
+ */
+table_instruction_data = g_array_new(FALSE, FALSE, sizeof(char));
+
+/* Serialization Instruction Entries */
+action = ACTION_BEGIN_WRITE_OPERATION;
+build_serialization_instruction_entry(table_instruction_data,
+action, INST_WRITE_REGISTER_VALUE, 0, 32,
+bar0 + ERST_ACTION_OFFSET, action, MASK8);
+
+action = ACTION_BEGIN_READ_OPERATION;
+build_serialization_instruction_entry(table_instruction_data,
+action, INST_WRITE_REGISTER_VALUE, 0, 32,
+bar0 + ERST_ACTION_OFFSET, action, MASK8);
+
+action = ACTION_BEGIN_CLEAR_OPERATION;
+build_serialization_instruction_entry(table_instruction_data,
+action, INST_WRITE_REGISTER_VALUE, 0, 32,
+bar0 + ERST_ACTION_OFFSET, action, MASK8);
+
+action = ACTION_END_OPERATION;
+build_serialization_instruction_entry(table_instruction_data,
+action, INST_WRITE_REGISTER_VALUE, 0, 32,
+bar0 + ERST_ACTION_OFFSET, action, MASK8);
+
+action =
[PATCH v6 00/10] acpi: Error Record Serialization Table, ERST, support for QEMU
This patchset introduces support for the ACPI Error Record Serialization Table, ERST. For background and implementation information, please see docs/specs/acpi_erst.txt, which is patch 2/10. Suggested-by: Konrad Wilk Signed-off-by: Eric DeVolder --- v6: 5aug2021 - Fixed compile warning/error, per Michael Tsirkin - Fixed mingw32 build error, per Michael - Converted exchange buffer to MemoryBackend, per Igor - Migrated test to PCI, per Igor - Significantly reduced amount of copying, per Igor - Corrections/enhancements to acpi_erst.txt, per Igor - Many misc/other small items, per Igor v5: 30jun2021 - Create docs/specs/acpi_erst.txt, per Igor - Separate PCI BARs for registers and memory, per Igor - Convert debugging to use trace infrastructure, per Igor - Various other fixups, per Igor v4: 11jun2021 - Converted to a PCI device, per Igor. - Updated qtest. - Rearranged patches, per Igor. v3: 28may2021 - Converted to using a TYPE_MEMORY_BACKEND_FILE object rather than internal array with explicit file operations, per Igor. - Changed the way the qdev and base address are handled, allowing ERST to be disabled at run-time. Also aligns better with other existing code. v2: 8feb2021 - Added qtest/smoke test per Paolo Bonzini - Split patch into smaller chunks, per Igor Mammedov - Did away with use of ACPI packed structures, per Igor Mammedov v1: 26oct2020 - initial post --- Eric DeVolder (10): ACPI ERST: bios-tables-test.c steps 1 and 2 ACPI ERST: specification for ERST support ACPI ERST: PCI device_id for ERST ACPI ERST: header file for ERST ACPI ERST: support for ACPI ERST feature ACPI ERST: build the ACPI ERST table ACPI ERST: create ACPI ERST table for pc/x86 machines ACPI ERST: qtest for ERST ACPI ERST: bios-tables-test testcase ACPI ERST: step 6 of bios-tables-test docs/specs/acpi_erst.txt | 147 ++ hw/acpi/erst.c| 989 ++ hw/acpi/meson.build | 1 + hw/acpi/trace-events | 15 + hw/i386/acpi-build.c | 9 + hw/i386/acpi-microvm.c| 9 + include/hw/acpi/erst.h| 24 + include/hw/pci/pci.h | 1 + tests/data/acpi/microvm/ERST | 0 tests/data/acpi/microvm/ERST.pcie | Bin 0 -> 912 bytes tests/data/acpi/pc/DSDT | Bin 6002 -> 6009 bytes tests/data/acpi/pc/ERST | Bin 0 -> 912 bytes tests/data/acpi/q35/DSDT | Bin 8289 -> 8306 bytes tests/data/acpi/q35/ERST | Bin 0 -> 912 bytes tests/qtest/bios-tables-test.c| 43 ++ tests/qtest/erst-test.c | 167 +++ tests/qtest/meson.build | 2 + 17 files changed, 1407 insertions(+) create mode 100644 docs/specs/acpi_erst.txt create mode 100644 hw/acpi/erst.c create mode 100644 include/hw/acpi/erst.h create mode 100644 tests/data/acpi/microvm/ERST create mode 100644 tests/data/acpi/microvm/ERST.pcie create mode 100644 tests/data/acpi/pc/ERST create mode 100644 tests/data/acpi/q35/ERST create mode 100644 tests/qtest/erst-test.c -- 1.8.3.1
[PATCH v6 10/10] ACPI ERST: step 6 of bios-tables-test
Following the guidelines in tests/qtest/bios-tables-test.c, this
is step 6, the re-generated ACPI tables binary blobs.
Signed-off-by: Eric DeVolder
---
tests/data/acpi/microvm/ERST.pcie | Bin 0 -> 912 bytes
tests/data/acpi/pc/DSDT | Bin 6002 -> 6009 bytes
tests/data/acpi/pc/ERST | Bin 0 -> 912 bytes
tests/data/acpi/q35/DSDT| Bin 8289 -> 8306 bytes
tests/data/acpi/q35/ERST| Bin 0 -> 912 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 6 --
6 files changed, 6 deletions(-)
create mode 100644 tests/data/acpi/microvm/ERST.pcie
diff --git a/tests/data/acpi/microvm/ERST.pcie
b/tests/data/acpi/microvm/ERST.pcie
new file mode 100644
index
..d9a2b3211ab5893a50751ad52be3782579e367f2
GIT binary patch
literal 912
zcmaKpO%8%E5QPUQ|KVrvh9h_c12J)@5f?5!k_Ygv*jGA8UW7?#`}+D#XXyDpKHiZ?
z@anI_W$gOrZRl(SB7!yMqx}#E4EC=}m^g_!0^0`kEl)DOuIXM6D@@*xq*8vyqH
z)b0KTlmlgmH~xt7vG=QZp?+M@&@5ljF8
diff --git a/tests/data/acpi/pc/ERST b/tests/data/acpi/pc/ERST
index
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..f24fadd345c798ee5c17cdb66e0e1703bd1b2f26
100644
GIT binary patch
literal 912
zcmaKpOAdlC6h#XZC=fn#CoF*_7>J28jW}>wF2KFG3zs9lTPTnl;U#=7r>E_sr(1u2
z21ttC4TZ!l<{$N9cc#e2SmmnSn
O1||j(wg6|p5C#C(xDBxY
delta 42
xcmez5@X&$FCDJ28jW}>wF2KFG3zs9lTPTnl;U#=7r>E_sr(1u2
z21
[PATCH v6 02/10] ACPI ERST: specification for ERST support
Information on the implementation of the ACPI ERST support. Signed-off-by: Eric DeVolder --- docs/specs/acpi_erst.txt | 147 +++ 1 file changed, 147 insertions(+) create mode 100644 docs/specs/acpi_erst.txt diff --git a/docs/specs/acpi_erst.txt b/docs/specs/acpi_erst.txt new file mode 100644 index 000..7f7544f --- /dev/null +++ b/docs/specs/acpi_erst.txt @@ -0,0 +1,147 @@ +ACPI ERST DEVICE + + +The ACPI ERST device is utilized to support the ACPI Error Record +Serialization Table, ERST, functionality. This feature is designed for +storing error records in persistent storage for future reference +and/or debugging. + +The ACPI specification[1], in Chapter "ACPI Platform Error Interfaces +(APEI)", and specifically subsection "Error Serialization", outlines a +method for storing error records into persistent storage. + +The format of error records is described in the UEFI specification[2], +in Appendix N "Common Platform Error Record". + +While the ACPI specification allows for an NVRAM "mode" (see +GET_ERROR_LOG_ADDRESS_RANGE_ATTRIBUTES) where non-volatile RAM is +directly exposed for direct access by the OS/guest, this device +implements the non-NVRAM "mode". This non-NVRAM "mode" is what is +implemented by most BIOS (since flash memory requires programming +operations in order to update its contents). Furthermore, as of the +time of this writing, Linux only supports the non-NVRAM "mode". + + +Background/Motivation +- + +Linux uses the persistent storage filesystem, pstore, to record +information (eg. dmesg tail) upon panics and shutdowns. Pstore is +independent of, and runs before, kdump. In certain scenarios (ie. +hosts/guests with root filesystems on NFS/iSCSI where networking +software and/or hardware fails), pstore may contain information +available for post-mortem debugging. + +Two common storage backends for the pstore filesystem are ACPI ERST +and UEFI. Most BIOS implement ACPI ERST. UEFI is not utilized in all +guests. With QEMU supporting ACPI ERST, it becomes a viable pstore +storage backend for virtual machines (as it is now for bare metal +machines). + +Enabling support for ACPI ERST facilitates a consistent method to +capture kernel panic information in a wide range of guests: from +resource-constrained microvms to very large guests, and in particular, +in direct-boot environments (which would lack UEFI run-time services). + +Note that Microsoft Windows also utilizes the ACPI ERST for certain +crash information, if available[3]. + + +Configuration|Usage +--- + +To use ACPI ERST, a memory-backend-file object and acpi-erst device +can be created, for example: + + qemu ... + -object memory-backend-file,id=erstnvram,mem-path=acpi-erst.backing,size=0x1,share=on \ + -device acpi-erst,memdev=erstnvram + +For proper operation, the ACPI ERST device needs a memory-backend-file +object with the following parameters: + + - id: The id of the memory-backend-file object is used to associate + this memory with the acpi-erst device. + - size: The size of the ACPI ERST backing storage. This parameter is + required. + - mem-path: The location of the ACPI ERST backing storage file. This + parameter is also required. + - share: The share=on parameter is required so that updates to the + ERST backing store are written to the file. + +and ERST device: + + - memdev: Is the object id of the memory-backend-file. + + +PCI Interface +- + +The ERST device is a PCI device with two BARs, one for accessing the +programming registers, and the other for accessing the record exchange +buffer. + +BAR0 contains the programming interface consisting of ACTION and VALUE +64-bit registers. All ERST actions/operations/side effects happen on +the write to the ACTION, by design. Any data needed by the action must +be placed into VALUE prior to writing ACTION. Reading the VALUE +simply returns the register contents, which can be updated by a +previous ACTION. + +BAR1 contains the 8KiB record exchange buffer, which is the +implemented maximum record size. + + +Backend Storage Format +-- + +The backend storage is divided into fixed size "slots", 8KiB in +length, with each slot storing a single record. Not all slots need to +be occupied, and they need not be occupied in a contiguous fashion. +The ability to clear/erase specific records allows for the formation +of unoccupied slots. + +Slot 0 is reserved for a backend storage header that identifies the +contents as ERST and also facilitates efficient access to the records. +Depending upon the size of the backend storage, additional slots will +be reserved to be a part of the slot 0 header. For example, at 8KiB, +the slot 0 header can accomodate 1024 records. Thus a storage size +above 8MiB (8KiB * 1024) requires an additional slot. In this +scenario, slot 0 and slot 1 form the backend storage header, and +records can be stored starting
[PATCH v6 03/10] ACPI ERST: PCI device_id for ERST
This change reserves the PCI device_id for the new ACPI ERST device. Signed-off-by: Eric DeVolder --- include/hw/pci/pci.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index d0f4266..58101d8 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -108,6 +108,7 @@ extern bool pci_available; #define PCI_DEVICE_ID_REDHAT_MDPY0x000f #define PCI_DEVICE_ID_REDHAT_NVME0x0010 #define PCI_DEVICE_ID_REDHAT_PVPANIC 0x0011 +#define PCI_DEVICE_ID_REDHAT_ACPI_ERST 0x0012 #define PCI_DEVICE_ID_REDHAT_QXL 0x0100 #define FMT_PCIBUS PRIx64 -- 1.8.3.1
[PATCH v6 04/10] ACPI ERST: header file for ERST
This change introduces the public defintions for ACPI ERST. Signed-off-by: Eric DeVolder --- include/hw/acpi/erst.h | 19 +++ 1 file changed, 19 insertions(+) create mode 100644 include/hw/acpi/erst.h diff --git a/include/hw/acpi/erst.h b/include/hw/acpi/erst.h new file mode 100644 index 000..9d63717 --- /dev/null +++ b/include/hw/acpi/erst.h @@ -0,0 +1,19 @@ +/* + * ACPI Error Record Serialization Table, ERST, Implementation + * + * ACPI ERST introduced in ACPI 4.0, June 16, 2009. + * ACPI Platform Error Interfaces : Error Serialization + * + * Copyright (c) 2021 Oracle and/or its affiliates. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#ifndef HW_ACPI_ERST_H +#define HW_ACPI_ERST_H + +void build_erst(GArray *table_data, BIOSLinker *linker, Object *erst_dev, +const char *oem_id, const char *oem_table_id); + +#define TYPE_ACPI_ERST "acpi-erst" + +#endif -- 1.8.3.1
[PATCH v7 09/10] hw/arm/virt: add ITS support in virt GIC
Included creation of ITS as part of virt platform GIC
initialization. This Emulated ITS model now co-exists with kvm
ITS and is enabled in absence of kvm irq kernel support in a
platform.
Signed-off-by: Shashi Mallela
Reviewed-by: Peter Maydell
---
hw/arm/virt.c | 28 ++--
include/hw/arm/virt.h | 2 ++
target/arm/kvm_arm.h | 4 ++--
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 81eda46b0b..99cf4f9dbd 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -583,6 +583,12 @@ static void create_its(VirtMachineState *vms)
const char *itsclass = its_class_name();
DeviceState *dev;
+if (!strcmp(itsclass, "arm-gicv3-its")) {
+if (!vms->tcg_its) {
+itsclass = NULL;
+}
+}
+
if (!itsclass) {
/* Do nothing if not supported */
return;
@@ -620,7 +626,7 @@ static void create_v2m(VirtMachineState *vms)
vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
}
-static void create_gic(VirtMachineState *vms)
+static void create_gic(VirtMachineState *vms, MemoryRegion *mem)
{
MachineState *ms = MACHINE(vms);
/* We create a standalone GIC */
@@ -654,6 +660,14 @@ static void create_gic(VirtMachineState *vms)
nb_redist_regions);
qdev_prop_set_uint32(vms->gic, "redist-region-count[0]",
redist0_count);
+if (!kvm_irqchip_in_kernel()) {
+if (vms->tcg_its) {
+object_property_set_link(OBJECT(vms->gic), "sysmem",
+ OBJECT(mem), _fatal);
+qdev_prop_set_bit(vms->gic, "has-lpi", true);
+}
+}
+
if (nb_redist_regions == 2) {
uint32_t redist1_capacity =
vms->memmap[VIRT_HIGH_GIC_REDIST2].size /
GICV3_REDIST_SIZE;
@@ -2043,7 +2057,7 @@ static void machvirt_init(MachineState *machine)
virt_flash_fdt(vms, sysmem, secure_sysmem ?: sysmem);
-create_gic(vms);
+create_gic(vms, sysmem);
virt_cpu_post_init(vms, sysmem);
@@ -2746,6 +2760,12 @@ static void virt_instance_init(Object *obj)
} else {
/* Default allows ITS instantiation */
vms->its = true;
+
+if (vmc->no_tcg_its) {
+vms->tcg_its = false;
+} else {
+vms->tcg_its = true;
+}
}
/* Default disallows iommu instantiation */
@@ -2795,8 +2815,12 @@ DEFINE_VIRT_MACHINE_AS_LATEST(6, 1)
static void virt_machine_6_0_options(MachineClass *mc)
{
+VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
virt_machine_6_1_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_0, hw_compat_6_0_len);
+/* qemu ITS was introduced with 6.1 */
+vmc->no_tcg_its = true;
}
DEFINE_VIRT_MACHINE(6, 0)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 9661c46699..b461b8d261 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -120,6 +120,7 @@ struct VirtMachineClass {
MachineClass parent;
bool disallow_affinity_adjustment;
bool no_its;
+bool no_tcg_its;
bool no_pmu;
bool claim_edge_triggered_timers;
bool smbios_old_sys_ver;
@@ -141,6 +142,7 @@ struct VirtMachineState {
bool highmem;
bool highmem_ecam;
bool its;
+bool tcg_its;
bool virt;
bool ras;
bool mte;
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 34f8daa377..0613454975 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -525,8 +525,8 @@ static inline const char *its_class_name(void)
/* KVM implementation requires this capability */
return kvm_direct_msi_enabled() ? "arm-its-kvm" : NULL;
} else {
-/* Software emulation is not implemented yet */
-return NULL;
+/* Software emulation based model */
+return "arm-gicv3-its";
}
}
--
2.27.0
[PATCH v6 07/10] ACPI ERST: create ACPI ERST table for pc/x86 machines
This change exposes ACPI ERST support for x86 guests.
Signed-off-by: Eric DeVolder
---
hw/i386/acpi-build.c | 9 +
hw/i386/acpi-microvm.c | 9 +
include/hw/acpi/erst.h | 5 +
3 files changed, 23 insertions(+)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a33ac8b..b55a548 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -43,6 +43,7 @@
#include "sysemu/tpm.h"
#include "hw/acpi/tpm.h"
#include "hw/acpi/vmgenid.h"
+#include "hw/acpi/erst.h"
#include "sysemu/tpm_backend.h"
#include "hw/rtc/mc146818rtc_regs.h"
#include "migration/vmstate.h"
@@ -2443,6 +2444,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState
*machine)
GArray *tables_blob = tables->table_data;
AcpiSlicOem slic_oem = { .id = NULL, .table_id = NULL };
Object *vmgenid_dev;
+Object *erst_dev;
char *oem_id;
char *oem_table_id;
@@ -2504,6 +2506,13 @@ void acpi_build(AcpiBuildTables *tables, MachineState
*machine)
ACPI_DEVICE_IF(x86ms->acpi_dev), x86ms->oem_id,
x86ms->oem_table_id);
+erst_dev = find_erst_dev();
+if (erst_dev) {
+acpi_add_table(table_offsets, tables_blob);
+build_erst(tables_blob, tables->linker, erst_dev,
+ x86ms->oem_id, x86ms->oem_table_id);
+}
+
vmgenid_dev = find_vmgenid_dev();
if (vmgenid_dev) {
acpi_add_table(table_offsets, tables_blob);
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
index 1a0f77b..6578254 100644
--- a/hw/i386/acpi-microvm.c
+++ b/hw/i386/acpi-microvm.c
@@ -30,6 +30,7 @@
#include "hw/acpi/bios-linker-loader.h"
#include "hw/acpi/generic_event_device.h"
#include "hw/acpi/utils.h"
+#include "hw/acpi/erst.h"
#include "hw/i386/fw_cfg.h"
#include "hw/i386/microvm.h"
#include "hw/pci/pci.h"
@@ -159,6 +160,7 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
X86MachineState *x86ms = X86_MACHINE(mms);
GArray *table_offsets;
GArray *tables_blob = tables->table_data;
+Object *erst_dev;
unsigned dsdt, xsdt;
AcpiFadtData pmfadt = {
/* ACPI 5.0: 4.1 Hardware-Reduced ACPI */
@@ -208,6 +210,13 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
ACPI_DEVICE_IF(x86ms->acpi_dev), x86ms->oem_id,
x86ms->oem_table_id);
+erst_dev = find_erst_dev();
+if (erst_dev) {
+acpi_add_table(table_offsets, tables_blob);
+build_erst(tables_blob, tables->linker, erst_dev,
+ x86ms->oem_id, x86ms->oem_table_id);
+}
+
xsdt = tables_blob->len;
build_xsdt(tables_blob, tables->linker, table_offsets, x86ms->oem_id,
x86ms->oem_table_id);
diff --git a/include/hw/acpi/erst.h b/include/hw/acpi/erst.h
index 9d63717..b747fe7 100644
--- a/include/hw/acpi/erst.h
+++ b/include/hw/acpi/erst.h
@@ -16,4 +16,9 @@ void build_erst(GArray *table_data, BIOSLinker *linker,
Object *erst_dev,
#define TYPE_ACPI_ERST "acpi-erst"
+/* returns NULL unless there is exactly one device */
+static inline Object *find_erst_dev(void)
+{
+return object_resolve_path_type("", TYPE_ACPI_ERST, NULL);
+}
#endif
--
1.8.3.1
[PATCH v7 10/10] tests/data/acpi/virt: Update IORT files for ITS
Updated expected IORT files applicable with latest GICv3 ITS changes. Full diff of new file disassembly: /* * Intel ACPI Component Architecture * AML/ASL+ Disassembler version 20180629 (64-bit version) * Copyright (c) 2000 - 2018 Intel Corporation * * Disassembly of tests/data/acpi/virt/IORT.pxb, Tue Jun 29 17:35:38 2021 * * ACPI Data Table [IORT] * * Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue */ [000h 4]Signature : "IORT"[IO Remapping Table] [004h 0004 4] Table Length : 007C [008h 0008 1] Revision : 00 [009h 0009 1] Checksum : 07 [00Ah 0010 6] Oem ID : "BOCHS " [010h 0016 8] Oem Table ID : "BXPC" [018h 0024 4] Oem Revision : 0001 [01Ch 0028 4] Asl Compiler ID : "BXPC" [020h 0032 4]Asl Compiler Revision : 0001 [024h 0036 4] Node Count : 0002 [028h 0040 4] Node Offset : 0030 [02Ch 0044 4] Reserved : [030h 0048 1] Type : 00 [031h 0049 2] Length : 0018 [033h 0051 1] Revision : 00 [034h 0052 4] Reserved : [038h 0056 4]Mapping Count : [03Ch 0060 4] Mapping Offset : [040h 0064 4] ItsCount : 0001 [044h 0068 4] Identifiers : [048h 0072 1] Type : 02 [049h 0073 2] Length : 0034 [04Bh 0075 1] Revision : 00 [04Ch 0076 4] Reserved : [050h 0080 4]Mapping Count : 0001 [054h 0084 4] Mapping Offset : 0020 [058h 0088 8]Memory Properties : [IORT Memory Access Properties] [058h 0088 4] Cache Coherency : 0001 [05Ch 0092 1]Hints (decoded below) : 00 Transient : 0 Write Allocate : 0 Read Allocate : 0 Override : 0 [05Dh 0093 2] Reserved : [05Fh 0095 1] Memory Flags (decoded below) : 03 Coherency : 1 Device Attribute : 1 [060h 0096 4]ATS Attribute : [064h 0100 4] PCI Segment Number : [068h 0104 1]Memory Size Limit : 00 [069h 0105 3] Reserved : 00 [068h 0104 4] Input base : [06Ch 0108 4] ID Count : [070h 0112 4] Output Base : [074h 0116 4] Output Reference : 0030 [078h 0120 4]Flags (decoded below) : Single Mapping : 0 Raw Table Data: Length 124 (0x7C) : 49 4F 52 54 7C 00 00 00 00 07 42 4F 43 48 53 20 // IORT|.BOCHS 0010: 42 58 50 43 20 20 20 20 01 00 00 00 42 58 50 43 // BXPCBXPC 0020: 01 00 00 00 02 00 00 00 30 00 00 00 00 00 00 00 // 0... 0030: 00 18 00 00 00 00 00 00 00 00 00 00 00 00 00 00 // 0040: 01 00 00 00 00 00 00 00 02 34 00 00 00 00 00 00 // .4.. 0050: 01 00 00 00 20 00 00 00 01 00 00 00 00 00 00 03 // ... 0060: 00 00 00 00 00 00 00 00 00 00 00 00 FF FF 00 00 // 0070: 00 00 00 00 30 00 00 00 00 00 00 00 // 0... Signed-off-by: Shashi Mallela --- tests/data/acpi/virt/IORT | Bin 0 -> 124 bytes tests/data/acpi/virt/IORT.memhp | Bin 0 -> 124 bytes tests/data/acpi/virt/IORT.numamem | Bin 0 -> 124 bytes tests/data/acpi/virt/IORT.pxb | Bin 0 -> 124 bytes tests/qtest/bios-tables-test-allowed-diff.h | 4 5 files changed, 4 deletions(-) diff --git a/tests/data/acpi/virt/IORT b/tests/data/acpi/virt/IORT index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..521acefe9ba66706c5607321a82d330586f3f280 100644 GIT binary patch literal 124 zcmebD4+^Pa00MR=e`k+i1*eDrX9XZ&1PX!JAesq?4S*O7Bw!2(4Uz`|CKCt^;wu0# QRGb+i3L*dhhtM#y0PN=p0RR91 literal 0 HcmV?d1 diff --git a/tests/data/acpi/virt/IORT.memhp b/tests/data/acpi/virt/IORT.memhp index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..521acefe9ba66706c5607321a82d330586f3f280 100644 GIT binary patch literal 124 zcmebD4+^Pa00MR=e`k+i1*eDrX9XZ&1PX!JAesq?4S*O7Bw!2(4Uz`|CKCt^;wu0# QRGb+i3L*dhhtM#y0PN=p0RR91 literal 0 HcmV?d1 diff --git a/tests/data/acpi/virt/IORT.numamem b/tests/data/acpi/virt/IORT.numamem index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..521acefe9ba66706c5607321a82d330586f3f280 100644 GIT binary patch literal 124 zcmebD4+^Pa00MR=e`k+i1*eDrX9XZ&1PX!JAesq?4S*O7Bw!2(4Uz`|CKCt^;wu0# QRGb+i3L*dhhtM#y0PN=p0RR91
[PATCH v7 06/10] hw/intc: GICv3 redistributor ITS processing
Implemented lpi processing at redistributor to get lpi config info
from lpi configuration table,determine priority,set pending state in
lpi pending table and forward the lpi to cpuif.Added logic to invoke
redistributor lpi processing with translated LPI which set/clear LPI
from ITS device as part of ITS INT,CLEAR,DISCARD command and
GITS_TRANSLATER processing.
Signed-off-by: Shashi Mallela
---
hw/intc/arm_gicv3.c| 14 +++
hw/intc/arm_gicv3_common.c | 1 +
hw/intc/arm_gicv3_cpuif.c | 7 +-
hw/intc/arm_gicv3_its.c| 23 +
hw/intc/arm_gicv3_redist.c | 141 +
hw/intc/gicv3_internal.h | 9 ++
include/hw/intc/arm_gicv3_common.h | 7 ++
7 files changed, 200 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gicv3.c b/hw/intc/arm_gicv3.c
index d63f8af604..3f24707838 100644
--- a/hw/intc/arm_gicv3.c
+++ b/hw/intc/arm_gicv3.c
@@ -165,6 +165,16 @@ static void gicv3_redist_update_noirqset(GICv3CPUState *cs)
cs->hppi.grp = gicv3_irq_group(cs->gic, cs, cs->hppi.irq);
}
+if ((cs->gicr_ctlr & GICR_CTLR_ENABLE_LPIS) && cs->gic->lpi_enable &&
+(cs->hpplpi.prio != 0xff)) {
+if (irqbetter(cs, cs->hpplpi.irq, cs->hpplpi.prio)) {
+cs->hppi.irq = cs->hpplpi.irq;
+cs->hppi.prio = cs->hpplpi.prio;
+cs->hppi.grp = cs->hpplpi.grp;
+seenbetter = true;
+}
+}
+
/* If the best interrupt we just found would preempt whatever
* was the previous best interrupt before this update, then
* we know it's definitely the best one now.
@@ -339,9 +349,13 @@ static void gicv3_set_irq(void *opaque, int irq, int level)
static void arm_gicv3_post_load(GICv3State *s)
{
+int i;
/* Recalculate our cached idea of the current highest priority
* pending interrupt, but don't set IRQ or FIQ lines.
*/
+for (i = 0; i < s->num_cpu; i++) {
+gicv3_redist_update_lpi(>cpu[i]);
+}
gicv3_full_update_noirqset(s);
/* Repopulate the cache of GICv3CPUState pointers for target CPUs */
gicv3_cache_all_target_cpustates(s);
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 53dea2a775..223db16fec 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -435,6 +435,7 @@ static void arm_gicv3_common_reset(DeviceState *dev)
memset(cs->gicr_ipriorityr, 0, sizeof(cs->gicr_ipriorityr));
cs->hppi.prio = 0xff;
+cs->hpplpi.prio = 0xff;
/* State in the CPU interface must *not* be reset here, because it
* is part of the CPU's reset domain, not the GIC device's.
diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c
index a032d505f5..462a35f66e 100644
--- a/hw/intc/arm_gicv3_cpuif.c
+++ b/hw/intc/arm_gicv3_cpuif.c
@@ -899,10 +899,12 @@ static void icc_activate_irq(GICv3CPUState *cs, int irq)
cs->gicr_iactiver0 = deposit32(cs->gicr_iactiver0, irq, 1, 1);
cs->gicr_ipendr0 = deposit32(cs->gicr_ipendr0, irq, 1, 0);
gicv3_redist_update(cs);
-} else {
+} else if (irq < GICV3_LPI_INTID_START) {
gicv3_gicd_active_set(cs->gic, irq);
gicv3_gicd_pending_clear(cs->gic, irq);
gicv3_update(cs->gic, irq, 1);
+} else {
+gicv3_redist_lpi_pending(cs, irq, 0);
}
}
@@ -1318,7 +1320,8 @@ static void icc_eoir_write(CPUARMState *env, const
ARMCPRegInfo *ri,
trace_gicv3_icc_eoir_write(is_eoir0 ? 0 : 1,
gicv3_redist_affid(cs), value);
-if (irq >= cs->gic->num_irq) {
+if ((irq >= cs->gic->num_irq) &&
+!(cs->gic->lpi_enable && (irq >= GICV3_LPI_INTID_START))) {
/* This handles two cases:
* 1. If software writes the ID of a spurious interrupt [ie 1020-1023]
* to the GICC_EOIR, the GIC ignores that write.
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 35308f1c32..efa43b0333 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -225,6 +225,7 @@ static MemTxResult process_its_cmd(GICv3ITSState *s,
uint64_t value,
bool ite_valid = false;
uint64_t cte = 0;
bool cte_valid = false;
+uint64_t rdbase;
if (cmd == NONE) {
devid = offset;
@@ -285,6 +286,18 @@ static MemTxResult process_its_cmd(GICv3ITSState *s,
uint64_t value,
* Current implementation only supports rdbase == procnum
* Hence rdbase physical address is ignored
*/
+rdbase = (cte & GITS_CTE_RDBASE_PROCNUM_MASK) >> 1U;
+
+if (rdbase > s->gicv3->num_cpu) {
+return res;
+}
+
+if ((cmd == CLEAR) || (cmd == DISCARD)) {
+gicv3_redist_process_lpi(>gicv3->cpu[rdbase], pIntid, 0);
+} else {
+gicv3_redist_process_lpi(>gicv3->cpu[rdbase], pIntid, 1);
+}
+
if (cmd == DISCARD) {
IteEntry ite = {};
/* remove mapping
[PATCH v7 04/10] hw/intc: GICv3 ITS Command processing
Added ITS command queue handling for MAPTI,MAPI commands,handled ITS
translation which triggers an LPI via INT command as well as write
to GITS_TRANSLATER register,defined enum to differentiate between ITS
command interrupt trigger and GITS_TRANSLATER based interrupt trigger.
Each of these commands make use of other functionalities implemented to
get device table entry,collection table entry or interrupt translation
table entry required for their processing.
Signed-off-by: Shashi Mallela
Reviewed-by: Peter Maydell
---
hw/intc/arm_gicv3_its.c| 348 +
hw/intc/gicv3_internal.h | 12 +
include/hw/intc/arm_gicv3_common.h | 2 +
3 files changed, 362 insertions(+)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 8bdbebbeca..35308f1c32 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -29,6 +29,22 @@ struct GICv3ITSClass {
void (*parent_reset)(DeviceState *dev);
};
+/*
+ * This is an internal enum used to distinguish between LPI triggered
+ * via command queue and LPI triggered via gits_translater write.
+ */
+typedef enum ItsCmdType {
+NONE = 0, /* internal indication for GITS_TRANSLATER write */
+CLEAR = 1,
+DISCARD = 2,
+INT = 3,
+} ItsCmdType;
+
+typedef struct {
+uint32_t iteh;
+uint64_t itel;
+} IteEntry;
+
static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz)
{
uint64_t result = 0;
@@ -50,6 +66,320 @@ static uint64_t baser_base_addr(uint64_t value, uint32_t
page_sz)
return result;
}
+static bool get_cte(GICv3ITSState *s, uint16_t icid, uint64_t *cte,
+MemTxResult *res)
+{
+AddressSpace *as = >gicv3->dma_as;
+uint64_t l2t_addr;
+uint64_t value;
+bool valid_l2t;
+uint32_t l2t_id;
+uint32_t max_l2_entries;
+
+if (s->ct.indirect) {
+l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
+
+value = address_space_ldq_le(as,
+ s->ct.base_addr +
+ (l2t_id * L1TABLE_ENTRY_SIZE),
+ MEMTXATTRS_UNSPECIFIED, res);
+
+if (*res == MEMTX_OK) {
+valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
+
+if (valid_l2t) {
+max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
+
+l2t_addr = value & ((1ULL << 51) - 1);
+
+*cte = address_space_ldq_le(as, l2t_addr +
+((icid % max_l2_entries) * GITS_CTE_SIZE),
+MEMTXATTRS_UNSPECIFIED, res);
+ }
+ }
+} else {
+/* Flat level table */
+*cte = address_space_ldq_le(as, s->ct.base_addr +
+ (icid * GITS_CTE_SIZE),
+ MEMTXATTRS_UNSPECIFIED, res);
+}
+
+return (*cte & TABLE_ENTRY_VALID_MASK) != 0;
+}
+
+static MemTxResult update_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
+ IteEntry ite)
+{
+AddressSpace *as = >gicv3->dma_as;
+uint64_t itt_addr;
+MemTxResult res = MEMTX_OK;
+
+itt_addr = (dte & GITS_DTE_ITTADDR_MASK) >> GITS_DTE_ITTADDR_SHIFT;
+itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
+
+address_space_stq_le(as, itt_addr + (eventid * sizeof(uint64_t)),
+ ite.itel, MEMTXATTRS_UNSPECIFIED, );
+
+if (res == MEMTX_OK) {
+address_space_stl_le(as, itt_addr + ((eventid + sizeof(uint64_t)) *
+ sizeof(uint32_t)), ite.iteh,
+ MEMTXATTRS_UNSPECIFIED, );
+}
+ return res;
+}
+
+static bool get_ite(GICv3ITSState *s, uint32_t eventid, uint64_t dte,
+uint16_t *icid, uint32_t *pIntid, MemTxResult *res)
+{
+AddressSpace *as = >gicv3->dma_as;
+uint64_t itt_addr;
+bool status = false;
+IteEntry ite = {};
+
+itt_addr = (dte & GITS_DTE_ITTADDR_MASK) >> GITS_DTE_ITTADDR_SHIFT;
+itt_addr <<= ITTADDR_SHIFT; /* 256 byte aligned */
+
+ite.itel = address_space_ldq_le(as, itt_addr +
+(eventid * sizeof(uint64_t)),
+MEMTXATTRS_UNSPECIFIED, res);
+
+if (*res == MEMTX_OK) {
+ite.iteh = address_space_ldl_le(as, itt_addr + ((eventid +
+sizeof(uint64_t)) * sizeof(uint32_t)),
+MEMTXATTRS_UNSPECIFIED, res);
+
+if (*res == MEMTX_OK) {
+if (ite.itel & TABLE_ENTRY_VALID_MASK) {
+if ((ite.itel >> ITE_ENTRY_INTTYPE_SHIFT) &
+GITS_TYPE_PHYSICAL) {
+*pIntid = (ite.itel & ITE_ENTRY_INTID_MASK) >>
+ ITE_ENTRY_INTID_SHIFT;
+*icid = ite.iteh & ITE_ENTRY_ICID_MASK;
+status = true;
+}
+}
+}
+}
+return status;
[PATCH v6 01/10] ACPI ERST: bios-tables-test.c steps 1 and 2
Following the guidelines in tests/qtest/bios-tables-test.c, this change adds empty placeholder files per step 1 for the new ERST table, and excludes resulting changed files in bios-tables-test-allowed-diff.h per step 2. Signed-off-by: Eric DeVolder --- tests/data/acpi/microvm/ERST| 0 tests/data/acpi/pc/ERST | 0 tests/data/acpi/q35/ERST| 0 tests/qtest/bios-tables-test-allowed-diff.h | 6 ++ 4 files changed, 6 insertions(+) create mode 100644 tests/data/acpi/microvm/ERST create mode 100644 tests/data/acpi/pc/ERST create mode 100644 tests/data/acpi/q35/ERST diff --git a/tests/data/acpi/microvm/ERST b/tests/data/acpi/microvm/ERST new file mode 100644 index 000..e69de29 diff --git a/tests/data/acpi/pc/ERST b/tests/data/acpi/pc/ERST new file mode 100644 index 000..e69de29 diff --git a/tests/data/acpi/q35/ERST b/tests/data/acpi/q35/ERST new file mode 100644 index 000..e69de29 diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523..b3aaf76 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,7 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/pc/ERST", +"tests/data/acpi/q35/ERST", +"tests/data/acpi/microvm/ERST", +"tests/data/acpi/pc/DSDT", +"tests/data/acpi/q35/DSDT", +"tests/data/acpi/microvm/DSDT", -- 1.8.3.1
[PATCH v7 08/10] tests/data/acpi/virt: Add IORT files for ITS
Added expected IORT files applicable with latest GICv3 ITS changes.Temporarily differences in these files are okay. Signed-off-by: Shashi Mallela --- tests/data/acpi/virt/IORT | 0 tests/data/acpi/virt/IORT.memhp | 0 tests/data/acpi/virt/IORT.numamem | 0 tests/data/acpi/virt/IORT.pxb | 0 tests/qtest/bios-tables-test-allowed-diff.h | 4 5 files changed, 4 insertions(+) create mode 100644 tests/data/acpi/virt/IORT create mode 100644 tests/data/acpi/virt/IORT.memhp create mode 100644 tests/data/acpi/virt/IORT.numamem create mode 100644 tests/data/acpi/virt/IORT.pxb diff --git a/tests/data/acpi/virt/IORT b/tests/data/acpi/virt/IORT new file mode 100644 index 00..e69de29bb2 diff --git a/tests/data/acpi/virt/IORT.memhp b/tests/data/acpi/virt/IORT.memhp new file mode 100644 index 00..e69de29bb2 diff --git a/tests/data/acpi/virt/IORT.numamem b/tests/data/acpi/virt/IORT.numamem new file mode 100644 index 00..e69de29bb2 diff --git a/tests/data/acpi/virt/IORT.pxb b/tests/data/acpi/virt/IORT.pxb new file mode 100644 index 00..e69de29bb2 diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h index dfb8523c8b..2ef211df59 100644 --- a/tests/qtest/bios-tables-test-allowed-diff.h +++ b/tests/qtest/bios-tables-test-allowed-diff.h @@ -1 +1,5 @@ /* List of comma-separated changed AML files to ignore */ +"tests/data/acpi/virt/IORT", +"tests/data/acpi/virt/IORT.memhp", +"tests/data/acpi/virt/IORT.numamem", +"tests/data/acpi/virt/IORT.pxb", -- 2.27.0
[PATCH v7 02/10] hw/intc: GICv3 ITS register definitions added
Defined descriptors for ITS device table,collection table and ITS
command queue entities.Implemented register read/write functions,
extract ITS table parameters and command queue parameters,extended
gicv3 common to capture qemu address space(which host the ITS table
platform memories required for subsequent ITS processing) and
initialize the same in ITS device.
Signed-off-by: Shashi Mallela
Reviewed-by: Peter Maydell
Reviewed-by: Eric Auger
---
hw/intc/arm_gicv3_its.c| 376 +
hw/intc/gicv3_internal.h | 29 ++
include/hw/intc/arm_gicv3_common.h | 3 +
include/hw/intc/arm_gicv3_its_common.h | 23 ++
4 files changed, 431 insertions(+)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index 2286b3f757..b2210dffdc 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -29,6 +29,160 @@ struct GICv3ITSClass {
void (*parent_reset)(DeviceState *dev);
};
+static uint64_t baser_base_addr(uint64_t value, uint32_t page_sz)
+{
+uint64_t result = 0;
+
+switch (page_sz) {
+case GITS_PAGE_SIZE_4K:
+case GITS_PAGE_SIZE_16K:
+result = FIELD_EX64(value, GITS_BASER, PHYADDR) << 12;
+break;
+
+case GITS_PAGE_SIZE_64K:
+result = FIELD_EX64(value, GITS_BASER, PHYADDRL_64K) << 16;
+result |= FIELD_EX64(value, GITS_BASER, PHYADDRH_64K) << 48;
+break;
+
+default:
+break;
+}
+return result;
+}
+
+/*
+ * This function extracts the ITS Device and Collection table specific
+ * parameters (like base_addr, size etc) from GITS_BASER register.
+ * It is called during ITS enable and also during post_load migration
+ */
+static void extract_table_params(GICv3ITSState *s)
+{
+uint16_t num_pages = 0;
+uint8_t page_sz_type;
+uint8_t type;
+uint32_t page_sz = 0;
+uint64_t value;
+
+for (int i = 0; i < 8; i++) {
+value = s->baser[i];
+
+if (!value) {
+continue;
+}
+
+page_sz_type = FIELD_EX64(value, GITS_BASER, PAGESIZE);
+
+switch (page_sz_type) {
+case 0:
+page_sz = GITS_PAGE_SIZE_4K;
+break;
+
+case 1:
+page_sz = GITS_PAGE_SIZE_16K;
+break;
+
+case 2:
+case 3:
+page_sz = GITS_PAGE_SIZE_64K;
+break;
+
+default:
+g_assert_not_reached();
+}
+
+num_pages = FIELD_EX64(value, GITS_BASER, SIZE) + 1;
+
+type = FIELD_EX64(value, GITS_BASER, TYPE);
+
+switch (type) {
+
+case GITS_BASER_TYPE_DEVICE:
+memset(>dt, 0 , sizeof(s->dt));
+s->dt.valid = FIELD_EX64(value, GITS_BASER, VALID);
+
+if (!s->dt.valid) {
+return;
+}
+
+s->dt.page_sz = page_sz;
+s->dt.indirect = FIELD_EX64(value, GITS_BASER, INDIRECT);
+s->dt.entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE);
+
+if (!s->dt.indirect) {
+s->dt.max_entries = (num_pages * page_sz) / s->dt.entry_sz;
+} else {
+s->dt.max_entries = (((num_pages * page_sz) /
+ L1TABLE_ENTRY_SIZE) *
+ (page_sz / s->dt.entry_sz));
+}
+
+s->dt.maxids.max_devids = (1UL << (FIELD_EX64(s->typer, GITS_TYPER,
+ DEVBITS) + 1));
+
+s->dt.base_addr = baser_base_addr(value, page_sz);
+
+break;
+
+case GITS_BASER_TYPE_COLLECTION:
+memset(>ct, 0 , sizeof(s->ct));
+s->ct.valid = FIELD_EX64(value, GITS_BASER, VALID);
+
+/*
+ * GITS_TYPER.HCC is 0 for this implementation
+ * hence writes are discarded if ct.valid is 0
+ */
+if (!s->ct.valid) {
+return;
+}
+
+s->ct.page_sz = page_sz;
+s->ct.indirect = FIELD_EX64(value, GITS_BASER, INDIRECT);
+s->ct.entry_sz = FIELD_EX64(value, GITS_BASER, ENTRYSIZE);
+
+if (!s->ct.indirect) {
+s->ct.max_entries = (num_pages * page_sz) / s->ct.entry_sz;
+} else {
+s->ct.max_entries = (((num_pages * page_sz) /
+ L1TABLE_ENTRY_SIZE) *
+ (page_sz / s->ct.entry_sz));
+}
+
+if (FIELD_EX64(s->typer, GITS_TYPER, CIL)) {
+s->ct.maxids.max_collids = (1UL << (FIELD_EX64(s->typer,
+GITS_TYPER, CIDBITS) + 1));
+} else {
+/* 16-bit CollectionId supported when CIL == 0 */
+s->ct.maxids.max_collids = (1UL << 16);
+}
+
+s->ct.base_addr = baser_base_addr(value, page_sz);
+
+break;
+
+default:
+break;
+}
+}
+}
+
+static void
[PATCH v7 03/10] hw/intc: GICv3 ITS command queue framework
Added functionality to trigger ITS command queue processing on
write to CWRITE register and process each command queue entry to
identify the command type and handle commands like MAPD,MAPC,SYNC.
Signed-off-by: Shashi Mallela
Reviewed-by: Peter Maydell
Reviewed-by: Eric Auger
---
hw/intc/arm_gicv3_its.c | 306 +++
hw/intc/gicv3_internal.h | 40 +
2 files changed, 346 insertions(+)
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
index b2210dffdc..8bdbebbeca 100644
--- a/hw/intc/arm_gicv3_its.c
+++ b/hw/intc/arm_gicv3_its.c
@@ -50,6 +50,305 @@ static uint64_t baser_base_addr(uint64_t value, uint32_t
page_sz)
return result;
}
+static MemTxResult update_cte(GICv3ITSState *s, uint16_t icid, bool valid,
+ uint64_t rdbase)
+{
+AddressSpace *as = >gicv3->dma_as;
+uint64_t value;
+uint64_t l2t_addr;
+bool valid_l2t;
+uint32_t l2t_id;
+uint32_t max_l2_entries;
+uint64_t cte = 0;
+MemTxResult res = MEMTX_OK;
+
+if (!s->ct.valid) {
+return res;
+}
+
+if (valid) {
+/* add mapping entry to collection table */
+cte = (valid & TABLE_ENTRY_VALID_MASK) | (rdbase << 1ULL);
+}
+
+/*
+ * The specification defines the format of level 1 entries of a
+ * 2-level table, but the format of level 2 entries and the format
+ * of flat-mapped tables is IMPDEF.
+ */
+if (s->ct.indirect) {
+l2t_id = icid / (s->ct.page_sz / L1TABLE_ENTRY_SIZE);
+
+value = address_space_ldq_le(as,
+ s->ct.base_addr +
+ (l2t_id * L1TABLE_ENTRY_SIZE),
+ MEMTXATTRS_UNSPECIFIED, );
+
+if (res != MEMTX_OK) {
+return res;
+}
+
+valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
+
+if (valid_l2t) {
+max_l2_entries = s->ct.page_sz / s->ct.entry_sz;
+
+l2t_addr = value & ((1ULL << 51) - 1);
+
+address_space_stq_le(as, l2t_addr +
+ ((icid % max_l2_entries) * GITS_CTE_SIZE),
+ cte, MEMTXATTRS_UNSPECIFIED, );
+}
+} else {
+/* Flat level table */
+address_space_stq_le(as, s->ct.base_addr + (icid * GITS_CTE_SIZE),
+ cte, MEMTXATTRS_UNSPECIFIED, );
+}
+return res;
+}
+
+static MemTxResult process_mapc(GICv3ITSState *s, uint32_t offset)
+{
+AddressSpace *as = >gicv3->dma_as;
+uint16_t icid;
+uint64_t rdbase;
+bool valid;
+MemTxResult res = MEMTX_OK;
+uint64_t value;
+
+offset += NUM_BYTES_IN_DW;
+offset += NUM_BYTES_IN_DW;
+
+value = address_space_ldq_le(as, s->cq.base_addr + offset,
+ MEMTXATTRS_UNSPECIFIED, );
+
+if (res != MEMTX_OK) {
+return res;
+}
+
+icid = value & ICID_MASK;
+
+rdbase = (value & R_MAPC_RDBASE_MASK) >> R_MAPC_RDBASE_SHIFT;
+rdbase &= RDBASE_PROCNUM_MASK;
+
+valid = (value & CMD_FIELD_VALID_MASK);
+
+if ((icid > s->ct.maxids.max_collids) || (rdbase > s->gicv3->num_cpu)) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "ITS MAPC: invalid collection table attributes "
+ "icid %d rdbase %lu\n", icid, rdbase);
+/*
+ * in this implementation, in case of error
+ * we ignore this command and move onto the next
+ * command in the queue
+ */
+} else {
+res = update_cte(s, icid, valid, rdbase);
+}
+
+return res;
+}
+
+static MemTxResult update_dte(GICv3ITSState *s, uint32_t devid, bool valid,
+ uint8_t size, uint64_t itt_addr)
+{
+AddressSpace *as = >gicv3->dma_as;
+uint64_t value;
+uint64_t l2t_addr;
+bool valid_l2t;
+uint32_t l2t_id;
+uint32_t max_l2_entries;
+uint64_t dte = 0;
+MemTxResult res = MEMTX_OK;
+
+if (s->dt.valid) {
+if (valid) {
+/* add mapping entry to device table */
+dte = (valid & TABLE_ENTRY_VALID_MASK) |
+ ((size & SIZE_MASK) << 1U) |
+ (itt_addr << GITS_DTE_ITTADDR_SHIFT);
+}
+} else {
+return res;
+}
+
+/*
+ * The specification defines the format of level 1 entries of a
+ * 2-level table, but the format of level 2 entries and the format
+ * of flat-mapped tables is IMPDEF.
+ */
+if (s->dt.indirect) {
+l2t_id = devid / (s->dt.page_sz / L1TABLE_ENTRY_SIZE);
+
+value = address_space_ldq_le(as,
+ s->dt.base_addr +
+ (l2t_id * L1TABLE_ENTRY_SIZE),
+ MEMTXATTRS_UNSPECIFIED, );
+
+if (res != MEMTX_OK) {
+return res;
+}
+
+valid_l2t = (value & L2_TABLE_VALID_MASK) != 0;
+
+if (valid_l2t) {
+
[PATCH v7 05/10] hw/intc: GICv3 ITS Feature enablement
Added properties to enable ITS feature and define qemu system
address space memory in gicv3 common,setup distributor and
redistributor registers to indicate LPI support.
Signed-off-by: Shashi Mallela
Reviewed-by: Peter Maydell
---
hw/intc/arm_gicv3_common.c | 12
hw/intc/arm_gicv3_dist.c | 5 -
hw/intc/arm_gicv3_redist.c | 12 +---
hw/intc/gicv3_internal.h | 2 ++
include/hw/intc/arm_gicv3_common.h | 1 +
5 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 58ef65f589..53dea2a775 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -345,6 +345,11 @@ static void arm_gicv3_common_realize(DeviceState *dev,
Error **errp)
return;
}
+if (s->lpi_enable && !s->dma) {
+error_setg(errp, "Redist-ITS: Guest 'sysmem' reference link not set");
+return;
+}
+
s->cpu = g_new0(GICv3CPUState, s->num_cpu);
for (i = 0; i < s->num_cpu; i++) {
@@ -381,6 +386,10 @@ static void arm_gicv3_common_realize(DeviceState *dev,
Error **errp)
(1 << 24) |
(i << 8) |
(last << 4);
+
+if (s->lpi_enable) {
+s->cpu[i].gicr_typer |= GICR_TYPER_PLPIS;
+}
}
}
@@ -494,9 +503,12 @@ static Property arm_gicv3_common_properties[] = {
DEFINE_PROP_UINT32("num-cpu", GICv3State, num_cpu, 1),
DEFINE_PROP_UINT32("num-irq", GICv3State, num_irq, 32),
DEFINE_PROP_UINT32("revision", GICv3State, revision, 3),
+DEFINE_PROP_BOOL("has-lpi", GICv3State, lpi_enable, 0),
DEFINE_PROP_BOOL("has-security-extensions", GICv3State, security_extn, 0),
DEFINE_PROP_ARRAY("redist-region-count", GICv3State, nb_redist_regions,
redist_region_count, qdev_prop_uint32, uint32_t),
+DEFINE_PROP_LINK("sysmem", GICv3State, dma, TYPE_MEMORY_REGION,
+ MemoryRegion *),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/intc/arm_gicv3_dist.c b/hw/intc/arm_gicv3_dist.c
index b65f56f903..43128b376d 100644
--- a/hw/intc/arm_gicv3_dist.c
+++ b/hw/intc/arm_gicv3_dist.c
@@ -371,7 +371,9 @@ static MemTxResult gicd_readl(GICv3State *s, hwaddr offset,
* A3V == 1 (non-zero values of Affinity level 3 supported)
* IDbits == 0xf (we support 16-bit interrupt identifiers)
* DVIS == 0 (Direct virtual LPI injection not supported)
- * LPIS == 0 (LPIs not supported)
+ * LPIS == 1 (LPIs are supported if affinity routing is enabled)
+ * num_LPIs == 0b0 (bits [15:11],Number of LPIs as indicated
+ * by GICD_TYPER.IDbits)
* MBIS == 0 (message-based SPIs not supported)
* SecurityExtn == 1 if security extns supported
* CPUNumber == 0 since for us ARE is always 1
@@ -386,6 +388,7 @@ static MemTxResult gicd_readl(GICv3State *s, hwaddr offset,
bool sec_extn = !(s->gicd_ctlr & GICD_CTLR_DS);
*data = (1 << 25) | (1 << 24) | (sec_extn << 10) |
+(s->lpi_enable << GICD_TYPER_LPIS_SHIFT) |
(0xf << 19) | itlinesnumber;
return MEMTX_OK;
}
diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
index 53da703ed8..2108abfe9c 100644
--- a/hw/intc/arm_gicv3_redist.c
+++ b/hw/intc/arm_gicv3_redist.c
@@ -248,10 +248,16 @@ static MemTxResult gicr_writel(GICv3CPUState *cs, hwaddr
offset,
case GICR_CTLR:
/* For our implementation, GICR_TYPER.DPGS is 0 and so all
* the DPG bits are RAZ/WI. We don't do anything asynchronously,
- * so UWP and RWP are RAZ/WI. And GICR_TYPER.LPIS is 0 (we don't
- * implement LPIs) so Enable_LPIs is RES0. So there are no writable
- * bits for us.
+ * so UWP and RWP are RAZ/WI. GICR_TYPER.LPIS is 1 (we
+ * implement LPIs) so Enable_LPIs is programmable.
*/
+if (cs->gicr_typer & GICR_TYPER_PLPIS) {
+if (value & GICR_CTLR_ENABLE_LPIS) {
+cs->gicr_ctlr |= GICR_CTLR_ENABLE_LPIS;
+} else {
+cs->gicr_ctlr &= ~GICR_CTLR_ENABLE_LPIS;
+}
+}
return MEMTX_OK;
case GICR_STATUSR:
/* RAZ/WI for our implementation */
diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h
index 1966444790..530d1c1789 100644
--- a/hw/intc/gicv3_internal.h
+++ b/hw/intc/gicv3_internal.h
@@ -68,6 +68,8 @@
#define GICD_CTLR_E1NWF (1U << 7)
#define GICD_CTLR_RWP (1U << 31)
+#define GICD_TYPER_LPIS_SHIFT 17
+
/* 16 bits EventId */
#define GICD_TYPER_IDBITS0xf
diff --git a/include/hw/intc/arm_gicv3_common.h
b/include/hw/intc/arm_gicv3_common.h
index 0715b0bc2a..c1348cc60a 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -221,6 +221,7 @@ struct GICv3State {
uint32_t num_cpu;
uint32_t num_irq;
uint32_t
[PATCH v7 07/10] hw/arm/sbsa-ref: add ITS support in SBSA GIC
Included creation of ITS as part of SBSA platform GIC
initialization.
Signed-off-by: Shashi Mallela
---
hw/arm/sbsa-ref.c | 79 ---
1 file changed, 75 insertions(+), 4 deletions(-)
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index c1629df603..feadae2f33 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -34,7 +34,7 @@
#include "hw/boards.h"
#include "hw/ide/internal.h"
#include "hw/ide/ahci_internal.h"
-#include "hw/intc/arm_gicv3_common.h"
+#include "hw/intc/arm_gicv3_its_common.h"
#include "hw/loader.h"
#include "hw/pci-host/gpex.h"
#include "hw/qdev-properties.h"
@@ -58,12 +58,26 @@
#define ARCH_TIMER_NS_EL1_IRQ 14
#define ARCH_TIMER_NS_EL2_IRQ 10
+/*
+ * Enumeration of the possible values of sbsa-ref version
+ * property. These are arbitrary QEMU-internal values.
+ * values are :-
+ * DEFAULT = without ITS memory map
+ * SBSA_GIC_ITS = with ITS memory map between distributor & redistributor
+ *regions. This is the current version supported.
+ */
+typedef enum SbsaRefVersion {
+SBSA_DEFAULT,
+SBSA_ITS,
+} SbsaRefVersion;
+
enum {
SBSA_FLASH,
SBSA_MEM,
SBSA_CPUPERIPHS,
SBSA_GIC_DIST,
SBSA_GIC_REDIST,
+SBSA_GIC_ITS,
SBSA_SECURE_EC,
SBSA_GWDT,
SBSA_GWDT_REFRESH,
@@ -91,6 +105,7 @@ struct SBSAMachineState {
void *fdt;
int fdt_size;
int psci_conduit;
+SbsaRefVersion version;
DeviceState *gic;
PFlashCFI01 *flash[2];
};
@@ -105,8 +120,11 @@ static const MemMapEntry sbsa_ref_memmap[] = {
[SBSA_SECURE_MEM] = { 0x2000, 0x2000 },
/* Space reserved for CPU peripheral devices */
[SBSA_CPUPERIPHS] = { 0x4000, 0x0004 },
+/* GIC components reserved space Start */
[SBSA_GIC_DIST] = { 0x4006, 0x0001 },
-[SBSA_GIC_REDIST] = { 0x4008, 0x0400 },
+[SBSA_GIC_ITS] ={ 0x4007, 0x0002 },
+[SBSA_GIC_REDIST] = { 0x400B, 0x0400 },
+/* GIC components reserved space End */
[SBSA_SECURE_EC] = { 0x5000, 0x1000 },
[SBSA_GWDT_REFRESH] = { 0x5001, 0x1000 },
[SBSA_GWDT_CONTROL] = { 0x50011000, 0x1000 },
@@ -377,7 +395,20 @@ static void create_secure_ram(SBSAMachineState *sms,
memory_region_add_subregion(secure_sysmem, base, secram);
}
-static void create_gic(SBSAMachineState *sms)
+static void create_its(SBSAMachineState *sms)
+{
+DeviceState *dev;
+
+dev = qdev_new(TYPE_ARM_GICV3_ITS);
+SysBusDevice *s = SYS_BUS_DEVICE(dev);
+
+object_property_set_link(OBJECT(dev), "parent-gicv3", OBJECT(sms->gic),
+ _abort);
+sysbus_realize_and_unref(s, _fatal);
+sysbus_mmio_map(s, 0, sbsa_ref_memmap[SBSA_GIC_ITS].base);
+}
+
+static void create_gic(SBSAMachineState *sms, MemoryRegion *mem)
{
unsigned int smp_cpus = MACHINE(sms)->smp.cpus;
SysBusDevice *gicbusdev;
@@ -404,6 +435,10 @@ static void create_gic(SBSAMachineState *sms)
qdev_prop_set_uint32(sms->gic, "len-redist-region-count", 1);
qdev_prop_set_uint32(sms->gic, "redist-region-count[0]", redist0_count);
+object_property_set_link(OBJECT(sms->gic), "sysmem", OBJECT(mem),
+ _fatal);
+qdev_prop_set_bit(sms->gic, "has-lpi", true);
+
gicbusdev = SYS_BUS_DEVICE(sms->gic);
sysbus_realize_and_unref(gicbusdev, _fatal);
sysbus_mmio_map(gicbusdev, 0, sbsa_ref_memmap[SBSA_GIC_DIST].base);
@@ -450,6 +485,7 @@ static void create_gic(SBSAMachineState *sms)
sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus,
qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
}
+create_its(sms);
}
static void create_uart(const SBSAMachineState *sms, int uart,
@@ -755,7 +791,7 @@ static void sbsa_ref_init(MachineState *machine)
create_secure_ram(sms, secure_sysmem);
-create_gic(sms);
+create_gic(sms, sysmem);
create_uart(sms, SBSA_UART, sysmem, serial_hd(0));
create_uart(sms, SBSA_SECURE_UART, secure_sysmem, serial_hd(1));
@@ -825,10 +861,39 @@ sbsa_ref_get_default_cpu_node_id(const MachineState *ms,
int idx)
return idx % ms->numa_state->num_nodes;
}
+static char *sbsa_get_version(Object *obj, Error **errp)
+{
+SBSAMachineState *sms = SBSA_MACHINE(obj);
+
+switch (sms->version) {
+case SBSA_DEFAULT:
+return g_strdup("default");
+case SBSA_ITS:
+return g_strdup("sbsaits");
+default:
+g_assert_not_reached();
+}
+}
+
+static void sbsa_set_version(Object *obj, const char *value, Error **errp)
+{
+SBSAMachineState *sms = SBSA_MACHINE(obj);
+
+if (!strcmp(value, "sbsaits")) {
+sms->version = SBSA_ITS;
+} else if (!strcmp(value, "default")) {
+sms->version = SBSA_DEFAULT;
+} else {
+error_setg(errp, "Invalid version value");
+error_append_hint(errp, "Valid values are
[PATCH v7 01/10] hw/intc: GICv3 ITS initial framework
Added register definitions relevant to ITS,implemented overall
ITS device framework with stubs for ITS control and translater
regions read/write,extended ITS common to handle mmio init between
existing kvm device and newer qemu device.
Signed-off-by: Shashi Mallela
Reviewed-by: Peter Maydell
Reviewed-by: Eric Auger
---
hw/intc/arm_gicv3_its.c| 245 +
hw/intc/arm_gicv3_its_common.c | 7 +-
hw/intc/arm_gicv3_its_kvm.c| 2 +-
hw/intc/gicv3_internal.h | 96 +-
hw/intc/meson.build| 1 +
include/hw/intc/arm_gicv3_its_common.h | 9 +-
6 files changed, 346 insertions(+), 14 deletions(-)
create mode 100644 hw/intc/arm_gicv3_its.c
diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
new file mode 100644
index 00..2286b3f757
--- /dev/null
+++ b/hw/intc/arm_gicv3_its.c
@@ -0,0 +1,245 @@
+/*
+ * ITS emulation for a GICv3-based system
+ *
+ * Copyright Linaro.org 2021
+ *
+ * Authors:
+ * Shashi Mallela
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at your
+ * option) any later version. See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/qdev-properties.h"
+#include "hw/intc/arm_gicv3_its_common.h"
+#include "gicv3_internal.h"
+#include "qom/object.h"
+#include "qapi/error.h"
+
+typedef struct GICv3ITSClass GICv3ITSClass;
+/* This is reusing the GICv3ITSState typedef from ARM_GICV3_ITS_COMMON */
+DECLARE_OBJ_CHECKERS(GICv3ITSState, GICv3ITSClass,
+ ARM_GICV3_ITS, TYPE_ARM_GICV3_ITS)
+
+struct GICv3ITSClass {
+GICv3ITSCommonClass parent_class;
+void (*parent_reset)(DeviceState *dev);
+};
+
+static MemTxResult gicv3_its_translation_write(void *opaque, hwaddr offset,
+ uint64_t data, unsigned size,
+ MemTxAttrs attrs)
+{
+MemTxResult result = MEMTX_OK;
+
+return result;
+}
+
+static MemTxResult its_writel(GICv3ITSState *s, hwaddr offset,
+ uint64_t value, MemTxAttrs attrs)
+{
+MemTxResult result = MEMTX_OK;
+
+return result;
+}
+
+static MemTxResult its_readl(GICv3ITSState *s, hwaddr offset,
+ uint64_t *data, MemTxAttrs attrs)
+{
+MemTxResult result = MEMTX_OK;
+
+return result;
+}
+
+static MemTxResult its_writell(GICv3ITSState *s, hwaddr offset,
+ uint64_t value, MemTxAttrs attrs)
+{
+MemTxResult result = MEMTX_OK;
+
+return result;
+}
+
+static MemTxResult its_readll(GICv3ITSState *s, hwaddr offset,
+ uint64_t *data, MemTxAttrs attrs)
+{
+MemTxResult result = MEMTX_OK;
+
+return result;
+}
+
+static MemTxResult gicv3_its_read(void *opaque, hwaddr offset, uint64_t *data,
+ unsigned size, MemTxAttrs attrs)
+{
+GICv3ITSState *s = (GICv3ITSState *)opaque;
+MemTxResult result;
+
+switch (size) {
+case 4:
+result = its_readl(s, offset, data, attrs);
+break;
+case 8:
+result = its_readll(s, offset, data, attrs);
+break;
+default:
+result = MEMTX_ERROR;
+break;
+}
+
+if (result == MEMTX_ERROR) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: invalid guest read at offset " TARGET_FMT_plx
+ "size %u\n", __func__, offset, size);
+/*
+ * The spec requires that reserved registers are RAZ/WI;
+ * so use MEMTX_ERROR returns from leaf functions as a way to
+ * trigger the guest-error logging but don't return it to
+ * the caller, or we'll cause a spurious guest data abort.
+ */
+result = MEMTX_OK;
+*data = 0;
+}
+return result;
+}
+
+static MemTxResult gicv3_its_write(void *opaque, hwaddr offset, uint64_t data,
+ unsigned size, MemTxAttrs attrs)
+{
+GICv3ITSState *s = (GICv3ITSState *)opaque;
+MemTxResult result;
+
+switch (size) {
+case 4:
+result = its_writel(s, offset, data, attrs);
+break;
+case 8:
+result = its_writell(s, offset, data, attrs);
+break;
+default:
+result = MEMTX_ERROR;
+break;
+}
+
+if (result == MEMTX_ERROR) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: invalid guest write at offset " TARGET_FMT_plx
+ "size %u\n", __func__, offset, size);
+/*
+ * The spec requires that reserved registers are RAZ/WI;
+ * so use MEMTX_ERROR returns from leaf functions as a way to
+ * trigger the guest-error logging but don't return it to
+ * the caller, or we'll cause a spurious guest data abort.
+ */
+result = MEMTX_OK;
+}
+return result;
+}
+
+static const MemoryRegionOps
[PATCH v7 00/10] GICv3 LPI and ITS feature implementation
This patchset implements qemu device model for enabling physical LPI support and ITS functionality in GIC as per GICv3 specification. Both flat table and 2 level tables are implemented.The ITS commands for adding/deleting ITS table entries,trigerring LPI interrupts are implemented.Translated LPI interrupt ids are processed by redistributor to determine priority and set pending state appropriately before forwarding the same to cpu interface. The ITS feature support has been added to sbsa-ref platform as well as virt platform,wherein the emulated functionality co-exists with kvm kernel functionality. Changes in v7: - Restructured the lpi pending table scan logic as per comments in v6 - Addressed minor comments from v6 - For sbsa-ref platform,ITS memory map has been inserted between distributor & redistributors region similar to GIC-600 layout. Also, introduced sbsa-ref versioning to reflect the latest its memory map change and subsequent future use for helper functions - All kvm_unit_tests PASS - Verified Linux Boot functionality Shashi Mallela (10): hw/intc: GICv3 ITS initial framework hw/intc: GICv3 ITS register definitions added hw/intc: GICv3 ITS command queue framework hw/intc: GICv3 ITS Command processing hw/intc: GICv3 ITS Feature enablement hw/intc: GICv3 redistributor ITS processing hw/arm/sbsa-ref: add ITS support in SBSA GIC tests/data/acpi/virt: Add IORT files for ITS hw/arm/virt: add ITS support in virt GIC tests/data/acpi/virt: Update IORT files for ITS hw/arm/sbsa-ref.c | 79 +- hw/arm/virt.c | 28 +- hw/intc/arm_gicv3.c| 14 + hw/intc/arm_gicv3_common.c | 13 + hw/intc/arm_gicv3_cpuif.c |7 +- hw/intc/arm_gicv3_dist.c |5 +- hw/intc/arm_gicv3_its.c| 1298 hw/intc/arm_gicv3_its_common.c |7 +- hw/intc/arm_gicv3_its_kvm.c|2 +- hw/intc/arm_gicv3_redist.c | 153 ++- hw/intc/gicv3_internal.h | 188 +++- hw/intc/meson.build|1 + include/hw/arm/virt.h |2 + include/hw/intc/arm_gicv3_common.h | 13 + include/hw/intc/arm_gicv3_its_common.h | 32 +- target/arm/kvm_arm.h |4 +- tests/data/acpi/virt/IORT | Bin 0 -> 124 bytes tests/data/acpi/virt/IORT.memhp| Bin 0 -> 124 bytes tests/data/acpi/virt/IORT.numamem | Bin 0 -> 124 bytes tests/data/acpi/virt/IORT.pxb | Bin 0 -> 124 bytes 20 files changed, 1818 insertions(+), 28 deletions(-) create mode 100644 hw/intc/arm_gicv3_its.c create mode 100644 tests/data/acpi/virt/IORT create mode 100644 tests/data/acpi/virt/IORT.memhp create mode 100644 tests/data/acpi/virt/IORT.numamem create mode 100644 tests/data/acpi/virt/IORT.pxb -- 2.27.0
[PATCH] Use EGL device extension in display initialization.
This patch enables running generic EGL devices such as Nvidia's in headless
mode. It assumes single device. More work is needed to support multiple devices.
Signed-off-by: Eugene Huang mailto:euge...@nvidia.com>>
---
ui/egl-helpers.c | 41 +
1 file changed, 37 insertions(+), 4 deletions(-)
diff --git a/ui/egl-helpers.c b/ui/egl-helpers.c
index 7c530c2825..c11610c083 100644
--- a/ui/egl-helpers.c
+++ b/ui/egl-helpers.c
@@ -1,6 +1,8 @@
/*
* Copyright (C) 2015-2016 Gerd Hoffmann
mailto:kra...@redhat.com>>
*
+ * Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
@@ -349,11 +351,26 @@ static EGLDisplay
qemu_egl_get_display(EGLNativeDisplayType native,
EGLDisplay dpy = EGL_NO_DISPLAY;
/* In practise any EGL 1.5 implementation would support the EXT extension
*/
-if (epoxy_has_egl_extension(NULL, "EGL_EXT_platform_base")) {
+if (epoxy_has_egl_extension(NULL, "EGL_EXT_platform_base")
+&& epoxy_has_egl_extension(NULL, "EGL_EXT_platform_device")
+&& (epoxy_has_egl_extension(NULL, "EGL_EXT_device_base")
+|| epoxy_has_egl_extension(NULL, "EGL_EXT_device_enumeration"))) {
PFNEGLGETPLATFORMDISPLAYEXTPROC getPlatformDisplayEXT =
(void *) eglGetProcAddress("eglGetPlatformDisplayEXT");
if (getPlatformDisplayEXT && platform != 0) {
-dpy = getPlatformDisplayEXT(platform, native, NULL);
+if (platform == EGL_PLATFORM_DEVICE_EXT) {
+static const int MAX_DEVICES = 4;
+EGLDeviceEXT eglDevs[MAX_DEVICES];
+EGLint numDevices;
+
+PFNEGLQUERYDEVICESEXTPROC eglQueryDevicesEXT =
+(PFNEGLQUERYDEVICESEXTPROC)
+eglGetProcAddress("eglQueryDevicesEXT");
+eglQueryDevicesEXT(MAX_DEVICES, eglDevs, );
+dpy = getPlatformDisplayEXT(platform, eglDevs[0], 0);
+} else {
+dpy = getPlatformDisplayEXT(platform, native, NULL);
+}
}
}
@@ -386,6 +403,17 @@ static int qemu_egl_init_dpy(EGLNativeDisplayType dpy,
EGL_ALPHA_SIZE, 0,
EGL_NONE,
};
+
+static const EGLint conf_att_pbuffer[] = {
+EGL_SURFACE_TYPE, EGL_PBUFFER_BIT,
+EGL_RED_SIZE, 8,
+EGL_GREEN_SIZE, 8,
+EGL_BLUE_SIZE, 8,
+EGL_DEPTH_SIZE, 1,
+EGL_RENDERABLE_TYPE, EGL_OPENGL_BIT,
+EGL_NONE
+};
+
EGLint major, minor;
EGLBoolean b;
EGLint n;
@@ -411,8 +439,8 @@ static int qemu_egl_init_dpy(EGLNativeDisplayType dpy,
}
b = eglChooseConfig(qemu_egl_display,
-gles ? conf_att_gles : conf_att_core,
-_egl_config, 1, );
+gles ? conf_att_gles : (platform == EGL_PLATFORM_DEVICE_EXT ?
conf_att_pbuffer : conf_att_core),
+_egl_config, 1, );
if (b == EGL_FALSE || n != 1) {
error_report("egl: eglChooseConfig failed (%s mode)",
gles ? "gles" : "core");
@@ -434,6 +462,11 @@ int qemu_egl_init_dpy_x11(EGLNativeDisplayType dpy,
DisplayGLMode mode)
int qemu_egl_init_dpy_mesa(EGLNativeDisplayType dpy, DisplayGLMode mode)
{
+// Try EGL Device Extension
+if (qemu_egl_init_dpy(dpy, EGL_PLATFORM_DEVICE_EXT, mode) == 0) {
+return 0;
+}
+
#ifdef EGL_MESA_platform_gbm
return qemu_egl_init_dpy(dpy, EGL_PLATFORM_GBM_MESA, mode);
#else
--
2.17.1
Re: [RFC PATCH 06/13] target/riscv: Fix div instructions
On 8/4/21 4:53 PM, LIU Zhiwei wrote: Don't overwrite global source register after https://lists.gnu.org/archive/html/qemu-riscv/2021-07/msg00058.html. Signed-off-by: LIU Zhiwei --- target/riscv/translate.c | 46 +++- 1 file changed, 26 insertions(+), 20 deletions(-) FWIW, I have a cleanup to these routines that does more than add temps. I've been slow to re-post the series; sorry about that. r~
Re: [RFC PATCH 05/13] target/riscv: Support UXL32 for shift instruction
On 8/4/21 4:53 PM, LIU Zhiwei wrote:
static bool trans_srli(DisasContext *ctx, arg_srli *a)
{
+if (ctx->uxl32) {
+return trans_srliw(ctx, a);
+}
return gen_shifti(ctx, a, tcg_gen_shr_tl);
}
First, trans_srliw begins with REQUIRE_64BIT, which *should* fail when RV32 is in effect.
This means there's a missing change to is_32bit().
Second, the current decode for srli allows 7 bits of shift, while srilw only allows 5. As
a consequence, gen_shifti contains a check for TARGET_LONG_BITS and trans_srliw does not
contain a check at all. We need to diagnose an out-of-range shift for RV32 somewhere.
I recommend extending the gen_shift* family of helpers.
static bool gen_shifti_imm(DisasContext *ctx, arg_shift *a, int width,
void (*func)(TCGv, TCGv, target_long))
{
TCGv dest, src1;
if (a->shamt >= width) {
return false;
}
dest = gpr_dst(ctx, a->rd);
src1 = gpr_src(ctx, a->rs1);
func(dest, src1, a->shamt);
return true;
}
static bool gen_shifti(DisasContext *ctx, arg_shift *a, int width,
void (*func)(TCGv, TCGv, TCGv))
{...}
static void gen_srliw(TCGv dest, TCGv src1, target_long shamt)
{
tcg_gen_extract_tl(dest, src1, shamt, 32 - shamt);
tcg_gen_ext32s_tl(dest, dest);
}
static bool trans_srliw(DisasContext *ctx, arg_shift *a)
{
REQUIRE_64BIT(ctx);
return gen_shifti_imm(ctx, a, 32, gen_srliw);
}
static bool trans_srli(DisasContext *ctx, arg_shift *a)
{
int xlen = is_32bit(ctx) ? 32 : 64;
return gen_shifti_imm(ctx, a, xlen,
xlen == TARGET_LONG_BITS
? tcg_gen_shri_tl : gen_srliw);
}
etc. Perhaps that is_32bit() check above could be consolidated into some
macro/inline.
r~
Re: [PATCH v3] hw/acpi: add an assertion check for non-null return from acpi_get_i386_pci_host
On Thu, Aug 05, 2021 at 07:42:35PM +0530, Ani Sinha wrote: > Also to be noted that there is a stub for acpi_get_i386_pci_host() which > simply returns NULL. This activates when CONFIG_PC is disabled. It is this > stub that gets called for mips and hence the check for non-null host is > needed in acpi_set_pci_info() function. > Frankly this is generating more discussion that it's worth imho. IMHO these tweaks will bring little benefit. Igor do you feel differently? -- MST
[PATCH] bcm2836: Remove redundant typedef and macros
commit 58b350280e97 ("hw/arm/bcm2836: Restrict BCM283XInfo
declaration to C source") didn't just move the struct
BCM283XClass definition to bcm2836.c. It also introduced a
typedef (BCM283XClass) and two type checking macros
(BCM283X_CLASS, BCM283X_GET_CLASS).
The typedef and macros duplicate what is already defined by the
OBJECT_DECLARE_TYPE declaration at bcm2836.h. Remove the
redundant declarations.
Signed-off-by: Eduardo Habkost
---
Cc: Philippe Mathieu-Daudé
Cc: Luc Michel
Cc: Peter Maydell
---
hw/arm/bcm2836.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 24354338cad..f894338fc6a 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -16,7 +16,7 @@
#include "hw/arm/raspi_platform.h"
#include "hw/sysbus.h"
-typedef struct BCM283XClass {
+struct BCM283XClass {
/*< private >*/
DeviceClass parent_class;
/*< public >*/
@@ -26,12 +26,7 @@ typedef struct BCM283XClass {
hwaddr peri_base; /* Peripheral base address seen by the CPU */
hwaddr ctrl_base; /* Interrupt controller and mailboxes etc. */
int clusterid;
-} BCM283XClass;
-
-#define BCM283X_CLASS(klass) \
-OBJECT_CLASS_CHECK(BCM283XClass, (klass), TYPE_BCM283X)
-#define BCM283X_GET_CLASS(obj) \
-OBJECT_GET_CLASS(BCM283XClass, (obj), TYPE_BCM283X)
+};
static Property bcm2836_enabled_cores_property =
DEFINE_PROP_UINT32("enabled-cpus", BCM283XState, enabled_cpus, 0);
--
2.31.1
Re: [PATCH v3 06/25] python/aqmp: add runstate state machine to AsyncProtocol
On Thu, Aug 5, 2021 at 5:02 PM Eric Blake wrote:
> On Tue, Aug 03, 2021 at 02:29:22PM -0400, John Snow wrote:
> > This serves a few purposes:
> >
> > 1. Protect interfaces when it's not safe to call them (via @require)
> >
> > 2. Add an interface by which an async client can determine if the state
> > has changed, for the purposes of connection management.
> >
> > Signed-off-by: John Snow
> > ---
> > python/qemu/aqmp/__init__.py | 6 +-
> > python/qemu/aqmp/protocol.py | 159 ++-
> > 2 files changed, 160 insertions(+), 5 deletions(-)
> >
>
> > +++ b/python/qemu/aqmp/protocol.py
> > +
> > +F = TypeVar('F', bound=Callable[..., Any]) # pylint:
> disable=invalid-name
> > +
> > +
> > +# Don't Panic.
> > +def require(required_state: Runstate) -> Callable[[F], F]:
>
> Technically, the definition of F is integral to the definition of
> require, so I might have put the 'Don't Panic' note a bit sooner. And
> it took me a while before I noticed...
>
> > +"""
> > +Decorator: protect a method so it can only be run in a certain
> `Runstate`.
> > +
> > +:param required_state: The `Runstate` required to invoke this
> method.
> > +:raise StateError: When the required `Runstate` is not met.
> > +"""
> > +def _decorator(func: F) -> F:
> > +# _decorator is the decorator that is built by calling the
> > +# require() decorator factory; e.g.:
> > +#
> > +# @require(Runstate.IDLE) def # foo(): ...
>
> Is that second # intentional?
>
>
No, that one is a mistake. Comment reflow mishap.
> > +# will replace 'foo' with the result of '_decorator(foo)'.
> > +
> > +@wraps(func)
> > +def _wrapper(proto: 'AsyncProtocol[Any]',
> > + *args: Any, **kwargs: Any) -> Any:
> > +# _wrapper is the function that gets executed prior to the
> > +# decorated method.
> > +
> > +name = type(proto).__name__
> > +
> > +if proto.runstate != required_state:
> > +if proto.runstate == Runstate.CONNECTING:
> > +emsg = f"{name} is currently connecting."
> > +elif proto.runstate == Runstate.DISCONNECTING:
> > +emsg = (f"{name} is disconnecting."
> > +" Call disconnect() to return to IDLE
> state.")
> > +elif proto.runstate == Runstate.RUNNING:
> > +emsg = f"{name} is already connected and running."
> > +elif proto.runstate == Runstate.IDLE:
> > +emsg = f"{name} is disconnected and idle."
> > +else:
> > +assert False
> > +raise StateError(emsg, proto.runstate, required_state)
> > +# No StateError, so call the wrapped method.
> > +return func(proto, *args, **kwargs)
> > +
> > +# Return the decorated method;
> > +# Transforming Func to Decorated[Func].
> > +return cast(F, _wrapper)
> > +
> > +# Return the decorator instance from the decorator factory. Phew!
> > +return _decorator
>
> ...that it paired with this comment, explaining what looks like a
> monstrosity, but in reality is a fairly typical and boilerplate
> implementation of a function decorator (not that you write one every
> day - the whole point of the decorator is to have a one-word
> abstraction already implemented so you DON'T have to reimplement the
> decoration yourself ;).
>
>
> +
> > +
> > class AsyncProtocol(Generic[T]):
> > """
> > AsyncProtocol implements a generic async message-based protocol.
> > @@ -118,7 +202,24 @@ def __init__(self) -> None:
> > #: exit.
> > self._dc_task: Optional[asyncio.Future[None]] = None
> >
> > +self._runstate = Runstate.IDLE
> > +self._runstate_changed: Optional[asyncio.Event] = None
> > +
> > +@property # @upper_half
>
> Is it intentional to leave the @upper_half decorator commented out?
>
>
In this case yes, because you can't decorate a property in Python. So for
the sake of leaving it greppable, especially when those decorators are just
a "notation only" thing anyway, I figured I'd stuff it behind a comment.
> But that's minor enough that I trust you to handle it as you see fit.
>
> Reviewed-by: Eric Blake
>
>
Thanks!
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
>
Re: [PATCH v5 02/10] hw/intc: GICv3 ITS register definitions added
On Thu, 2021-07-08 at 19:27 +0200, Eric Auger wrote:
> Hi Shashi,
>
> On 7/6/21 11:29 AM, Eric Auger wrote:
> > Hi,
> >
> > On 6/30/21 5:31 PM, Shashi Mallela wrote:
> > > Defined descriptors for ITS device table,collection table and ITS
> > > command queue entities.Implemented register read/write functions,
> > > extract ITS table parameters and command queue
> > > parameters,extended
> > > gicv3 common to capture qemu address space(which host the ITS
> > > table
> > > platform memories required for subsequent ITS processing) and
> > > initialize the same in ITS device.
> > >
> > > Signed-off-by: Shashi Mallela
> > > Reviewed-by: Peter Maydell
> > > ---
> > > hw/intc/arm_gicv3_its.c| 376
> > > +
> > > hw/intc/gicv3_internal.h | 31 +-
> > > include/hw/intc/arm_gicv3_common.h | 3 +
> > > include/hw/intc/arm_gicv3_its_common.h | 23 ++
> > > 4 files changed, 432 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/intc/arm_gicv3_its.c b/hw/intc/arm_gicv3_its.c
> > > index 545cda3665..2d786a1e21 100644
> > > --- a/hw/intc/arm_gicv3_its.c
> > > +++ b/hw/intc/arm_gicv3_its.c
> > > @@ -28,6 +28,160 @@ struct GICv3ITSClass {
> > > void (*parent_reset)(DeviceState *dev);
> > > };
> > >
> > > +static uint64_t baser_base_addr(uint64_t value, uint32_t
> > > page_sz)
> > > +{
> > > +uint64_t result = 0;
> > > +
> > > +switch (page_sz) {
> > > +case GITS_PAGE_SIZE_4K:
> > > +case GITS_PAGE_SIZE_16K:
> > > +result = FIELD_EX64(value, GITS_BASER, PHYADDR);
> > << 12 ?
> Did you check that? Seems unchanged in v6?
>
> Thanks
>
> Eric
Have taken care of it in the v7 patch
> > > +break;
> > > +
> > > +case GITS_PAGE_SIZE_64K:
> > > +result = FIELD_EX64(value, GITS_BASER, PHYADDRL_64K) <<
> > > 16;
> > > +result |= FIELD_EX64(value, GITS_BASER, PHYADDRH_64K) <<
> > > 48;
> > > +break;
> > > +
> > > +default:
> > > +break;
> > > +}
> > > +return result;
> > > +}
> > > +
> > > +/*
> > > + * This function extracts the ITS Device and Collection table
> > > specific
> > > + * parameters (like base_addr, size etc) from GITS_BASER
> > > register.
> > > + * It is called during ITS enable and also during post_load
> > > migration
> > > + */
> > > +static void extract_table_params(GICv3ITSState *s)
> > > +{
> > > +uint16_t num_pages = 0;
> > > +uint8_t page_sz_type;
> > > +uint8_t type;
> > > +uint32_t page_sz = 0;
> > > +uint64_t value;
> > > +
> > > +for (int i = 0; i < 8; i++) {
> > > +value = s->baser[i];
> > > +
> > > +if (!value) {
> > > +continue;
> > > +}
> > > +
> > > +page_sz_type = FIELD_EX64(value, GITS_BASER, PAGESIZE);
> > > +
> > > +switch (page_sz_type) {
> > > +case 0:
> > > +page_sz = GITS_PAGE_SIZE_4K;
> > > +break;
> > > +
> > > +case 1:
> > > +page_sz = GITS_PAGE_SIZE_16K;
> > > +break;
> > > +
> > > +case 2:
> > > +case 3:
> > > +page_sz = GITS_PAGE_SIZE_64K;
> > > +break;
> > > +
> > > +default:
> > > +g_assert_not_reached();
> > > +}
> > > +
> > > +num_pages = FIELD_EX64(value, GITS_BASER, SIZE) + 1;
> > > +
> > > +type = FIELD_EX64(value, GITS_BASER, TYPE);
> > > +
> > > +switch (type) {
> > > +
> > > +case GITS_ITT_TYPE_DEVICE:
> > > +memset(>dt, 0 , sizeof(s->dt));
> > > +s->dt.valid = FIELD_EX64(value, GITS_BASER, VALID);
> > > +
> > > +if (!s->dt.valid) {
> > > +return;
> > > +}
> > > +
> > > +s->dt.page_sz = page_sz;
> > > +s->dt.indirect = FIELD_EX64(value, GITS_BASER,
> > > INDIRECT);
> > > +s->dt.entry_sz = FIELD_EX64(value, GITS_BASER,
> > > ENTRYSIZE);
> > > +
> > > +if (!s->dt.indirect) {
> > > +s->dt.max_entries = (num_pages * page_sz) / s-
> > > >dt.entry_sz;
> > > +} else {
> > > +s->dt.max_entries = (((num_pages * page_sz) /
> > > + L1TABLE_ENTRY_SIZE) *
> > > + (page_sz / s-
> > > >dt.entry_sz));
> > > +}
> > > +
> > > +s->dt.maxids.max_devids = (1UL << (FIELD_EX64(s-
> > > >typer, GITS_TYPER,
> > > + DEVBITS) + 1));
> > > +
> > > +s->dt.base_addr = baser_base_addr(value, page_sz);
> > > +
> > > +break;
> > > +
> > > +case GITS_ITT_TYPE_COLLECTION:
> > > +memset(>ct, 0 , sizeof(s->ct));
> > > +s->ct.valid = FIELD_EX64(value, GITS_BASER, VALID);
> > > +
> > > +/*
> > > + * GITS_TYPER.HCC is 0 for this implementation
> > > + * hence writes are discarded if ct.valid is 0
> > > + */
> > > +if
Re: [PATCH v3 06/25] python/aqmp: add runstate state machine to AsyncProtocol
On Tue, Aug 03, 2021 at 02:29:22PM -0400, John Snow wrote:
> This serves a few purposes:
>
> 1. Protect interfaces when it's not safe to call them (via @require)
>
> 2. Add an interface by which an async client can determine if the state
> has changed, for the purposes of connection management.
>
> Signed-off-by: John Snow
> ---
> python/qemu/aqmp/__init__.py | 6 +-
> python/qemu/aqmp/protocol.py | 159 ++-
> 2 files changed, 160 insertions(+), 5 deletions(-)
>
> +++ b/python/qemu/aqmp/protocol.py
> +
> +F = TypeVar('F', bound=Callable[..., Any]) # pylint: disable=invalid-name
> +
> +
> +# Don't Panic.
> +def require(required_state: Runstate) -> Callable[[F], F]:
Technically, the definition of F is integral to the definition of
require, so I might have put the 'Don't Panic' note a bit sooner. And
it took me a while before I noticed...
> +"""
> +Decorator: protect a method so it can only be run in a certain
> `Runstate`.
> +
> +:param required_state: The `Runstate` required to invoke this method.
> +:raise StateError: When the required `Runstate` is not met.
> +"""
> +def _decorator(func: F) -> F:
> +# _decorator is the decorator that is built by calling the
> +# require() decorator factory; e.g.:
> +#
> +# @require(Runstate.IDLE) def # foo(): ...
Is that second # intentional?
> +# will replace 'foo' with the result of '_decorator(foo)'.
> +
> +@wraps(func)
> +def _wrapper(proto: 'AsyncProtocol[Any]',
> + *args: Any, **kwargs: Any) -> Any:
> +# _wrapper is the function that gets executed prior to the
> +# decorated method.
> +
> +name = type(proto).__name__
> +
> +if proto.runstate != required_state:
> +if proto.runstate == Runstate.CONNECTING:
> +emsg = f"{name} is currently connecting."
> +elif proto.runstate == Runstate.DISCONNECTING:
> +emsg = (f"{name} is disconnecting."
> +" Call disconnect() to return to IDLE state.")
> +elif proto.runstate == Runstate.RUNNING:
> +emsg = f"{name} is already connected and running."
> +elif proto.runstate == Runstate.IDLE:
> +emsg = f"{name} is disconnected and idle."
> +else:
> +assert False
> +raise StateError(emsg, proto.runstate, required_state)
> +# No StateError, so call the wrapped method.
> +return func(proto, *args, **kwargs)
> +
> +# Return the decorated method;
> +# Transforming Func to Decorated[Func].
> +return cast(F, _wrapper)
> +
> +# Return the decorator instance from the decorator factory. Phew!
> +return _decorator
...that it paired with this comment, explaining what looks like a
monstrosity, but in reality is a fairly typical and boilerplate
implementation of a function decorator (not that you write one every
day - the whole point of the decorator is to have a one-word
abstraction already implemented so you DON'T have to reimplement the
decoration yourself ;).
> +
> +
> class AsyncProtocol(Generic[T]):
> """
> AsyncProtocol implements a generic async message-based protocol.
> @@ -118,7 +202,24 @@ def __init__(self) -> None:
> #: exit.
> self._dc_task: Optional[asyncio.Future[None]] = None
>
> +self._runstate = Runstate.IDLE
> +self._runstate_changed: Optional[asyncio.Event] = None
> +
> +@property # @upper_half
Is it intentional to leave the @upper_half decorator commented out?
But that's minor enough that I trust you to handle it as you see fit.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
[PATCH v3] accel/tcg: Clear PAGE_WRITE before translation
translate_insn() implementations fetch instruction bytes piecemeal,
which can cause qemu-user to generate inconsistent translations if
another thread modifies them concurrently [1].
Fix by making pages containing translated instruction non-writable
right before loading instruction bytes from them.
[1] https://lists.nongnu.org/archive/html/qemu-devel/2021-08/msg00644.html
Signed-off-by: Ilya Leoshkevich
---
v2: https://lists.nongnu.org/archive/html/qemu-devel/2021-08/msg00819.html
v2 -> v3: Move translator_ld*_swap() functions from translator.h into
translator.c for a better size trade-off (Richard).
accel/tcg/translate-all.c | 59 +--
accel/tcg/translator.c| 30
include/exec/translate-all.h | 1 +
include/exec/translator.h | 52 +---
target/alpha/translate.c | 2 +-
target/arm/arm_ldst.h | 12 ++---
target/arm/translate-a64.c| 2 +-
target/arm/translate.c| 9 ++--
target/hexagon/translate.c| 3 +-
target/hppa/translate.c | 2 +-
target/i386/tcg/translate.c | 10 ++--
target/m68k/translate.c | 2 +-
target/mips/tcg/micromips_translate.c.inc | 2 +-
target/mips/tcg/mips16e_translate.c.inc | 4 +-
target/mips/tcg/nanomips_translate.c.inc | 4 +-
target/mips/tcg/translate.c | 8 +--
target/openrisc/translate.c | 2 +-
target/ppc/translate.c| 5 +-
target/riscv/translate.c | 5 +-
target/s390x/tcg/translate.c | 16 +++---
target/sh4/translate.c| 4 +-
target/sparc/translate.c | 2 +-
target/xtensa/translate.c | 5 +-
23 files changed, 153 insertions(+), 88 deletions(-)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index bbfcfb698c..fb9ebfad9e 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1297,31 +1297,8 @@ static inline void tb_page_add(PageDesc *p,
TranslationBlock *tb,
invalidate_page_bitmap(p);
#if defined(CONFIG_USER_ONLY)
-if (p->flags & PAGE_WRITE) {
-target_ulong addr;
-PageDesc *p2;
-int prot;
-
-/* force the host page as non writable (writes will have a
- page fault + mprotect overhead) */
-page_addr &= qemu_host_page_mask;
-prot = 0;
-for (addr = page_addr; addr < page_addr + qemu_host_page_size;
-addr += TARGET_PAGE_SIZE) {
-
-p2 = page_find(addr >> TARGET_PAGE_BITS);
-if (!p2) {
-continue;
-}
-prot |= p2->flags;
-p2->flags &= ~PAGE_WRITE;
- }
-mprotect(g2h_untagged(page_addr), qemu_host_page_size,
- (prot & PAGE_BITS) & ~PAGE_WRITE);
-if (DEBUG_TB_INVALIDATE_GATE) {
-printf("protecting code page: 0x" TB_PAGE_ADDR_FMT "\n",
page_addr);
-}
-}
+/* translator_loop() must have made all TB pages non-writable */
+assert(!(p->flags & PAGE_WRITE));
#else
/* if some code is already present, then the pages are already
protected. So we handle the case where only the first TB is
@@ -2394,6 +2371,38 @@ int page_check_range(target_ulong start, target_ulong
len, int flags)
return 0;
}
+void page_protect(tb_page_addr_t page_addr)
+{
+target_ulong addr;
+PageDesc *p;
+int prot;
+
+p = page_find(page_addr >> TARGET_PAGE_BITS);
+if (p && (p->flags & PAGE_WRITE)) {
+/*
+ * Force the host page as non writable (writes will have a page fault +
+ * mprotect overhead).
+ */
+page_addr &= qemu_host_page_mask;
+prot = 0;
+for (addr = page_addr; addr < page_addr + qemu_host_page_size;
+ addr += TARGET_PAGE_SIZE) {
+
+p = page_find(addr >> TARGET_PAGE_BITS);
+if (!p) {
+continue;
+}
+prot |= p->flags;
+p->flags &= ~PAGE_WRITE;
+}
+mprotect(g2h_untagged(page_addr), qemu_host_page_size,
+ (prot & PAGE_BITS) & ~PAGE_WRITE);
+if (DEBUG_TB_INVALIDATE_GATE) {
+printf("protecting code page: 0x" TB_PAGE_ADDR_FMT "\n",
page_addr);
+}
+}
+}
+
/* called from signal handler: invalidate the code and unprotect the
* page. Return 0 if the fault was not handled, 1 if it was handled,
* and 2 if it was handled but the caller must cause the TB to be
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index c53a7f8e44..93423949e7 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -56,6 +56,7 @@ void translator_loop(const TranslatorOps *ops,
DisasContextBase *db,
db->num_insns = 0;
db->max_insns = max_insns;
db->singlestep_enabled = cflags &
Re: [PATCH RFC 1/1] accel/tcg: Clear PAGE_WRITE before translation
On Thu, 2021-08-05 at 06:59 -1000, Richard Henderson wrote: > On 8/5/21 12:56 AM, Ilya Leoshkevich wrote: > > On Wed, 2021-08-04 at 14:30 -1000, Richard Henderson wrote: > > > I think that the translator_ld*_swap functions should be moved > > > out of > > > include/exec/translator.h into accel/tcg/translator.c. > > > > Do we really need this? In the end, the added code is not that > > large. > > I suppose it's not required, but they're already larger than they > need to be. > > In my opinion, if you're going to swap one out-of-line function call > for two out-of-line > function calls (cpu_ld*_code + plugin_insn_append), you've probably > made a bad size trade-off. > > With your added code, it'll be 3 out-of-line calls. > > > r~ Fair enough. Let me send a v3 then. Best regards, Ilya
[PATCH v0] kvm: unsigned datatype in ioctl wrapper
Dear all,
in my opinion the `type` argument in the kvm ioctl wrappers should be of
type unsigned. Please correct me if I am wrong.
-
Due to the same reason as explained in the comment on the
`irq_set_ioctl` field in `struct KVMState` (accel/kvm/kvm-all.c),
the kvm ioctl wrapper should take `type` as an unsigned.
Signed-off-by: johannst
---
accel/kvm/kvm-all.c| 8
accel/kvm/trace-events | 8
include/sysemu/kvm.h | 8
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 0125c17edb..45cd6edce3 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2967,7 +2967,7 @@ int kvm_cpu_exec(CPUState *cpu)
return ret;
}
-int kvm_ioctl(KVMState *s, int type, ...)
+int kvm_ioctl(KVMState *s, unsigned type, ...)
{
int ret;
void *arg;
@@ -2985,7 +2985,7 @@ int kvm_ioctl(KVMState *s, int type, ...)
return ret;
}
-int kvm_vm_ioctl(KVMState *s, int type, ...)
+int kvm_vm_ioctl(KVMState *s, unsigned type, ...)
{
int ret;
void *arg;
@@ -3003,7 +3003,7 @@ int kvm_vm_ioctl(KVMState *s, int type, ...)
return ret;
}
-int kvm_vcpu_ioctl(CPUState *cpu, int type, ...)
+int kvm_vcpu_ioctl(CPUState *cpu, unsigned type, ...)
{
int ret;
void *arg;
@@ -3021,7 +3021,7 @@ int kvm_vcpu_ioctl(CPUState *cpu, int type, ...)
return ret;
}
-int kvm_device_ioctl(int fd, int type, ...)
+int kvm_device_ioctl(int fd, unsigned type, ...)
{
int ret;
void *arg;
diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events
index 399aaeb0ec..d78c2eff31 100644
--- a/accel/kvm/trace-events
+++ b/accel/kvm/trace-events
@@ -1,11 +1,11 @@
# See docs/devel/tracing.rst for syntax documentation.
# kvm-all.c
-kvm_ioctl(int type, void *arg) "type 0x%x, arg %p"
-kvm_vm_ioctl(int type, void *arg) "type 0x%x, arg %p"
-kvm_vcpu_ioctl(int cpu_index, int type, void *arg) "cpu_index %d, type 0x%x,
arg %p"
+kvm_ioctl(unsigned type, void *arg) "type 0x%x, arg %p"
+kvm_vm_ioctl(unsigned type, void *arg) "type 0x%x, arg %p"
+kvm_vcpu_ioctl(int cpu_index, unsigned type, void *arg) "cpu_index %d, type
0x%x, arg %p"
kvm_run_exit(int cpu_index, uint32_t reason) "cpu_index %d, reason %d"
-kvm_device_ioctl(int fd, int type, void *arg) "dev fd %d, type 0x%x, arg %p"
+kvm_device_ioctl(int fd, unsigned type, void *arg) "dev fd %d, type 0x%x, arg
%p"
kvm_failed_reg_get(uint64_t id, const char *msg) "Warning: Unable to retrieve
ONEREG %" PRIu64 " from KVM: %s"
kvm_failed_reg_set(uint64_t id, const char *msg) "Warning: Unable to set
ONEREG %" PRIu64 " to KVM: %s"
kvm_init_vcpu(int cpu_index, unsigned long arch_cpu_id) "index: %d id: %lu"
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index a1ab1ee12d..6d6ed4acf9 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -251,11 +251,11 @@ int kvm_on_sigbus(int code, void *addr);
/* internal API */
-int kvm_ioctl(KVMState *s, int type, ...);
+int kvm_ioctl(KVMState *s, unsigned type, ...);
-int kvm_vm_ioctl(KVMState *s, int type, ...);
+int kvm_vm_ioctl(KVMState *s, unsigned type, ...);
-int kvm_vcpu_ioctl(CPUState *cpu, int type, ...);
+int kvm_vcpu_ioctl(CPUState *cpu, unsigned type, ...);
/**
* kvm_device_ioctl - call an ioctl on a kvm device
@@ -264,7 +264,7 @@ int kvm_vcpu_ioctl(CPUState *cpu, int type, ...);
*
* Returns: -errno on error, nonnegative on success
*/
-int kvm_device_ioctl(int fd, int type, ...);
+int kvm_device_ioctl(int fd, unsigned type, ...);
/**
* kvm_vm_check_attr - check for existence of a specific vm attribute
--
2.32.0
Re: [PATCH v4 03/14] migration.json: add AMD SEV specific migration parameters
On Wed, Aug 04, 2021 at 11:54:43AM +, Ashish Kalra wrote: > From: Brijesh Singh > > AMD SEV migration flow requires that target machine's public Diffie-Hellman > key (PDH) and certificate chain must be passed before initiating the guest > migration. User can use QMP 'migrate-set-parameters' to pass the certificate > chain. The certificate chain will be used while creating the outgoing > encryption context. > > Signed-off-by: Brijesh Singh > Signed-off-by: Ashish Kalra > --- > +++ b/qapi/migration.json > @@ -743,6 +743,15 @@ > #block device name if there is one, and to their > node name > #otherwise. (Since 5.2) > # > +# @sev-pdh: The target host platform diffie-hellman key encoded in base64 > +# (Since 4.2) 6.2, not 4.2. > +# > +# @sev-plat-cert: The target host platform certificate chain encoded in > base64 > +# (Since 4.2) And again; I'll quit pointing it out. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH v4 7/8] hw/arm/sbsa-ref: add ITS support in SBSA GIC
On Thu, 2021-07-08 at 23:05 +0100, Leif Lindholm wrote: > On Thu, Jul 08, 2021 at 21:05:02 +0100, Peter Maydell wrote: > > On Thu, 8 Jul 2021 at 20:40, Leif Lindholm > > wrote: > > > I think my summary-summary would be: > > > - I think we will need to introduce a compatiblity-breaking > > > change to > > > sbsa-ref. > > > - I think we will need to have support for more than one ITS if > > > we're > > > going to be able to use QEMU to prototype real systems. > > > - I think we should then start versioning sbsa-ref (like many > > > other > > > platforms already are). And there are other reasons why I would > > > want > > > to do this. > > > - But I think it would be unfair to hold this set back for it. > > > > FWIW, I do not currently expect this series to make 6.1, so we > > have some time to get things right. > > Ah, ok. > > Then I would ideally like to see this patch add the ITS block between > Distributor and Redistributors regions. I think it makes the most > sense > for this version to match the GIC-600 layout. Have added ITS block between Distributor and Redistributors regions,with comments. Also,added sbsa versioning support (as discussed in last call) > > I am also going to rework those two remaining gicv4-ish patches based > on my acquired understanding, to be sent out post v6.1. > > And, I would like to switch the default CPU of sbsa-ref to "max" as > part of that platform versioning, now that is supported in TF-A > (from v2.5). > > / > Leif
Re: [PATCH v3 06/13] python/aqmp-tui: Added type annotations for aqmp-tui
On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu
wrote:
> This patch adds type annotations for aqmp-tui using
> the mypy library.
>
>
Awesome, thanks for taking a swing at this. Looks like it wasn't as bad as
I was fearing.
> Signed-off-by: G S Niteesh Babu
> ---
> python/qemu/aqmp/aqmp_tui.py | 79
> python/setup.cfg | 3 --
> 2 files changed, 43 insertions(+), 39 deletions(-)
>
> diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
> index ec9eba0aa7..ab9ada793a 100644
> --- a/python/qemu/aqmp/aqmp_tui.py
> +++ b/python/qemu/aqmp/aqmp_tui.py
> @@ -9,8 +9,15 @@
> import argparse
> import asyncio
> import logging
> -from logging import Handler
> +from logging import Handler, LogRecord
> import signal
> +from typing import (
> +Any,
> +List,
> +Optional,
> +Tuple,
> +Union,
> +)
>
> import urwid
> import urwid_readline
> @@ -22,13 +29,13 @@
> from .util import create_task, pretty_traceback
>
>
> -UPDATE_MSG = 'UPDATE_MSG'
> +UPDATE_MSG: str = 'UPDATE_MSG'
>
> # Using root logger to enable all loggers under qemu and asyncio
> LOGGER = logging.getLogger()
>
>
> -def format_json(msg):
> +def format_json(msg: str) -> str:
> """
> Formats given multiline JSON message into a single line message.
> Converting into single line is more asthetically pleasing when looking
> @@ -43,17 +50,17 @@ def format_json(msg):
>
>
> class App(QMPClient):
> -def __init__(self, address):
> +def __init__(self, address: Union[str, Tuple[str, int]]) -> None:
> urwid.register_signal(type(self), UPDATE_MSG)
> self.window = Window(self)
> self.address = address
> -self.aloop = None
> +self.aloop: Optional[Any] = None # FIXME: Use more concrete type.
>
I ran into this in util.py; you want Optional[asyncio.AbstractEventLoop]
here.
> super().__init__()
>
> -def add_to_history(self, msg):
> +def add_to_history(self, msg: str) -> None:
> urwid.emit_signal(self, UPDATE_MSG, msg)
>
> -def _cb_outbound(self, msg):
> +def _cb_outbound(self, msg: Message) -> Message:
> # FIXME: I think the ideal way to omit these messages during
> in-TUI
> # logging will be to add a filter to the logger. We can use regex
> to
> # filter out messages starting with 'Request:' or 'Response:' but
> I
> @@ -67,25 +74,25 @@ def _cb_outbound(self, msg):
> self.add_to_history('<-- ' + str(msg))
> return msg
>
> -def _cb_inbound(self, msg):
> +def _cb_inbound(self, msg: Message) -> Message:
> handler = LOGGER.handlers[0]
> if not isinstance(handler, TUILogHandler):
> LOGGER.debug('Response: %s', str(msg))
> self.add_to_history('--> ' + str(msg))
> return msg
>
> -async def wait_for_events(self):
> +async def wait_for_events(self) -> None:
> async for event in self.events:
> self.handle_event(event)
>
> -async def _send_to_server(self, raw_msg):
> +async def _send_to_server(self, raw_msg: str) -> None:
> # FIXME: Format the raw_msg in history view to one line. It is not
> # pleasing to see multiple lines JSON object with an error
> statement.
> try:
> msg = Message(bytes(raw_msg, encoding='utf-8'))
> # Format multiline json into a single line JSON, since it is
> more
> # pleasing to look along with err message in TUI.
> -raw_msg = self.format_json(raw_msg)
> +raw_msg = format_json(raw_msg)
> await self._raw(msg, assign_id='id' not in msg)
> except (ValueError, TypeError) as err:
> LOGGER.info('Invalid message: %s', str(err))
> @@ -102,18 +109,18 @@ def _cb_inbound(self, msg):
> LOGGER.error('Exception from _send_to_server: %s', str(err))
> raise err
>
> -def cb_send_to_server(self, msg):
> +def cb_send_to_server(self, msg: str) -> None:
> create_task(self._send_to_server(msg))
>
> -def unhandled_input(self, key):
> +def unhandled_input(self, key: str) -> None:
> if key == 'esc':
> self.kill_app()
>
> -def kill_app(self):
> +def kill_app(self) -> None:
> # TODO: Work on the disconnect logic
> create_task(self._kill_app())
>
> -async def _kill_app(self):
> +async def _kill_app(self) -> None:
> # It is ok to call disconnect even in disconnect state
> try:
> await self.disconnect()
> @@ -124,7 +131,7 @@ def kill_app(self):
> raise err
> raise urwid.ExitMainLoop()
>
> -def handle_event(self, event):
> +def handle_event(self, event: Message) -> None:
> # FIXME: Consider all states present in qapi/run-state.json
> if event['event'] == 'SHUTDOWN':
> self._set_status('Server shutdown')
> @@ -139,7 +146,7 @@ def
Re: [PATCH 2/9] hw/sd: Extract address_in_range() helper, log invalid accesses
On Wed, Jun 23, 2021 at 08:00:14PM +0200, Philippe Mathieu-Daudé wrote:
> Multiple commands have to check the address requested is valid.
check that the
> Extract this code pattern as a new address_in_range() helper, and
> log invalid accesses as guest errors.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/sd/sd.c | 32
> 1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index d8fdf84f4db..9c8dd11bad1 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -937,6 +937,18 @@ static void sd_lock_command(SDState *sd)
> sd->card_status &= ~CARD_IS_LOCKED;
> }
>
> +static bool address_in_range(SDState *sd, const char *desc,
> + uint64_t addr, uint32_t length)
> +{
> +if (addr + length > sd->size) {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s offset %lu > card %lu [%%%u]\n",
> + desc, addr, sd->size, length);
For a (fictitiously small) device with 2048 bytes and a read request
of 2k at offset 1k, this results in the odd message:
READ_BLOCK offset 1024 > card 2048 [%2048]
Would it be any better as:
"%s offset+length %lu+%lu > card size %lu\n"
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
Re: About two-dimensional page translation (e.g., Intel EPT) and shadow page table in Linux QEMU/KVM
Sean, understood with many thanks! Good luck, Harry On Wed, Jul 28, 2021 at 3:01 PM Sean Christopherson wrote: > > On Wed, Jul 28, 2021, harry harry wrote: > > Sean, sorry for the late reply. Thanks for your careful explanations. > > > > > For emulation of any instruction/flow that starts with a guest virtual > > > address. > > > On Intel CPUs, that includes quite literally any "full" instruction > > > emulation, > > > since KVM needs to translate CS:RIP to a guest physical address in order > > > to fetch > > > the guest's code stream. KVM can't avoid "full" emulation unless the > > > guest is > > > heavily enlightened, e.g. to avoid string I/O, among many other things. > > > > Do you mean the emulated MMU is needed when it *only* wants to > > translate GVAs to GPAs in the guest level? > > Not quite, though gva_to_gpa() is the main use. The emulated MMU is also > used to > inject guest #PF and to load/store guest PDTPRs. > > > In such cases, the hardware MMU cannot be used because hardware MMU > > can only translate GVAs to HPAs, right? > > Sort of. The hardware MMU does translate GVA to GPA, but the GPA value is not > visible to software (unless the GPA->HPA translation faults). That's also > true > for VA to PA (and GVA to HPA). Irrespective of virtualization, x86 ISA > doesn't > provide an instruction to retrive the PA for a given VA. > > If such an instruction did exist, and it was to be usable for a VMM to do a > GVA->GPA translation, the magic instruction would need to take all MMU params > as > operands, e.g. CR0, CR3, CR4, and EFER. When KVM is active (not the guest), > the > hardware MMU is loaded with the host MMU configuration, not the guest. In > both > VMX and SVM, vCPU state is mostly ephemeral in the sense that it ceases to > exist > in hardware when the vCPU exits to the host. Some state is retained in > hardware, > e.g. TLB and cache entries, but those are associated with select properties of > the vCPU, e.g. EPTP, CR3, etc..., not with the vCPU itself, i.e. not with the > VMCS (VMX) / VMCB (SVM).
Re: [PATCH 1/9] hw/sd: When card is in wrong state, log which state it is
On Wed, Jun 23, 2021 at 08:00:13PM +0200, Philippe Mathieu-Daudé wrote: > We report the card is in an inconsistent state, but don't precise s/don't/aren't/ > in which state it is. Add this information, as it is useful when > debugging problems. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/sd/sd.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake > > diff --git a/hw/sd/sd.c b/hw/sd/sd.c > index 282d39a7042..d8fdf84f4db 100644 > --- a/hw/sd/sd.c > +++ b/hw/sd/sd.c > @@ -1504,7 +1504,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, > SDRequest req) > return sd_illegal; > } > > -qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state\n", req.cmd); > +qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s\n", > + req.cmd, sd_state_name(sd->state)); > return sd_illegal; > } > > -- > 2.31.1 > > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[PATCH for-6.2 4/6] s390-sclp-events-bus: Use OBJECT_DECLARE_SIMPLE_TYPE
We have a SCLPEventsBus struct type defined, but no QOM type
checkers are declared for the type.
Use OBJECT_DECLARE_SIMPLE_TYPE to declare the struct type and
have a SCLP_EVENT_BUS typecast wrapper defined.
Signed-off-by: Eduardo Habkost
---
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 0a65e16cdd9..9f7883d6e20 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -328,6 +328,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB
*sccb)
/* qemu object creation and initialization functions */
#define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"
+OBJECT_DECLARE_SIMPLE_TYPE(SCLPEventsBus, SCLP_EVENT_BUS)
static const TypeInfo sclp_events_bus_info = {
.name = TYPE_SCLP_EVENTS_BUS,
--
2.31.1
[PATCH for-6.2 6/6] Use PCI_HOST_BRIDGE macro
OBJECT_CHECK(PciHostState, ..., TYPE_PCI_HOST_BRIDGE) is exactly
what the PCI_HOST_BRIDGE macro does. We can just use the macro
instead of using OBJECT_CHECK manually.
Signed-off-by: Eduardo Habkost
---
Cc: "Michael S. Tsirkin"
Cc: Igor Mammedov
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Eduardo Habkost
Cc: Marcel Apfelbaum
Cc: qemu-devel@nongnu.org
---
hw/i386/acpi-build.c | 8 ++--
hw/pci-host/i440fx.c | 4 +---
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a33ac8b91e1..3c6bbb1beb3 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -303,13 +303,9 @@ Object *acpi_get_i386_pci_host(void)
{
PCIHostState *host;
-host = OBJECT_CHECK(PCIHostState,
-object_resolve_path("/machine/i440fx", NULL),
-TYPE_PCI_HOST_BRIDGE);
+host = PCI_HOST_BRIDGE(object_resolve_path("/machine/i440fx", NULL));
if (!host) {
-host = OBJECT_CHECK(PCIHostState,
-object_resolve_path("/machine/q35", NULL),
-TYPE_PCI_HOST_BRIDGE);
+host = PCI_HOST_BRIDGE(object_resolve_path("/machine/q35", NULL));
}
return OBJECT(host);
diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 28c9bae8994..cd87e21a9b2 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -316,9 +316,7 @@ PCIBus *i440fx_init(const char *host_type, const char
*pci_type,
PCIBus *find_i440fx(void)
{
-PCIHostState *s = OBJECT_CHECK(PCIHostState,
- object_resolve_path("/machine/i440fx",
NULL),
- TYPE_PCI_HOST_BRIDGE);
+PCIHostState *s = PCI_HOST_BRIDGE(object_resolve_path("/machine/i440fx",
NULL));
return s ? s->bus : NULL;
}
--
2.31.1
Re: [PULL 0/5] QAPI patches patches for 2021-08-05
On Thu, 5 Aug 2021 at 15:09, Markus Armbruster wrote: > > The following changes since commit cb2f4b8750b7e1c954570d19b104d4fdbeb8739a: > > Merge remote-tracking branch > 'remotes/thuth-gitlab/tags/pull-request-2021-08-03' into staging (2021-08-03 > 19:50:43 +0100) > > are available in the Git repository at: > > git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2021-08-05 > > for you to fetch changes up to 68e6dc594a44a7077657f2ea878806e38dfa50cf: > > docs: convert writing-qmp-commands.txt to writing-qmp-commands.rst > (2021-08-04 11:18:05 +0200) > > > QAPI patches patches for 2021-08-05 > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1 for any user-visible changes. -- PMM
Re: [PATCH] gluster: Align block-status tail
On Thu, Aug 05, 2021 at 04:36:03PM +0200, Max Reitz wrote: > gluster's block-status implementation is basically a copy of that in > block/file-posix.c, there is only one thing missing, and that is > aligning trailing data extents to the request alignment (as added by > commit 9c3db310ff0). > > Note that 9c3db310ff0 mentions that "there seems to be no other block > driver that sets request_alignment and [...]", but while block/gluster.c > does indeed not set request_alignment, block/io.c's > bdrv_refresh_limits() will still default to an alignment of 512 because > block/gluster.c does not provide a byte-aligned read function. > Therefore, unaligned tails can conceivably occur, and so we should apply > the change from 9c3db310ff0 to gluster's block-status implementation. > > Reported-by: Vladimir Sementsov-Ogievskiy > Signed-off-by: Max Reitz > --- > block/gluster.c | 16 > 1 file changed, 16 insertions(+) Probably not a show-stopper for 6.1, so I'm fine if it sits until 6.2. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[PATCH for-6.2 3/6] s390-sclp-events-bus: Set instance_size
We have a SCLPEventsBus struct defined, but the struct is not
used at the TypeInfo definition. This works today but will break
silently if anybody adds a new field to SCLPEventsBus.
Set instance_size properly to avoid problems in the future.
Signed-off-by: Eduardo Habkost
---
Cc: Cornelia Huck
Cc: Thomas Huth
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index ed92ce510d9..0a65e16cdd9 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -332,6 +332,7 @@ static void write_event_mask(SCLPEventFacility *ef, SCCB
*sccb)
static const TypeInfo sclp_events_bus_info = {
.name = TYPE_SCLP_EVENTS_BUS,
.parent = TYPE_BUS,
+.instance_size = sizeof(SCLPEventsBus),
};
static void command_handler(SCLPEventFacility *ef, SCCB *sccb, uint64_t code)
--
2.31.1
[PATCH for-6.2 5/6] s390x: event-facility: Use SCLP_EVENT_BUS macro
Use the SCLP_EVENT_BUS macro instead of manually calling
OBJECT_CHECK.
Signed-off-by: Eduardo Habkost
---
Cc: Cornelia Huck
Cc: Thomas Huth
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/s390x/event-facility.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index 9f7883d6e20..bc706bd19b4 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -532,7 +532,7 @@ BusState *sclp_get_event_facility_bus(void)
SCLPEventsBus *sbus;
busobj = object_resolve_path_type("", TYPE_SCLP_EVENTS_BUS, NULL);
-sbus = OBJECT_CHECK(SCLPEventsBus, busobj, TYPE_SCLP_EVENTS_BUS);
+sbus = SCLP_EVENT_BUS(busobj);
if (!sbus) {
return NULL;
}
--
2.31.1
[PATCH for-6.2 2/6] sbsa_gwdt: Delete broken SBSA_*CLASS macros
Those macros never worked and never will, because the SBSA_GWDTClass type never existed. Signed-off-by: Eduardo Habkost --- Cc: qemu-devel@nongnu.org Cc: Peter Maydell Cc: Shashi Mallela --- include/hw/watchdog/sbsa_gwdt.h | 4 1 file changed, 4 deletions(-) diff --git a/include/hw/watchdog/sbsa_gwdt.h b/include/hw/watchdog/sbsa_gwdt.h index 70b137de301..dcb13bc29dc 100644 --- a/include/hw/watchdog/sbsa_gwdt.h +++ b/include/hw/watchdog/sbsa_gwdt.h @@ -19,10 +19,6 @@ #define TYPE_WDT_SBSA "sbsa_gwdt" #define SBSA_GWDT(obj) \ OBJECT_CHECK(SBSA_GWDTState, (obj), TYPE_WDT_SBSA) -#define SBSA_GWDT_CLASS(klass) \ -OBJECT_CLASS_CHECK(SBSA_GWDTClass, (klass), TYPE_WDT_SBSA) -#define SBSA_GWDT_GET_CLASS(obj) \ -OBJECT_GET_CLASS(SBSA_GWDTClass, (obj), TYPE_WDT_SBSA) /* SBSA Generic Watchdog register definitions */ /* refresh frame */ -- 2.31.1
[PATCH for-6.2 1/6] acpi: Delete broken ACPI_GED_X86 macro
The macro never worked and never will, because the AcpiGedX86State type never existed. Signed-off-by: Eduardo Habkost --- Cc: "Michael S. Tsirkin" Cc: Igor Mammedov Cc: qemu-devel@nongnu.org --- include/hw/acpi/generic_event_device.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h index 6bed92e8fc5..d49217c445f 100644 --- a/include/hw/acpi/generic_event_device.h +++ b/include/hw/acpi/generic_event_device.h @@ -70,8 +70,6 @@ OBJECT_DECLARE_SIMPLE_TYPE(AcpiGedState, ACPI_GED) #define TYPE_ACPI_GED_X86 "acpi-ged-x86" -#define ACPI_GED_X86(obj) \ -OBJECT_CHECK(AcpiGedX86State, (obj), TYPE_ACPI_GED_X86) #define ACPI_GED_EVT_SEL_OFFSET0x0 #define ACPI_GED_EVT_SEL_LEN 0x4 -- 2.31.1
[PATCH for-6.2 0/6] qom: Fix broken OBJECT_CHECK usage
This series removes some broken OBJECT_CHECK macros and fix cases where OBJECT_CHECK is being used directly in the code. Eduardo Habkost (6): acpi: Delete broken ACPI_GED_X86 macro sbsa_gwdt: Delete broken SBSA_*CLASS macros s390-sclp-events-bus: Set instance_size s390-sclp-events-bus: Use OBJECT_DECLARE_SIMPLE_TYPE s390x: event-facility: Use SCLP_EVENT_BUS macro Use PCI_HOST_BRIDGE macro include/hw/acpi/generic_event_device.h | 2 -- include/hw/watchdog/sbsa_gwdt.h| 4 hw/i386/acpi-build.c | 8 ++-- hw/pci-host/i440fx.c | 4 +--- hw/s390x/event-facility.c | 4 +++- 5 files changed, 6 insertions(+), 16 deletions(-) -- 2.31.1
[PATCH v2] Partially revert "build: -no-pie is no functional linker flag"
This partially reverts commit bbd2d5a8120771ec59b86a80a1f51884e0a26e53.
This commit was misguided and broke using --disable-pie on any distro
that enables PIE by default in their compiler driver, including Debian
and its derivatives. Whilst -no-pie is not a linker flag, it is a
compiler driver flag that ensures -pie is not automatically passed by it
to the linker. Without it, all compile_prog checks will fail as any code
built with the explicit -fno-pie will fail to link with the implicit
default -pie due to trying to use position-dependent relocations. The
only bug that needed fixing was LDFLAGS_NOPIE being used as a flag for
the linker itself in pc-bios/optionrom/Makefile.
Note this does not reinstate exporting LDFLAGS_NOPIE, as it is unused,
since the only previous use was the one that should not have existed. I
have also updated the comment for the -fno-pie and -no-pie checks to
reflect what they're actually needed for.
Fixes: bbd2d5a8120771ec59b86a80a1f51884e0a26e53
Cc: Christian Ehrhardt
Cc: Paolo Bonzini
Cc: qemu-sta...@nongnu.org
Signed-off-by: Jessica Clarke
---
Changes in v2:
* Actually include the comment change; didn't add the hunk when
amending...
configure | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 9a79a004d7..8aecd277ed 100755
--- a/configure
+++ b/configure
@@ -2246,9 +2246,11 @@ static THREAD int tls_var;
int main(void) { return tls_var; }
EOF
-# Check we support --no-pie first; we will need this for building ROMs.
+# Check we support -fno-pie and -no-pie first; we will need the former for
+# building ROMs, and both for everything if --disable-pie is passed.
if compile_prog "-Werror -fno-pie" "-no-pie"; then
CFLAGS_NOPIE="-fno-pie"
+ LDFLAGS_NOPIE="-no-pie"
fi
if test "$static" = "yes"; then
@@ -2264,6 +2266,7 @@ if test "$static" = "yes"; then
fi
elif test "$pie" = "no"; then
CONFIGURE_CFLAGS="$CFLAGS_NOPIE $CONFIGURE_CFLAGS"
+ CONFIGURE_LDFLAGS="$LDFLAGS_NOPIE $CONFIGURE_LDFLAGS"
elif compile_prog "-Werror -fPIE -DPIE" "-pie"; then
CONFIGURE_CFLAGS="-fPIE -DPIE $CONFIGURE_CFLAGS"
CONFIGURE_LDFLAGS="-pie $CONFIGURE_LDFLAGS"
--
2.17.1
Re: [PATCH v7 10/10] qapi: make 'if' condition strings simple identifiers
On Wed, Aug 04, 2021 at 12:31:05PM +0400, marcandre.lur...@redhat.com wrote: > From: Marc-André Lureau > > Change the 'if' condition strings to be C-agnostic. It will accept > '[A-Z][A-Z0-9_]*' identifiers. This allows to express configuration This allows the expression of configuration > conditions in other languages (Rust or Python for ex) or other more I'd spell out 'example' > suitable forms. > > Signed-off-by: Marc-André Lureau > Reviewed-by: Stefan Hajnoczi > Tested-by: John Snow > --- > @@ -4307,8 +4307,8 @@ > # @size: Size of the virtual disk in bytes > # @preallocation: Preallocation mode for the new image (default: off; > # allowed values: off, > -# falloc (if defined CONFIG_POSIX_FALLOCATE), > -# full (if defined CONFIG_POSIX)) > +# falloc (if CONFIG_POSIX_FALLOCATE), > +# full (if CONFIG_POSIX)) Pre-existing, but the (double (nesting)) is a bit lisp-y considering this comment is user-facing, where using [] for one of the layers might help readability. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 05/13] python: add entry point for aqmp-tui
On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu wrote: > Add an entry point for aqmp-tui. This will allow it to be run from > the command line using "aqmp-tui localhost:1234" > More options available in the TUI can be found using "aqmp-tui -h" > > Signed-off-by: G S Niteesh Babu > Reviewed-by: John Snow
[PATCH] Partially revert "build: -no-pie is no functional linker flag"
This partially reverts commit bbd2d5a8120771ec59b86a80a1f51884e0a26e53. This commit was misguided and broke using --disable-pie on any distro that enables PIE by default in their compiler driver, including Debian and its derivatives. Whilst -no-pie is not a linker flag, it is a compiler driver flag that ensures -pie is not automatically passed by it to the linker. Without it, all compile_prog checks will fail as any code built with the explicit -fno-pie will fail to link with the implicit default -pie due to trying to use position-dependent relocations. The only bug that needed fixing was LDFLAGS_NOPIE being used as a flag for the linker itself in pc-bios/optionrom/Makefile. Note this does not reinstate exporting LDFLAGS_NOPIE, as it is unused, since the only previous use was the one that should not have existed. I have also updated the comment for the -fno-pie and -no-pie checks to reflect what they're actually needed for. Fixes: bbd2d5a8120771ec59b86a80a1f51884e0a26e53 Cc: Christian Ehrhardt Cc: Paolo Bonzini Cc: qemu-sta...@nongnu.org Signed-off-by: Jessica Clarke --- configure | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index 9a79a004d7..b8b29e1eee 100755 --- a/configure +++ b/configure @@ -2249,6 +2249,7 @@ EOF # Check we support --no-pie first; we will need this for building ROMs. if compile_prog "-Werror -fno-pie" "-no-pie"; then CFLAGS_NOPIE="-fno-pie" + LDFLAGS_NOPIE="-no-pie" fi if test "$static" = "yes"; then @@ -2264,6 +2265,7 @@ if test "$static" = "yes"; then fi elif test "$pie" = "no"; then CONFIGURE_CFLAGS="$CFLAGS_NOPIE $CONFIGURE_CFLAGS" + CONFIGURE_LDFLAGS="$LDFLAGS_NOPIE $CONFIGURE_LDFLAGS" elif compile_prog "-Werror -fPIE -DPIE" "-pie"; then CONFIGURE_CFLAGS="-fPIE -DPIE $CONFIGURE_CFLAGS" CONFIGURE_LDFLAGS="-pie $CONFIGURE_LDFLAGS" -- 2.17.1
Re: [PATCH v3 04/13] python/aqmp-tui: Add AQMP TUI draft
On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu
wrote:
> Added a draft of AQMP TUI.
>
> Implements the follwing basic features:
> 1) Command transmission/reception.
> 2) Shows events asynchronously.
> 3) Shows server status in the bottom status bar.
>
> Also added necessary pylint, mypy configurations
>
> Signed-off-by: G S Niteesh Babu
> ---
> python/qemu/aqmp/aqmp_tui.py | 333 +++
> python/setup.cfg | 16 +-
> 2 files changed, 348 insertions(+), 1 deletion(-)
> create mode 100644 python/qemu/aqmp/aqmp_tui.py
>
> diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
> new file mode 100644
> index 00..ec9eba0aa7
> --- /dev/null
> +++ b/python/qemu/aqmp/aqmp_tui.py
> @@ -0,0 +1,333 @@
> +# Copyright (c) 2021
> +#
> +# Authors:
> +# Niteesh Babu G S
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later. See the COPYING file in the top-level directory.
> +
> +import argparse
> +import asyncio
> +import logging
> +from logging import Handler
> +import signal
> +
> +import urwid
> +import urwid_readline
> +
> +from ..qmp import QEMUMonitorProtocol, QMPBadPortError
> +from .message import DeserializationError, Message, UnexpectedTypeError
> +from .protocol import ConnectError
> +from .qmp_client import ExecInterruptedError, QMPClient
> +from .util import create_task, pretty_traceback
> +
> +
> +UPDATE_MSG = 'UPDATE_MSG'
> +
> +# Using root logger to enable all loggers under qemu and asyncio
> +LOGGER = logging.getLogger()
> +
> +
> +def format_json(msg):
> +"""
> +Formats given multiline JSON message into a single line message.
> +Converting into single line is more asthetically pleasing when looking
> +along with error messages compared to multiline JSON.
> +"""
> +# FIXME: Use better formatting mechanism. Might break at more complex
> JSON
> +# data.
> +msg = msg.replace('\n', '')
> +words = msg.split(' ')
> +words = [word for word in words if word != '']
> +return ' '.join(words)
> +
> +
> +class App(QMPClient):
> +def __init__(self, address):
> +urwid.register_signal(type(self), UPDATE_MSG)
> +self.window = Window(self)
> +self.address = address
> +self.aloop = None
> +super().__init__()
> +
> +def add_to_history(self, msg):
> +urwid.emit_signal(self, UPDATE_MSG, msg)
> +
> +def _cb_outbound(self, msg):
> +# FIXME: I think the ideal way to omit these messages during
> in-TUI
> +# logging will be to add a filter to the logger. We can use regex
> to
> +# filter out messages starting with 'Request:' or 'Response:' but
> I
> +# think a better approach will be encapsulate the message in an
> object
> +# and filter based on the object. Encapsulation of the message
> will
> +# also be necessary when we want different formatting of messages
> +# inside TUI.
> +handler = LOGGER.handlers[0]
> +if not isinstance(handler, TUILogHandler):
> +LOGGER.debug('Request: %s', str(msg))
> +self.add_to_history('<-- ' + str(msg))
> +return msg
> +
> +def _cb_inbound(self, msg):
> +handler = LOGGER.handlers[0]
> +if not isinstance(handler, TUILogHandler):
> +LOGGER.debug('Response: %s', str(msg))
> +self.add_to_history('--> ' + str(msg))
> +return msg
> +
> +async def wait_for_events(self):
> +async for event in self.events:
> +self.handle_event(event)
> +
> +async def _send_to_server(self, raw_msg):
> +# FIXME: Format the raw_msg in history view to one line. It is not
> +# pleasing to see multiple lines JSON object with an error
> statement.
> +try:
> +msg = Message(bytes(raw_msg, encoding='utf-8'))
> +# Format multiline json into a single line JSON, since it is
> more
> +# pleasing to look along with err message in TUI.
> +raw_msg = self.format_json(raw_msg)
> +await self._raw(msg, assign_id='id' not in msg)
> +except (ValueError, TypeError) as err:
> +LOGGER.info('Invalid message: %s', str(err))
> +self.add_to_history(f'{raw_msg}: {err}')
> +except (DeserializationError, UnexpectedTypeError) as err:
> +LOGGER.info('Invalid message: %s', err.error_message)
> +self.add_to_history(f'{raw_msg}: {err.error_message}')
> +except ExecInterruptedError:
> +LOGGER.info('Error server disconnected before reply')
> +urwid.emit_signal(self, UPDATE_MSG,
> + '{"error": "Server disconnected before
> reply"}')
> +self._set_status("Server disconnected")
> +except Exception as err:
> +LOGGER.error('Exception from _send_to_server: %s', str(err))
> +raise err
> +
> +def cb_send_to_server(self, msg):
> +
Re: [RFC PATCH 04/13] target/riscv: Support UXL32 for slit/sltiu
On 8/4/21 4:53 PM, LIU Zhiwei wrote:
+static bool gen_arith_simm_tl(DisasContext *ctx, arg_i *a,
+ void (*func)(TCGv, TCGv, TCGv))
+{
+TCGv dest = gpr_dst(ctx, a->rd);
+TCGv src1 = gpr_src_s(ctx, a->rs1);
+TCGv src2 = tcg_constant_tl(a->imm);
+
+(*func)(dest, src1, src2);
+return true;
+}
+
+static bool gen_arith_uimm_tl(DisasContext *ctx, arg_i *a,
+ void (*func)(TCGv, TCGv, TCGv))
+{
+TCGv dest = gpr_dst(ctx, a->rd);
+TCGv src1 = gpr_src_u(ctx, a->rs1);
+TCGv src2 = tcg_constant_tl(ctx->uxl32 ? a->imm & UINT32_MAX : a->imm);
+
+(*func)(dest, src1, src2);
+return true;
+}
Again, unsigned comparisions work fine with sign-extended inputs.
r~
Re: [RFC PATCH 03/13] target/riscv: Support UXL32 on 64-bit cpu for load/store
On 8/4/21 4:53 PM, LIU Zhiwei wrote:
Get the LSB 32 bits and zero-extend as the base address.
Signed-off-by: LIU Zhiwei
---
target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
b/target/riscv/insn_trans/trans_rvi.c.inc
index ea41d1de2d..6823a6b3e0 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -163,7 +163,7 @@ static bool trans_bgeu(DisasContext *ctx, arg_bgeu *a)
static bool gen_load(DisasContext *ctx, arg_lb *a, MemOp memop)
{
TCGv dest = gpr_dst(ctx, a->rd);
-TCGv addr = gpr_src(ctx, a->rs1);
+TCGv addr = gpr_src_u(ctx, a->rs1);
TCGv temp = NULL;
if (a->imm) {
@@ -207,7 +207,7 @@ static bool trans_lhu(DisasContext *ctx, arg_lhu *a)
static bool gen_store(DisasContext *ctx, arg_sb *a, MemOp memop)
{
-TCGv addr = gpr_src(ctx, a->rs1);
+TCGv addr = gpr_src_u(ctx, a->rs1);
TCGv data = gpr_src(ctx, a->rs2);
TCGv temp = NULL;
This is incorrect. The zero-extension should happen after the addition of the immediate
offset.
r~
Re: [PATCH for-6.2 01/10] docs: qom: Replace old GTK-Doc #symbol syntax with `symbol`
On Thu, Aug 05, 2021 at 12:36:11PM -0400, Eduardo Habkost wrote: > On Wed, Aug 04, 2021 at 08:26:10PM -0400, John Snow wrote: > > On Wed, Aug 4, 2021 at 5:00 PM Eduardo Habkost wrote: > > > > > On Wed, Aug 04, 2021 at 09:42:24PM +0100, Peter Maydell wrote: > > > > Is there a sensible default-role we can use as the default for the whole > > > manual, > > > > rather than only declaring it in individual .rst files ? One of the > > > > things I don't > > > > like about the change here is that it means that `thing` in this > > > individual .rst > > > > file is different from `thing` in every other .rst file in our docs. > > > > > > I believe "any" would be a very sensible default role for all > > > documents, but I don't know how to set default-role globally. > > > I'll try to find out. > > > > Oh, I actually fixed that issue I referenced there back in May -- I keep a > > patchset up to date whenever I work on modernizing the QAPI python code > > that actually DOES switch our default role to "any" ... I updated it just > > today, in fact. I will send it to the list if there's an appetite for it > > now. > > If you already have a patch that makes it possible to change the > default role to "any" globally, I'd be glad to include it in v2 > of this series. John had submitted the patches at: https://lore.kernel.org/qemu-devel/20210805004837.1775306-1-js...@redhat.com/ (Thanks!) If John's patches are merged, the only change needed in this series is the removal of the "default-role" line in patch 01/10. Instead of submitting v2 for a one-line change, I'm hoping I can just get Reviewed-by lines for this version. (the reviews can be conditional on the removal of the "default-role" line in patch 01/10) -- Eduardo
Re: Re: [PATCH v4] virtio/vsock: add two more queues for datagram types
On Wed, Aug 4, 2021 at 1:13 AM Stefano Garzarella wrote:
>
> On Tue, Aug 03, 2021 at 11:41:32PM +, Jiang Wang wrote:
> >Datagram sockets are connectionless and unreliable.
> >The sender does not know the capacity of the receiver
> >and may send more packets than the receiver can handle.
> >
> >Add two more dedicate virtqueues for datagram sockets,
> >so that it will not unfairly steal resources from
> >stream and future connection-oriented sockets.
> >
> >Signed-off-by: Jiang Wang
> >---
> >v1 -> v2: use qemu cmd option to control number of queues,
> > removed configuration settings for dgram.
> >v2 -> v3: use ioctl to get features and decide number of
> >virt queues, instead of qemu cmd option.
> >v3 -> v4: change DGRAM feature bit value to 2. Add an argument
> > in vhost_vsock_common_realize to indicate dgram is supported or not.
> >
> > hw/virtio/vhost-user-vsock.c | 2 +-
> > hw/virtio/vhost-vsock-common.c| 58 ++-
> > hw/virtio/vhost-vsock.c | 5 +-
> > include/hw/virtio/vhost-vsock-common.h| 6 +-
> > include/hw/virtio/vhost-vsock.h | 4 ++
> > include/standard-headers/linux/virtio_vsock.h | 1 +
> > 6 files changed, 69 insertions(+), 7 deletions(-)
> >
> >diff --git a/hw/virtio/vhost-user-vsock.c b/hw/virtio/vhost-user-vsock.c
> >index 6095ed7349..e9ec0e1c00 100644
> >--- a/hw/virtio/vhost-user-vsock.c
> >+++ b/hw/virtio/vhost-user-vsock.c
> >@@ -105,7 +105,7 @@ static void vuv_device_realize(DeviceState *dev, Error
> >**errp)
> > return;
> > }
> >
> >-vhost_vsock_common_realize(vdev, "vhost-user-vsock");
> >+vhost_vsock_common_realize(vdev, "vhost-user-vsock", false);
> >
> > vhost_dev_set_config_notifier(>vhost_dev, _ops);
> >
> >diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c
> >index 4ad6e234ad..c78536911a 100644
> >--- a/hw/virtio/vhost-vsock-common.c
> >+++ b/hw/virtio/vhost-vsock-common.c
> >@@ -17,6 +17,8 @@
> > #include "hw/virtio/vhost-vsock.h"
> > #include "qemu/iov.h"
> > #include "monitor/monitor.h"
> >+#include
> >+#include
> >
> > int vhost_vsock_common_start(VirtIODevice *vdev)
> > {
> >@@ -196,9 +198,39 @@ int vhost_vsock_common_post_load(void *opaque, int
> >version_id)
> > return 0;
> > }
> >
> >-void vhost_vsock_common_realize(VirtIODevice *vdev, const char *name)
> >+static int vhost_vsock_get_max_qps(bool enable_dgram)
> >+{
> >+uint64_t features;
> >+int ret;
> >+int fd = -1;
> >+
> >+if (!enable_dgram)
> >+return MAX_VQS_WITHOUT_DGRAM;
> >+
> >+fd = qemu_open_old("/dev/vhost-vsock", O_RDONLY);
>
>
> As I said in the previous version, we cannot directly open
> /dev/vhost-vsock, for two reasons:
>
>1. this code is common with vhost-user-vsock which does not use
>/dev/vhost-vsock.
>
>2. the fd may have been passed from the management layer and qemu may
>not be able to directly open /dev/vhost-vsock.
>
> I think is better to move this function in hw/virtio/vhost-vsock.c,
> using the `vhostfd`, returning true or false if dgram is supported, then
> you can use it for `enable_dgram` param ...
>
Yes, you are right. Now I remember you said that before but I forgot about that
when I changed the code. I will fix it. Sorry about that.
> >+if (fd == -1) {
> >+error_report("vhost-vsock: failed to open device. %s",
> >strerror(errno));
> >+return -1;
> >+}
> >+
> >+ret = ioctl(fd, VHOST_GET_FEATURES, );
> >+if (ret) {
> >+error_report("vhost-vsock: failed to read device. %s",
> >strerror(errno));
> >+qemu_close(fd);
> >+return ret;
> >+}
> >+
> >+qemu_close(fd);
> >+if (features & (1 << VIRTIO_VSOCK_F_DGRAM))
> >+return MAX_VQS_WITH_DGRAM;
> >+
> >+return MAX_VQS_WITHOUT_DGRAM;
> >+}
> >+
> >+void vhost_vsock_common_realize(VirtIODevice *vdev, const char *name, bool
> >enable_dgram)
> > {
> > VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
> >+int nvqs = MAX_VQS_WITHOUT_DGRAM;
> >
> > virtio_init(vdev, name, VIRTIO_ID_VSOCK,
> > sizeof(struct virtio_vsock_config));
> >@@ -209,12 +241,24 @@ void vhost_vsock_common_realize(VirtIODevice
> >*vdev, const char *name)
> > vvc->trans_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> >vhost_vsock_common_handle_output);
> >
> >+nvqs = vhost_vsock_get_max_qps(enable_dgram);
> >+
> >+if (nvqs < 0)
> >+nvqs = MAX_VQS_WITHOUT_DGRAM;
>
> ... and here, if `enable_dgram` is true, you can set `nvqs =
> MAX_VQS_WITH_DGRAM``
>
sure.
> >+
> >+if (nvqs == MAX_VQS_WITH_DGRAM) {
> >+vvc->dgram_recv_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> >+
> >vhost_vsock_common_handle_output);
> >+vvc->dgram_trans_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> >+
>
Re: [RFC PATCH 02/13] target/riscv: Support UXL32 for branch instructions
On 8/4/21 4:53 PM, LIU Zhiwei wrote:
+static TCGv gpr_src_u(DisasContext *ctx, int reg_num)
+{
+if (reg_num == 0) {
+return ctx->zero;
+}
+if (ctx->uxl32) {
+tcg_gen_ext32u_tl(cpu_gpr[reg_num], cpu_gpr[reg_num]);
+}
+return cpu_gpr[reg_num];
+}
+
+static TCGv gpr_src_s(DisasContext *ctx, int reg_num)
+{
+if (reg_num == 0) {
+return ctx->zero;
+}
+if (ctx->uxl32) {
+tcg_gen_ext32s_tl(cpu_gpr[reg_num], cpu_gpr[reg_num]);
+}
+return cpu_gpr[reg_num];
+}
This is bad: you cannot modify the source registers like this.
These incorrect modifications will be visible to the kernel on transition back
to S-mode.
+static bool gen_branch_u(DisasContext *ctx, arg_b *a, TCGCond cond)
+{
+TCGv src1 = gpr_src_u(ctx, a->rs1);
+TCGv src2 = gpr_src_u(ctx, a->rs2);
+
+return gen_branch_internal(ctx, a, cond, src1, src2);
+}
This is unnecessary. Unsigned comparisons work just fine with sign-extended values. It
will be simpler to keep all values sign-extended.
r~
Re: [RFC PATCH 01/13] target/riscv: Add UXL to tb flags
On 8/4/21 4:53 PM, LIU Zhiwei wrote:
For 32-bit applications run on 64-bit cpu, it may share some code
with other 64-bit applictions. Thus we should distinguish the translated
cache of the share code with a tb flag.
Signed-off-by: LIU Zhiwei
---
target/riscv/cpu.h | 15 +++
target/riscv/translate.c | 3 +++
2 files changed, 18 insertions(+)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index bf1c899c00..2b3ba21a78 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -394,9 +394,20 @@ FIELD(TB_FLAGS, SEW, 5, 3)
FIELD(TB_FLAGS, VILL, 8, 1)
/* Is a Hypervisor instruction load/store allowed? */
FIELD(TB_FLAGS, HLSX, 9, 1)
+FIELD(TB_FLAGS, UXL, 10, 2)
Are you intending to reserve space for RV128 here?
Otherwise this could be a single bit.
Also, you probably don't want to name it "UXL", since it should indicate the current
operating XLEN, taking MXL, SXL and UXL into account.
Perhaps just name the field XLEN32, and make it a single bit?
+static inline bool riscv_cpu_is_uxl32(CPURISCVState *env)
+{
+#ifndef CONFIG_USER_ONLY
+return (get_field(env->mstatus, MSTATUS64_UXL) == 1) &&
+ !riscv_cpu_is_32bit(env) &&
+ (env->priv == PRV_U);
+#endif
+return false;
+}
Again, naming could be better?
It seems trivial to handle all of the fields here. Perhaps
static inline bool riscv_cpu_is_xlen32(env)
{
#if defined(TARGET_RISCV32)
return true;
#elif defined(CONFIG_USER_ONLY)
return false;
#else
/* When emulating a 32-bit-only cpu, use RV32. */
if (riscv_cpu_is_32bit(env)) {
return true;
}
/*
* If MXL has been reduced to RV32, MSTATUSH doesn't have UXL/SXL,
* therefore, XLEN cannot be widened back to RV64 for lower privs.
*/
if (get_field(env->misa, MISA64_MXL) == 1) {
return true;
}
switch (env->priv) {
case PRV_M:
return false;
case PRV_U:
return get_field(env->mstatus, MSTATUS64_UXL) == 1;
default: /* PRV_S & PRV_H */
return get_field(env->mstatus, MSTATUS64_SXL) == 1;
}
#endif
}
@@ -451,6 +462,10 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState
*env, target_ulong *pc,
flags = FIELD_DP32(flags, TB_FLAGS, HLSX, 1);
}
}
+if (riscv_cpu_is_uxl32(env)) {
+flags = FIELD_DP32(flags, TB_FLAGS, UXL,
+ get_field(env->mstatus, MSTATUS64_UXL));
flags = FIELD_DP32(flags, TB_FLAGS, XLEN32,
riscv_cpu_is_xlen32(env));
r~
Re: [PATCH 2/2] docs/sphinx: change default role to "any"
On Wed, Aug 04, 2021 at 08:48:37PM -0400, John Snow wrote: > This interprets single-backtick syntax in all of our Sphinx docs as a > cross-reference to *something*, including Python symbols. If it doesn't > resolve, or resolves to too more than one thing, Sphinx will emit a s/too // > warning and the build will fail. > > Signed-off-by: John Snow > --- > docs/conf.py | 3 +++ > 1 file changed, 3 insertions(+) > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: Re: [RFC v3] virtio/vsock: add two more queues for datagram types
On Tue, Aug 3, 2021 at 11:49 PM Stefano Garzarella wrote:
>
> On Wed, Aug 4, 2021 at 8:41 AM Stefano Garzarella
> wrote:
> >
> > On Tue, Aug 03, 2021 at 11:58:27AM -0700, Jiang Wang . wrote:
> > >On Wed, Jul 7, 2021 at 10:27 AM Stefano Garzarella
> > >wrote:
> > >> On Wed, Jul 07, 2021 at 09:52:46AM -0700, Jiang Wang . wrote:
> > >> >On Wed, Jul 7, 2021 at 1:33 AM Stefano Garzarella
> > >> >wrote:
> > >> >> On Tue, Jul 06, 2021 at 10:26:07PM +, Jiang Wang wrote:
> >
> > [...]
> >
> > >> >> >+
> > >> >> >+if (nvqs < 0)
> > >> >> >+nvqs = MAX_VQS_WITHOUT_DGRAM;
> > >> >> >+
> > >> >> >+if (nvqs == MAX_VQS_WITH_DGRAM) {
> > >> >> >+vvc->dgram_recv_vq = virtio_add_queue(vdev,
> > >> >> >VHOST_VSOCK_QUEUE_SIZE,
> > >> >> >+
> > >> >> >vhost_vsock_common_handle_output);
> > >> >> >+vvc->dgram_trans_vq = virtio_add_queue(vdev,
> > >> >> >VHOST_VSOCK_QUEUE_SIZE,
> > >> >> >+
> > >> >> >vhost_vsock_common_handle_output);
> > >> >> >+}
> > >> >> >+
> > >> >> > /* The event queue belongs to QEMU */
> > >> >> > vvc->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
> > >> >> >
> > >> >> > vhost_vsock_common_handle_output);
> > >> >>
> > >> >> Did you do a test with a guest that doesn't support datagram with QEMU
> > >> >> and hosts that do?
> > >> >>
> > >> >Yes, and it works.
> > >> >
> > >> >> I repost my thoughts that I had on v2:
> > >> >>
> > >> >> What happen if the guest doesn't support dgram?
> > >> >>
> > >> >> I think we should dynamically use the 3rd queue or the 5th queue
> > >> >> for
> > >> >> the events at runtime after the guest acked the features.
> > >> >>
> > >> >> Maybe better to switch to an array of VirtQueue.
> > >> >>
> > >> >I think in current V3, it already dynamically use 3rd or 5th queue
> > >> >depending
> > >> >on the feature bit.
> > >>
> > >> I'm not sure. IIUC when vhost_vsock_common_realize() is called, we don't
> > >> know the features acked by the guest, so how can it be dynamic?
> > >>
> > >> Here we should know only if the host kernel supports it.
> > >>
> > >> Maybe it works, because in QEMU we use the event queue only after a
> > >> migration to send a reset event, so you can try to migrate a guest to
> > >> check this path.
> > >>
> > >I tried VM migration and didn't see any problems. The migration looks fine
> > >and vsock dgram still works after migration. Is there any more specific
> > >test
> > >you want to run to check for this code path?
> > >
> >
> > I meant a migration of a guest from QEMU without this patch to a QEMU
> > with this patch. Of course in that case testing a socket stream.
> >
>
> Sorry, I meant the opposite.
>
> You should try to migrate a guest that does not support dgrams starting
> from a QEMU with this patch (and kernel that supports dgram, so qemu
> uses the 5th queue for event), to a QEMU without this patch.
>
Got it. I tried what you said and saw errors on the destination qemu. Then
I moved event queue up to be number 3 (before adding dgram vqs),
as the following:
+/* The event queue belongs to QEMU */
+vvc->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
+ vhost_vsock_common_handle_output);
+
nvqs = vhost_vsock_get_max_qps(enable_dgram);
@@ -253,10 +257,6 @@ void vhost_vsock_common_realize(VirtIODevice
*vdev, const char *name, bool enabl
vhost_vsock_common_handle_output);
}
-/* The event queue belongs to QEMU */
-vvc->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
- vhost_vsock_common_handle_output);
-
But I still see following errors on the destination qemu:
qemu-system-x86_64: Error starting vhost vsock: 14
Any idea if my above code change is wrong or missing something?
Take one step back, what should be the host kernel version? With or
without dgram support? I tried both. The new dest kernel shows the above error.
The old dest kernel shows a msr error probably not related to vsock.
To explain the above qemu 14 error, I think the issue is that the
source host kernel
supports dgram by always setting the DGRAM feature bit(in my
implementation). Then the source
qemu query the source host kernel, and use 5 for event vq. Even if the source
guest kernel does not support dgram, it currently has no way to tell the source
host or the source qemu.
On the source machine, when we start qemu process, and the guest VM
is still in BIOS or early boot process ( before vsock is initialized), I think
at this time, the qemu vhost_vsock_common_realize() is already called.
So qemu can only check if the host kernel supports dgram or not, but has
no knowledge about the guest kernel. After the guest kernel is fully boot up,
it can tell qemu or the host if it supports dgram or not ( or the host or qemu
detect for that).
Re: [PATCH v3 04/13] python/aqmp-tui: Add AQMP TUI draft
On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu
wrote:
> Added a draft of AQMP TUI.
>
> Implements the follwing basic features:
> 1) Command transmission/reception.
> 2) Shows events asynchronously.
> 3) Shows server status in the bottom status bar.
>
> Also added necessary pylint, mypy configurations
>
> Signed-off-by: G S Niteesh Babu
> ---
> python/qemu/aqmp/aqmp_tui.py | 333 +++
> python/setup.cfg | 16 +-
> 2 files changed, 348 insertions(+), 1 deletion(-)
> create mode 100644 python/qemu/aqmp/aqmp_tui.py
>
> diff --git a/python/qemu/aqmp/aqmp_tui.py b/python/qemu/aqmp/aqmp_tui.py
> new file mode 100644
> index 00..ec9eba0aa7
> --- /dev/null
> +++ b/python/qemu/aqmp/aqmp_tui.py
> @@ -0,0 +1,333 @@
> +# Copyright (c) 2021
> +#
> +# Authors:
> +# Niteesh Babu G S
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later. See the COPYING file in the top-level directory.
> +
> +import argparse
> +import asyncio
> +import logging
> +from logging import Handler
> +import signal
> +
> +import urwid
> +import urwid_readline
> +
> +from ..qmp import QEMUMonitorProtocol, QMPBadPortError
> +from .message import DeserializationError, Message, UnexpectedTypeError
> +from .protocol import ConnectError
> +from .qmp_client import ExecInterruptedError, QMPClient
> +from .util import create_task, pretty_traceback
> +
> +
> +UPDATE_MSG = 'UPDATE_MSG'
>
This line still feels kind of "naked" on a cold read. It could use a small
comment.
> +
> +# Using root logger to enable all loggers under qemu and asyncio
> +LOGGER = logging.getLogger()
>
The comment isn't quite true; this is the root logger -- but you go on to
use it to directly log messages. I don't think you should; use a
module-level logger.
(1) Create a module-level logger that is named after the current module
name (e.g. qemu.aqmp.aqmp_tui) and use that for logging messages relating
to this module:
LOGGER = logging.getLogger(__name__)
(2) Where you need to address the root logger, just use `root_logger =
logging.getLogger() ` I think the main() function is the only place
you might need this.
> +
> +
> +def format_json(msg):
> +"""
> +Formats given multiline JSON message into a single line message.
> +Converting into single line is more asthetically pleasing when looking
> +along with error messages compared to multiline JSON.
> +"""
> +# FIXME: Use better formatting mechanism. Might break at more complex
> JSON
> +# data.
> +msg = msg.replace('\n', '')
> +words = msg.split(' ')
> +words = [word for word in words if word != '']
> +return ' '.join(words)
> +
>
You can use the JSON module to do this,
https://docs.python.org/3/library/json.html#json.dumps
Message._serialize uses this technique to send JSON messages over the wire
that have no newlines:
https://gitlab.com/jsnow/qemu/-/blob/python-async-qmp-aqmp/python/qemu/aqmp/message.py#L132
by not specifying an indent and including separators with no spaces, we can
get a JSON representation with all the space squeezed out. You can add
spaces and still omit the indent/newlines and so on.
> +
> +class App(QMPClient):
> +def __init__(self, address):
> +urwid.register_signal(type(self), UPDATE_MSG)
>
I found some hidden documentation about this -- I was struggling to find it
before:
https://github.com/urwid/urwid/blob/master/urwid/signals.py#L62
So the first argument is meant to be "The class of an object sending the
signal". Alright. And it looks like we only emit from add_to_history and in
_send_to_server, so I guess that checks out!
One thing I am noticing is that these signals are global and not
per-instance, so if for some reason we ever create a second App I wonder if
the __init__ method here will explode ... Well, it's probably not too
important right now.
>
> +self.window = Window(self)
> +self.address = address
> +self.aloop = None
> +super().__init__()
> +
> +def add_to_history(self, msg):
> +urwid.emit_signal(self, UPDATE_MSG, msg)
> +
> +def _cb_outbound(self, msg):
> +# FIXME: I think the ideal way to omit these messages during
> in-TUI
> +# logging will be to add a filter to the logger. We can use regex
> to
> +# filter out messages starting with 'Request:' or 'Response:' but
> I
> +# think a better approach will be encapsulate the message in an
> object
> +# and filter based on the object. Encapsulation of the message
> will
> +# also be necessary when we want different formatting of messages
> +# inside TUI.
>
+handler = LOGGER.handlers[0]
> +if not isinstance(handler, TUILogHandler):
> +LOGGER.debug('Request: %s', str(msg))
> +self.add_to_history('<-- ' + str(msg))
> +return msg
> +
> +def _cb_inbound(self, msg):
> +handler = LOGGER.handlers[0]
> +if not
[RFC PATCH v4 3/4] Adding Andes AX25 and A25 CPU model
From: Ruinalnd ChuanTzu Tsai
Adding Andes AX25 and A25 CPU model into cpu.h and cpu.c without
enhanced features (yet).
Signed-off-by: Dylan Jhong
---
target/riscv/cpu.c | 16
target/riscv/cpu.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3a638b5..9eb1e3a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -182,6 +182,13 @@ static void rv64_base_cpu_init(Object *obj)
set_misa(env, RV64);
}
+static void ax25_cpu_init(Object *obj)
+{
+CPURISCVState *env = _CPU(obj)->env;
+set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
+set_priv_version(env, PRIV_VERSION_1_10_0);
+}
+
static void rv64_sifive_u_cpu_init(Object *obj)
{
CPURISCVState *env = _CPU(obj)->env;
@@ -235,6 +242,13 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
set_resetvec(env, DEFAULT_RSTVEC);
qdev_prop_set_bit(DEVICE(obj), "mmu", false);
}
+
+static void a25_cpu_init(Object *obj)
+{
+CPURISCVState *env = _CPU(obj)->env;
+set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
+set_priv_version(env, PRIV_VERSION_1_10_0);
+}
#endif
static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
@@ -726,8 +740,10 @@ static const TypeInfo riscv_cpu_type_infos[] = {
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31, rv32_sifive_e_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34, rv32_imafcu_nommu_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34, rv32_sifive_u_cpu_init),
+DEFINE_CPU(TYPE_RISCV_CPU_A25, a25_cpu_init),
#elif defined(TARGET_RISCV64)
DEFINE_CPU(TYPE_RISCV_CPU_BASE64, rv64_base_cpu_init),
+DEFINE_CPU(TYPE_RISCV_CPU_AX25, ax25_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51, rv64_sifive_e_cpu_init),
DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54, rv64_sifive_u_cpu_init),
#endif
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 52df9bb..bd79d63 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -37,6 +37,8 @@
#define TYPE_RISCV_CPU_ANY RISCV_CPU_TYPE_NAME("any")
#define TYPE_RISCV_CPU_BASE32 RISCV_CPU_TYPE_NAME("rv32")
#define TYPE_RISCV_CPU_BASE64 RISCV_CPU_TYPE_NAME("rv64")
+#define TYPE_RISCV_CPU_A25 RISCV_CPU_TYPE_NAME("andes-a25")
+#define TYPE_RISCV_CPU_AX25 RISCV_CPU_TYPE_NAME("andes-ax25")
#define TYPE_RISCV_CPU_IBEX RISCV_CPU_TYPE_NAME("lowrisc-ibex")
#define TYPE_RISCV_CPU_SIFIVE_E31 RISCV_CPU_TYPE_NAME("sifive-e31")
#define TYPE_RISCV_CPU_SIFIVE_E34 RISCV_CPU_TYPE_NAME("sifive-e34")
--
2.32.0
[RFC PATCH v4 0/4] Add basic support for custom CSR
From: Ruinland ChuanTzu Tsai
Dear all,
In this patch, the implementation of custom CSR handling logic is introduced.
If --enable-riscv-custom is set during configuration, custom CSR logic will be
turned on. During CPU model initialization, setup_custom_csr() is invoked to
register vendor-provided custom CSR opsets into a hash table.
When accessing a CSR, in riscv_csrrw(), is_custom_csr() will be called to check
whether the encountering csrno is a custom CSR. If that's a custom one, a
struct riscv_csr_operations will be returned and such CSR will be served
accordingly.
The performance slowdown could be easily tested with a simple program running
on linux-user mode.
/* test_csr.c */
#include
#include
#include
int main (int ac, char *av[]) {
struct timeval start;
struct timeval end;
gettimeofday(,NULL);
unsigned int loop_n = 99 ;
unsigned char i;
unsigned char o;
do {
for(i=0; i<32; i++) {
#if defined(FCSR)
__asm__("csrw fcsr, %0;"::"r"(i));
__asm__("csrr %0, fcsr;":"=r"(o));
#elif defined(UITB)
__asm__("csrw 0x800, %0;"::"r"(i));
__asm__("csrr %0, 0x800;":"=r"(o));
#endif
}
--loop_n;
} while (loop_n > 0);
gettimeofday(,NULL);
unsigned long diff = 100 *
(end.tv_sec-start.tv_sec)+end.tv_usec-start.tv_usec;
printf("%f\n", (double)(diff)/100);
return 0;
}
$ riscv64-linux-gnu-gcc -static -DUITB ./test_csr.c -o ./u
$ riscv64-linux-gnu-gcc -static -DFCSR ./test_csr.c -o ./f
For a custom CSR, uitb, being accessed on andes-ax25 :
$ ./build/qemu-riscv64 -cpu andes-ax25 ./u
4.283091
For a stock CSR, fcsr, being accessed on andes-ax25:
$ ./build/qemu-riscv64 ./f
3.875519
For a custom CSR being accessed on stock rv64:
$ ./build/qemu-riscv64 -cpu rv64 ./u
Illegal instruction (core dumped)
# This is expected to fail.
Currently, the statics on my hands shows that :
When the custom CSR handling mechanism is activated, we will suffer a 17% slow-
down on stock CSRs and the penalty of accessing to a custom CSR will be another
7% more.
Cordially yours,
Ruinland ChuanTzu Tsai
Changes from v3 :
* Adding options in configure and meson files to turn on/off custom CSR logic.
* Adding unlikely() to check if custom_csr_map is set.
* Moving any32 and any out of !(CONFIG_USER_ONLY) for enabling linux-user mode.
* Fix comment style, add missing license boilerplate.
Ruinalnd ChuanTzu Tsai (4):
Adding basic custom/vendor CSR handling mechanism
Adding Andes AX25 and A25 CPU model
Enable custom CSR logic for Andes AX25 and A25
Add options to config/meson files for custom CSR
configure | 6 ++
meson.build| 2 +
meson_options.txt | 2 +
target/riscv/andes_cpu_bits.h | 124 +
target/riscv/cpu.c | 51 +++
target/riscv/cpu.h | 33 ++-
target/riscv/cpu_bits.h| 4 +
target/riscv/csr.c | 83 ++---
target/riscv/csr_andes.inc.c | 160 +
target/riscv/custom_cpu_bits.h | 12 +++
10 files changed, 462 insertions(+), 15 deletions(-)
create mode 100644 target/riscv/andes_cpu_bits.h
create mode 100644 target/riscv/csr_andes.inc.c
create mode 100644 target/riscv/custom_cpu_bits.h
--
2.32.0
[RFC PATCH v4 2/4] Adding basic custom/vendor CSR handling mechanism
From: Ruinalnd ChuanTzu Tsai
For now we add a custom CSR handling mechanism to handle non-standard CSR read
or write.
The write_stub() and read_zero() are provided for quick placeholder usage if
such CSRs' behavior are expected to fail-over in its user code.
Signed-off-by: Dylan Jhong
---
target/riscv/cpu.c | 23 ++
target/riscv/cpu.h | 31 -
target/riscv/cpu_bits.h| 4 ++
target/riscv/csr.c | 83 --
target/riscv/custom_cpu_bits.h | 8
5 files changed, 134 insertions(+), 15 deletions(-)
create mode 100644 target/riscv/custom_cpu_bits.h
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7401325..3a638b5 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -144,6 +144,29 @@ static void set_resetvec(CPURISCVState *env, target_ulong
resetvec)
#endif
}
+#if defined(CONFIG_RISCV_CUSTOM)
+static void setup_custom_csr(CPURISCVState *env,
+ riscv_custom_csr_operations csr_map_struct[]
+ ) {
+
+env->custom_csr_map = g_hash_table_new_full(g_direct_hash, \
+g_direct_equal, \
+NULL, NULL);
+
+
+int i;
+for (i = 0; i < MAX_CUSTOM_CSR_NUM; i++) {
+if (csr_map_struct[i].csrno != 0) {
+g_hash_table_insert(env->custom_csr_map,
+GINT_TO_POINTER(csr_map_struct[i].csrno),
+_map_struct[i].csr_opset);
+} else {
+break;
+}
+}
+}
+#endif
+
static void riscv_any_cpu_init(Object *obj)
{
CPURISCVState *env = _CPU(obj)->env;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0edb282..52df9bb 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -239,6 +239,16 @@ struct CPURISCVState {
/* Fields from here on are preserved across CPU reset. */
QEMUTimer *timer; /* Internal timer */
+
+/*
+ * The reason why we have an opset map for custom CSRs and a seperated
+ * storage map is that we might have heterogeneous architecture, in which
+ * different harts have different custom CSRs.
+ * Custom CSR opset map
+ */
+GHashTable *custom_csr_map;
+/* Custom CSR val holder */
+void *custom_csr_val;
};
OBJECT_DECLARE_TYPE(RISCVCPU, RISCVCPUClass,
@@ -485,17 +495,36 @@ typedef struct {
riscv_csr_op_fn op;
} riscv_csr_operations;
+typedef struct {
+int csrno;
+riscv_csr_operations csr_opset;
+} riscv_custom_csr_operations;
+
+/*
+ * The reason why we have an abstraction here is that : We could have CSR
+ * number M on hart A is an alias of CSR number N on hart B. So we make a
+ * CSR number to value address map.
+ */
+typedef struct {
+int csrno;
+target_ulong val;
+} riscv_custom_csr_vals;
+
/* CSR function table constants */
enum {
-CSR_TABLE_SIZE = 0x1000
+CSR_TABLE_SIZE = 0x1000,
+MAX_CUSTOM_CSR_NUM = 100
};
/* CSR function table */
+extern int andes_custom_csr_size;
+extern riscv_custom_csr_operations andes_custom_csr_table[MAX_CUSTOM_CSR_NUM];
extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops);
void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops);
+
void riscv_cpu_register_gdb_regs_for_features(CPUState *cs);
#endif /* RISCV_CPU_H */
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index caf4599..de77242 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -593,3 +593,7 @@
#define MIE_SSIE (1 << IRQ_S_SOFT)
#define MIE_USIE (1 << IRQ_U_SOFT)
#endif
+
+#if defined(CONFIG_RISCV_CUSTOM)
+#include "custom_cpu_bits.h"
+#endif
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fd2e636..1c4dc94 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -137,7 +137,8 @@ static int ctr32(CPURISCVState *env, int csrno)
return ctr(env, csrno);
}
-#if !defined(CONFIG_USER_ONLY)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-function"
static int any(CPURISCVState *env, int csrno)
{
return 0;
@@ -152,6 +153,25 @@ static int any32(CPURISCVState *env, int csrno)
return any(env, csrno);
}
+#pragma GCC diagnostic pop
+
+/* Machine Information Registers */
+static int read_zero(CPURISCVState *env, int csrno, target_ulong *val)
+{
+return *val = 0;
+}
+
+/*
+ * XXX: This is just a write stub for developing custom CSR handler,
+ * if the behavior of writting such CSR is not presentable in QEMU and doesn't
+ * affect the functionality, just stub it.
+ */
+static int write_stub(CPURISCVState *env, int csrno, target_ulong val)
+{
+return 0;
+}
+
+#if !defined(CONFIG_USER_ONLY)
static int smode(CPURISCVState *env, int csrno)
{
@@ -435,11 +455,6 @@ static const char valid_vm_1_10_64[16] = {
[VM_1_10_SV57] = 1
};
-/* Machine Information
[RFC PATCH v4 4/4] Enable custom CSR logic for Andes AX25 and A25
From: Ruinalnd ChuanTzu Tsai In this patch we enabled custom CSR logic for Andes AX25 and A25 logic. Hence csr_andes.inc.c and andes_cpu_bits.h is added. Signed-off-by: Dylan Jhong --- target/riscv/andes_cpu_bits.h | 124 + target/riscv/cpu.c | 12 +++ target/riscv/csr.c | 2 +- target/riscv/csr_andes.inc.c | 160 + target/riscv/custom_cpu_bits.h | 6 +- 5 files changed, 302 insertions(+), 2 deletions(-) create mode 100644 target/riscv/andes_cpu_bits.h create mode 100644 target/riscv/csr_andes.inc.c diff --git a/target/riscv/andes_cpu_bits.h b/target/riscv/andes_cpu_bits.h new file mode 100644 index 000..bd2b7d1 --- /dev/null +++ b/target/riscv/andes_cpu_bits.h @@ -0,0 +1,124 @@ +/* + * Andes custom CSRs bit definitions + * + * Copyright (c) 2021 Andes Technology Corp. + * SPDX-License-Identifier: GPL-2.0+ + */ + +/* = Missing drafted/standard CSR definitions */ +/* Although TINFO is in official debug sepc, it's not in cpu_bits.h yet. */ +#define CSR_TINFO 0x7a4 + +/* = AndeStar V5 machine mode CSRs = */ +/* Configuration Registers */ +#define CSR_MICM_CFG0xfc0 +#define CSR_MDCM_CFG0xfc1 +#define CSR_MMSC_CFG0xfc2 +#define CSR_MMSC_CFG2 0xfc3 +#define CSR_MVEC_CFG0xfc7 + +/* Crash Debug CSRs */ +#define CSR_MCRASH_STATESAVE0xfc8 +#define CSR_MSTATUS_CRASHSAVE 0xfc9 + +/* Memory CSRs */ +#define CSR_MILMB 0x7c0 +#define CSR_MDLMB 0x7c1 +#define CSR_MECC_CODE 0x7C2 +#define CSR_MNVEC 0x7c3 +#define CSR_MCACHE_CTL 0x7ca +#define CSR_MCCTLBEGINADDR 0x7cb +#define CSR_MCCTLCOMMAND0x7cc +#define CSR_MCCTLDATA 0x7cd +#define CSR_MPPIB 0x7f0 +#define CSR_MFIOB 0x7f1 + +/* Hardware Stack Protection & Recording */ +#define CSR_MHSP_CTL0x7c6 +#define CSR_MSP_BOUND 0x7c7 +#define CSR_MSP_BASE0x7c8 +#define CSR_MXSTATUS0x7c4 +#define CSR_MDCAUSE 0x7c9 +#define CSR_MSLIDELEG 0x7d5 +#define CSR_MSAVESTATUS 0x7d6 +#define CSR_MSAVEEPC1 0x7d7 +#define CSR_MSAVECAUSE1 0x7d8 +#define CSR_MSAVEEPC2 0x7d9 +#define CSR_MSAVECAUSE2 0x7da +#define CSR_MSAVEDCAUSE10x7db +#define CSR_MSAVEDCAUSE20x7dc + +/* Control CSRs */ +#define CSR_MPFT_CTL0x7c5 +#define CSR_MMISC_CTL 0x7d0 +#define CSR_MCLK_CTL0x7df + +/* Counter related CSRs */ +#define CSR_MCOUNTERWEN 0x7ce +#define CSR_MCOUNTERINTEN 0x7cf +#define CSR_MCOUNTERMASK_M 0x7d1 +#define CSR_MCOUNTERMASK_S 0x7d2 +#define CSR_MCOUNTERMASK_U 0x7d3 +#define CSR_MCOUNTEROVF 0x7d4 + +/* Enhanced CLIC CSRs */ +#define CSR_MIRQ_ENTRY 0x7ec +#define CSR_MINTSEL_JAL 0x7ed +#define CSR_PUSHMCAUSE 0x7ee +#define CSR_PUSHMEPC0x7ef +#define CSR_PUSHMXSTATUS0x7eb + +/* Andes Physical Memory Attribute(PMA) CSRs */ +#define CSR_PMACFG0 0xbc0 +#define CSR_PMACFG1 0xbc1 +#define CSR_PMACFG2 0xbc2 +#define CSR_PMACFG3 0xbc3 +#define CSR_PMAADDR00xbd0 +#define CSR_PMAADDR10xbd1 +#define CSR_PMAADDR20xbd2 +#define CSR_PMAADDR30xbd2 +#define CSR_PMAADDR40xbd4 +#define CSR_PMAADDR50xbd5 +#define CSR_PMAADDR60xbd6 +#define CSR_PMAADDR70xbd7 +#define CSR_PMAADDR80xbd8 +#define CSR_PMAADDR90xbd9 +#define CSR_PMAADDR10 0xbda +#define CSR_PMAADDR11 0xbdb +#define CSR_PMAADDR12 0xbdc +#define CSR_PMAADDR13 0xbdd +#define CSR_PMAADDR14 0xbde +#define CSR_PMAADDR15 0xbdf + +/* = AndeStar V5 supervisor mode CSRs = */ +/* Supervisor trap registers */ +#define CSR_SLIE0x9c4 +#define CSR_SLIP0x9c5 +#define CSR_SDCAUSE 0x9c9 + +/* Supervisor counter registers */ +#define CSR_SCOUNTERINTEN 0x9cf +#define CSR_SCOUNTERMASK_M 0x9d1 +#define CSR_SCOUNTERMASK_S 0x9d2 +#define CSR_SCOUNTERMASK_U 0x9d3 +#define CSR_SCOUNTEROVF 0x9d4 +#define CSR_SCOUNTINHIBIT 0x9e0 +#define CSR_SHPMEVENT3 0x9e3 +#define CSR_SHPMEVENT4 0x9e4 +#define CSR_SHPMEVENT5 0x9e5 +#define CSR_SHPMEVENT6 0x9e6 + +/* Supervisor control registers */ +#define CSR_SCCTLDATA 0x9cd +#define CSR_SMISC_CTL 0x9d0 + +/* = AndeStar V5 user mode CSRs = */ +/* User mode control registers */ +#define CSR_UITB0x800 +#define CSR_UCODE 0x801 +#define CSR_UDCAUSE 0x809 +#define CSR_UCCTLBEGINADDR 0x80b +#define CSR_UCCTLCOMMAND0x80c +#define CSR_WFE 0x810 +#define
[RFC PATCH v4 1/4] Add options to config/meson files for custom CSR
From: Ruinland ChuanTzu Tsai
Adding option `riscv_custom` to configure script, meson.build and
meson_options.txt so as to toggle custom CSR and will-be-upstreamed custom
instructions handling logic.
Signed-off-by: Dylan Jhong
---
configure | 6 ++
meson.build | 2 ++
meson_options.txt | 2 ++
3 files changed, 10 insertions(+)
diff --git a/configure b/configure
index 34fccaa..88f6584 100755
--- a/configure
+++ b/configure
@@ -324,6 +324,7 @@ virtiofsd="auto"
virtfs="auto"
libudev="auto"
mpath="auto"
+riscv_custom="auto"
vnc="enabled"
sparse="auto"
vde="$default_feature"
@@ -1016,6 +1017,10 @@ for opt do
;;
--enable-vnc) vnc="enabled"
;;
+ --enable-riscv-custom) riscv_custom="enabled"
+ ;;
+ --disable-riscv-custom) riscv_custom="disabled"
+ ;;
--disable-gettext) gettext="disabled"
;;
--enable-gettext) gettext="enabled"
@@ -6416,6 +6421,7 @@ NINJA=$ninja $meson setup \
-Dcocoa=$cocoa -Dgtk=$gtk -Dmpath=$mpath -Dsdl=$sdl
-Dsdl_image=$sdl_image \
-Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg
-Dvnc_png=$vnc_png \
-Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f
-Dvirtiofsd=$virtiofsd \
+-Driscv_custom=$riscv_custom \
-Dcapstone=$capstone -Dslirp=$slirp -Dfdt=$fdt -Dbrlapi=$brlapi \
-Dcurl=$curl -Dglusterfs=$glusterfs -Dbzip2=$bzip2
-Dlibiscsi=$libiscsi \
-Dlibnfs=$libnfs -Diconv=$iconv -Dcurses=$curses -Dlibudev=$libudev\
diff --git a/meson.build b/meson.build
index adeec15..736810e 100644
--- a/meson.build
+++ b/meson.build
@@ -1151,6 +1151,7 @@ config_host_data.set('HAVE_SYS_KCOV_H',
cc.has_header('sys/kcov.h'))
config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix:
'#include '))
config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix:
'#include '))
+config_host_data.set('CONFIG_RISCV_CUSTOM',
get_option('riscv_custom').enabled())
ignored = ['CONFIG_QEMU_INTERP_PREFIX'] # actually per-target
arrays = ['CONFIG_AUDIO_DRIVERS', 'CONFIG_BDRV_RW_WHITELIST',
'CONFIG_BDRV_RO_WHITELIST']
@@ -2694,6 +2695,7 @@ summary_info += {'libpmem support':
config_host.has_key('CONFIG_LIBPMEM')}
summary_info += {'libdaxctl support': config_host.has_key('CONFIG_LIBDAXCTL')}
summary_info += {'libudev': libudev.found()}
summary_info += {'FUSE lseek':fuse_lseek.found()}
+summary_info += {'RISC-V custom CSRs/instructions':
get_option('riscv_custom').enabled()}
summary(summary_info, bool_yn: true, section: 'Dependencies')
if not supported_cpus.contains(cpu)
diff --git a/meson_options.txt b/meson_options.txt
index 9734019..470ef23 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -125,3 +125,5 @@ option('slirp', type: 'combo', value: 'auto',
option('fdt', type: 'combo', value: 'auto',
choices: ['disabled', 'enabled', 'auto', 'system', 'internal'],
description: 'Whether and how to find the libfdt library')
+option('riscv_custom', type: 'feature', value: 'auto',
+ description: 'RISC-V custom')
--
2.32.0
Re: [PULL 0/8] chardev fixes for 6.1
On Thu, 5 Aug 2021 at 13:53, wrote: > > From: Marc-André Lureau > > The following changes since commit bccabb3a5d60182645c7749e89f21a9ff307a9eb: > > Update version for v6.1.0-rc2 release (2021-08-04 16:56:14 +0100) > > are available in the Git repository at: > > g...@gitlab.com:marcandre.lureau/qemu.git tags/chr-fix-pull-request > > for you to fetch changes up to a68403b0a6843f106e381b0bbeaacb29f6d27255: > > chardev: report a simpler error about duplicated id (2021-08-05 16:15:33 > +0400) > > > Chardev-related fixes > > Hi > > Here are some bug fixes worthy for 6.1. > > thanks Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1 for any user-visible changes. -- PMM
Re: [PATCH v3 03/13] python: Add dependencies for AQMP TUI
On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu wrote: > Added dependencies for the upcoming AQMP TUI under the optional > 'tui' group. > > The same dependencies have also been added under the devel group > since no work around has been found for optional groups to imply > other optional groups. > > Signed-off-by: G S Niteesh Babu > Reviewed-by: John Snow
Re: intermittent failure, s390 host, x86-64 guest, vhost-user-blk test
On Wed, 4 Aug 2021 at 15:33, Peter Maydell wrote: > > On Wed, 4 Aug 2021 at 14:43, Michael S. Tsirkin wrote: > > > > On Wed, Aug 04, 2021 at 01:40:37PM +0100, Peter Maydell wrote: > > > Saw this intermittent as part of my ad-hoc CI test setup. > > > The backtrace says that the QEMU process has somehow hung > > > during 'realize' of the vhost-user device... > > > > Hmm seems to have to do with the daemon not responding ... > > Cc a bunch more people ... > > Here's another one; same thing, but a ppc64 guest, I think. Ran into this again; I'll spare you the backtraces but it was the same 'hang in realize', this one for qemu-system-i386. Same s390 host. This seems to be a fairly frequent intermittent -- can somebody have a look at it ? -- PMM
Re: [PATCH v3 02/13] python: disable pylint errors for aqmp-tui
On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu wrote: > Disable missing-docstring and fixme pylint warnings. > This is because since the AQMP is just a prototype > it is currently not documented properly and lot > of todo and fixme's are still in place. > > Signed-off-by: G S Niteesh Babu > --- > python/setup.cfg | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/python/setup.cfg b/python/setup.cfg > index 2573cd7bfb..7a30dd5b09 100644 > --- a/python/setup.cfg > +++ b/python/setup.cfg > @@ -90,6 +90,8 @@ ignore_missing_imports = True > # --disable=W". > disable=too-many-function-args, # mypy handles this with less false > positives. > no-member, # mypy also handles this better. > +missing-docstring, # FIXME > +fixme, # FIXME > > Please put some attention into removing the missing-docstring flag. At this point, anything that is "FIXME" should either be fixed or re-worded as a "TODO" and an exemption added to the pylint configuration such that "TODO" is allowed but "FIXME" is not. Take a look at pylint --generate-rcfile and find this section: [MISCELLANEOUS] # List of note tags to take in consideration, separated by a comma. notes=FIXME, XXX, TODO > [pylint.basic] > # Good variable names which should always be accepted, separated by a > comma. > -- > 2.17.1 > >
Re: [PATCH v7 00/10] qapi: untie 'if' conditions from C preprocessor
marcandre.lur...@redhat.com writes: > From: Marc-André Lureau > > Hi, > > This series makes the 'if' conditions less liberal, by formalizing a simple > expression tree based on bare boolean logic of configure option identifiers. > > (this allows to express conditions in Rust in my QAPI-Rust PoC series) > > thanks Only a few trivial things left to correct or improve. I'll take care of it in my tree. Series Reviewed-by: Markus Armbruster Thanks!
Re: [PATCH v7 05/10] qapidoc: introduce QAPISchemaIfCond.docgen()
Marc-André Lureau writes: > Hi > > On Thu, Aug 5, 2021 at 3:55 PM Markus Armbruster wrote: > >> marcandre.lur...@redhat.com writes: >> >> > From: Marc-André Lureau >> > >> > Instead of building the condition documentation from a list of string, >> > use the result generated from QAPISchemaIfCond.docgen(). >> > >> > This changes the generated documentation from: >> > - COND1, COND2... (where COND1, COND2 are Literal nodes, and ',' is Text) >> > to: >> > - COND1 and COND2 (the whole string as a Literal node) >> > >> > This will allow us to generate more complex conditions in the following >> > patches, such as "(COND1 and COND2) or COND3". >> > >> > Adding back the differentiated formatting is left to the wish list. >> >> What about a TODO comment? you suggest a suitable spot? >> > > I don't think this matters much, it will never be a user-friendly text. But > we can leave a comment in the docgen() function to say that the sphinx > build could benefit from a formatted string. Function docgen_ifcond(), I presume. Method docgen() is a simple wrapper. Something like # TODO Doc generated for conditions needs polish
