[PATCH] pcie_sriov: Remove g_new assertion
g_new() aborts if the allocation fails so it returns NULL only if the
requested allocation size is zero. register_vfs() makes such an
allocation if NumVFs is zero so it should not assert that g_new()
returns a non-NULL value.
Fixes: 7c0fa8dff8 ("pcie: Add support for Single Root I/O Virtualization
(SR/IOV)")
Buglink: https://issues.redhat.com/browse/RHEL-17209
Signed-off-by: Akihiko Odaki
---
hw/pci/pcie_sriov.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 5ef8950940..a1fe65f5d8 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -178,7 +178,6 @@ static void register_vfs(PCIDevice *dev)
num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
-assert(dev->exp.sriov_pf.vf);
trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
PCI_FUNC(dev->devfn), num_vfs);
--
2.42.1
RE: [PATCH] hw/ppc: Improve build for PPC VFIO
>-Original Message-
>From: Cédric Le Goater
>Sent: Thursday, November 23, 2023 3:33 PM
>Subject: Re: [PATCH] hw/ppc: Improve build for PPC VFIO
>
>On 11/23/23 07:01, Zhenzhong Duan wrote:
>> VFIO is not a required subsystem for the pseries machine but it's
>> force enabled currently. When --without-default-devices is used
>> to drop some default devices including vfio-pci, vfio core code
>> is still kept which is unnecessary.
>>
>> Introduce a stub file to hold stub functions of VFIO EEH hooks,
>> then vfio core could be compiled out.
>>
>> Suggested-by: Cédric Le Goater
>> Signed-off-by: Zhenzhong Duan
>
>
>Nick,
>
>I will take this patch through the vfio tree if that's OK for you.
Sure.
>
>> ---
>> Based on vfio-next/vfio-8.2
>>
>> hw/ppc/spapr_pci_vfio_stub.c | 33 +
>> hw/ppc/Kconfig | 2 +-
>> hw/ppc/meson.build | 6 +++---
>> 3 files changed, 37 insertions(+), 4 deletions(-)
>> create mode 100644 hw/ppc/spapr_pci_vfio_stub.c
>>
>> diff --git a/hw/ppc/spapr_pci_vfio_stub.c b/hw/ppc/spapr_pci_vfio_stub.c
>> new file mode 100644
>> index 00..adb3fb5e35
>> --- /dev/null
>> +++ b/hw/ppc/spapr_pci_vfio_stub.c
>> @@ -0,0 +1,33 @@
>> +#include "qemu/osdep.h"
>> +#include "hw/pci-host/spapr.h"
>> +
>> +bool spapr_phb_eeh_available(SpaprPhbState *sphb)
>> +{
>> +return false;
>> +}
>> +
>> +void spapr_phb_vfio_reset(DeviceState *qdev)
>> +{
>> +}
>> +
>> +int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
>> + unsigned int addr, int option)
>> +{
>> +return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> +int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
>> +{
>> +return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> +int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
>> +{
>> +return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> +int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
>> +{
>> +return RTAS_OUT_NOT_SUPPORTED;
>> +}
>> +
>> diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
>> index edc6d2d139..b8dabdbfbe 100644
>> --- a/hw/ppc/Kconfig
>> +++ b/hw/ppc/Kconfig
>> @@ -3,11 +3,11 @@ config PSERIES
>> imply PCI_DEVICES
>> imply TEST_DEVICES
>> imply VIRTIO_VGA
>> +imply VFIO if LINUX # needed by spapr_pci_vfio.c
>
>Zhenzhong,
>
>I changed VFIO to VFIO_PCI because PPC only supports this type
>of passthrough devices.
Oh, I see, thanks
BRs.
Zhenzhong
>
>With that,
>
>Reviewed-by: Cédric Le Goater
>
>Thanks,
>
>C.
>
>
>
>> select NVDIMM
>> select DIMM
>> select PCI
>> select SPAPR_VSCSI
>> -select VFIO_PCI if LINUX # needed by spapr_pci_vfio.c
>> select XICS
>> select XIVE
>> select MSI_NONBROKEN
>> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
>> index ea44856d43..2df5db2eef 100644
>> --- a/hw/ppc/meson.build
>> +++ b/hw/ppc/meson.build
>> @@ -34,9 +34,9 @@ ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'],
>if_true: files(
>> 'spapr_softmmu.c',
>> ))
>> ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
>> -ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
>> - 'spapr_pci_vfio.c',
>> -))
>> +ppc_ss.add(when: [ 'CONFIG_VFIO_PCI', 'CONFIG_PSERIES', 'CONFIG_LINUX'],
>> + if_true: files('spapr_pci_vfio.c'),
>> + if_false: files('spapr_pci_vfio_stub.c'))
>>
>> # IBM PowerNV
>> ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
Re: [PATCH] hw/ppc: Improve build for PPC VFIO
On 11/23/23 07:01, Zhenzhong Duan wrote:
VFIO is not a required subsystem for the pseries machine but it's
force enabled currently. When --without-default-devices is used
to drop some default devices including vfio-pci, vfio core code
is still kept which is unnecessary.
Introduce a stub file to hold stub functions of VFIO EEH hooks,
then vfio core could be compiled out.
Suggested-by: Cédric Le Goater
Signed-off-by: Zhenzhong Duan
Nick,
I will take this patch through the vfio tree if that's OK for you.
---
Based on vfio-next/vfio-8.2
hw/ppc/spapr_pci_vfio_stub.c | 33 +
hw/ppc/Kconfig | 2 +-
hw/ppc/meson.build | 6 +++---
3 files changed, 37 insertions(+), 4 deletions(-)
create mode 100644 hw/ppc/spapr_pci_vfio_stub.c
diff --git a/hw/ppc/spapr_pci_vfio_stub.c b/hw/ppc/spapr_pci_vfio_stub.c
new file mode 100644
index 00..adb3fb5e35
--- /dev/null
+++ b/hw/ppc/spapr_pci_vfio_stub.c
@@ -0,0 +1,33 @@
+#include "qemu/osdep.h"
+#include "hw/pci-host/spapr.h"
+
+bool spapr_phb_eeh_available(SpaprPhbState *sphb)
+{
+return false;
+}
+
+void spapr_phb_vfio_reset(DeviceState *qdev)
+{
+}
+
+int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
+ unsigned int addr, int option)
+{
+return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
+{
+return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
+{
+return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
+{
+return RTAS_OUT_NOT_SUPPORTED;
+}
+
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index edc6d2d139..b8dabdbfbe 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -3,11 +3,11 @@ config PSERIES
imply PCI_DEVICES
imply TEST_DEVICES
imply VIRTIO_VGA
+imply VFIO if LINUX # needed by spapr_pci_vfio.c
Zhenzhong,
I changed VFIO to VFIO_PCI because PPC only supports this type
of passthrough devices.
With that,
Reviewed-by: Cédric Le Goater
Thanks,
C.
select NVDIMM
select DIMM
select PCI
select SPAPR_VSCSI
-select VFIO_PCI if LINUX # needed by spapr_pci_vfio.c
select XICS
select XIVE
select MSI_NONBROKEN
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index ea44856d43..2df5db2eef 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -34,9 +34,9 @@ ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], if_true:
files(
'spapr_softmmu.c',
))
ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
-ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
- 'spapr_pci_vfio.c',
-))
+ppc_ss.add(when: [ 'CONFIG_VFIO_PCI', 'CONFIG_PSERIES', 'CONFIG_LINUX'],
+ if_true: files('spapr_pci_vfio.c'),
+ if_false: files('spapr_pci_vfio_stub.c'))
# IBM PowerNV
ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
Re: [PATCH v6 0/5] gdbstub and TCG plugin improvements
On 2023/11/23 12:10, Alistair Francis wrote:
On Mon, Oct 30, 2023 at 3:49 PM Akihiko Odaki wrote:
Based-on: <20231029145033.592566-1-alex.ben...@linaro.org>
("[PATCH v2 00/19] Maintainer updates for testing, gdb, semihosting and
plugins (pre-PR)")
This series extracts fixes and refactorings that can be applied
independently from "[PATCH v9 00/23] plugins: Allow to read registers".
The patch "target/riscv: Move MISA limits to class" was replaced with
patch "target/riscv: Move misa_mxl_max to class" since I found instances
may have different misa_ext_mask.
What instances? Couldn't MXL have the same differences? >
Alistair
Instances of subclasses of riscv-cpu. While misa_ext_mask are
configurable for each instances with properties, the misa_mxl_max value
is hardcoded for each classes and will not have such differences.
Regards,
Akihiko Odaki
Re: [PATCH v6 1/5] hw/riscv: Use misa_mxl instead of misa_mxl_max
On 2023/11/23 12:04, Alistair Francis wrote: On Mon, Oct 30, 2023 at 3:50 PM Akihiko Odaki wrote: The effective MXL value matters when booting. This doesn't sound right. Surely the max is what matters here Also, this was specifically changed to misa_mxl_max in db23e5d981a "target/riscv: Replace riscv_cpu_is_32bit with riscv_cpu_mxl". This needs a much better description of why this change should be made > > Alistair The kernel will be executed with the current MXL rather than the initial MXL value so the current MXL should be used here. For example, if you are going to emulate a system that has a RV64 CPU and a firmware that sets the MXL to RV32, then mxl_max should be MXL_RV64 and mxl should be MXL_RV32, and the kernel should be assumed as a RV32 binary. Loading a 64-bit kernel will not work in such a case. You can find a similar example in x86_64: x86_64 systems typically starts in 16-bit mode, and the firmware switches to 64-bit mode. When emulating those systems, QEMU switches to 64-bit mode and loads a 64-bit kernel. Regards, Akihiko Odaki
Re: [PATCH-for-9.0] hw/mips/cps: Simplify access to 'start-powered-off' property
Philippe Mathieu-Daudé writes:
> Since commit c1b701587e ("target/arm: Move start-powered-off
> property to generic CPUState"), all target CPUs have the
> 'start-powered-off' property.
>
> This object_property_set_bool() call can not fail. Use _abort
> to simplify.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/mips/cps.c | 7 +++
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/hw/mips/cps.c b/hw/mips/cps.c
> index b6612c1762..4f12e23ab5 100644
> --- a/hw/mips/cps.c
> +++ b/hw/mips/cps.c
> @@ -78,10 +78,9 @@ static void mips_cps_realize(DeviceState *dev, Error
> **errp)
> CPUMIPSState *env = >env;
>
> /* All VPs are halted on reset. Leave powering up to CPC. */
> -if (!object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
> - errp)) {
> -return;
> -}
> +object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
> + _abort);
> +
> /* All cores use the same clock tree */
> qdev_connect_clock_in(DEVICE(cpu), "clk-in", s->clock);
There are more:
$ git-grep -A 1 'object_prop.*start-powered-off'
hw/arm/armsse.c:1025:if (!object_property_set_bool(cpuobj,
"start-powered-off", true,
hw/arm/armsse.c-1026- errp)) {
--
hw/arm/armv7m.c:321:if (object_property_find(OBJECT(s->cpu),
"start-powered-off")) {
hw/arm/armv7m.c:322:if (!object_property_set_bool(OBJECT(s->cpu),
"start-powered-off",
hw/arm/armv7m.c-323-
s->start_powered_off, errp)) {
--
hw/arm/boot.c:1290:object_property_set_bool(cpuobj,
"start-powered-off", true,
hw/arm/boot.c-1291- _abort);
--
hw/arm/fsl-imx6.c:131:
object_property_set_bool(OBJECT(>cpu[i]), "start-powered-off",
hw/arm/fsl-imx6.c-132- true,
_abort);
--
hw/arm/fsl-imx7.c:195:object_property_set_bool(o,
"start-powered-off", true,
hw/arm/fsl-imx7.c-196- _abort);
--
hw/arm/xlnx-versal.c:51:object_property_set_bool(obj,
"start-powered-off", true,
hw/arm/xlnx-versal.c-52- _abort);
--
hw/arm/xlnx-versal.c:153:object_property_set_bool(obj,
"start-powered-off", true,
hw/arm/xlnx-versal.c-154- _abort);
--
hw/mips/cps.c:81:if (!object_property_set_bool(OBJECT(cpu),
"start-powered-off", true,
hw/mips/cps.c-82- errp)) {
--
hw/ppc/e500.c:957:object_property_set_bool(OBJECT(cs),
"start-powered-off", i != 0,
hw/ppc/e500.c-958- _fatal);
--
hw/sparc/sun4m.c:806:object_property_set_bool(OBJECT(cpu),
"start-powered-off", id != 0,
hw/sparc/sun4m.c-807- _fatal);
We also set the property with qdev_prop_set_bit() in places, which is a
trivial wrapper around object_property_set_bool() that passes
_abort. Either is fine, I think.
Re: [PATCH for-8.2] ui/vnc-clipboard: fix inflate_buffer
Hi
On Wed, Nov 22, 2023 at 5:25 PM Fiona Ebner wrote:
>
> Am 22.11.23 um 14:06 schrieb Marc-André Lureau:
> > Hi
> >
> > On Wed, Nov 22, 2023 at 5:00 PM Fiona Ebner wrote:
> >>
> >> Commit d921fea338 ("ui/vnc-clipboard: fix infinite loop in
> >> inflate_buffer (CVE-2023-3255)") removed this hunk, but it is still
> >> required, because it can happen that stream.avail_in becomes zero
> >> before coming across a return value of Z_STREAM_END in the loop.
> >
> > Isn't this an error from the client side then?
> >
>
> In my test just now I get Z_BUF_ERROR twice and after the second one,
> stream.avail_in is zero. Maybe if you'd call inflate() again, you'd get
> Z_STREAM_END, but no such call is made, because we exit the loop.
It should exit the loop after calling inflate() again though.
Or do you mean that it goes to Z_BUF_ERROR a second time with
stream.avail_in == 0, thus exit the loop quickly after ?
That could mean that the input buffer is not complete.
"Note that Z_BUF_ERROR is not fatal, and inflate() can be called again
with more input..."
Something is fishy.. Is it easy to reproduce?
> Would it be better/more correct to ensure that inflate is called again
> in such a scenario?
>
> Best Regards,
> Fiona
>
--
Marc-André Lureau
[PATCH] hw/ppc: Improve build for PPC VFIO
VFIO is not a required subsystem for the pseries machine but it's
force enabled currently. When --without-default-devices is used
to drop some default devices including vfio-pci, vfio core code
is still kept which is unnecessary.
Introduce a stub file to hold stub functions of VFIO EEH hooks,
then vfio core could be compiled out.
Suggested-by: Cédric Le Goater
Signed-off-by: Zhenzhong Duan
---
Based on vfio-next/vfio-8.2
hw/ppc/spapr_pci_vfio_stub.c | 33 +
hw/ppc/Kconfig | 2 +-
hw/ppc/meson.build | 6 +++---
3 files changed, 37 insertions(+), 4 deletions(-)
create mode 100644 hw/ppc/spapr_pci_vfio_stub.c
diff --git a/hw/ppc/spapr_pci_vfio_stub.c b/hw/ppc/spapr_pci_vfio_stub.c
new file mode 100644
index 00..adb3fb5e35
--- /dev/null
+++ b/hw/ppc/spapr_pci_vfio_stub.c
@@ -0,0 +1,33 @@
+#include "qemu/osdep.h"
+#include "hw/pci-host/spapr.h"
+
+bool spapr_phb_eeh_available(SpaprPhbState *sphb)
+{
+return false;
+}
+
+void spapr_phb_vfio_reset(DeviceState *qdev)
+{
+}
+
+int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
+ unsigned int addr, int option)
+{
+return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
+{
+return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
+{
+return RTAS_OUT_NOT_SUPPORTED;
+}
+
+int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
+{
+return RTAS_OUT_NOT_SUPPORTED;
+}
+
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index edc6d2d139..b8dabdbfbe 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -3,11 +3,11 @@ config PSERIES
imply PCI_DEVICES
imply TEST_DEVICES
imply VIRTIO_VGA
+imply VFIO if LINUX # needed by spapr_pci_vfio.c
select NVDIMM
select DIMM
select PCI
select SPAPR_VSCSI
-select VFIO_PCI if LINUX # needed by spapr_pci_vfio.c
select XICS
select XIVE
select MSI_NONBROKEN
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index ea44856d43..2df5db2eef 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -34,9 +34,9 @@ ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_TCG'], if_true:
files(
'spapr_softmmu.c',
))
ppc_ss.add(when: 'CONFIG_SPAPR_RNG', if_true: files('spapr_rng.c'))
-ppc_ss.add(when: ['CONFIG_PSERIES', 'CONFIG_LINUX'], if_true: files(
- 'spapr_pci_vfio.c',
-))
+ppc_ss.add(when: [ 'CONFIG_VFIO_PCI', 'CONFIG_PSERIES', 'CONFIG_LINUX'],
+ if_true: files('spapr_pci_vfio.c'),
+ if_false: files('spapr_pci_vfio_stub.c'))
# IBM PowerNV
ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
--
2.34.1
[PATCH v3 1/2] ppc/spapr: Introduce SPAPR_IRQ_NR_IPIS to refer IRQ range for CPU IPIs.
spapr_irq_init currently uses existing macro SPAPR_XIRQ_BASE to refer to
the range of CPU IPIs during initialization of nr-irqs property.
It is more appropriate to have its own define which can be further
reused as appropriate for correct interpretation.
Signed-off-by: Harsh Prateek Bora
Suggested-by: Cedric Le Goater
---
include/hw/ppc/spapr_irq.h | 14 +-
hw/ppc/spapr_irq.c | 6 --
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index c22a72c9e2..4fd2d5853d 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -14,9 +14,21 @@
#include "qom/object.h"
/*
- * IRQ range offsets per device type
+ * The XIVE IRQ backend uses the same layout as the XICS backend but
+ * covers the full range of the IRQ number space. The IRQ numbers for
+ * the CPU IPIs are allocated at the bottom of this space, below 4K,
+ * to preserve compatibility with XICS which does not use that range.
+ */
+
+/*
+ * CPU IPI range (XIVE only)
*/
#define SPAPR_IRQ_IPI0x0
+#define SPAPR_IRQ_NR_IPIS0x1000
+
+/*
+ * IRQ range offsets per device type
+ */
#define SPAPR_XIRQ_BASE XICS_IRQ_BASE /* 0x1000 */
#define SPAPR_IRQ_EPOW (SPAPR_XIRQ_BASE + 0x)
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index a0d1e1298e..97b2fc42ab 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -23,6 +23,8 @@
#include "trace.h"
+QEMU_BUILD_BUG_ON(SPAPR_IRQ_NR_IPIS > SPAPR_XIRQ_BASE);
+
static const TypeInfo spapr_intc_info = {
.name = TYPE_SPAPR_INTC,
.parent = TYPE_INTERFACE,
@@ -329,7 +331,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
int i;
dev = qdev_new(TYPE_SPAPR_XIVE);
-qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_XIRQ_BASE);
+qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs +
SPAPR_IRQ_NR_IPIS);
/*
* 8 XIVE END structures per CPU. One for each available
* priority
@@ -356,7 +358,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
}
spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr,
- smc->nr_xirqs + SPAPR_XIRQ_BASE);
+ smc->nr_xirqs + SPAPR_IRQ_NR_IPIS);
/*
* Mostly we don't actually need this until reset, except that not
--
2.39.3
[PATCH v3 2/2] ppc/spapr: Initialize max_cpus limit to SPAPR_IRQ_NR_IPIS.
Initialize the machine specific max_cpus limit as per the maximum range of CPU IPIs available. Keeping between 4096 to 8192 will throw IRQ not free error due to XIVE/XICS limitation and keeping beyond 8192 will hit assert in tcg_region_init or spapr_xive_claim_irq. Logs: Without patch fix: [root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=4097 qemu-system-ppc64: IRQ 4096 is not free [root@host build]# On LPAR: [root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=8193 ** ERROR:../tcg/region.c:774:tcg_region_init: assertion failed: (region_size >= 2 * page_size) Bail out! ERROR:../tcg/region.c:774:tcg_region_init: assertion failed: (region_size >= 2 * page_size) Aborted (core dumped) [root@host build]# On x86: [root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=8193 qemu-system-ppc64: ../hw/intc/spapr_xive.c:596: spapr_xive_claim_irq: Assertion `lisn < xive->nr_irqs' failed. Aborted (core dumped) [root@host build]# With patch fix: [root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=4097 qemu-system-ppc64: Invalid SMP CPUs 4097. The max CPUs supported by machine 'pseries-8.2' is 4096 [root@host build]# Signed-off-by: Harsh Prateek Bora --- hw/ppc/spapr.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index df09aa9d6a..222d926f46 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -4647,13 +4647,10 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) mc->block_default_type = IF_SCSI; /* - * Setting max_cpus to INT32_MAX. Both KVM and TCG max_cpus values - * should be limited by the host capability instead of hardcoded. - * max_cpus for KVM guests will be checked in kvm_init(), and TCG - * guests are welcome to have as many CPUs as the host are capable - * of emulate. + * While KVM determines max cpus in kvm_init() using kvm_max_vcpus(), + * In TCG the limit is restricted by the range of CPU IPIs available. */ -mc->max_cpus = INT32_MAX; +mc->max_cpus = SPAPR_IRQ_NR_IPIS; mc->no_parallel = 1; mc->default_boot_order = ""; -- 2.39.3
[PATCH v3 0/2] Introduce SPAPR_IRQ_NR_IPIS and fix max-cpus
On spapr, the max number of CPU IPIs are 4096 which is accounted during spapr_irq_init but currently existing macro SPAPR_XIRQ_BASE is being used to refer to that. Introducing SPAPR_IRQ_NR_IPIS to refer to the range of CPU IPIS which is being further used to initialize mc->max_cpus during spapr_machine_class_init(). v3: - addressed further review comments from Cedric v2: - addressed review comments from Cedric Harsh Prateek Bora (2): ppc/spapr: Introduce SPAPR_IRQ_NR_IPIS to refer IRQ range for CPU IPIs. ppc/spapr: Initialize max_cpus limit to SPAPR_IRQ_NR_IPIS. include/hw/ppc/spapr_irq.h | 14 +- hw/ppc/spapr.c | 9 +++-- hw/ppc/spapr_irq.c | 6 -- 3 files changed, 20 insertions(+), 9 deletions(-) -- 2.39.3
[PATCH 2/2] vhost-user-scsi: free the inflight area when reset
Keep it the same to vhost-user-blk.
At the same time, fix the vhost_reset_device.
Signed-off-by: Li Feng
---
hw/scsi/vhost-user-scsi.c | 16
hw/virtio/virtio.c| 2 +-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 2060f9f94b..780f10559d 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -360,6 +360,20 @@ static Property vhost_user_scsi_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void vhost_user_scsi_reset(VirtIODevice *vdev)
+{
+VHostUserSCSI *s = VHOST_USER_SCSI(vdev);
+VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
+
+vhost_dev_free_inflight(vsc->inflight);
+}
+
+static struct vhost_dev *vhost_user_scsi_get_vhost(VirtIODevice *vdev)
+{
+VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev);
+return >dev;
+}
+
static const VMStateDescription vmstate_vhost_scsi = {
.name = "virtio-scsi",
.minimum_version_id = 1,
@@ -385,6 +399,8 @@ static void vhost_user_scsi_class_init(ObjectClass *klass,
void *data)
vdc->set_config = vhost_scsi_common_set_config;
vdc->set_status = vhost_user_scsi_set_status;
fwc->get_dev_path = vhost_scsi_common_get_fw_dev_path;
+vdc->reset = vhost_user_scsi_reset;
+vdc->get_vhost = vhost_user_scsi_get_vhost;
}
static void vhost_user_scsi_instance_init(Object *obj)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 4259fefeb6..d0a640af63 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2137,7 +2137,7 @@ void virtio_reset(void *opaque)
vdev->device_endian = virtio_default_endian();
}
-if (vdev->vhost_started) {
+if (vdev->vhost_started && k->get_vhost) {
vhost_reset_device(k->get_vhost(vdev));
}
--
2.42.0
[PATCH 0/2] fix some vhost-user issues
Li Feng (2): vhost-user: fix the reconnect error vhost-user-scsi: free the inflight area when reset hw/block/vhost-user-blk.c | 8 +++- hw/scsi/vhost-user-scsi.c | 19 ++- hw/virtio/vhost-user-gpio.c | 3 ++- hw/virtio/virtio.c | 2 +- 4 files changed, 24 insertions(+), 8 deletions(-) -- 2.42.0
[PATCH 1/2] vhost-user: fix the reconnect error
If the error occurs in vhost_dev_init, the value of s->connected is set to true
in advance, and there is no chance to enter this function execution again
in the future.
Signed-off-by: Li Feng
---
hw/block/vhost-user-blk.c | 8 +++-
hw/scsi/vhost-user-scsi.c | 3 ++-
hw/virtio/vhost-user-gpio.c | 3 ++-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 818b833108..2863d80d15 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -326,7 +326,6 @@ static int vhost_user_blk_connect(DeviceState *dev, Error
**errp)
if (s->connected) {
return 0;
}
-s->connected = true;
s->dev.num_queues = s->num_queues;
s->dev.nvqs = s->num_queues;
@@ -343,15 +342,14 @@ static int vhost_user_blk_connect(DeviceState *dev, Error
**errp)
return ret;
}
+s->connected = true;
+
/* restore vhost state */
if (virtio_device_started(vdev, vdev->status)) {
ret = vhost_user_blk_start(vdev, errp);
-if (ret < 0) {
-return ret;
-}
}
-return 0;
+return ret;
}
static void vhost_user_blk_disconnect(DeviceState *dev)
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 4486500cac..2060f9f94b 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -147,7 +147,6 @@ static int vhost_user_scsi_connect(DeviceState *dev, Error
**errp)
if (s->connected) {
return 0;
}
-s->connected = true;
vsc->dev.num_queues = vs->conf.num_queues;
vsc->dev.nvqs = VIRTIO_SCSI_VQ_NUM_FIXED + vs->conf.num_queues;
@@ -161,6 +160,8 @@ static int vhost_user_scsi_connect(DeviceState *dev, Error
**errp)
return ret;
}
+s->connected = true;
+
/* restore vhost state */
if (virtio_device_started(vdev, vdev->status)) {
ret = vhost_user_scsi_start(s, errp);
diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
index aff2d7eff6..a83437a5da 100644
--- a/hw/virtio/vhost-user-gpio.c
+++ b/hw/virtio/vhost-user-gpio.c
@@ -229,7 +229,6 @@ static int vu_gpio_connect(DeviceState *dev, Error **errp)
if (gpio->connected) {
return 0;
}
-gpio->connected = true;
vhost_dev_set_config_notifier(vhost_dev, _ops);
gpio->vhost_user.supports_config = true;
@@ -243,6 +242,8 @@ static int vu_gpio_connect(DeviceState *dev, Error **errp)
return ret;
}
+gpio->connected = true;
+
/* restore vhost state */
if (virtio_device_started(vdev, vdev->status)) {
vu_gpio_start(vdev);
--
2.42.0
Re: [PATCH v2 1/2] ppc/spapr: Introduce SPAPR_NR_IPIS to refer IRQ range for CPU IPIs.
On 11/22/23 17:01, Cédric Le Goater wrote: Hello Harsh, Please add to your .git/config file: [diff] orderFile = /path/to/qemu/scripts/git.orderfile Sure, thanks for the suggestion. On 11/22/23 10:28, Harsh Prateek Bora wrote: spapr_irq_init currently uses existing macro SPAPR_XIRQ_BASE to refer to the range of CPU IPIs during initialization of nr-irqs property. It is more appropriate to have its own define which can be further reused as appropriate for correct interpretation. Signed-off-by: Harsh Prateek Bora Suggested-by: Cedric Le Goater --- hw/ppc/spapr_irq.c | 4 ++-- include/hw/ppc/spapr_irq.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index a0d1e1298e..0c5db6b161 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -329,7 +329,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) int i; dev = qdev_new(TYPE_SPAPR_XIVE); - qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_XIRQ_BASE); + qdev_prop_set_uint32(dev, "nr-irqs", smc->nr_xirqs + SPAPR_NR_IPIS); SPAPR_IRQ_NR_IPIS ? /* * 8 XIVE END structures per CPU. One for each available * priority @@ -356,7 +356,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) } spapr->qirqs = qemu_allocate_irqs(spapr_set_irq, spapr, - smc->nr_xirqs + SPAPR_XIRQ_BASE); + smc->nr_xirqs + SPAPR_NR_IPIS); /* * Mostly we don't actually need this until reset, except that not diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index c22a72c9e2..e7a80a8349 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -28,6 +28,7 @@ In include/hw/ppc/spapr_irq.h, we should describe the ranges a bit more. See commit dcc345b61ebe and ad8de98636e7 for more info. Something like : /* * The XIVE IRQ backend uses the same layout as the XICS backend but * covers the full range of the IRQ number space. The IRQ numbers for * the CPU IPIs are allocated at the bottom of this space, below 4K, * to preserve compatibility with XICS which does not use that range. */ /* * CPU IPI range (XIVE only) */ #define SPAPR_IRQ_IPI 0x0 #define SPAPR_IRQ_NR_IPIS 0x1000 /* * IRQ range offsets per device type */ #define SPAPR_XIRQ_BASE XICS_IRQ_BASE /* 0x1000 */ And to make sure the ranges don't overlap, let's add : QEMU_BUILD_BUG_ON(SPAPR_IRQ_NR_IPIS > SPAPR_XIRQ_BASE) Yeh, this looks much better. Will update and post. regards, Harsh Thanks, C.
Re: [PATCH v2 2/2] ppc/spapr: Initialize max_cpus limit to SPAPR_NR_IPIS.
Hi Philippe, On 11/22/23 16:46, Philippe Mathieu-Daudé wrote: Hi Harsh, On 22/11/23 10:28, Harsh Prateek Bora wrote: Initialize the machine specific max_cpus limit as per the maximum range of CPU IPIs available. Keeping between 4096 to 8192 will throw IRQ not free error due to XIVE/XICS limitation and keeping beyond 8192 will hit assert in tcg_region_init or spapr_xive_claim_irq. Logs: Without patch fix: [root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=4097 qemu-system-ppc64: IRQ 4096 is not free [root@host build]# On LPAR: [root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=8193 ** ERROR:../tcg/region.c:774:tcg_region_init: assertion failed: (region_size >= 2 * page_size) Bail out! ERROR:../tcg/region.c:774:tcg_region_init: assertion failed: (region_size >= 2 * page_size) Aborted (core dumped) [root@host build]# On x86: [root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=8193 qemu-system-ppc64: ../hw/intc/spapr_xive.c:596: spapr_xive_claim_irq: Assertion `lisn < xive->nr_irqs' failed. Aborted (core dumped) [root@host build]# With patch fix: [root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=4097 qemu-system-ppc64: Invalid SMP CPUs 4097. The max CPUs supported by machine 'pseries-8.2' is 4096 [root@host build]# Signed-off-by: Harsh Prateek Bora --- hw/ppc/spapr.c | 9 +++-- 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index df09aa9d6a..0de11a4458 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -4647,13 +4647,10 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) mc->block_default_type = IF_SCSI; /* - * Setting max_cpus to INT32_MAX. Both KVM and TCG max_cpus values - * should be limited by the host capability instead of hardcoded. - * max_cpus for KVM guests will be checked in kvm_init(), and TCG - * guests are welcome to have as many CPUs as the host are capable - * of emulate. + * While KVM determines max cpus in kvm_init() using kvm_max_vcpus(), + * In TCG the limit is restricted by the range of CPU IPIs available. */ - mc->max_cpus = INT32_MAX; + mc->max_cpus = SPAPR_NR_IPIS; Is SPAPR_NR_IPIS also the upper limit for KVM? In KVM mode, the limit is restricted to what is supported by KVM which is checked using kvm_ioctl via wrappers in kvm_init and appears to be evaluating to 2048. So, having a larger default works for both case. regards, Harsh mc->no_parallel = 1; mc->default_boot_order = "";
[PATCH 14/21] target/arm/kvm: Merge kvm64.c into kvm.c
Since kvm32.c was removed, there is no need to keep them separate.
This will allow more symbols to be unexported.
Signed-off-by: Richard Henderson
---
target/arm/kvm.c | 789 +++
target/arm/kvm64.c | 820 -
target/arm/meson.build | 2 +-
3 files changed, 790 insertions(+), 821 deletions(-)
delete mode 100644 target/arm/kvm64.c
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 9bca6baf35..466b848156 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -19,6 +19,7 @@
#include "qom/object.h"
#include "qapi/error.h"
#include "sysemu/sysemu.h"
+#include "sysemu/runstate.h"
#include "sysemu/kvm.h"
#include "sysemu/kvm_int.h"
#include "kvm_arm.h"
@@ -28,10 +29,13 @@
#include "hw/pci/pci.h"
#include "exec/memattrs.h"
#include "exec/address-spaces.h"
+#include "exec/gdbstub.h"
#include "hw/boards.h"
#include "hw/irq.h"
#include "qapi/visitor.h"
#include "qemu/log.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/ghes.h"
const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
KVM_CAP_LAST_INFO
@@ -1614,3 +1618,788 @@ void kvm_arch_accel_class_init(ObjectClass *oc)
object_class_property_set_description(oc, "eager-split-size",
"Eager Page Split chunk size for hugepages. (default: 0, disabled)");
}
+
+int kvm_arch_insert_hw_breakpoint(vaddr addr, vaddr len, int type)
+{
+switch (type) {
+case GDB_BREAKPOINT_HW:
+return insert_hw_breakpoint(addr);
+break;
+case GDB_WATCHPOINT_READ:
+case GDB_WATCHPOINT_WRITE:
+case GDB_WATCHPOINT_ACCESS:
+return insert_hw_watchpoint(addr, len, type);
+default:
+return -ENOSYS;
+}
+}
+
+int kvm_arch_remove_hw_breakpoint(vaddr addr, vaddr len, int type)
+{
+switch (type) {
+case GDB_BREAKPOINT_HW:
+return delete_hw_breakpoint(addr);
+case GDB_WATCHPOINT_READ:
+case GDB_WATCHPOINT_WRITE:
+case GDB_WATCHPOINT_ACCESS:
+return delete_hw_watchpoint(addr, len, type);
+default:
+return -ENOSYS;
+}
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+if (cur_hw_wps > 0) {
+g_array_remove_range(hw_watchpoints, 0, cur_hw_wps);
+}
+if (cur_hw_bps > 0) {
+g_array_remove_range(hw_breakpoints, 0, cur_hw_bps);
+}
+}
+
+static bool kvm_arm_set_device_attr(CPUState *cs, struct kvm_device_attr *attr,
+const char *name)
+{
+int err;
+
+err = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, attr);
+if (err != 0) {
+error_report("%s: KVM_HAS_DEVICE_ATTR: %s", name, strerror(-err));
+return false;
+}
+
+err = kvm_vcpu_ioctl(cs, KVM_SET_DEVICE_ATTR, attr);
+if (err != 0) {
+error_report("%s: KVM_SET_DEVICE_ATTR: %s", name, strerror(-err));
+return false;
+}
+
+return true;
+}
+
+void kvm_arm_pmu_init(CPUState *cs)
+{
+struct kvm_device_attr attr = {
+.group = KVM_ARM_VCPU_PMU_V3_CTRL,
+.attr = KVM_ARM_VCPU_PMU_V3_INIT,
+};
+
+if (!ARM_CPU(cs)->has_pmu) {
+return;
+}
+if (!kvm_arm_set_device_attr(cs, , "PMU")) {
+error_report("failed to init PMU");
+abort();
+}
+}
+
+void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
+{
+struct kvm_device_attr attr = {
+.group = KVM_ARM_VCPU_PMU_V3_CTRL,
+.addr = (intptr_t),
+.attr = KVM_ARM_VCPU_PMU_V3_IRQ,
+};
+
+if (!ARM_CPU(cs)->has_pmu) {
+return;
+}
+if (!kvm_arm_set_device_attr(cs, , "PMU")) {
+error_report("failed to set irq for PMU");
+abort();
+}
+}
+
+void kvm_arm_pvtime_init(CPUState *cs, uint64_t ipa)
+{
+struct kvm_device_attr attr = {
+.group = KVM_ARM_VCPU_PVTIME_CTRL,
+.attr = KVM_ARM_VCPU_PVTIME_IPA,
+.addr = (uint64_t),
+};
+
+if (ARM_CPU(cs)->kvm_steal_time == ON_OFF_AUTO_OFF) {
+return;
+}
+if (!kvm_arm_set_device_attr(cs, , "PVTIME IPA")) {
+error_report("failed to init PVTIME IPA");
+abort();
+}
+}
+
+void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp)
+{
+bool has_steal_time = kvm_check_extension(kvm_state, KVM_CAP_STEAL_TIME);
+
+if (cpu->kvm_steal_time == ON_OFF_AUTO_AUTO) {
+if (!has_steal_time || !arm_feature(>env, ARM_FEATURE_AARCH64)) {
+cpu->kvm_steal_time = ON_OFF_AUTO_OFF;
+} else {
+cpu->kvm_steal_time = ON_OFF_AUTO_ON;
+}
+} else if (cpu->kvm_steal_time == ON_OFF_AUTO_ON) {
+if (!has_steal_time) {
+error_setg(errp, "'kvm-steal-time' cannot be enabled "
+ "on this host");
+return;
+} else if (!arm_feature(>env, ARM_FEATURE_AARCH64)) {
+/*
+ * DEN0057A chapter 2 says "This specification only covers
+ * systems in which the Execution state of the hypervisor
+ * as well as EL1 of virtual
[PATCH 15/21] target/arm/kvm: Unexport kvm_arm_vcpu_init
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 12
target/arm/kvm.c | 12 +++-
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 1043123cc7..b96ff35e34 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -18,18 +18,6 @@
#define KVM_ARM_VGIC_V2 (1 << 0)
#define KVM_ARM_VGIC_V3 (1 << 1)
-/**
- * kvm_arm_vcpu_init:
- * @cs: CPUState
- *
- * Initialize (or reinitialize) the VCPU by invoking the
- * KVM_ARM_VCPU_INIT ioctl with the CPU type and feature
- * bitmask specified in the CPUState.
- *
- * Returns: 0 if success else < 0 error code
- */
-int kvm_arm_vcpu_init(CPUState *cs);
-
/**
* kvm_arm_vcpu_finalize:
* @cs: CPUState
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 466b848156..a864d0852f 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -58,7 +58,17 @@ typedef struct ARMHostCPUFeatures {
static ARMHostCPUFeatures arm_host_cpu_features;
-int kvm_arm_vcpu_init(CPUState *cs)
+/**
+ * kvm_arm_vcpu_init:
+ * @cs: CPUState
+ *
+ * Initialize (or reinitialize) the VCPU by invoking the
+ * KVM_ARM_VCPU_INIT ioctl with the CPU type and feature
+ * bitmask specified in the CPUState.
+ *
+ * Returns: 0 if success else < 0 error code
+ */
+static int kvm_arm_vcpu_init(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
struct kvm_vcpu_init init;
--
2.34.1
[PATCH 16/21] target/arm/kvm: Unexport kvm_arm_vcpu_finalize
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 14 --
target/arm/kvm.c | 14 +-
2 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index b96ff35e34..9b630a1631 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -18,20 +18,6 @@
#define KVM_ARM_VGIC_V2 (1 << 0)
#define KVM_ARM_VGIC_V3 (1 << 1)
-/**
- * kvm_arm_vcpu_finalize:
- * @cs: CPUState
- * @feature: feature to finalize
- *
- * Finalizes the configuration of the specified VCPU feature by
- * invoking the KVM_ARM_VCPU_FINALIZE ioctl. Features requiring
- * this are documented in the "KVM_ARM_VCPU_FINALIZE" section of
- * KVM's API documentation.
- *
- * Returns: 0 if success else < 0 error code
- */
-int kvm_arm_vcpu_finalize(CPUState *cs, int feature);
-
/**
* kvm_arm_register_device:
* @mr: memory region for this device
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index a864d0852f..20d8c81667 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -79,7 +79,19 @@ static int kvm_arm_vcpu_init(CPUState *cs)
return kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, );
}
-int kvm_arm_vcpu_finalize(CPUState *cs, int feature)
+/**
+ * kvm_arm_vcpu_finalize:
+ * @cs: CPUState
+ * @feature: feature to finalize
+ *
+ * Finalizes the configuration of the specified VCPU feature by
+ * invoking the KVM_ARM_VCPU_FINALIZE ioctl. Features requiring
+ * this are documented in the "KVM_ARM_VCPU_FINALIZE" section of
+ * KVM's API documentation.
+ *
+ * Returns: 0 if success else < 0 error code
+ */
+static int kvm_arm_vcpu_finalize(CPUState *cs, int feature)
{
return kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_FINALIZE, );
}
--
2.34.1
[PATCH 20/21] target/arm/kvm: Unexport and tidy kvm_arm_sync_mpstate_to_{kvm, qemu}
Drop fprintfs and actually use the return values in the callers.
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 20
target/arm/kvm.c | 23 ++-
2 files changed, 6 insertions(+), 37 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index b4339d49d1..8a44a6b762 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -200,26 +200,6 @@ bool kvm_arm_sve_supported(void);
*/
int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa);
-/**
- * kvm_arm_sync_mpstate_to_kvm:
- * @cpu: ARMCPU
- *
- * If supported set the KVM MP_STATE based on QEMU's model.
- *
- * Returns 0 on success and -1 on failure.
- */
-int kvm_arm_sync_mpstate_to_kvm(ARMCPU *cpu);
-
-/**
- * kvm_arm_sync_mpstate_to_qemu:
- * @cpu: ARMCPU
- *
- * If supported get the MP_STATE from KVM and store in QEMU's model.
- *
- * Returns 0 on success and aborts on failure.
- */
-int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu);
-
void kvm_arm_vm_state_change(void *opaque, bool running, RunState state);
int kvm_arm_vgic_probe(void);
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index f9aa55b1a0..19454f432a 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -1006,41 +1006,32 @@ void kvm_arm_reset_vcpu(ARMCPU *cpu)
/*
* Update KVM's MP_STATE based on what QEMU thinks it is
*/
-int kvm_arm_sync_mpstate_to_kvm(ARMCPU *cpu)
+static int kvm_arm_sync_mpstate_to_kvm(ARMCPU *cpu)
{
if (cap_has_mp_state) {
struct kvm_mp_state mp_state = {
.mp_state = (cpu->power_state == PSCI_OFF) ?
KVM_MP_STATE_STOPPED : KVM_MP_STATE_RUNNABLE
};
-int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, _state);
-if (ret) {
-fprintf(stderr, "%s: failed to set MP_STATE %d/%s\n",
-__func__, ret, strerror(-ret));
-return -1;
-}
+return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MP_STATE, _state);
}
-
return 0;
}
/*
* Sync the KVM MP_STATE into QEMU
*/
-int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
+static int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
{
if (cap_has_mp_state) {
struct kvm_mp_state mp_state;
int ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MP_STATE, _state);
if (ret) {
-fprintf(stderr, "%s: failed to get MP_STATE %d/%s\n",
-__func__, ret, strerror(-ret));
-abort();
+return ret;
}
cpu->power_state = (mp_state.mp_state == KVM_MP_STATE_STOPPED) ?
PSCI_OFF : PSCI_ON;
}
-
return 0;
}
@@ -2184,9 +2175,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
return ret;
}
-kvm_arm_sync_mpstate_to_kvm(cpu);
-
-return ret;
+return kvm_arm_sync_mpstate_to_kvm(cpu);
}
static int kvm_arch_get_fpsimd(CPUState *cs)
@@ -2367,7 +2356,7 @@ int kvm_arch_get_registers(CPUState *cs)
*/
write_list_to_cpustate(cpu);
-kvm_arm_sync_mpstate_to_qemu(cpu);
+ret = kvm_arm_sync_mpstate_to_qemu(cpu);
/* TODO: other registers */
return ret;
--
2.34.1
[PATCH 10/21] target/arm/kvm: Move kvm_arm_get_host_cpu_features and unexport
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 22
target/arm/kvm.c | 265 +++
target/arm/kvm64.c | 254 -
3 files changed, 265 insertions(+), 276 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 58c087207f..e59d713973 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -214,28 +214,6 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t
*cpus_to_try,
*/
void kvm_arm_destroy_scratch_host_vcpu(int *fdarray);
-/**
- * ARMHostCPUFeatures: information about the host CPU (identified
- * by asking the host kernel)
- */
-typedef struct ARMHostCPUFeatures {
-ARMISARegisters isar;
-uint64_t features;
-uint32_t target;
-const char *dtb_compatible;
-} ARMHostCPUFeatures;
-
-/**
- * kvm_arm_get_host_cpu_features:
- * @ahcf: ARMHostCPUClass to fill in
- *
- * Probe the capabilities of the host kernel's preferred CPU and fill
- * in the ARMHostCPUClass struct accordingly.
- *
- * Returns true on success and false otherwise.
- */
-bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf);
-
/**
* kvm_arm_sve_get_vls:
* @cs: CPUState
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 84f300c602..ffe0db4293 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -41,6 +41,17 @@ static bool cap_has_mp_state;
static bool cap_has_inject_serror_esr;
static bool cap_has_inject_ext_dabt;
+/**
+ * ARMHostCPUFeatures: information about the host CPU (identified
+ * by asking the host kernel)
+ */
+typedef struct ARMHostCPUFeatures {
+ARMISARegisters isar;
+uint64_t features;
+uint32_t target;
+const char *dtb_compatible;
+} ARMHostCPUFeatures;
+
static ARMHostCPUFeatures arm_host_cpu_features;
int kvm_arm_vcpu_init(CPUState *cs)
@@ -167,6 +178,260 @@ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray)
}
}
+static int read_sys_reg32(int fd, uint32_t *pret, uint64_t id)
+{
+uint64_t ret;
+struct kvm_one_reg idreg = { .id = id, .addr = (uintptr_t) };
+int err;
+
+assert((id & KVM_REG_SIZE_MASK) == KVM_REG_SIZE_U64);
+err = ioctl(fd, KVM_GET_ONE_REG, );
+if (err < 0) {
+return -1;
+}
+*pret = ret;
+return 0;
+}
+
+static int read_sys_reg64(int fd, uint64_t *pret, uint64_t id)
+{
+struct kvm_one_reg idreg = { .id = id, .addr = (uintptr_t)pret };
+
+assert((id & KVM_REG_SIZE_MASK) == KVM_REG_SIZE_U64);
+return ioctl(fd, KVM_GET_ONE_REG, );
+}
+
+static bool kvm_arm_pauth_supported(void)
+{
+return (kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_ADDRESS) &&
+kvm_check_extension(kvm_state, KVM_CAP_ARM_PTRAUTH_GENERIC));
+}
+
+static bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
+{
+/* Identify the feature bits corresponding to the host CPU, and
+ * fill out the ARMHostCPUClass fields accordingly. To do this
+ * we have to create a scratch VM, create a single CPU inside it,
+ * and then query that CPU for the relevant ID registers.
+ */
+int fdarray[3];
+bool sve_supported;
+bool pmu_supported = false;
+uint64_t features = 0;
+int err;
+
+/* Old kernels may not know about the PREFERRED_TARGET ioctl: however
+ * we know these will only support creating one kind of guest CPU,
+ * which is its preferred CPU type. Fortunately these old kernels
+ * support only a very limited number of CPUs.
+ */
+static const uint32_t cpus_to_try[] = {
+KVM_ARM_TARGET_AEM_V8,
+KVM_ARM_TARGET_FOUNDATION_V8,
+KVM_ARM_TARGET_CORTEX_A57,
+QEMU_KVM_ARM_TARGET_NONE
+};
+/*
+ * target = -1 informs kvm_arm_create_scratch_host_vcpu()
+ * to use the preferred target
+ */
+struct kvm_vcpu_init init = { .target = -1, };
+
+/*
+ * Ask for SVE if supported, so that we can query ID_AA64ZFR0,
+ * which is otherwise RAZ.
+ */
+sve_supported = kvm_arm_sve_supported();
+if (sve_supported) {
+init.features[0] |= 1 << KVM_ARM_VCPU_SVE;
+}
+
+/*
+ * Ask for Pointer Authentication if supported, so that we get
+ * the unsanitized field values for AA64ISAR1_EL1.
+ */
+if (kvm_arm_pauth_supported()) {
+init.features[0] |= (1 << KVM_ARM_VCPU_PTRAUTH_ADDRESS |
+ 1 << KVM_ARM_VCPU_PTRAUTH_GENERIC);
+}
+
+if (kvm_arm_pmu_supported()) {
+init.features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
+pmu_supported = true;
+}
+
+if (!kvm_arm_create_scratch_host_vcpu(cpus_to_try, fdarray, )) {
+return false;
+}
+
+ahcf->target = init.target;
+ahcf->dtb_compatible = "arm,arm-v8";
+
+err = read_sys_reg64(fdarray[2], >isar.id_aa64pfr0,
+ ARM64_SYS_REG(3, 0, 0, 4, 0));
+if (unlikely(err < 0)) {
+/*
+ * Before v4.15, the kernel only exposed a limited number of system
+ * registers, not including any
[PATCH 19/21] target/arm/kvm: Unexport kvm_{get,put}_vcpu_events
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 20
target/arm/kvm.c | 20 ++--
2 files changed, 18 insertions(+), 22 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 1ec2476de7..b4339d49d1 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -98,26 +98,6 @@ void kvm_arm_cpu_post_load(ARMCPU *cpu);
*/
void kvm_arm_reset_vcpu(ARMCPU *cpu);
-/**
- * kvm_get_vcpu_events:
- * @cpu: ARMCPU
- *
- * Get VCPU related state from kvm.
- *
- * Returns: 0 if success else < 0 error code
- */
-int kvm_get_vcpu_events(ARMCPU *cpu);
-
-/**
- * kvm_put_vcpu_events:
- * @cpu: ARMCPU
- *
- * Put VCPU related state to kvm.
- *
- * Returns: 0 if success else < 0 error code
- */
-int kvm_put_vcpu_events(ARMCPU *cpu);
-
#ifdef CONFIG_KVM
/**
* kvm_arm_create_scratch_host_vcpu:
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 3250919273..f9aa55b1a0 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -1092,7 +1092,15 @@ static void kvm_arm_put_virtual_time(CPUState *cs)
cpu->kvm_vtime_dirty = false;
}
-int kvm_put_vcpu_events(ARMCPU *cpu)
+/**
+ * kvm_put_vcpu_events:
+ * @cpu: ARMCPU
+ *
+ * Put VCPU related state to kvm.
+ *
+ * Returns: 0 if success else < 0 error code
+ */
+static int kvm_put_vcpu_events(ARMCPU *cpu)
{
CPUARMState *env = >env;
struct kvm_vcpu_events events;
@@ -1121,7 +1129,15 @@ int kvm_put_vcpu_events(ARMCPU *cpu)
return ret;
}
-int kvm_get_vcpu_events(ARMCPU *cpu)
+/**
+ * kvm_get_vcpu_events:
+ * @cpu: ARMCPU
+ *
+ * Get VCPU related state from kvm.
+ *
+ * Returns: 0 if success else < 0 error code
+ */
+static int kvm_get_vcpu_events(ARMCPU *cpu)
{
CPUARMState *env = >env;
struct kvm_vcpu_events events;
--
2.34.1
[PATCH 08/21] target/arm/kvm: Unexport kvm_arm_{get, put}_virtual_time
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 16
target/arm/kvm.c | 16 ++--
2 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 9fa9cb7f76..e7c32f6ed0 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -335,22 +335,6 @@ int kvm_arm_sync_mpstate_to_kvm(ARMCPU *cpu);
*/
int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu);
-/**
- * kvm_arm_get_virtual_time:
- * @cs: CPUState
- *
- * Gets the VCPU's virtual counter and stores it in the KVM CPU state.
- */
-void kvm_arm_get_virtual_time(CPUState *cs);
-
-/**
- * kvm_arm_put_virtual_time:
- * @cs: CPUState
- *
- * Sets the VCPU's virtual counter to the value stored in the KVM CPU state.
- */
-void kvm_arm_put_virtual_time(CPUState *cs);
-
void kvm_arm_vm_state_change(void *opaque, bool running, RunState state);
int kvm_arm_vgic_probe(void);
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 55e1b4f26e..84f300c602 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -704,7 +704,13 @@ int kvm_arm_sync_mpstate_to_qemu(ARMCPU *cpu)
return 0;
}
-void kvm_arm_get_virtual_time(CPUState *cs)
+/**
+ * kvm_arm_get_virtual_time:
+ * @cs: CPUState
+ *
+ * Gets the VCPU's virtual counter and stores it in the KVM CPU state.
+ */
+static void kvm_arm_get_virtual_time(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
int ret;
@@ -722,7 +728,13 @@ void kvm_arm_get_virtual_time(CPUState *cs)
cpu->kvm_vtime_dirty = true;
}
-void kvm_arm_put_virtual_time(CPUState *cs)
+/**
+ * kvm_arm_put_virtual_time:
+ * @cs: CPUState
+ *
+ * Sets the VCPU's virtual counter to the value stored in the KVM CPU state.
+ */
+static void kvm_arm_put_virtual_time(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
int ret;
--
2.34.1
[PATCH 13/21] target/arm/kvm: Move kvm_arm_reg_syncs_via_cpreg_list and unexport
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 10 --
target/arm/kvm.c | 23 +++
target/arm/kvm64.c | 15 ---
3 files changed, 23 insertions(+), 25 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 2755ee8366..1043123cc7 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -77,16 +77,6 @@ void kvm_arm_register_device(MemoryRegion *mr, uint64_t
devid, uint64_t group,
*/
int kvm_arm_init_cpreg_list(ARMCPU *cpu);
-/**
- * kvm_arm_reg_syncs_via_cpreg_list:
- * @regidx: KVM register index
- *
- * Return true if this KVM register should be synchronized via the
- * cpreg list of arbitrary system registers, false if it is synchronized
- * by hand using code in kvm_arch_get/put_registers().
- */
-bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx);
-
/**
* write_list_to_kvmstate:
* @cpu: ARMCPU
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index dadc3fd755..9bca6baf35 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -740,6 +740,29 @@ static uint64_t *kvm_arm_get_cpreg_ptr(ARMCPU *cpu,
uint64_t regidx)
return >cpreg_values[res - cpu->cpreg_indexes];
}
+/**
+ * kvm_arm_reg_syncs_via_cpreg_list:
+ * @regidx: KVM register index
+ *
+ * Return true if this KVM register should be synchronized via the
+ * cpreg list of arbitrary system registers, false if it is synchronized
+ * by hand using code in kvm_arch_get/put_registers().
+ */
+static bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
+{
+/* Return true if the regidx is a register we should synchronize
+ * via the cpreg_tuples array (ie is not a core or sve reg that
+ * we sync by hand in kvm_arch_get/put_registers())
+ */
+switch (regidx & KVM_REG_ARM_COPROC_MASK) {
+case KVM_REG_ARM_CORE:
+case KVM_REG_ARM64_SVE:
+return false;
+default:
+return true;
+}
+}
+
/* Initialize the ARMCPU cpreg list according to the kernel's
* definition of what CPU registers it knows about (and throw away
* the previous TCG-created cpreg list).
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index a184cca4dc..52c0a6d3af 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -346,21 +346,6 @@ int kvm_arch_destroy_vcpu(CPUState *cs)
return 0;
}
-bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
-{
-/* Return true if the regidx is a register we should synchronize
- * via the cpreg_tuples array (ie is not a core or sve reg that
- * we sync by hand in kvm_arch_get/put_registers())
- */
-switch (regidx & KVM_REG_ARM_COPROC_MASK) {
-case KVM_REG_ARM_CORE:
-case KVM_REG_ARM64_SVE:
-return false;
-default:
-return true;
-}
-}
-
/* Callers must hold the iothread mutex lock */
static void kvm_inject_arm_sea(CPUState *c)
{
--
2.34.1
[PATCH 17/21] target/arm/kvm: Unexport kvm_arm_init_cpreg_list
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 12
target/arm/kvm.c | 10 --
2 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 9b630a1631..350ba6cb96 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -39,18 +39,6 @@
void kvm_arm_register_device(MemoryRegion *mr, uint64_t devid, uint64_t group,
uint64_t attr, int dev_fd, uint64_t addr_ormask);
-/**
- * kvm_arm_init_cpreg_list:
- * @cpu: ARMCPU
- *
- * Initialize the ARMCPU cpreg list according to the kernel's
- * definition of what CPU registers it knows about (and throw away
- * the previous TCG-created cpreg list).
- *
- * Returns: 0 if success, else < 0 error code
- */
-int kvm_arm_init_cpreg_list(ARMCPU *cpu);
-
/**
* write_list_to_kvmstate:
* @cpu: ARMCPU
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 20d8c81667..bc4ba7628b 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -789,11 +789,17 @@ static bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t
regidx)
}
}
-/* Initialize the ARMCPU cpreg list according to the kernel's
+/**
+ * kvm_arm_init_cpreg_list:
+ * @cpu: ARMCPU
+ *
+ * Initialize the ARMCPU cpreg list according to the kernel's
* definition of what CPU registers it knows about (and throw away
* the previous TCG-created cpreg list).
+ *
+ * Returns: 0 if success, else < 0 error code
*/
-int kvm_arm_init_cpreg_list(ARMCPU *cpu)
+static int kvm_arm_init_cpreg_list(ARMCPU *cpu)
{
struct kvm_reg_list rl;
struct kvm_reg_list *rlp;
--
2.34.1
[PATCH 01/21] accel/kvm: Make kvm_has_guest_debug static
This variable is not used or declared outside kvm-all.c. Signed-off-by: Richard Henderson --- accel/kvm/kvm-all.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index e39a810a4e..f138e7fefe 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -98,7 +98,7 @@ bool kvm_allowed; bool kvm_readonly_mem_allowed; bool kvm_vm_attributes_allowed; bool kvm_msi_use_devid; -bool kvm_has_guest_debug; +static bool kvm_has_guest_debug; static int kvm_sstep_flags; static bool kvm_immediate_exit; static hwaddr kvm_max_slot_size = ~0; -- 2.34.1
[PATCH 12/21] target/arm/kvm: Move kvm_arm_cpreg_level and unexport
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 9 -
target/arm/kvm.c | 22 ++
target/arm/kvm64.c | 15 ---
3 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index e59d713973..2755ee8366 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -87,15 +87,6 @@ int kvm_arm_init_cpreg_list(ARMCPU *cpu);
*/
bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx);
-/**
- * kvm_arm_cpreg_level:
- * @regidx: KVM register index
- *
- * Return the level of this coprocessor/system register. Return value is
- * either KVM_PUT_RUNTIME_STATE, KVM_PUT_RESET_STATE, or KVM_PUT_FULL_STATE.
- */
-int kvm_arm_cpreg_level(uint64_t regidx);
-
/**
* write_list_to_kvmstate:
* @cpu: ARMCPU
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index ffe0db4293..dadc3fd755 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -817,6 +817,28 @@ out:
return ret;
}
+/**
+ * kvm_arm_cpreg_level:
+ * @regidx: KVM register index
+ *
+ * Return the level of this coprocessor/system register. Return value is
+ * either KVM_PUT_RUNTIME_STATE, KVM_PUT_RESET_STATE, or KVM_PUT_FULL_STATE.
+ */
+static int kvm_arm_cpreg_level(uint64_t regidx)
+{
+/*
+ * All system registers are assumed to be level KVM_PUT_RUNTIME_STATE.
+ * If a register should be written less often, you must add it here
+ * with a state of either KVM_PUT_RESET_STATE or KVM_PUT_FULL_STATE.
+ */
+switch (regidx) {
+case KVM_REG_ARM_TIMER_CNT:
+case KVM_REG_ARM_PTIMER_CNT:
+return KVM_PUT_FULL_STATE;
+}
+return KVM_PUT_RUNTIME_STATE;
+}
+
bool write_kvmstate_to_list(ARMCPU *cpu)
{
CPUState *cs = CPU(cpu);
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 61fb9dbde0..a184cca4dc 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -361,21 +361,6 @@ bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
}
}
-int kvm_arm_cpreg_level(uint64_t regidx)
-{
-/*
- * All system registers are assumed to be level KVM_PUT_RUNTIME_STATE.
- * If a register should be written less often, you must add it here
- * with a state of either KVM_PUT_RESET_STATE or KVM_PUT_FULL_STATE.
- */
-switch (regidx) {
-case KVM_REG_ARM_TIMER_CNT:
-case KVM_REG_ARM_PTIMER_CNT:
-return KVM_PUT_FULL_STATE;
-}
-return KVM_PUT_RUNTIME_STATE;
-}
-
/* Callers must hold the iothread mutex lock */
static void kvm_inject_arm_sea(CPUState *c)
{
--
2.34.1
[PATCH 07/21] target/arm/kvm: Move kvm_arm_handle_debug and unexport
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 9 --
target/arm/kvm.c | 77
target/arm/kvm64.c | 70
3 files changed, 77 insertions(+), 79 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index ac4856cb46..9fa9cb7f76 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -445,13 +445,4 @@ static inline uint32_t kvm_arm_sve_get_vls(CPUState *cs)
#endif
-/**
- * kvm_arm_handle_debug:
- * @cs: CPUState
- * @debug_exit: debug part of the KVM exit structure
- *
- * Returns: TRUE if the debug exception was handled.
- */
-bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch
*debug_exit);
-
#endif
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 4608bea7df..55e1b4f26e 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -988,6 +988,83 @@ static int kvm_arm_handle_dabt_nisv(CPUState *cs, uint64_t
esr_iss,
return -1;
}
+/**
+ * kvm_arm_handle_debug:
+ * @cs: CPUState
+ * @debug_exit: debug part of the KVM exit structure
+ *
+ * Returns: TRUE if the debug exception was handled.
+ *
+ * See v8 ARM ARM D7.2.27 ESR_ELx, Exception Syndrome Register
+ *
+ * To minimise translating between kernel and user-space the kernel
+ * ABI just provides user-space with the full exception syndrome
+ * register value to be decoded in QEMU.
+ */
+static bool kvm_arm_handle_debug(CPUState *cs,
+ struct kvm_debug_exit_arch *debug_exit)
+{
+int hsr_ec = syn_get_ec(debug_exit->hsr);
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = >env;
+
+/* Ensure PC is synchronised */
+kvm_cpu_synchronize_state(cs);
+
+switch (hsr_ec) {
+case EC_SOFTWARESTEP:
+if (cs->singlestep_enabled) {
+return true;
+} else {
+/*
+ * The kernel should have suppressed the guest's ability to
+ * single step at this point so something has gone wrong.
+ */
+error_report("%s: guest single-step while debugging unsupported"
+ " (%"PRIx64", %"PRIx32")",
+ __func__, env->pc, debug_exit->hsr);
+return false;
+}
+break;
+case EC_AA64_BKPT:
+if (kvm_find_sw_breakpoint(cs, env->pc)) {
+return true;
+}
+break;
+case EC_BREAKPOINT:
+if (find_hw_breakpoint(cs, env->pc)) {
+return true;
+}
+break;
+case EC_WATCHPOINT:
+{
+CPUWatchpoint *wp = find_hw_watchpoint(cs, debug_exit->far);
+if (wp) {
+cs->watchpoint_hit = wp;
+return true;
+}
+break;
+}
+default:
+error_report("%s: unhandled debug exit (%"PRIx32", %"PRIx64")",
+ __func__, debug_exit->hsr, env->pc);
+}
+
+/* If we are not handling the debug exception it must belong to
+ * the guest. Let's re-use the existing TCG interrupt code to set
+ * everything up properly.
+ */
+cs->exception_index = EXCP_BKPT;
+env->exception.syndrome = debug_exit->hsr;
+env->exception.vaddress = debug_exit->far;
+env->exception.target_el = 1;
+qemu_mutex_lock_iothread();
+arm_cpu_do_interrupt(cs);
+qemu_mutex_unlock_iothread();
+
+return false;
+}
+
int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
{
int ret = 0;
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 352643e066..6b6db9374c 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -1121,73 +1121,3 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct
kvm_sw_breakpoint *bp)
}
return 0;
}
-
-/* See v8 ARM ARM D7.2.27 ESR_ELx, Exception Syndrome Register
- *
- * To minimise translating between kernel and user-space the kernel
- * ABI just provides user-space with the full exception syndrome
- * register value to be decoded in QEMU.
- */
-
-bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit)
-{
-int hsr_ec = syn_get_ec(debug_exit->hsr);
-ARMCPU *cpu = ARM_CPU(cs);
-CPUARMState *env = >env;
-
-/* Ensure PC is synchronised */
-kvm_cpu_synchronize_state(cs);
-
-switch (hsr_ec) {
-case EC_SOFTWARESTEP:
-if (cs->singlestep_enabled) {
-return true;
-} else {
-/*
- * The kernel should have suppressed the guest's ability to
- * single step at this point so something has gone wrong.
- */
-error_report("%s: guest single-step while debugging unsupported"
- " (%"PRIx64", %"PRIx32")",
- __func__, env->pc, debug_exit->hsr);
-return false;
-}
-break;
-case EC_AA64_BKPT:
-if (kvm_find_sw_breakpoint(cs, env->pc)) {
-return true;
-}
-break;
-case EC_BREAKPOINT:
-if
[PATCH 03/21] target/arm/kvm: Merge kvm_arm_init_debug into kvm_arch_init
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 8
target/arm/kvm.c | 8 +++-
target/arm/kvm64.c | 12
3 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 051a0da41c..fe6d824a52 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -18,14 +18,6 @@
#define KVM_ARM_VGIC_V2 (1 << 0)
#define KVM_ARM_VGIC_V3 (1 << 1)
-/**
- * kvm_arm_init_debug() - initialize guest debug capabilities
- * @s: KVMState
- *
- * Should be called only once before using guest debug capabilities.
- */
-void kvm_arm_init_debug(KVMState *s);
-
/**
* kvm_arm_vcpu_init:
* @cs: CPUState
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 7903e2ddde..b4836da6b2 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -308,7 +308,13 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
}
}
-kvm_arm_init_debug(s);
+max_hw_wps = kvm_check_extension(s, KVM_CAP_GUEST_DEBUG_HW_WPS);
+hw_watchpoints = g_array_sized_new(true, true,
+ sizeof(HWWatchpoint), max_hw_wps);
+
+max_hw_bps = kvm_check_extension(s, KVM_CAP_GUEST_DEBUG_HW_BPS);
+hw_breakpoints = g_array_sized_new(true, true,
+ sizeof(HWBreakpoint), max_hw_bps);
return ret;
}
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index b8bb25a1ea..40f459b786 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -33,18 +33,6 @@
#include "hw/acpi/ghes.h"
-void kvm_arm_init_debug(KVMState *s)
-{
-max_hw_wps = kvm_check_extension(s, KVM_CAP_GUEST_DEBUG_HW_WPS);
-hw_watchpoints = g_array_sized_new(true, true,
- sizeof(HWWatchpoint), max_hw_wps);
-
-max_hw_bps = kvm_check_extension(s, KVM_CAP_GUEST_DEBUG_HW_BPS);
-hw_breakpoints = g_array_sized_new(true, true,
- sizeof(HWBreakpoint), max_hw_bps);
-return;
-}
-
int kvm_arch_insert_hw_breakpoint(vaddr addr, vaddr len, int type)
{
switch (type) {
--
2.34.1
[PATCH 21/21] target/arm/kvm: Unexport kvm_arm_vm_state_change
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 2 --
target/arm/kvm.c | 2 +-
2 files changed, 1 insertion(+), 3 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 8a44a6b762..2037b2d7ea 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -200,8 +200,6 @@ bool kvm_arm_sve_supported(void);
*/
int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa);
-void kvm_arm_vm_state_change(void *opaque, bool running, RunState state);
-
int kvm_arm_vgic_probe(void);
void kvm_arm_pmu_set_irq(CPUState *cs, int irq);
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 19454f432a..6e3fea1879 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -1290,7 +1290,7 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run
*run)
return MEMTXATTRS_UNSPECIFIED;
}
-void kvm_arm_vm_state_change(void *opaque, bool running, RunState state)
+static void kvm_arm_vm_state_change(void *opaque, bool running, RunState state)
{
CPUState *cs = opaque;
ARMCPU *cpu = ARM_CPU(cs);
--
2.34.1
[PATCH 11/21] target/arm/kvm: Use a switch for kvm_arm_cpreg_level
Use a switch instead of a linear search through data.
Signed-off-by: Richard Henderson
---
target/arm/kvm64.c | 32 +---
1 file changed, 9 insertions(+), 23 deletions(-)
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 504526b24c..61fb9dbde0 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -361,32 +361,18 @@ bool kvm_arm_reg_syncs_via_cpreg_list(uint64_t regidx)
}
}
-typedef struct CPRegStateLevel {
-uint64_t regidx;
-int level;
-} CPRegStateLevel;
-
-/* All system registers not listed in the following table are assumed to be
- * of the level KVM_PUT_RUNTIME_STATE. If a register should be written less
- * often, you must add it to this table with a state of either
- * KVM_PUT_RESET_STATE or KVM_PUT_FULL_STATE.
- */
-static const CPRegStateLevel non_runtime_cpregs[] = {
-{ KVM_REG_ARM_TIMER_CNT, KVM_PUT_FULL_STATE },
-{ KVM_REG_ARM_PTIMER_CNT, KVM_PUT_FULL_STATE },
-};
-
int kvm_arm_cpreg_level(uint64_t regidx)
{
-int i;
-
-for (i = 0; i < ARRAY_SIZE(non_runtime_cpregs); i++) {
-const CPRegStateLevel *l = _runtime_cpregs[i];
-if (l->regidx == regidx) {
-return l->level;
-}
+/*
+ * All system registers are assumed to be level KVM_PUT_RUNTIME_STATE.
+ * If a register should be written less often, you must add it here
+ * with a state of either KVM_PUT_RESET_STATE or KVM_PUT_FULL_STATE.
+ */
+switch (regidx) {
+case KVM_REG_ARM_TIMER_CNT:
+case KVM_REG_ARM_PTIMER_CNT:
+return KVM_PUT_FULL_STATE;
}
-
return KVM_PUT_RUNTIME_STATE;
}
--
2.34.1
[PATCH for-9.0 00/21] target/arm: kvm cleanups
This is primarily concerned with merging kvm64.c with kvm.c
and then unexporting everything that is not required outside.
r~
Chao Du (1):
target/arm: kvm64: remove a redundant KVM_CAP_SET_GUEST_DEBUG probe
Richard Henderson (20):
accel/kvm: Make kvm_has_guest_debug static
target/arm/kvm: Merge kvm_arm_init_debug into kvm_arch_init
target/arm/kvm: Move kvm_arm_verify_ext_dabt_pending and unexport
target/arm/kvm: Move kvm_arm_copy_hw_debug_data and unexport
target/arm/kvm: Move kvm_arm_hw_debug_active and unexport
target/arm/kvm: Move kvm_arm_handle_debug and unexport
target/arm/kvm: Unexport kvm_arm_{get,put}_virtual_time
target/arm/kvm: Inline kvm_arm_steal_time_supported
target/arm/kvm: Move kvm_arm_get_host_cpu_features and unexport
target/arm/kvm: Use a switch for kvm_arm_cpreg_level
target/arm/kvm: Move kvm_arm_cpreg_level and unexport
target/arm/kvm: Move kvm_arm_reg_syncs_via_cpreg_list and unexport
target/arm/kvm: Merge kvm64.c into kvm.c
target/arm/kvm: Unexport kvm_arm_vcpu_init
target/arm/kvm: Unexport kvm_arm_vcpu_finalize
target/arm/kvm: Unexport kvm_arm_init_cpreg_list
target/arm/kvm: Init cap_has_inject_serror_esr in kvm_arch_init
target/arm/kvm: Unexport kvm_{get,put}_vcpu_events
target/arm/kvm: Unexport and tidy kvm_arm_sync_mpstate_to_{kvm,qemu}
target/arm/kvm: Unexport kvm_arm_vm_state_change
target/arm/kvm_arm.h | 203 --
accel/kvm/kvm-all.c|2 +-
target/arm/kvm.c | 1372 +++-
target/arm/kvm64.c | 1290 -
target/arm/meson.build |2 +-
5 files changed, 1345 insertions(+), 1524 deletions(-)
delete mode 100644 target/arm/kvm64.c
--
2.34.1
[PATCH 18/21] target/arm/kvm: Init cap_has_inject_serror_esr in kvm_arch_init
There is no need to do this in kvm_arch_init_vcpu per vcpu.
Inline kvm_arm_init_serror_injection rather than keep separate.
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 8
target/arm/kvm.c | 13 -
2 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 350ba6cb96..1ec2476de7 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -98,14 +98,6 @@ void kvm_arm_cpu_post_load(ARMCPU *cpu);
*/
void kvm_arm_reset_vcpu(ARMCPU *cpu);
-/**
- * kvm_arm_init_serror_injection:
- * @cs: CPUState
- *
- * Check whether KVM can set guest SError syndrome.
- */
-void kvm_arm_init_serror_injection(CPUState *cs);
-
/**
* kvm_get_vcpu_events:
* @cpu: ARMCPU
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index bc4ba7628b..3250919273 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -96,12 +96,6 @@ static int kvm_arm_vcpu_finalize(CPUState *cs, int feature)
return kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_FINALIZE, );
}
-void kvm_arm_init_serror_injection(CPUState *cs)
-{
-cap_has_inject_serror_esr = kvm_check_extension(cs->kvm_state,
-KVM_CAP_ARM_INJECT_SERROR_ESR);
-}
-
bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try,
int *fdarray,
struct kvm_vcpu_init *init)
@@ -562,6 +556,10 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
cap_has_mp_state = kvm_check_extension(s, KVM_CAP_MP_STATE);
+/* Check whether user space can specify guest syndrome value */
+cap_has_inject_serror_esr =
+kvm_check_extension(s, KVM_CAP_ARM_INJECT_SERROR_ESR);
+
if (ms->smp.cpus > 256 &&
!kvm_check_extension(s, KVM_CAP_ARM_IRQ_LINE_LAYOUT_2)) {
error_report("Using more than 256 vcpus requires a host kernel "
@@ -1948,9 +1946,6 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
cpu->mp_affinity = mpidr & ARM64_AFFINITY_MASK;
-/* Check whether user space can specify guest syndrome value */
-kvm_arm_init_serror_injection(cs);
-
return kvm_arm_init_cpreg_list(cpu);
}
--
2.34.1
[PATCH 04/21] target/arm/kvm: Move kvm_arm_verify_ext_dabt_pending and unexport
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 10
target/arm/kvm.c | 57
target/arm/kvm64.c | 49 -
3 files changed, 57 insertions(+), 59 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index fe6d824a52..bb284a47de 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -472,14 +472,4 @@ bool kvm_arm_hw_debug_active(CPUState *cs);
struct kvm_guest_debug_arch;
void kvm_arm_copy_hw_debug_data(struct kvm_guest_debug_arch *ptr);
-/**
- * kvm_arm_verify_ext_dabt_pending:
- * @cs: CPUState
- *
- * Verify the fault status code wrt the Ext DABT injection
- *
- * Returns: true if the fault status code is as expected, false otherwise
- */
-bool kvm_arm_verify_ext_dabt_pending(CPUState *cs);
-
#endif
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index b4836da6b2..696bc63e86 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -793,6 +793,63 @@ int kvm_get_vcpu_events(ARMCPU *cpu)
return 0;
}
+#define ARM64_REG_ESR_EL1 ARM64_SYS_REG(3, 0, 5, 2, 0)
+#define ARM64_REG_TCR_EL1 ARM64_SYS_REG(3, 0, 2, 0, 2)
+
+/*
+ * ESR_EL1
+ * ISS encoding
+ * AARCH64: DFSC, bits [5:0]
+ * AARCH32:
+ * TTBCR.EAE == 0
+ * FS[4] - DFSR[10]
+ * FS[3:0] - DFSR[3:0]
+ * TTBCR.EAE == 1
+ * FS, bits [5:0]
+ */
+#define ESR_DFSC(aarch64, lpae, v)\
+((aarch64 || (lpae)) ? ((v) & 0x3F) \
+ : (((v) >> 6) | ((v) & 0x1F)))
+
+#define ESR_DFSC_EXTABT(aarch64, lpae) \
+((aarch64) ? 0x10 : (lpae) ? 0x10 : 0x8)
+
+/**
+ * kvm_arm_verify_ext_dabt_pending:
+ * @cs: CPUState
+ *
+ * Verify the fault status code wrt the Ext DABT injection
+ *
+ * Returns: true if the fault status code is as expected, false otherwise
+ */
+static bool kvm_arm_verify_ext_dabt_pending(CPUState *cs)
+{
+uint64_t dfsr_val;
+
+if (!kvm_get_one_reg(cs, ARM64_REG_ESR_EL1, _val)) {
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = >env;
+int aarch64_mode = arm_feature(env, ARM_FEATURE_AARCH64);
+int lpae = 0;
+
+if (!aarch64_mode) {
+uint64_t ttbcr;
+
+if (!kvm_get_one_reg(cs, ARM64_REG_TCR_EL1, )) {
+lpae = arm_feature(env, ARM_FEATURE_LPAE)
+&& (ttbcr & TTBCR_EAE);
+}
+}
+/*
+ * The verification here is based on the DFSC bits
+ * of the ESR_EL1 reg only
+ */
+ return (ESR_DFSC(aarch64_mode, lpae, dfsr_val) ==
+ESR_DFSC_EXTABT(aarch64_mode, lpae));
+}
+return false;
+}
+
void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run)
{
ARMCPU *cpu = ARM_CPU(cs);
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 40f459b786..7d937e2539 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -1213,52 +1213,3 @@ bool kvm_arm_handle_debug(CPUState *cs, struct
kvm_debug_exit_arch *debug_exit)
return false;
}
-
-#define ARM64_REG_ESR_EL1 ARM64_SYS_REG(3, 0, 5, 2, 0)
-#define ARM64_REG_TCR_EL1 ARM64_SYS_REG(3, 0, 2, 0, 2)
-
-/*
- * ESR_EL1
- * ISS encoding
- * AARCH64: DFSC, bits [5:0]
- * AARCH32:
- * TTBCR.EAE == 0
- * FS[4] - DFSR[10]
- * FS[3:0] - DFSR[3:0]
- * TTBCR.EAE == 1
- * FS, bits [5:0]
- */
-#define ESR_DFSC(aarch64, lpae, v)\
-((aarch64 || (lpae)) ? ((v) & 0x3F) \
- : (((v) >> 6) | ((v) & 0x1F)))
-
-#define ESR_DFSC_EXTABT(aarch64, lpae) \
-((aarch64) ? 0x10 : (lpae) ? 0x10 : 0x8)
-
-bool kvm_arm_verify_ext_dabt_pending(CPUState *cs)
-{
-uint64_t dfsr_val;
-
-if (!kvm_get_one_reg(cs, ARM64_REG_ESR_EL1, _val)) {
-ARMCPU *cpu = ARM_CPU(cs);
-CPUARMState *env = >env;
-int aarch64_mode = arm_feature(env, ARM_FEATURE_AARCH64);
-int lpae = 0;
-
-if (!aarch64_mode) {
-uint64_t ttbcr;
-
-if (!kvm_get_one_reg(cs, ARM64_REG_TCR_EL1, )) {
-lpae = arm_feature(env, ARM_FEATURE_LPAE)
-&& (ttbcr & TTBCR_EAE);
-}
-}
-/*
- * The verification here is based on the DFSC bits
- * of the ESR_EL1 reg only
- */
- return (ESR_DFSC(aarch64_mode, lpae, dfsr_val) ==
-ESR_DFSC_EXTABT(aarch64_mode, lpae));
-}
-return false;
-}
--
2.34.1
[PATCH 06/21] target/arm/kvm: Move kvm_arm_hw_debug_active and unexport
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 8
target/arm/kvm.c | 11 +++
target/arm/kvm64.c | 5 -
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index 207b7f21b0..ac4856cb46 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -454,12 +454,4 @@ static inline uint32_t kvm_arm_sve_get_vls(CPUState *cs)
*/
bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch
*debug_exit);
-/**
- * kvm_arm_hw_debug_active:
- * @cs: CPU State
- *
- * Return: TRUE if any hardware breakpoints in use.
- */
-bool kvm_arm_hw_debug_active(CPUState *cs);
-
#endif
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 2898e680fc..4608bea7df 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -1021,6 +1021,17 @@ int kvm_arch_process_async_events(CPUState *cs)
return 0;
}
+/**
+ * kvm_arm_hw_debug_active:
+ * @cs: CPU State
+ *
+ * Return: TRUE if any hardware breakpoints in use.
+ */
+static bool kvm_arm_hw_debug_active(CPUState *cs)
+{
+return ((cur_hw_wps > 0) || (cur_hw_bps > 0));
+}
+
/**
* kvm_arm_copy_hw_debug_data:
* @ptr: kvm_guest_debug_arch structure
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index ac3120adaf..352643e066 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -73,11 +73,6 @@ void kvm_arch_remove_all_hw_breakpoints(void)
}
}
-bool kvm_arm_hw_debug_active(CPUState *cs)
-{
-return ((cur_hw_wps > 0) || (cur_hw_bps > 0));
-}
-
static bool kvm_arm_set_device_attr(CPUState *cs, struct kvm_device_attr *attr,
const char *name)
{
--
2.34.1
[PATCH 09/21] target/arm/kvm: Inline kvm_arm_steal_time_supported
This function is only used once, and is quite simple.
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 13 -
target/arm/kvm64.c | 7 +--
2 files changed, 1 insertion(+), 19 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index e7c32f6ed0..58c087207f 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -274,14 +274,6 @@ void kvm_arm_add_vcpu_properties(Object *obj);
*/
void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp);
-/**
- * kvm_arm_steal_time_supported:
- *
- * Returns: true if KVM can enable steal time reporting
- * and false otherwise.
- */
-bool kvm_arm_steal_time_supported(void);
-
/**
* kvm_arm_aarch32_supported:
*
@@ -374,11 +366,6 @@ static inline bool kvm_arm_sve_supported(void)
return false;
}
-static inline bool kvm_arm_steal_time_supported(void)
-{
-return false;
-}
-
/*
* These functions should never actually be called without KVM support.
*/
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 6b6db9374c..fca4864b73 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -399,7 +399,7 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
void kvm_arm_steal_time_finalize(ARMCPU *cpu, Error **errp)
{
-bool has_steal_time = kvm_arm_steal_time_supported();
+bool has_steal_time = kvm_check_extension(kvm_state, KVM_CAP_STEAL_TIME);
if (cpu->kvm_steal_time == ON_OFF_AUTO_AUTO) {
if (!has_steal_time || !arm_feature(>env, ARM_FEATURE_AARCH64)) {
@@ -437,11 +437,6 @@ bool kvm_arm_sve_supported(void)
return kvm_check_extension(kvm_state, KVM_CAP_ARM_SVE);
}
-bool kvm_arm_steal_time_supported(void)
-{
-return kvm_check_extension(kvm_state, KVM_CAP_STEAL_TIME);
-}
-
QEMU_BUILD_BUG_ON(KVM_ARM64_SVE_VQ_MIN != 1);
uint32_t kvm_arm_sve_get_vls(CPUState *cs)
--
2.34.1
[PATCH 02/21] target/arm: kvm64: remove a redundant KVM_CAP_SET_GUEST_DEBUG probe
From: Chao Du
The KVM_CAP_SET_GUEST_DEBUG is probed during kvm_init().
gdbserver will fail to start if the CAP is not supported.
So no need to make another probe here, like other targets.
Signed-off-by: Chao Du
Reviewed-by: Richard Henderson
Message-Id: <20231025070726.22689-1-duc...@eswincomputing.com>
Signed-off-by: Richard Henderson
---
target/arm/kvm64.c | 28 +++-
1 file changed, 7 insertions(+), 21 deletions(-)
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 3c175c93a7..b8bb25a1ea 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -32,13 +32,9 @@
#include "hw/acpi/acpi.h"
#include "hw/acpi/ghes.h"
-static bool have_guest_debug;
void kvm_arm_init_debug(KVMState *s)
{
-have_guest_debug = kvm_check_extension(s,
- KVM_CAP_SET_GUEST_DEBUG);
-
max_hw_wps = kvm_check_extension(s, KVM_CAP_GUEST_DEBUG_HW_WPS);
hw_watchpoints = g_array_sized_new(true, true,
sizeof(HWWatchpoint), max_hw_wps);
@@ -1141,33 +1137,23 @@ static const uint32_t brk_insn = 0xd420;
int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
{
-if (have_guest_debug) {
-if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)>saved_insn, 4, 0)
||
-cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)_insn, 4, 1)) {
-return -EINVAL;
-}
-return 0;
-} else {
-error_report("guest debug not supported on this kernel");
+if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)>saved_insn, 4, 0) ||
+cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)_insn, 4, 1)) {
return -EINVAL;
}
+return 0;
}
int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
{
static uint32_t brk;
-if (have_guest_debug) {
-if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *), 4, 0) ||
-brk != brk_insn ||
-cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)>saved_insn, 4, 1))
{
-return -EINVAL;
-}
-return 0;
-} else {
-error_report("guest debug not supported on this kernel");
+if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *), 4, 0) ||
+brk != brk_insn ||
+cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)>saved_insn, 4, 1)) {
return -EINVAL;
}
+return 0;
}
/* See v8 ARM ARM D7.2.27 ESR_ELx, Exception Syndrome Register
--
2.34.1
[PATCH 05/21] target/arm/kvm: Move kvm_arm_copy_hw_debug_data and unexport
Signed-off-by: Richard Henderson
---
target/arm/kvm_arm.h | 10 --
target/arm/kvm.c | 24
target/arm/kvm64.c | 17 -
3 files changed, 24 insertions(+), 27 deletions(-)
diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
index bb284a47de..207b7f21b0 100644
--- a/target/arm/kvm_arm.h
+++ b/target/arm/kvm_arm.h
@@ -462,14 +462,4 @@ bool kvm_arm_handle_debug(CPUState *cs, struct
kvm_debug_exit_arch *debug_exit);
*/
bool kvm_arm_hw_debug_active(CPUState *cs);
-/**
- * kvm_arm_copy_hw_debug_data:
- * @ptr: kvm_guest_debug_arch structure
- *
- * Copy the architecture specific debug registers into the
- * kvm_guest_debug ioctl structure.
- */
-struct kvm_guest_debug_arch;
-void kvm_arm_copy_hw_debug_data(struct kvm_guest_debug_arch *ptr);
-
#endif
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 696bc63e86..2898e680fc 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -1021,6 +1021,30 @@ int kvm_arch_process_async_events(CPUState *cs)
return 0;
}
+/**
+ * kvm_arm_copy_hw_debug_data:
+ * @ptr: kvm_guest_debug_arch structure
+ *
+ * Copy the architecture specific debug registers into the
+ * kvm_guest_debug ioctl structure.
+ */
+static void kvm_arm_copy_hw_debug_data(struct kvm_guest_debug_arch *ptr)
+{
+int i;
+memset(ptr, 0, sizeof(struct kvm_guest_debug_arch));
+
+for (i = 0; i < max_hw_wps; i++) {
+HWWatchpoint *wp = get_hw_wp(i);
+ptr->dbg_wcr[i] = wp->wcr;
+ptr->dbg_wvr[i] = wp->wvr;
+}
+for (i = 0; i < max_hw_bps; i++) {
+HWBreakpoint *bp = get_hw_bp(i);
+ptr->dbg_bcr[i] = bp->bcr;
+ptr->dbg_bvr[i] = bp->bvr;
+}
+}
+
void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
{
if (kvm_sw_breakpoints_active(cs)) {
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 7d937e2539..ac3120adaf 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -73,23 +73,6 @@ void kvm_arch_remove_all_hw_breakpoints(void)
}
}
-void kvm_arm_copy_hw_debug_data(struct kvm_guest_debug_arch *ptr)
-{
-int i;
-memset(ptr, 0, sizeof(struct kvm_guest_debug_arch));
-
-for (i = 0; i < max_hw_wps; i++) {
-HWWatchpoint *wp = get_hw_wp(i);
-ptr->dbg_wcr[i] = wp->wcr;
-ptr->dbg_wvr[i] = wp->wvr;
-}
-for (i = 0; i < max_hw_bps; i++) {
-HWBreakpoint *bp = get_hw_bp(i);
-ptr->dbg_bcr[i] = bp->bcr;
-ptr->dbg_bvr[i] = bp->bvr;
-}
-}
-
bool kvm_arm_hw_debug_active(CPUState *cs)
{
return ((cur_hw_wps > 0) || (cur_hw_bps > 0));
--
2.34.1
Re: [PATCH v6 0/5] gdbstub and TCG plugin improvements
On Mon, Oct 30, 2023 at 3:49 PM Akihiko Odaki wrote:
>
> Based-on: <20231029145033.592566-1-alex.ben...@linaro.org>
> ("[PATCH v2 00/19] Maintainer updates for testing, gdb, semihosting and
> plugins (pre-PR)")
>
> This series extracts fixes and refactorings that can be applied
> independently from "[PATCH v9 00/23] plugins: Allow to read registers".
>
> The patch "target/riscv: Move MISA limits to class" was replaced with
> patch "target/riscv: Move misa_mxl_max to class" since I found instances
> may have different misa_ext_mask.
What instances? Couldn't MXL have the same differences?
Alistair
>
> V5 -> V6:
> Added patch "default-configs: Add TARGET_XML_FILES definition".
> Rebased.
>
> V4 -> V5:
> Added patch "hw/riscv: Use misa_mxl instead of misa_mxl_max".
>
> V3 -> V4:
> Added patch "gdbstub: Check if gdb_regs is NULL".
>
> V2 -> V3:
> Restored patch sets from the previous version.
> Rebased to commit 800485762e6564e04e2ab315132d477069562d91.
>
> V1 -> V2:
> Added patch "target/riscv: Do not allow MXL_RV32 for TARGET_RISCV64".
> Added patch "target/riscv: Initialize gdb_core_xml_file only once".
> Dropped patch "target/riscv: Remove misa_mxl validation".
> Dropped patch "target/riscv: Move misa_mxl_max to class".
> Dropped patch "target/riscv: Validate misa_mxl_max only once".
>
> Akihiko Odaki (5):
> hw/riscv: Use misa_mxl instead of misa_mxl_max
> target/riscv: Remove misa_mxl validation
> target/riscv: Move misa_mxl_max to class
> target/riscv: Validate misa_mxl_max only once
> default-configs: Add TARGET_XML_FILES definition
>
> configs/targets/loongarch64-linux-user.mak | 1 +
> target/riscv/cpu-qom.h | 1 +
> target/riscv/cpu.h | 3 +-
> hw/riscv/boot.c| 2 +-
> target/riscv/cpu.c | 139 -
> target/riscv/gdbstub.c | 12 +-
> target/riscv/kvm/kvm-cpu.c | 10 +-
> target/riscv/machine.c | 7 +-
> target/riscv/tcg/tcg-cpu.c | 42 +--
> target/riscv/translate.c | 3 +-
> 10 files changed, 109 insertions(+), 111 deletions(-)
>
> --
> 2.42.0
>
>
Re: [PATCH v6 2/5] target/riscv: Remove misa_mxl validation
On Mon, Oct 30, 2023 at 3:50 PM Akihiko Odaki wrote:
>
> It is initialized with a simple assignment and there is little room for
> error. In fact, the validation is even more complex.
>
> Signed-off-by: Akihiko Odaki
> Acked-by: LIU Zhiwei
> Reviewed-by: Daniel Henrique Barboza
Acked-by: Alistair Francis
Alistair
> ---
> target/riscv/tcg/tcg-cpu.c | 13 ++---
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index a28918ab30..7f45e42000 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -148,7 +148,7 @@ static void riscv_cpu_validate_misa_priv(CPURISCVState
> *env, Error **errp)
> }
> }
>
> -static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu, Error **errp)
> +static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu)
> {
> RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(cpu);
> CPUClass *cc = CPU_CLASS(mcc);
> @@ -168,11 +168,6 @@ static void riscv_cpu_validate_misa_mxl(RISCVCPU *cpu,
> Error **errp)
> default:
> g_assert_not_reached();
> }
> -
> -if (env->misa_mxl_max != env->misa_mxl) {
> -error_setg(errp, "misa_mxl_max must be equal to misa_mxl");
> -return;
> -}
> }
>
> static void riscv_cpu_validate_priv_spec(RISCVCPU *cpu, Error **errp)
> @@ -573,11 +568,7 @@ static bool tcg_cpu_realize(CPUState *cs, Error **errp)
> return false;
> }
>
> -riscv_cpu_validate_misa_mxl(cpu, _err);
> -if (local_err != NULL) {
> -error_propagate(errp, local_err);
> -return false;
> -}
> +riscv_cpu_validate_misa_mxl(cpu);
>
> riscv_cpu_validate_priv_spec(cpu, _err);
> if (local_err != NULL) {
> --
> 2.42.0
>
>
Re: [PATCH v6 1/5] hw/riscv: Use misa_mxl instead of misa_mxl_max
On Mon, Oct 30, 2023 at 3:50 PM Akihiko Odaki wrote:
>
> The effective MXL value matters when booting.
This doesn't sound right. Surely the max is what matters here
Also, this was specifically changed to misa_mxl_max in db23e5d981a
"target/riscv: Replace riscv_cpu_is_32bit with riscv_cpu_mxl".
This needs a much better description of why this change should be made
Alistair
>
> Signed-off-by: Akihiko Odaki
> ---
> hw/riscv/boot.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 52bf8e67de..dad3f6e7b1 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -36,7 +36,7 @@
>
> bool riscv_is_32bit(RISCVHartArrayState *harts)
> {
> -return harts->harts[0].env.misa_mxl_max == MXL_RV32;
> +return harts->harts[0].env.misa_mxl == MXL_RV32;
> }
>
> /*
> --
> 2.42.0
>
>
[PATCH v3 3/3] hw/nvme: Add SPDM over DOE support
From: Wilfred Mallawa Setup Data Object Exchance (DOE) as an extended capability for the NVME controller and connect SPDM to it (CMA) to it. Signed-off-by: Wilfred Mallawa Signed-off-by: Alistair Francis Reviewed-by: Jonathan Cameron Acked-by: Klaus Jensen --- docs/specs/index.rst| 1 + docs/specs/spdm.rst | 121 include/hw/pci/pci_device.h | 5 ++ include/hw/pci/pcie_doe.h | 3 + hw/nvme/ctrl.c | 53 5 files changed, 183 insertions(+) create mode 100644 docs/specs/spdm.rst diff --git a/docs/specs/index.rst b/docs/specs/index.rst index b3f482b0aa..fbbb702c7d 100644 --- a/docs/specs/index.rst +++ b/docs/specs/index.rst @@ -28,6 +28,7 @@ guest hardware that is specific to QEMU. edu ivshmem-spec pvpanic + spdm standard-vga virt-ctlr vmcoreinfo diff --git a/docs/specs/spdm.rst b/docs/specs/spdm.rst new file mode 100644 index 00..a592181c7e --- /dev/null +++ b/docs/specs/spdm.rst @@ -0,0 +1,121 @@ +== +QEMU Security Protocols and Data Models (SPDM) Support +== + +SPDM enables authentication, attestation and key exchange to assist in +providing infrastructure security enablement. It's a standard published +by the `DMTF`_. + +QEMU supports connecting to a SPDM responder implementation. This allows an +external application to emulate the SPDM responder logic for an SPDM device. + +Setting up a SPDM server + + +When using QEMU with SPDM devices QEMU will connect to a server which +implements the SPDM functionality. + +SPDM-Utils +-- + +You can use `SPDM Utils`_ to emulate a responder. + +SPDM-Utils is a Linux applications to manage, test and develop devices +supporting DMTF Security Protocol and Data Model (SPDM). It is written in Rust +and utilises libspdm. + +To use SPDM-Utils you will need to do the following: + + 1. `Build SPDM Utils`_ + 2. `Generate the certificates`_ + 3. `Run it as a server`_ + +spdm-emu + + +You can use `spdm emu`_ to model the +SPDM responder. + +.. code-block:: shell + +$ cd spdm-emu +$ git submodule init; git submodule update --recursive +$ mkdir build; cd build +$ cmake -DARCH=x64 -DTOOLCHAIN=GCC -DTARGET=Debug -DCRYPTO=openssl .. +$ make -j32 +$ make copy_sample_key # Build certificates, required for SPDM authentication. + +It is worth noting that the certificates should be in compliance with +PCIe r6.1 sec 6.31.3. This means you will need to add the following to +openssl.cnf + +.. code-block:: + +subjectAltName = otherName:2.23.147;UTF8:Vendor=1b36:Device=0010:CC=010802:REV=02:SSVID=1af4:SSID=1100 +2.23.147 = ASN1:OID:2.23.147 + +and then manually regenerate some certificates with: + +.. code-block:: shell + +$ openssl req -nodes -newkey ec:param.pem -keyout end_responder.key \ +-out end_responder.req -sha384 -batch \ +-subj "/CN=DMTF libspdm ECP384 responder cert" + +$ openssl x509 -req -in end_responder.req -out end_responder.cert \ +-CA inter.cert -CAkey inter.key -sha384 -days 3650 -set_serial 3 \ +-extensions v3_end -extfile ../openssl.cnf + +$ openssl asn1parse -in end_responder.cert -out end_responder.cert.der + +$ cat ca.cert.der inter.cert.der end_responder.cert.der > bundle_responder.certchain.der + +You can use SPDM-Utils instead as it will generate the correct certificates +automatically. + +The responder can then be launched with + +.. code-block:: shell + +$ cd bin +$ ./spdm_responder_emu --trans PCI_DOE + +Connecting an SPDM NVMe device +== + +Once a SPDM server is running we can start QEMU and connect to the server. + +For an NVMe device first let's setup a block we can use + +.. code-block:: shell + +$ cd qemu-spdm/linux/image +$ dd if=/dev/zero of=blknvme bs=1M count=2096 # 2GB NNMe Drive + +Then you can add this to your QEMU command line: + +.. code-block:: shell + +-drive file=blknvme,if=none,id=mynvme,format=raw \ +-device nvme,drive=mynvme,serial=deadbeef,spdm=2323 + +At which point QEMU will try to connect to the SPDM server. + + +.. _DMTF: + https://www.dmtf.org/standards/SPDM + +.. _SPDM Utils: + https://github.com/westerndigitalcorporation/spdm-utils + +.. _spdm emu: + https://github.com/dmtf/spdm-emu + +.. _Build SPDM Utils: + https://github.com/westerndigitalcorporation/spdm-utils#building + +.. _Generate the certificates: + https://github.com/westerndigitalcorporation/spdm-utils#generate-mutable-certificates + +.. _Run it as a server: + https://github.com/westerndigitalcorporation/spdm-utils#qemu-spdm-device-emulation diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h index d3dd0f64b2..b8379c78f1 100644 --- a/include/hw/pci/pci_device.h +++ b/include/hw/pci/pci_device.h @@ -3,6 +3,7 @@ #include
[PATCH v3 2/3] backends: Initial support for SPDM socket support
From: Huai-Cheng Kuo
SPDM enables authentication, attestation and key exchange to assist in
providing infrastructure security enablement. It's a standard published
by the DMTF [1].
SPDM supports multiple transports, including PCIe DOE and MCTP.
This patch adds support to QEMU to connect to an external SPDM
instance.
SPDM support can be added to any QEMU device by exposing a
TCP socket to a SPDM server. The server can then implement the SPDM
decoding/encoding support, generally using libspdm [2].
This is similar to how the current TPM implementation works and means
that the heavy lifting of setting up certificate chains, capabilities,
measurements and complex crypto can be done outside QEMU by a well
supported and tested library.
1: https://www.dmtf.org/standards/SPDM
2: https://github.com/DMTF/libspdm
Signed-off-by: Huai-Cheng Kuo
Signed-off-by: Chris Browy
Co-developed-by: Jonathan Cameron
Signed-off-by: Jonathan Cameron
[ Changes by WM
- Bug fixes from testing
]
Signed-off-by: Wilfred Mallawa
[ Changes by AF:
- Convert to be more QEMU-ified
- Move to backends as it isn't PCIe specific
]
Signed-off-by: Alistair Francis
---
include/sysemu/spdm-socket.h | 44 +++
backends/spdm-socket.c | 216 +++
backends/Kconfig | 4 +
backends/meson.build | 2 +
4 files changed, 266 insertions(+)
create mode 100644 include/sysemu/spdm-socket.h
create mode 100644 backends/spdm-socket.c
diff --git a/include/sysemu/spdm-socket.h b/include/sysemu/spdm-socket.h
new file mode 100644
index 00..24e6fccb83
--- /dev/null
+++ b/include/sysemu/spdm-socket.h
@@ -0,0 +1,44 @@
+/*
+ * QEMU SPDM socket support
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef SPDM_REQUESTER_H
+#define SPDM_REQUESTER_H
+
+int spdm_socket_connect(uint16_t port, Error **errp);
+uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type,
+ void *req, uint32_t req_len,
+ void *rsp, uint32_t rsp_len);
+void spdm_socket_close(const int socket, uint32_t transport_type);
+
+#define SPDM_SOCKET_COMMAND_NORMAL0x0001
+#define SPDM_SOCKET_COMMAND_OOB_ENCAP_KEY_UPDATE 0x8001
+#define SPDM_SOCKET_COMMAND_CONTINUE 0xFFFD
+#define SPDM_SOCKET_COMMAND_SHUTDOWN 0xFFFE
+#define SPDM_SOCKET_COMMAND_UNKOWN0x
+#define SPDM_SOCKET_COMMAND_TEST 0xDEAD
+
+#define SPDM_SOCKET_TRANSPORT_TYPE_MCTP 0x01
+#define SPDM_SOCKET_TRANSPORT_TYPE_PCI_DOE0x02
+
+#define SPDM_SOCKET_MAX_MESSAGE_BUFFER_SIZE 0x1200
+
+#endif
diff --git a/backends/spdm-socket.c b/backends/spdm-socket.c
new file mode 100644
index 00..d0663d696c
--- /dev/null
+++ b/backends/spdm-socket.c
@@ -0,0 +1,216 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * QEMU SPDM socket support
+ *
+ * This is based on:
+ *
https://github.com/DMTF/spdm-emu/blob/07c0a838bcc1c6207c656ac75885c0603e344b6f/spdm_emu/spdm_emu_common/command.c
+ * but has been re-written to match QEMU style
+ *
+ * Copyright (c) 2021, DMTF. All rights reserved.
+ * Copyright (c) 2023. Western Digital Corporation or its affiliates.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/spdm-socket.h"
+#include "qapi/error.h"
+
+static bool read_bytes(const int socket, uint8_t *buffer,
+ size_t number_of_bytes)
+{
+ssize_t number_received = 0;
+ssize_t result;
+
+while (number_received < number_of_bytes) {
+result = recv(socket, buffer + number_received,
+ number_of_bytes - number_received, 0);
+if (result <= 0) {
+return false;
+}
+number_received += result;
+}
+return true;
+}
+
+static bool read_data32(const int socket, uint32_t *data)
+{
+bool result;
+
+result = read_bytes(socket, (uint8_t *)data, sizeof(uint32_t));
+if (!result) {
+
[PATCH v3 1/3] hw/pci: Add all Data Object Types defined in PCIe r6.0
Add all of the defined protocols/features from the PCIe-SIG r6.0 "Table 6-32 PCI-SIG defined Data Object Types (Vendor ID = 0001h)" table. Signed-off-by: Alistair Francis Reviewed-by: Jonathan Cameron --- include/hw/pci/pcie_doe.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/hw/pci/pcie_doe.h b/include/hw/pci/pcie_doe.h index 87dc17dcef..15d94661f9 100644 --- a/include/hw/pci/pcie_doe.h +++ b/include/hw/pci/pcie_doe.h @@ -46,6 +46,8 @@ REG32(PCI_DOE_CAP_STATUS, 0) /* PCI-SIG defined Data Object Types - r6.0 Table 6-32 */ #define PCI_SIG_DOE_DISCOVERY 0x00 +#define PCI_SIG_DOE_CMA 0x01 +#define PCI_SIG_DOE_SECURED_CMA 0x02 #define PCI_DOE_DW_SIZE_MAX (1 << 18) #define PCI_DOE_PROTOCOL_NUM_MAX256 -- 2.42.0
[PATCH v3 0/3] Initial support for SPDM Responders
The Security Protocol and Data Model (SPDM) Specification defines messages, data objects, and sequences for performing message exchanges over a variety of transport and physical media. - https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf SPDM currently supports PCIe DOE and MCTP transports, but it can be extended to support others in the future. This series adds support to QEMU to connect to an external SPDM instance. SPDM support can be added to any QEMU device by exposing a TCP socket to a SPDM server. The server can then implement the SPDM decoding/encoding support, generally using libspdm [1]. This is similar to how the current TPM implementation works and means that the heavy lifting of setting up certificate chains, capabilities, measurements and complex crypto can be done outside QEMU by a well supported and tested library. This series implements socket support and exposes SPDM for a NVMe device. 1: https://github.com/DMTF/libspdm v3: - Spelling fixes - Support for SPDM-Utils v2: - Add cover letter - A few code fixes based on comments - Document SPDM-Utils - A few tweaks and clarifications to the documentation Alistair Francis (1): hw/pci: Add all Data Object Types defined in PCIe r6.0 Huai-Cheng Kuo (1): backends: Initial support for SPDM socket support Wilfred Mallawa (1): hw/nvme: Add SPDM over DOE support docs/specs/index.rst | 1 + docs/specs/spdm.rst | 121 include/hw/pci/pci_device.h | 5 + include/hw/pci/pcie_doe.h| 5 + include/sysemu/spdm-socket.h | 44 +++ backends/spdm-socket.c | 216 +++ hw/nvme/ctrl.c | 53 + backends/Kconfig | 4 + backends/meson.build | 2 + 9 files changed, 451 insertions(+) create mode 100644 docs/specs/spdm.rst create mode 100644 include/sysemu/spdm-socket.h create mode 100644 backends/spdm-socket.c -- 2.42.0
Re: [PATCH] tests/avocado/reverse_debugging: Disable the ppc64 tests by default
On Tue Nov 21, 2023 at 5:18 AM AEST, John Snow wrote:
> On Wed, Nov 15, 2023 at 12:23 PM Daniel P. Berrangé
> wrote:
> >
> > On Wed, Nov 15, 2023 at 01:14:53PM +, Daniel P. Berrangé wrote:
> > > On Wed, Nov 15, 2023 at 07:23:01AM +0100, Thomas Huth wrote:
> > > > On 15/11/2023 02.15, Nicholas Piggin wrote:
> > > > > On Wed Nov 15, 2023 at 4:29 AM AEST, Thomas Huth wrote:
> > > > > > On 14/11/2023 17.37, Philippe Mathieu-Daudé wrote:
> > > > > > > On 14/11/23 17:31, Thomas Huth wrote:
> > > > > > > > The tests seem currently to be broken. Disable them by default
> > > > > > > > until someone fixes them.
> > > > > > > >
> > > > > > > > Signed-off-by: Thomas Huth
> > > > > > > > ---
> > > > > > > >tests/avocado/reverse_debugging.py | 7 ---
> > > > > > > >1 file changed, 4 insertions(+), 3 deletions(-)
> > > > > > >
> > > > > > > Similarly, I suspect
> > > > > > > https://gitlab.com/qemu-project/qemu/-/issues/1961
> > > > > > > which has a fix ready:
> > > > > > > https://lore.kernel.org/qemu-devel/20231110170831.185001-1-richard.hender...@linaro.org/
> > > > > > >
> > > > > > > Maybe wait the fix gets in first?
> > > > > >
> > > > > > No, I applied Richard's patch, but the problem persists. Does this
> > > > > > test
> > > > > > still work for you?
> > > > >
> > > > > I bisected it to 1d4796cd008373 ("python/machine: use socketpair() for
> > > > > console connections"),
> > > >
> > > > Maybe John (who wrote that commit) can help?
> > >
> > > I find it hard to believe this commit is a direct root cause of the
> > > problem since all it does is change the QEMU startup sequence so that
> > > instead of QEMU listening for a monitor connection, it is given a
> > > pre-opened monitor connection.
> > >
> > > At the very most that should affect the startup timing a little.
> > >
> > > I notice all the reverse debugging tests have a skip on gitlab
> > > with a comment:
> > >
> > > # unidentified gitlab timeout problem
> > >
> > > this makes be suspicious that John's patch has merely made this
> > > (henceforth undiagnosed) timeout more likely to ocurr.
> >
> > After an absolutely horrendous hours long debugging session I think
> > I figured out the problem. The QEMU process is blocking in
> >
> > qemu_chr_write_buffer
> >
> > spinning in the loop on EAGAIN.
> >
> > The Python Machine() class has passed one of a pre-created socketpair
> > FDs for the serial port chardev. The guest is trying to write to this
> > and blocking. Nothing in the Machine() class is reading from the
> > other end of the serial port console.
> >
> >
> > Before John's change, the serial port uses a chardev in server mode
> > and crucially 'wait=off', and the Machine() class never opened the
> > console socket unless the test case wanted to read from it.
> >
> > IOW, QEMU had a background job setting there waiting for a connection
> > that would never come.
> >
> > As a result when QEMU started executing the guest, all the serial port
> > writes get sent into to the void.
> >
> >
> > So John's patch has had a semantic change in behaviour, because the
> > console socket is permanently open, and thus socket buffers are liable
> > to fill up.
> >
> > As a demo I increased the socket buffers to 1MB and everything then
> > succeeded.
> >
> > @@ -357,6 +360,10 @@ def _pre_launch(self) -> None:
> >
> > if self._console_set:
> > self._cons_sock_pair = socket.socketpair()
> > +self._cons_sock_pair[0].setsockopt(socket.SOL_SOCKET,
> > socket.SO_SNDBUF, 1024*1024);
> > +self._cons_sock_pair[0].setsockopt(socket.SOL_SOCKET,
> > socket.SO_RCVBUF, 1024*1024);
> > +self._cons_sock_pair[1].setsockopt(socket.SOL_SOCKET,
> > socket.SO_SNDBUF, 1024*1024);
> > +self._cons_sock_pair[1].setsockopt(socket.SOL_SOCKET,
> > socket.SO_RCVBUF, 1024*1024);
> > os.set_inheritable(self._cons_sock_pair[0].fileno(), True)
> >
> > # NOTE: Make sure any opened resources are *definitely* freed in
> >
> >
> > The Machine class doesn't know if anything will ever use the console,
> > so as is the change is unsafe.
> >
> > The original goal of John's change was to guarantee we capture early
> > boot messages as some test need that.
> >
> > I think we need to be able to have a flag to say whether the caller needs
> > an "early console" facility, and only use the pre-opened FD passing for
> > that case. Tests we need early console will have to ask for that guarantee
> > explicitly.
>
> Tch. I see. Thank you for diagnosing this.
>
> From the machine.py perspective, you have to *opt in* to having a
> console, so I hadn't considered that a caller would enable the console
> and then ... not read from it. Surely that's a bug in the caller?
>
> If you don't intend to read from the console, you shouldn't call
> set_console().
Agree, hence the fix patch for the test case.
Although most tests wait for console, ones like this that control the
machine with gdb/qmp are
Re: [PATCH v4 03/11] ppc/pnv: New powernv10-rainier machine type
On Tue Nov 21, 2023 at 5:29 PM AEST, Cédric Le Goater wrote: > On 11/21/23 02:33, Nicholas Piggin wrote: > > On Tue Nov 21, 2023 at 9:51 AM AEST, Glenn Miles wrote: > >> Create a new powernv machine type, powernv10-rainier, that > >> will contain rainier-specific devices. > > > > Is the plan to have a base powernv10 common to all and then > > powernv10-rainier looks like a Rainier? Or would powernv10 > > just be a rainier? > > > > It's fine to structure code this way, I'm just wondering about > > the machine types available to user. Is a base powernv10 machine > > useful to run? > > There are multiple P10 boards defined in Linux : > >aspeed-bmc-ibm-bonnell.dts >aspeed-bmc-ibm-everest.dts >aspeed-bmc-ibm-rainier-1s4u.dts >aspeed-bmc-ibm-rainier-4u.dts >aspeed-bmc-ibm-rainier.dts > > and we could model the machines above with a fixed number of sockets. > The "powernv10" would be the generic system that can be customized > at will on the command line, even I2C devices. If a bare qemu machine could be useful, I don't have a problem with it. I'm more thinking of what an average OPAL/PowerNV Linux user developer would want, they (I) would probably want to use powernv, powernv9, or powernv10, and just get a reasonable "realistic" machine. The bare system could be powernv10-generic or powernv10-minimal for those who know what they're doing. > There is also the > P10 Denali which is FSP based. This QEMU machine would certainly be > very different. I thought of doing the same for P9 with a -zaius > and include NPU2 models for it. I lacked time and the interest was > small at the time of OpenPOWER. > > Anyhow, adding a new machine makes sense and it prepares ground for > possible new ones. I am OK with or without. As primary users, you are > the ones that can tell if there will be a second machine. Yeah we will want to add other machines at some point, I think this does make sense, my only real concern is what we call them. Thanks, Nick
[PATCH] RISC-V: Increase max vlen to 4096
QEMU currently limits the max vlenb to 1024. GCC sets the upper bound
to 4096 [1]. There doesn't seem to be an upper bound set by the spec [2]
so this patch just changes QEMU to match GCC's upper bound.
[1]
https://github.com/gcc-mirror/gcc/blob/5d2a360f0a541646abb11efdbabc33c6a04de7ee/gcc/testsuite/gcc.target/riscv/rvv/base/zvl-unimplemented-2.c#L4
[2] https://github.com/riscv/riscv-v-spec/issues/204
Signed-off-by: Patrick O'Neill
---
Tested by applying to QEMU v8.1.2 and running the GCC testsuite in QEMU
user mode with rv64gcv_zvl4096b. Failures are somewhat reasonable and on
first inspection appear to be in the same ballpark as failures for
rv64gcv_zvl1024b. Since I used tip-of-tree GCC I'm expecting those
failures to be GCC-caused & from skimming the debug log they appear to
be.
---
target/riscv/cpu.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6ea22e0eea..2ff3a72fc0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -97,7 +97,7 @@ typedef enum {
#include "debug.h"
#endif
-#define RV_VLEN_MAX 1024
+#define RV_VLEN_MAX 4096
#define RV_MAX_MHPMEVENTS 32
#define RV_MAX_MHPMCOUNTERS 32
--
2.34.1
Re: [PATCH] target/ppc: Update gdbstub to read SPR's CFAR, DEC, HDEC, TB-L/U
On Mon Sep 18, 2023 at 7:26 PM AEST, Saif Abrar wrote:
> SPR's CFAR, DEC, HDEC, TB-L/U are not implemented as part of CPUPPCState.
> Hence, gdbstub is not able to access them using (CPUPPCState *)env->spr[]
> array.
> Update gdb_get_spr_reg() method to handle these SPR's specifically.
>
> Signed-off-by: Saif Abrar
Thanks for this, I have been annoyed by missing those regs when using
gdb at times :)
> ---
> target/ppc/gdbstub.c | 40 ++--
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
> index 2ad11510bf..eb086c0168 100644
> --- a/target/ppc/gdbstub.c
> +++ b/target/ppc/gdbstub.c
> @@ -412,7 +412,32 @@ static int gdb_get_spr_reg(CPUPPCState *env, GByteArray
> *buf, int n)
> }
>
> len = TARGET_LONG_SIZE;
> -gdb_get_regl(buf, env->spr[reg]);
> +
> +/* Handle those SPRs that are not part of the env->spr[] array */
> +target_ulong val;
Could you move this to the of the block with other declarations?
> +switch (reg) {
> +#if defined(TARGET_PPC64)
> +case SPR_CFAR:
> +val = env->cfar;
> +break;
> +#endif
> +case SPR_HDEC:
> +val = cpu_ppc_load_hdecr(env);
> +break;
> +case SPR_TBL:
> +val = cpu_ppc_load_tbl(env);
> +break;
> +case SPR_TBU:
> +val = cpu_ppc_load_tbu(env);
> +break;
> +case SPR_DECR:
> +val = cpu_ppc_load_decr(env);
> +break;
> +default:
> +val = env->spr[reg];
> +}
> +gdb_get_regl(buf, val);
> +
> ppc_maybe_bswap_register(env, gdb_get_reg_ptr(buf, len), len);
> return len;
> }
> @@ -429,7 +454,18 @@ static int gdb_set_spr_reg(CPUPPCState *env, uint8_t
> *mem_buf, int n)
>
> len = TARGET_LONG_SIZE;
> ppc_maybe_bswap_register(env, mem_buf, len);
> -env->spr[reg] = ldn_p(mem_buf, len);
> +
> +/* Handle those SPRs that are not part of the env->spr[] array */
> +target_ulong val = ldn_p(mem_buf, len);
> +switch (reg) {
> +#if defined(TARGET_PPC64)
> +case SPR_CFAR:
> +env->cfar = val;
> +break;
> +#endif
I suppose we could store some time regs here too. (h)decr I have found
useful to change at times to interrupts when debugging Linux timer code.
TB could be similar.
We have a bit of weirdness with our timebase registers though, I'm going
to send out some patches for them. Maybe hold off changing this until
we agree on those.
I'll take this for next release, sorry didn't get to it earlier.
Thanks,
Nick
> +default:
> +env->spr[reg] = val;
> +}
>
> return len;
> }
Re: [PATCH v2 3/6] xen: xenstore: add possibility to preserve owner
Hi David,
David Woodhouse writes:
> [[S/MIME Signed Part:Undecided]]
> On Tue, 2023-11-21 at 22:10 +, Volodymyr Babchuk wrote:
>>
>> --- a/hw/xen/xen-operations.c
>> +++ b/hw/xen/xen-operations.c
>> @@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle
>> *h, xs_transaction_t t,
>> return false;
>> }
>>
>> + if (owner == XS_PRESERVE_OWNER) {
>> + struct xs_permissions *tmp;
>> + unsigned int num;
>> +
>> + tmp = xs_get_permissions(h->xsh, t, path, );
>> + if (tmp == NULL) {
>> + return false;
>> + }
>> + perms_list[0].id = tmp[0].id;
>> + free(tmp);
>> + }
>> +
>> return xs_set_permissions(h->xsh, t, path, perms_list,
>> ARRAY_SIZE(perms_list));
>> }
>
> If the existing transaction is XBT_NULL I think you want to create a
> new transaction for it, don't you?
I must say that your comment is valid even without my
changes. xenstore_mkdir() calls qemu_xen_xs_create, providing just plain
"0" (not even XBT_NULL) as a transaction, but actual xenstore interface
implementation, like xs_be_create(), issue multiple calls to lower
layer, passing "t" downwards. For example, xs_be_create() calls
xs_impl_read, xs_impl_write and xs_impl_set_perms(). If called from
xesntore_mkdir(), those three operations will be non-atomic. I don't
know if this can lead to a problem, because apparently it was so for a
long time...
--
WBR, Volodymyr
Re: [PATCH v2 6/6] xen_arm: Add virtual PCIe host bridge support
Hi Vikram, Vikram Garhwal writes: > Hi Volodymyr, > Thank you sharing this patch. I have few comments below > On Wed, Nov 22, 2023 at 02:39:46PM -0800, Stefano Stabellini wrote: >> +Vikram >> >> On Tue, 21 Nov 2023, Volodymyr Babchuk wrote: >> > From: Oleksandr Tyshchenko >> > >> > The bridge is needed for virtio-pci support, as QEMU can emulate the >> > whole bridge with any virtio-pci devices connected to it. >> > >> > This patch provides a flexible way to configure PCIe brige resources >> > with xenstore. We made this for several reasons: >> > >> > - We don't want to clash with vPCI devices, so we need information >> > from Xen toolstack on which PCI bus to use. >> > - The guest memory layout that describes these resources is not stable >> > and may vary between guests, so we cannot rely on static resources >> > to be always the same for both ends. >> > - Also the device-models which run in different domains and serve >> > virtio-pci devices for the same guest should use different host >> > bridge resources for Xen to distinguish. The rule for the guest >> > device-tree generation is one PCI host bridge per backend domain. >> > >> > Signed-off-by: Oleksandr Tyshchenko >> > Signed-off-by: Volodymyr Babchuk >> >> There is still a discussion ongoing on xen-devel if / how to register a >> PCI Root Complex or individual BDFs. In the meantime a couple of >> comments. >> >> Typically emulated devices are configured in QEMU via QEMU command line >> parameters, not xenstore. If you are running QEMU in a domU (instead of >> Dom0) you can always read config parameters from xenstore from a wrapper >> bash script (using xenstore-read) and then pass the right command line >> options to QEMU. >> >> If you need help in adding new QEMU command line options, Vikram (CCed) >> can help. >> >> > I agree with Stefano here. Setting properties would be better and easier. Okay, I'll look at this. > I have one questions here: > 1. If there are multiple QEMU backends, which means xen tools will end up > creating lot of entries in xenstore and we need to remove those xenstore > entries when backend goes away. Is this already handled in Xen tools? Well, this is not a classic PV backend, so common code in Xen Tools does not handle those entries. I am not sure if tools remove entries right now, because I am not the original author. But we definitely will remove them in the final version of patches. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi, Volodymyr Babchuk writes: > Hi Stefano, > > Stefano Stabellini writes: > >> On Wed, 22 Nov 2023, David Woodhouse wrote: >>> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >>> > On Wed, 22 Nov 2023, David Woodhouse wrote: >>> > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >>> > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >>> > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >>> > > > > > From: Oleksandr Tyshchenko >>> > > > > > >>> > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >>> > > > > > inherit the owner of the directory. >>> > > > > >>> > > > > Ah... so that's why the previous patch is there. >>> > > > > >>> > > > > This is not the right way to fix it. The QEMU Xen support is >>> > > > > *assuming* that >>> > > > > QEMU is either running in, or emulating, dom0. In the emulation >>> > > > > case this is >>> > > > > probably fine, but the 'real Xen' case it should be using the >>> > > > > correct domid >>> > > > > for node creation. I guess this could either be supplied on the >>> > > > > command line >>> > > > > or discerned by reading the local domain 'domid' node. >>> > > > >>> > > > yes, it should be passed as command line option to QEMU >>> > > >>> > > I'm not sure I like the idea of a command line option for something >>> > > which QEMU could discover for itself. >>> > >>> > That's fine too. I meant to say "yes, as far as I know the toolstack >>> > passes the domid to QEMU as a command line option today". >>> >>> The -xen-domid argument on the QEMU command line today is the *guest* >>> domain ID, not the domain ID in which QEMU itself is running. >>> >>> Or were you thinking of something different? >> >> Ops, you are right and I understand your comment better now. The backend >> domid is not on the command line but it should be discoverable (on >> xenstore if I remember right). > > Yes, it is just "~/domid". I'll add a function that reads it. Just a quick question to QEMU folks: is it better to add a global variable where we will store own Domain ID or it will be okay to read domid from Xenstore every time we need it? If global variable variant is better, what is proffered place to define this variable? system/globals.c ? -- WBR, Volodymyr
Re: [PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
On Wed, 2023-11-22 at 23:49 +, Volodymyr Babchuk wrote: > > I can just pull it from this link, if you don't mind. Please do; thank you! smime.p7s Description: S/MIME cryptographic signature Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.
Re: [PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
Hi David, "Woodhouse, David" writes: > On Wed, 2023-11-22 at 17:05 +, Paul Durrant wrote: >> On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> > From: David Woodhouse >> > >> > This allows a XenDevice implementation to know whether it was created >> > by QEMU, or merely discovered in XenStore after the toolstack created >> > it. This will allow us to create frontend/backend nodes only when we >> > should, rather than unconditionally attempting to overwrite them from >> > a driver domain which doesn't have privileges to do so. >> > >> > As an added benefit, it also means we no longer have to call the >> > xen_backend_set_device() function from the device models immediately >> > after calling qdev_realize_and_unref(). Even though we could make >> > the argument that it's safe to do so, and the pointer to the unreffed >> > device *will* actually still be valid, it still made my skin itch to >> > look at it. >> > >> > Signed-off-by: David Woodhouse >> > --- >> > hw/block/xen-block.c | 3 +-- >> > hw/char/xen_console.c | 2 +- >> > hw/net/xen_nic.c | 2 +- >> > hw/xen/xen-bus.c | 4 >> > include/hw/xen/xen-backend.h | 2 -- >> > include/hw/xen/xen-bus.h | 2 ++ >> > 6 files changed, 9 insertions(+), 6 deletions(-) >> > >> >> Actually, I think you should probably update >> xen_backend_try_device_destroy() in this patch too. It currently uses >> xen_backend_list_find() to check whether the specified XenDevice has an >> associated XenBackendInstance. > > I think I did that in > https://git.infradead.org/users/dwmw2/qemu.git/commitdiff/94f1b474478ce0ede > but didn't get round to sending it out again because of travel. I can just pull it from this link, if you don't mind. -- WBR, Volodymyr
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
Hi Stefano, Stefano Stabellini writes: > On Wed, 22 Nov 2023, David Woodhouse wrote: >> On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: >> > On Wed, 22 Nov 2023, David Woodhouse wrote: >> > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: >> > > > On Wed, 22 Nov 2023, Paul Durrant wrote: >> > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> > > > > > From: Oleksandr Tyshchenko >> > > > > > >> > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to >> > > > > > inherit the owner of the directory. >> > > > > >> > > > > Ah... so that's why the previous patch is there. >> > > > > >> > > > > This is not the right way to fix it. The QEMU Xen support is >> > > > > *assuming* that >> > > > > QEMU is either running in, or emulating, dom0. In the emulation case >> > > > > this is >> > > > > probably fine, but the 'real Xen' case it should be using the >> > > > > correct domid >> > > > > for node creation. I guess this could either be supplied on the >> > > > > command line >> > > > > or discerned by reading the local domain 'domid' node. >> > > > >> > > > yes, it should be passed as command line option to QEMU >> > > >> > > I'm not sure I like the idea of a command line option for something >> > > which QEMU could discover for itself. >> > >> > That's fine too. I meant to say "yes, as far as I know the toolstack >> > passes the domid to QEMU as a command line option today". >> >> The -xen-domid argument on the QEMU command line today is the *guest* >> domain ID, not the domain ID in which QEMU itself is running. >> >> Or were you thinking of something different? > > Ops, you are right and I understand your comment better now. The backend > domid is not on the command line but it should be discoverable (on > xenstore if I remember right). Yes, it is just "~/domid". I'll add a function that reads it. -- WBR, Volodymyr
Re: [PATCH v2 6/6] xen_arm: Add virtual PCIe host bridge support
Hi Volodymyr,
Thank you sharing this patch. I have few comments below
On Wed, Nov 22, 2023 at 02:39:46PM -0800, Stefano Stabellini wrote:
> +Vikram
>
> On Tue, 21 Nov 2023, Volodymyr Babchuk wrote:
> > From: Oleksandr Tyshchenko
> >
> > The bridge is needed for virtio-pci support, as QEMU can emulate the
> > whole bridge with any virtio-pci devices connected to it.
> >
> > This patch provides a flexible way to configure PCIe brige resources
> > with xenstore. We made this for several reasons:
> >
> > - We don't want to clash with vPCI devices, so we need information
> > from Xen toolstack on which PCI bus to use.
> > - The guest memory layout that describes these resources is not stable
> > and may vary between guests, so we cannot rely on static resources
> > to be always the same for both ends.
> > - Also the device-models which run in different domains and serve
> > virtio-pci devices for the same guest should use different host
> > bridge resources for Xen to distinguish. The rule for the guest
> > device-tree generation is one PCI host bridge per backend domain.
> >
> > Signed-off-by: Oleksandr Tyshchenko
> > Signed-off-by: Volodymyr Babchuk
>
> There is still a discussion ongoing on xen-devel if / how to register a
> PCI Root Complex or individual BDFs. In the meantime a couple of
> comments.
>
> Typically emulated devices are configured in QEMU via QEMU command line
> parameters, not xenstore. If you are running QEMU in a domU (instead of
> Dom0) you can always read config parameters from xenstore from a wrapper
> bash script (using xenstore-read) and then pass the right command line
> options to QEMU.
>
> If you need help in adding new QEMU command line options, Vikram (CCed)
> can help.
>
>
I agree with Stefano here. Setting properties would be better and easier.
I have one questions here:
1. If there are multiple QEMU backends, which means xen tools will end up
creating lot of entries in xenstore and we need to remove those xenstore
entries when backend goes away. Is this already handled in Xen tools?
Regards,
Vikram
> > ---
> >
> > Changes from v1:
> >
> > - Renamed virtio_pci_host to pcie_host entries in XenStore, because
> > there is nothing specific to virtio-pci: any PCI device can be
> > emulated via this newly created bridge.
> > ---
> > hw/arm/xen_arm.c| 186
> > hw/xen/xen-hvm-common.c | 9 +-
> > include/hw/xen/xen_native.h | 8 +-
> > 3 files changed, 200 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
> > index b9c3ae14b6..d506d55d0f 100644
> > --- a/hw/arm/xen_arm.c
> > +++ b/hw/arm/xen_arm.c
> > @@ -22,6 +22,7 @@
> > */
> >
> > #include "qemu/osdep.h"
> > +#include "qemu/cutils.h"
> > #include "qemu/error-report.h"
> > #include "qapi/qapi-commands-migration.h"
> > #include "qapi/visitor.h"
> > @@ -34,6 +35,9 @@
> > #include "hw/xen/xen-hvm-common.h"
> > #include "sysemu/tpm.h"
> > #include "hw/xen/arch_hvm.h"
> > +#include "exec/address-spaces.h"
> > +#include "hw/pci-host/gpex.h"
> > +#include "hw/virtio/virtio-pci.h"
> >
> > #define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpvh")
> > OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
> > @@ -58,6 +62,11 @@ struct XenArmState {
> > struct {
> > uint64_t tpm_base_addr;
> > } cfg;
> > +
> > +MemMapEntry pcie_mmio;
> > +MemMapEntry pcie_ecam;
> > +MemMapEntry pcie_mmio_high;
> > +int pcie_irq;
> > };
> >
> > static MemoryRegion ram_lo, ram_hi;
> > @@ -73,6 +82,7 @@ static MemoryRegion ram_lo, ram_hi;
> > #define NR_VIRTIO_MMIO_DEVICES \
> > (GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
> >
> > +/* TODO It should be xendevicemodel_set_pci_intx_level() for PCI
> > interrupts. */
> > static void xen_set_irq(void *opaque, int irq, int level)
> > {
> > if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
> > @@ -129,6 +139,176 @@ static void xen_init_ram(MachineState *machine)
> > }
> > }
> >
> > +static void xen_create_pcie(XenArmState *xam)
> > +{
> > +MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
> > +MemoryRegion *ecam_alias, *ecam_reg;
> > +DeviceState *dev;
> > +int i;
> > +
> > +dev = qdev_new(TYPE_GPEX_HOST);
> > +sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
> > +
> > +/* Map ECAM space */
> > +ecam_alias = g_new0(MemoryRegion, 1);
> > +ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> > +memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
> > + ecam_reg, 0, xam->pcie_ecam.size);
> > +memory_region_add_subregion(get_system_memory(), xam->pcie_ecam.base,
> > +ecam_alias);
> > +
> > +/* Map the MMIO space */
> > +mmio_alias = g_new0(MemoryRegion, 1);
> > +mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
> > +
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Wed, 22 Nov 2023, David Woodhouse wrote: > On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: > > On Wed, 22 Nov 2023, David Woodhouse wrote: > > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > > > > On Wed, 22 Nov 2023, Paul Durrant wrote: > > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > > > > From: Oleksandr Tyshchenko > > > > > > > > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to > > > > > > inherit the owner of the directory. > > > > > > > > > > Ah... so that's why the previous patch is there. > > > > > > > > > > This is not the right way to fix it. The QEMU Xen support is > > > > > *assuming* that > > > > > QEMU is either running in, or emulating, dom0. In the emulation case > > > > > this is > > > > > probably fine, but the 'real Xen' case it should be using the correct > > > > > domid > > > > > for node creation. I guess this could either be supplied on the > > > > > command line > > > > > or discerned by reading the local domain 'domid' node. > > > > > > > > yes, it should be passed as command line option to QEMU > > > > > > I'm not sure I like the idea of a command line option for something > > > which QEMU could discover for itself. > > > > That's fine too. I meant to say "yes, as far as I know the toolstack > > passes the domid to QEMU as a command line option today". > > The -xen-domid argument on the QEMU command line today is the *guest* > domain ID, not the domain ID in which QEMU itself is running. > > Or were you thinking of something different? Ops, you are right and I understand your comment better now. The backend domid is not on the command line but it should be discoverable (on xenstore if I remember right).
Re: [PATCH v2 2/6] xen: backends: touch some XenStore nodes only if device...
On Wed, 2023-11-22 at 22:49 +, Volodymyr Babchuk wrote: > > > On 21/11/23 23:10, Volodymyr Babchuk wrote: > > > was created by QEMU > > > > Please do not split lines between subject and content. Rewrite the > > full line. Preferably restrict the subject to 72 chars. > > I tried to come with shorter description, but failed. I'll rewrite it in > the next version. Even if you just put those last four words *into* the subject, it's only 75 characters once the leaving [PATCH...] is stripped. xen: backends: touch some XenStore nodes only if device was created by QEMU And this would be 9 characters fewer: xen: backends: don't overwrite XenStore nodes created by toolstack smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Wed, 2023-11-22 at 15:09 -0800, Stefano Stabellini wrote: > On Wed, 22 Nov 2023, David Woodhouse wrote: > > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > > > On Wed, 22 Nov 2023, Paul Durrant wrote: > > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > > > From: Oleksandr Tyshchenko > > > > > > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to > > > > > inherit the owner of the directory. > > > > > > > > Ah... so that's why the previous patch is there. > > > > > > > > This is not the right way to fix it. The QEMU Xen support is *assuming* > > > > that > > > > QEMU is either running in, or emulating, dom0. In the emulation case > > > > this is > > > > probably fine, but the 'real Xen' case it should be using the correct > > > > domid > > > > for node creation. I guess this could either be supplied on the command > > > > line > > > > or discerned by reading the local domain 'domid' node. > > > > > > yes, it should be passed as command line option to QEMU > > > > I'm not sure I like the idea of a command line option for something > > which QEMU could discover for itself. > > That's fine too. I meant to say "yes, as far as I know the toolstack > passes the domid to QEMU as a command line option today". The -xen-domid argument on the QEMU command line today is the *guest* domain ID, not the domain ID in which QEMU itself is running. Or were you thinking of something different? smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Wed, 22 Nov 2023, David Woodhouse wrote: > On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > > On Wed, 22 Nov 2023, Paul Durrant wrote: > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > > From: Oleksandr Tyshchenko > > > > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to > > > > inherit the owner of the directory. > > > > > > Ah... so that's why the previous patch is there. > > > > > > This is not the right way to fix it. The QEMU Xen support is *assuming* > > > that > > > QEMU is either running in, or emulating, dom0. In the emulation case this > > > is > > > probably fine, but the 'real Xen' case it should be using the correct > > > domid > > > for node creation. I guess this could either be supplied on the command > > > line > > > or discerned by reading the local domain 'domid' node. > > > > yes, it should be passed as command line option to QEMU > > I'm not sure I like the idea of a command line option for something > which QEMU could discover for itself. That's fine too. I meant to say "yes, as far as I know the toolstack passes the domid to QEMU as a command line option today".
Re: [PATCH v2 3/6] xen: xenstore: add possibility to preserve owner
Hi David,
David Woodhouse writes:
> [[S/MIME Signed Part:Undecided]]
> On Tue, 2023-11-21 at 22:10 +, Volodymyr Babchuk wrote:
>>
>> --- a/hw/xen/xen-operations.c
>> +++ b/hw/xen/xen-operations.c
>> @@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle
>> *h, xs_transaction_t t,
>> return false;
>> }
>>
>> + if (owner == XS_PRESERVE_OWNER) {
>> + struct xs_permissions *tmp;
>> + unsigned int num;
>> +
>> + tmp = xs_get_permissions(h->xsh, t, path, );
>> + if (tmp == NULL) {
>> + return false;
>> + }
>> + perms_list[0].id = tmp[0].id;
>> + free(tmp);
>> + }
>> +
>> return xs_set_permissions(h->xsh, t, path, perms_list,
>> ARRAY_SIZE(perms_list));
>> }
>
> If the existing transaction is XBT_NULL I think you want to create a
> new transaction for it, don't you?
As per Stefano's and Paul's comments I'll drop this patch
completely. Thanks for review, thought.
--
WBR, Volodymyr
Re: [PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
On Wed, 2023-11-22 at 22:56 +, Volodymyr Babchuk wrote: > > > Paul Durrant writes: > > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > From: David Woodhouse > > > This allows a XenDevice implementation to know whether it was > > > created > > > by QEMU, or merely discovered in XenStore after the toolstack created > > > it. This will allow us to create frontend/backend nodes only when we > > > should, rather than unconditionally attempting to overwrite them from > > > a driver domain which doesn't have privileges to do so. > > > As an added benefit, it also means we no longer have to call the > > > xen_backend_set_device() function from the device models immediately > > > after calling qdev_realize_and_unref(). Even though we could make > > > the argument that it's safe to do so, and the pointer to the unreffed > > > device *will* actually still be valid, it still made my skin itch to > > > look at it. > > > Signed-off-by: David Woodhouse > > > --- > > > hw/block/xen-block.c | 3 +-- > > > hw/char/xen_console.c | 2 +- > > > hw/net/xen_nic.c | 2 +- > > > hw/xen/xen-bus.c | 4 > > > include/hw/xen/xen-backend.h | 2 -- > > > include/hw/xen/xen-bus.h | 2 ++ > > > 6 files changed, 9 insertions(+), 6 deletions(-) > > > > > > > Actually, I think you should probably update > > xen_backend_try_device_destroy() in this patch too. It currently uses > > xen_backend_list_find() to check whether the specified XenDevice has > > an associated XenBackendInstance. > > Sure. Looks like it is the only user of xen_backend_list_find(), so we > can get rid of it as well. > > I'll drop your R-b tag, because of those additional changes in the new > version. I think it's fine to keep. He told me to do it :) smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Wed, 2023-11-22 at 14:29 -0800, Stefano Stabellini wrote: > On Wed, 22 Nov 2023, Paul Durrant wrote: > > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > > From: Oleksandr Tyshchenko > > > > > > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to > > > inherit the owner of the directory. > > > > Ah... so that's why the previous patch is there. > > > > This is not the right way to fix it. The QEMU Xen support is *assuming* that > > QEMU is either running in, or emulating, dom0. In the emulation case this is > > probably fine, but the 'real Xen' case it should be using the correct domid > > for node creation. I guess this could either be supplied on the command line > > or discerned by reading the local domain 'domid' node. > > yes, it should be passed as command line option to QEMU I'm not sure I like the idea of a command line option for something which QEMU could discover for itself. smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH v2 3/6] xen: xenstore: add possibility to preserve owner
On Tue, 2023-11-21 at 22:10 +, Volodymyr Babchuk wrote:
>
> --- a/hw/xen/xen-operations.c
> +++ b/hw/xen/xen-operations.c
> @@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle *h,
> xs_transaction_t t,
> return false;
> }
>
> + if (owner == XS_PRESERVE_OWNER) {
> + struct xs_permissions *tmp;
> + unsigned int num;
> +
> + tmp = xs_get_permissions(h->xsh, t, path, );
> + if (tmp == NULL) {
> + return false;
> + }
> + perms_list[0].id = tmp[0].id;
> + free(tmp);
> + }
> +
> return xs_set_permissions(h->xsh, t, path, perms_list,
> ARRAY_SIZE(perms_list));
> }
If the existing transaction is XBT_NULL I think you want to create a
new transaction for it, don't you?
smime.p7s
Description: S/MIME cryptographic signature
Re: [PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
Paul Durrant writes: > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> From: David Woodhouse >> This allows a XenDevice implementation to know whether it was >> created >> by QEMU, or merely discovered in XenStore after the toolstack created >> it. This will allow us to create frontend/backend nodes only when we >> should, rather than unconditionally attempting to overwrite them from >> a driver domain which doesn't have privileges to do so. >> As an added benefit, it also means we no longer have to call the >> xen_backend_set_device() function from the device models immediately >> after calling qdev_realize_and_unref(). Even though we could make >> the argument that it's safe to do so, and the pointer to the unreffed >> device *will* actually still be valid, it still made my skin itch to >> look at it. >> Signed-off-by: David Woodhouse >> --- >> hw/block/xen-block.c | 3 +-- >> hw/char/xen_console.c| 2 +- >> hw/net/xen_nic.c | 2 +- >> hw/xen/xen-bus.c | 4 >> include/hw/xen/xen-backend.h | 2 -- >> include/hw/xen/xen-bus.h | 2 ++ >> 6 files changed, 9 insertions(+), 6 deletions(-) >> > > Actually, I think you should probably update > xen_backend_try_device_destroy() in this patch too. It currently uses > xen_backend_list_find() to check whether the specified XenDevice has > an associated XenBackendInstance. Sure. Looks like it is the only user of xen_backend_list_find(), so we can get rid of it as well. I'll drop your R-b tag, because of those additional changes in the new version. -- WBR, Volodymyr
Re: [PATCH v2 2/6] xen: backends: touch some XenStore nodes only if device...
Hi Paul, Paul Durrant writes: > On 21/11/2023 22:10, Volodymyr Babchuk wrote: >> was created by QEMU >> Xen PV devices in QEMU can be created in two ways: either by QEMU >> itself, if they were passed via command line, or by Xen toolstack. In >> the latter case, QEMU scans XenStore entries and configures devices >> accordingly. >> In the second case we don't want QEMU to write/delete front-end >> entries for two reasons: it might have no access to those entries if >> it is running in un-privileged domain and it is just incorrect to >> overwrite entries already provided by Xen toolstack, because toolstack >> manages those nodes. For example, it might read backend- or frontend- >> state to be sure that they are both disconnected and it is safe to >> destroy a domain. >> This patch checks presence of xendev->backend to check if Xen PV >> device is acting as a backend (i.e. it was configured by Xen > > Technally *all* XenDevice objects are backends. > Yes, you are right of course. I'll rephrase this paragraph in the next version. -- WBR, Volodymyr
Re: [PATCH v2 2/6] xen: backends: touch some XenStore nodes only if device...
Hi Philippe, Philippe Mathieu-Daudé writes: > Hi Volodymyr, > > On 21/11/23 23:10, Volodymyr Babchuk wrote: >> was created by QEMU > > Please do not split lines between subject and content. Rewrite the > full line. Preferably restrict the subject to 72 chars. I tried to come with shorter description, but failed. I'll rewrite it in the next version. -- WBR, Volodymyr
Re: [PATCH v2 2/6] xen: backends: touch some XenStore nodes only if device...
On Wed, 2023-11-22 at 17:03 +, Paul Durrant wrote: > > > This patch checks presence of xendev->backend to check if Xen PV > > device is acting as a backend (i.e. it was configured by Xen > > Technally *all* XenDevice objects are backends. Right. The key criterion is whether the backend was created by QEMU, or merely discovered by QEMU after the toolstack created it. smime.p7s Description: S/MIME cryptographic signature Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.
Re: [PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
On Wed, 2023-11-22 at 17:05 +, Paul Durrant wrote: > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > From: David Woodhouse > > > > This allows a XenDevice implementation to know whether it was created > > by QEMU, or merely discovered in XenStore after the toolstack created > > it. This will allow us to create frontend/backend nodes only when we > > should, rather than unconditionally attempting to overwrite them from > > a driver domain which doesn't have privileges to do so. > > > > As an added benefit, it also means we no longer have to call the > > xen_backend_set_device() function from the device models immediately > > after calling qdev_realize_and_unref(). Even though we could make > > the argument that it's safe to do so, and the pointer to the unreffed > > device *will* actually still be valid, it still made my skin itch to > > look at it. > > > > Signed-off-by: David Woodhouse > > --- > > hw/block/xen-block.c | 3 +-- > > hw/char/xen_console.c | 2 +- > > hw/net/xen_nic.c | 2 +- > > hw/xen/xen-bus.c | 4 > > include/hw/xen/xen-backend.h | 2 -- > > include/hw/xen/xen-bus.h | 2 ++ > > 6 files changed, 9 insertions(+), 6 deletions(-) > > > > Actually, I think you should probably update > xen_backend_try_device_destroy() in this patch too. It currently uses > xen_backend_list_find() to check whether the specified XenDevice has an > associated XenBackendInstance. I think I did that in https://git.infradead.org/users/dwmw2/qemu.git/commitdiff/94f1b474478ce0ede but didn't get round to sending it out again because of travel. smime.p7s Description: S/MIME cryptographic signature Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.
Re: [PATCH v2 6/6] xen_arm: Add virtual PCIe host bridge support
+Vikram
On Tue, 21 Nov 2023, Volodymyr Babchuk wrote:
> From: Oleksandr Tyshchenko
>
> The bridge is needed for virtio-pci support, as QEMU can emulate the
> whole bridge with any virtio-pci devices connected to it.
>
> This patch provides a flexible way to configure PCIe brige resources
> with xenstore. We made this for several reasons:
>
> - We don't want to clash with vPCI devices, so we need information
> from Xen toolstack on which PCI bus to use.
> - The guest memory layout that describes these resources is not stable
> and may vary between guests, so we cannot rely on static resources
> to be always the same for both ends.
> - Also the device-models which run in different domains and serve
> virtio-pci devices for the same guest should use different host
> bridge resources for Xen to distinguish. The rule for the guest
> device-tree generation is one PCI host bridge per backend domain.
>
> Signed-off-by: Oleksandr Tyshchenko
> Signed-off-by: Volodymyr Babchuk
There is still a discussion ongoing on xen-devel if / how to register a
PCI Root Complex or individual BDFs. In the meantime a couple of
comments.
Typically emulated devices are configured in QEMU via QEMU command line
parameters, not xenstore. If you are running QEMU in a domU (instead of
Dom0) you can always read config parameters from xenstore from a wrapper
bash script (using xenstore-read) and then pass the right command line
options to QEMU.
If you need help in adding new QEMU command line options, Vikram (CCed)
can help.
> ---
>
> Changes from v1:
>
> - Renamed virtio_pci_host to pcie_host entries in XenStore, because
> there is nothing specific to virtio-pci: any PCI device can be
> emulated via this newly created bridge.
> ---
> hw/arm/xen_arm.c| 186
> hw/xen/xen-hvm-common.c | 9 +-
> include/hw/xen/xen_native.h | 8 +-
> 3 files changed, 200 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
> index b9c3ae14b6..d506d55d0f 100644
> --- a/hw/arm/xen_arm.c
> +++ b/hw/arm/xen_arm.c
> @@ -22,6 +22,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
> #include "qemu/error-report.h"
> #include "qapi/qapi-commands-migration.h"
> #include "qapi/visitor.h"
> @@ -34,6 +35,9 @@
> #include "hw/xen/xen-hvm-common.h"
> #include "sysemu/tpm.h"
> #include "hw/xen/arch_hvm.h"
> +#include "exec/address-spaces.h"
> +#include "hw/pci-host/gpex.h"
> +#include "hw/virtio/virtio-pci.h"
>
> #define TYPE_XEN_ARM MACHINE_TYPE_NAME("xenpvh")
> OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
> @@ -58,6 +62,11 @@ struct XenArmState {
> struct {
> uint64_t tpm_base_addr;
> } cfg;
> +
> +MemMapEntry pcie_mmio;
> +MemMapEntry pcie_ecam;
> +MemMapEntry pcie_mmio_high;
> +int pcie_irq;
> };
>
> static MemoryRegion ram_lo, ram_hi;
> @@ -73,6 +82,7 @@ static MemoryRegion ram_lo, ram_hi;
> #define NR_VIRTIO_MMIO_DEVICES \
> (GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
>
> +/* TODO It should be xendevicemodel_set_pci_intx_level() for PCI interrupts.
> */
> static void xen_set_irq(void *opaque, int irq, int level)
> {
> if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
> @@ -129,6 +139,176 @@ static void xen_init_ram(MachineState *machine)
> }
> }
>
> +static void xen_create_pcie(XenArmState *xam)
> +{
> +MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
> +MemoryRegion *ecam_alias, *ecam_reg;
> +DeviceState *dev;
> +int i;
> +
> +dev = qdev_new(TYPE_GPEX_HOST);
> +sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
> +
> +/* Map ECAM space */
> +ecam_alias = g_new0(MemoryRegion, 1);
> +ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
> +memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
> + ecam_reg, 0, xam->pcie_ecam.size);
> +memory_region_add_subregion(get_system_memory(), xam->pcie_ecam.base,
> +ecam_alias);
> +
> +/* Map the MMIO space */
> +mmio_alias = g_new0(MemoryRegion, 1);
> +mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
> +memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
> + mmio_reg,
> + xam->pcie_mmio.base,
> + xam->pcie_mmio.size);
> +memory_region_add_subregion(get_system_memory(), xam->pcie_mmio.base,
> +mmio_alias);
> +
> +/* Map the MMIO_HIGH space */
> +mmio_alias_high = g_new0(MemoryRegion, 1);
> +memory_region_init_alias(mmio_alias_high, OBJECT(dev), "pcie-mmio-high",
> + mmio_reg,
> + xam->pcie_mmio_high.base,
> + xam->pcie_mmio_high.size);
> +memory_region_add_subregion(get_system_memory(),
>
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On Wed, 22 Nov 2023, Paul Durrant wrote:
> On 21/11/2023 22:10, Volodymyr Babchuk wrote:
> > From: Oleksandr Tyshchenko
> >
> > Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to
> > inherit the owner of the directory.
>
> Ah... so that's why the previous patch is there.
>
> This is not the right way to fix it. The QEMU Xen support is *assuming* that
> QEMU is either running in, or emulating, dom0. In the emulation case this is
> probably fine, but the 'real Xen' case it should be using the correct domid
> for node creation. I guess this could either be supplied on the command line
> or discerned by reading the local domain 'domid' node.
yes, it should be passed as command line option to QEMU
> > Note that for other than Dom0 domain (non toolstack domain) the
> > "driver_domain" property should be set in domain config file for the
> > toolstack to create required directories in advance.
> >
> > Signed-off-by: Oleksandr Tyshchenko
> > Signed-off-by: Volodymyr Babchuk
> > ---
> > hw/xen/xen_pvdev.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
> > index c5ad71e8dc..42bdd4f6c8 100644
> > --- a/hw/xen/xen_pvdev.c
> > +++ b/hw/xen/xen_pvdev.c
> > @@ -60,7 +60,8 @@ void xen_config_cleanup(void)
> > int xenstore_mkdir(char *path, int p)
> > {
> > -if (!qemu_xen_xs_create(xenstore, 0, 0, xen_domid, p, path)) {
> > +if (!qemu_xen_xs_create(xenstore, 0, XS_PRESERVE_OWNER,
> > +xen_domid, p, path)) {
> > xen_pv_printf(NULL, 0, "xs_mkdir %s: failed\n", path);
> > return -1;
> > }
>
Re: [PATCH v2 3/6] xen: xenstore: add possibility to preserve owner
On Wed, 22 Nov 2023, Paul Durrant wrote: > On 21/11/2023 22:10, Volodymyr Babchuk wrote: > > Add option to preserve owner when creating an entry in Xen Store. This > > may be needed in cases when Qemu is working as device model in a > > *may* be needed? > > I don't undertstand why this patch is necessary and the commit comment isn't > really helping me. > > > domain that is Domain-0, e.g. in driver domain. I think Volodymyr meant: domain that is *not* Domain-0 So I am guessing this patch is needed otherwise QEMU will run into permissions errors doing xenstore operations > > "owner" parameter for qemu_xen_xs_create() function can have special > > value XS_PRESERVE_OWNER, which will make specific implementation to > > get original owner of an entry and pass it back to > > set_permissions() call. > > > > Please note, that XenStore inherits permissions, so even if entry is > > newly created by, it already has the owner set to match owner of entry > > at previous level. > > > > Signed-off-by: Volodymyr Babchuk >
Re: [PATCH-for-8.2 v2 1/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping TX FIFOs
Hi,
On Sun, Nov 19, 2023 at 11:51:01PM +0100, Philippe Mathieu-Daudé wrote:
> Per https://docs.xilinx.com/r/en-US/ug1085-zynq-ultrascale-trm/Message-Format
>
> Message Format
>
> The same message format is used for RXFIFO, TXFIFO, and TXHPB.
> Each message includes four words (16 bytes). Software must read
> and write all four words regardless of the actual number of data
> bytes and valid fields in the message.
>
> There is no mention in this reference manual about what the
> hardware does when not all four words are written. To fix the
> reported underflow behavior when DATA2 register is written,
> I choose to fill the data with the previous content of the
> ID / DLC / DATA1 registers, which is how I expect hardware
> would do.
>
> Note there is no hardware flag raised under such condition.
>
> Reported-by: Qiang Liu
> Fixes: 98e5d7a2b7 ("hw/net/can: Introduce Xilinx ZynqMP CAN controller")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1425
> Signed-off-by: Philippe Mathieu-Daudé
> Francisco Iglesias
> ---
> hw/net/can/xlnx-zynqmp-can.c | 49 +---
> 1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
> index e93e6c5e19..58938b574e 100644
> --- a/hw/net/can/xlnx-zynqmp-can.c
> +++ b/hw/net/can/xlnx-zynqmp-can.c
> @@ -434,6 +434,51 @@ static bool tx_ready_check(XlnxZynqMPCANState *s)
> return true;
> }
>
> +static void read_tx_frame(XlnxZynqMPCANState *s, Fifo32 *fifo, uint32_t
> *data)
> +{
> +unsigned used = fifo32_num_used(fifo);
> +bool is_txhpb = fifo == >txhpb_fifo;
> +
> +assert(used > 0);
> +used %= CAN_FRAME_SIZE;
> +
> +/*
> + * Frame Message Format
> + *
> + * Each frame includes four words (16 bytes). Software must read and
> write
> + * all four words regardless of the actual number of data bytes and valid
> + * fields in the message.
> + * If software misbehave (not writting all four words), we use the
> previous
%s/writting/writing
> + * registers content to initialize each missing word.
> + */
> +if (used > 0) {
> +/* ID, DLC, DATA1 missing */
Code is correct but i feel This comment is confusing. ID is missing for sure
if user > 0 but same is not the case for DLC and DATA1.
> +data[0] = s->regs[is_txhpb ? R_TXHPB_ID : R_TXFIFO_ID];
> +} else {
> +data[0] = fifo32_pop(fifo);
> +}
> +if (used == 1 || used == 2) {
> +/* DLC, DATA1 missing */
Same here DLC is missing for sure but DATA1 is not for used == 2.
> +data[1] = s->regs[is_txhpb ? R_TXHPB_DLC : R_TXFIFO_DLC];
> +} else {
> +data[1] = fifo32_pop(fifo);
> +}
> +if (used == 1) {
> +/* DATA1 missing */
May be we remove all these individual comments and write a common comment before
the first check(if(used > 0)). Something like this:
/*
* If used is 1 then ID, DLC and DATA1 are missing.
*
* if used is 2 then ID and DLC are missing.
*
* if used is 3 then only ID is missing.
*/
Code looks correct to me. So, With above minor changes in code comments:
Reviewed-by: Vikram Garhwal
> +data[2] = s->regs[is_txhpb ? R_TXHPB_DATA1 : R_TXFIFO_DATA1];
> +} else {
> +data[2] = fifo32_pop(fifo);
> +}
> +/* DATA2 triggered the transfer thus is always available */
> +data[3] = fifo32_pop(fifo);
> +
> +if (used) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Incomplete CAN frame (only %u/%u slots used)\n",
> + TYPE_XLNX_ZYNQMP_CAN, used, CAN_FRAME_SIZE);
> +}
> +}
> +
> static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo)
> {
> qemu_can_frame frame;
> @@ -451,9 +496,7 @@ static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32
> *fifo)
> }
>
> while (!fifo32_is_empty(fifo)) {
> -for (i = 0; i < CAN_FRAME_SIZE; i++) {
> -data[i] = fifo32_pop(fifo);
> -}
> +read_tx_frame(s, fifo, data);
>
> if (ARRAY_FIELD_EX32(s->regs, STATUS_REGISTER, LBACK)) {
> /*
> --
> 2.41.0
>
Re: [PATCH-for-8.2 v2 2/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping RX FIFO
On Wed, Nov 22, 2023 at 08:45:56PM +0100, Francisco Iglesias wrote:
> On Sun, Nov 19, 2023 at 11:51:02PM +0100, Philippe Mathieu-Daudé wrote:
> > Per
> > https://docs.xilinx.com/r/en-US/ug1085-zynq-ultrascale-trm/Message-Format
> >
> > Message Format
> >
> > The same message format is used for RXFIFO, TXFIFO, and TXHPB.
> > Each message includes four words (16 bytes). Software must read
> > and write all four words regardless of the actual number of data
> > bytes and valid fields in the message.
> >
> > There is no mention in this reference manual about what the
> > hardware does when not all four words are read. To fix the
> > reported underflow behavior, I choose to fill the 4 frame data
> > registers when the first register (ID) is accessed, which is how
> > I expect hardware would do.
> >
> > Reported-by: Qiang Liu
> > Fixes: 98e5d7a2b7 ("hw/net/can: Introduce Xilinx ZynqMP CAN controller")
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1427
> > Signed-off-by: Philippe Mathieu-Daudé
>
> Reviewed-by: Francisco Iglesias
Reviewed-by: Vikram Garhwal
>
>
> > ---
> > hw/net/can/xlnx-zynqmp-can.c | 17 +
> > 1 file changed, 9 insertions(+), 8 deletions(-)
> >
> > diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
> > index 58938b574e..c63fb4a83c 100644
> > --- a/hw/net/can/xlnx-zynqmp-can.c
> > +++ b/hw/net/can/xlnx-zynqmp-can.c
> > @@ -777,14 +777,18 @@ static void update_rx_fifo(XlnxZynqMPCANState *s,
> > const qemu_can_frame *frame)
> > }
> > }
> >
> > -static uint64_t can_rxfifo_pre_read(RegisterInfo *reg, uint64_t val)
> > +static uint64_t can_rxfifo_post_read_id(RegisterInfo *reg, uint64_t val)
> > {
> > XlnxZynqMPCANState *s = XLNX_ZYNQMP_CAN(reg->opaque);
> > +unsigned used = fifo32_num_used(>rx_fifo);
> >
> > -if (!fifo32_is_empty(>rx_fifo)) {
> > -val = fifo32_pop(>rx_fifo);
> > -} else {
> > +if (used < CAN_FRAME_SIZE) {
> > ARRAY_FIELD_DP32(s->regs, INTERRUPT_STATUS_REGISTER, RXUFLW, 1);
> > +} else {
> > +val = s->regs[R_RXFIFO_ID] = fifo32_pop(>rx_fifo);
> > +s->regs[R_RXFIFO_DLC] = fifo32_pop(>rx_fifo);
> > +s->regs[R_RXFIFO_DATA1] = fifo32_pop(>rx_fifo);
> > +s->regs[R_RXFIFO_DATA2] = fifo32_pop(>rx_fifo);
> > }
> >
> > can_update_irq(s);
> > @@ -945,14 +949,11 @@ static const RegisterAccessInfo can_regs_info[] = {
> > .post_write = can_tx_post_write,
> > },{ .name = "RXFIFO_ID", .addr = A_RXFIFO_ID,
> > .ro = 0x,
> > -.post_read = can_rxfifo_pre_read,
> > +.post_read = can_rxfifo_post_read_id,
> > },{ .name = "RXFIFO_DLC", .addr = A_RXFIFO_DLC,
> > .rsvd = 0xfff,
> > -.post_read = can_rxfifo_pre_read,
> > },{ .name = "RXFIFO_DATA1", .addr = A_RXFIFO_DATA1,
> > -.post_read = can_rxfifo_pre_read,
> > },{ .name = "RXFIFO_DATA2", .addr = A_RXFIFO_DATA2,
> > -.post_read = can_rxfifo_pre_read,
> > },{ .name = "AFR", .addr = A_AFR,
> > .rsvd = 0xfff0,
> > .post_write = can_filter_enable_post_write,
> > --
> > 2.41.0
> >
Re: [PATCH-for-9.0] iothread: Remove unused Error** argument in aio_context_set_aio_params
On Mon, Nov 20, 2023 at 06:18:06PM +0100, Philippe Mathieu-Daudé wrote: > aio_context_set_aio_params() doesn't use its undocumented > Error** argument. Remove it to simplify. > > Note this removes a use of "unchecked Error**" in > iothread_set_aio_context_params(). > > Signed-off-by: Philippe Mathieu-Daudé > --- > include/block/aio.h | 3 +-- > iothread.c | 3 +-- > util/aio-posix.c| 3 +-- > util/aio-win32.c| 3 +-- > util/main-loop.c| 5 + > 5 files changed, 5 insertions(+), 12 deletions(-) Thanks, applied to my block tree: https://gitlab.com/stefanha/qemu/commits/block Stefan signature.asc Description: PGP signature
Re: [PATCH-for-9.0] hw/ppc/xive2_regs: Remove unnecessary 'cpu.h' inclusion
On 11/22/23 19:39, Philippe Mathieu-Daudé wrote: xive2_regs.h only requires declarations from "qemu/bswap.h". Include it instead of the huge target-specific "cpu.h". Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Cédric Le Goater Thanks, C. --- include/hw/ppc/xive2_regs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/ppc/xive2_regs.h b/include/hw/ppc/xive2_regs.h index b7adbdb7b9..816f5d0e84 100644 --- a/include/hw/ppc/xive2_regs.h +++ b/include/hw/ppc/xive2_regs.h @@ -10,7 +10,7 @@ #ifndef PPC_XIVE2_REGS_H #define PPC_XIVE2_REGS_H -#include "cpu.h" +#include "qemu/bswap.h" /* * Thread Interrupt Management Area (TIMA)
Re: [PATCH-for-8.2 v2 2/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping RX FIFO
On Sun, Nov 19, 2023 at 11:51:02PM +0100, Philippe Mathieu-Daudé wrote:
> Per https://docs.xilinx.com/r/en-US/ug1085-zynq-ultrascale-trm/Message-Format
>
> Message Format
>
> The same message format is used for RXFIFO, TXFIFO, and TXHPB.
> Each message includes four words (16 bytes). Software must read
> and write all four words regardless of the actual number of data
> bytes and valid fields in the message.
>
> There is no mention in this reference manual about what the
> hardware does when not all four words are read. To fix the
> reported underflow behavior, I choose to fill the 4 frame data
> registers when the first register (ID) is accessed, which is how
> I expect hardware would do.
>
> Reported-by: Qiang Liu
> Fixes: 98e5d7a2b7 ("hw/net/can: Introduce Xilinx ZynqMP CAN controller")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1427
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Francisco Iglesias
> ---
> hw/net/can/xlnx-zynqmp-can.c | 17 +
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
> index 58938b574e..c63fb4a83c 100644
> --- a/hw/net/can/xlnx-zynqmp-can.c
> +++ b/hw/net/can/xlnx-zynqmp-can.c
> @@ -777,14 +777,18 @@ static void update_rx_fifo(XlnxZynqMPCANState *s, const
> qemu_can_frame *frame)
> }
> }
>
> -static uint64_t can_rxfifo_pre_read(RegisterInfo *reg, uint64_t val)
> +static uint64_t can_rxfifo_post_read_id(RegisterInfo *reg, uint64_t val)
> {
> XlnxZynqMPCANState *s = XLNX_ZYNQMP_CAN(reg->opaque);
> +unsigned used = fifo32_num_used(>rx_fifo);
>
> -if (!fifo32_is_empty(>rx_fifo)) {
> -val = fifo32_pop(>rx_fifo);
> -} else {
> +if (used < CAN_FRAME_SIZE) {
> ARRAY_FIELD_DP32(s->regs, INTERRUPT_STATUS_REGISTER, RXUFLW, 1);
> +} else {
> +val = s->regs[R_RXFIFO_ID] = fifo32_pop(>rx_fifo);
> +s->regs[R_RXFIFO_DLC] = fifo32_pop(>rx_fifo);
> +s->regs[R_RXFIFO_DATA1] = fifo32_pop(>rx_fifo);
> +s->regs[R_RXFIFO_DATA2] = fifo32_pop(>rx_fifo);
> }
>
> can_update_irq(s);
> @@ -945,14 +949,11 @@ static const RegisterAccessInfo can_regs_info[] = {
> .post_write = can_tx_post_write,
> },{ .name = "RXFIFO_ID", .addr = A_RXFIFO_ID,
> .ro = 0x,
> -.post_read = can_rxfifo_pre_read,
> +.post_read = can_rxfifo_post_read_id,
> },{ .name = "RXFIFO_DLC", .addr = A_RXFIFO_DLC,
> .rsvd = 0xfff,
> -.post_read = can_rxfifo_pre_read,
> },{ .name = "RXFIFO_DATA1", .addr = A_RXFIFO_DATA1,
> -.post_read = can_rxfifo_pre_read,
> },{ .name = "RXFIFO_DATA2", .addr = A_RXFIFO_DATA2,
> -.post_read = can_rxfifo_pre_read,
> },{ .name = "AFR", .addr = A_AFR,
> .rsvd = 0xfff0,
> .post_write = can_filter_enable_post_write,
> --
> 2.41.0
>
Re: [PATCH-for-8.2 v2 1/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping TX FIFOs
On Sun, Nov 19, 2023 at 11:51:01PM +0100, Philippe Mathieu-Daudé wrote:
> Per https://docs.xilinx.com/r/en-US/ug1085-zynq-ultrascale-trm/Message-Format
>
> Message Format
>
> The same message format is used for RXFIFO, TXFIFO, and TXHPB.
> Each message includes four words (16 bytes). Software must read
> and write all four words regardless of the actual number of data
> bytes and valid fields in the message.
>
> There is no mention in this reference manual about what the
> hardware does when not all four words are written. To fix the
> reported underflow behavior when DATA2 register is written,
> I choose to fill the data with the previous content of the
> ID / DLC / DATA1 registers, which is how I expect hardware
> would do.
>
> Note there is no hardware flag raised under such condition.
>
> Reported-by: Qiang Liu
> Fixes: 98e5d7a2b7 ("hw/net/can: Introduce Xilinx ZynqMP CAN controller")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1425
> Signed-off-by: Philippe Mathieu-Daudé
> Francisco Iglesias
-above line
+below line
Reviewed-by: Francisco Iglesias
> ---
> hw/net/can/xlnx-zynqmp-can.c | 49 +---
> 1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
> index e93e6c5e19..58938b574e 100644
> --- a/hw/net/can/xlnx-zynqmp-can.c
> +++ b/hw/net/can/xlnx-zynqmp-can.c
> @@ -434,6 +434,51 @@ static bool tx_ready_check(XlnxZynqMPCANState *s)
> return true;
> }
>
> +static void read_tx_frame(XlnxZynqMPCANState *s, Fifo32 *fifo, uint32_t
> *data)
> +{
> +unsigned used = fifo32_num_used(fifo);
> +bool is_txhpb = fifo == >txhpb_fifo;
> +
> +assert(used > 0);
> +used %= CAN_FRAME_SIZE;
> +
> +/*
> + * Frame Message Format
> + *
> + * Each frame includes four words (16 bytes). Software must read and
> write
> + * all four words regardless of the actual number of data bytes and valid
> + * fields in the message.
> + * If software misbehave (not writting all four words), we use the
> previous
> + * registers content to initialize each missing word.
> + */
> +if (used > 0) {
> +/* ID, DLC, DATA1 missing */
> +data[0] = s->regs[is_txhpb ? R_TXHPB_ID : R_TXFIFO_ID];
> +} else {
> +data[0] = fifo32_pop(fifo);
> +}
> +if (used == 1 || used == 2) {
> +/* DLC, DATA1 missing */
> +data[1] = s->regs[is_txhpb ? R_TXHPB_DLC : R_TXFIFO_DLC];
> +} else {
> +data[1] = fifo32_pop(fifo);
> +}
> +if (used == 1) {
> +/* DATA1 missing */
> +data[2] = s->regs[is_txhpb ? R_TXHPB_DATA1 : R_TXFIFO_DATA1];
> +} else {
> +data[2] = fifo32_pop(fifo);
> +}
> +/* DATA2 triggered the transfer thus is always available */
> +data[3] = fifo32_pop(fifo);
> +
> +if (used) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> + "%s: Incomplete CAN frame (only %u/%u slots used)\n",
> + TYPE_XLNX_ZYNQMP_CAN, used, CAN_FRAME_SIZE);
> +}
> +}
> +
> static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo)
> {
> qemu_can_frame frame;
> @@ -451,9 +496,7 @@ static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32
> *fifo)
> }
>
> while (!fifo32_is_empty(fifo)) {
> -for (i = 0; i < CAN_FRAME_SIZE; i++) {
> -data[i] = fifo32_pop(fifo);
> -}
> +read_tx_frame(s, fifo, data);
>
> if (ARRAY_FIELD_EX32(s->regs, STATUS_REGISTER, LBACK)) {
> /*
> --
> 2.41.0
>
[PATCH-for-9.0] hw/mips: Inline 'bios.h' definitions
There is no universal BIOS, each machine needs a specific one.
Move the machine-specific definitions to each machine code and
remove this bogus header.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/mips/bios.h | 14 --
hw/mips/jazz.c | 10 +-
hw/mips/malta.c| 9 -
hw/mips/mipssim.c | 10 +-
4 files changed, 26 insertions(+), 17 deletions(-)
delete mode 100644 include/hw/mips/bios.h
diff --git a/include/hw/mips/bios.h b/include/hw/mips/bios.h
deleted file mode 100644
index 44acb6815b..00
--- a/include/hw/mips/bios.h
+++ /dev/null
@@ -1,14 +0,0 @@
-#ifndef HW_MIPS_BIOS_H
-#define HW_MIPS_BIOS_H
-
-#include "qemu/units.h"
-#include "cpu.h"
-
-#define BIOS_SIZE (4 * MiB)
-#if TARGET_BIG_ENDIAN
-#define BIOS_FILENAME "mips_bios.bin"
-#else
-#define BIOS_FILENAME "mipsel_bios.bin"
-#endif
-
-#endif
diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index d33a76ad4d..0d2348aa5a 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -36,7 +36,6 @@
#include "hw/boards.h"
#include "net/net.h"
#include "hw/scsi/esp.h"
-#include "hw/mips/bios.h"
#include "hw/loader.h"
#include "hw/rtc/mc146818rtc.h"
#include "hw/timer/i8254.h"
@@ -53,12 +52,19 @@
#ifdef CONFIG_TCG
#include "hw/core/tcg-cpu-ops.h"
#endif /* CONFIG_TCG */
+#include "cpu.h"
enum jazz_model_e {
JAZZ_MAGNUM,
JAZZ_PICA61,
};
+#if TARGET_BIG_ENDIAN
+#define BIOS_FILENAME "mips_bios.bin"
+#else
+#define BIOS_FILENAME "mipsel_bios.bin"
+#endif
+
static void main_cpu_reset(void *opaque)
{
MIPSCPU *cpu = opaque;
@@ -147,6 +153,8 @@ static void mips_jazz_init_net(NICInfo *nd,
IOMMUMemoryRegion *rc4030_dma_mr,
prom[7] = 0xff - checksum;
}
+#define BIOS_SIZE (4 * MiB)
+
#define MAGNUM_BIOS_SIZE_MAX 0x7e000
#define MAGNUM_BIOS_SIZE
\
(BIOS_SIZE < MAGNUM_BIOS_SIZE_MAX ? BIOS_SIZE : MAGNUM_BIOS_SIZE_MAX)
diff --git a/hw/mips/malta.c b/hw/mips/malta.c
index 049de46a9e..d22bb1edef 100644
--- a/hw/mips/malta.c
+++ b/hw/mips/malta.c
@@ -40,7 +40,6 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_bus.h"
#include "qemu/log.h"
-#include "hw/mips/bios.h"
#include "hw/ide/pci.h"
#include "hw/irq.h"
#include "hw/loader.h"
@@ -59,6 +58,7 @@
#include "hw/qdev-clock.h"
#include "target/mips/internal.h"
#include "trace.h"
+#include "cpu.h"
#define ENVP_PADDR 0x2000
#define ENVP_VADDR cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
@@ -71,6 +71,7 @@
#define RESET_ADDRESS 0x1fc0ULL
#define FLASH_SIZE 0x40
+#define BIOS_SIZE (4 * MiB)
#define PIIX4_PCI_DEVFN PCI_DEVFN(10, 0)
@@ -91,6 +92,12 @@ typedef struct {
bool display_inited;
} MaltaFPGAState;
+#if TARGET_BIG_ENDIAN
+#define BIOS_FILENAME "mips_bios.bin"
+#else
+#define BIOS_FILENAME "mipsel_bios.bin"
+#endif
+
#define TYPE_MIPS_MALTA "mips-malta"
OBJECT_DECLARE_SIMPLE_TYPE(MaltaState, MIPS_MALTA)
diff --git a/hw/mips/mipssim.c b/hw/mips/mipssim.c
index 4f743f37eb..01e323904d 100644
--- a/hw/mips/mipssim.c
+++ b/hw/mips/mipssim.c
@@ -35,7 +35,6 @@
#include "net/net.h"
#include "sysemu/sysemu.h"
#include "hw/boards.h"
-#include "hw/mips/bios.h"
#include "hw/loader.h"
#include "elf.h"
#include "hw/sysbus.h"
@@ -43,6 +42,15 @@
#include "qemu/error-report.h"
#include "sysemu/qtest.h"
#include "sysemu/reset.h"
+#include "cpu.h"
+
+#define BIOS_SIZE (4 * MiB)
+
+#if TARGET_BIG_ENDIAN
+#define BIOS_FILENAME "mips_bios.bin"
+#else
+#define BIOS_FILENAME "mipsel_bios.bin"
+#endif
static struct _loaderparams {
int ram_size;
--
2.41.0
[PATCH-for-9.0] hw/ppc/xive2_regs: Remove unnecessary 'cpu.h' inclusion
xive2_regs.h only requires declarations from "qemu/bswap.h". Include it instead of the huge target-specific "cpu.h". Signed-off-by: Philippe Mathieu-Daudé --- include/hw/ppc/xive2_regs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/hw/ppc/xive2_regs.h b/include/hw/ppc/xive2_regs.h index b7adbdb7b9..816f5d0e84 100644 --- a/include/hw/ppc/xive2_regs.h +++ b/include/hw/ppc/xive2_regs.h @@ -10,7 +10,7 @@ #ifndef PPC_XIVE2_REGS_H #define PPC_XIVE2_REGS_H -#include "cpu.h" +#include "qemu/bswap.h" /* * Thread Interrupt Management Area (TIMA) -- 2.41.0
[PATCH-for-9.0] hw/mips/cps: Simplify access to 'start-powered-off' property
Since commit c1b701587e ("target/arm: Move start-powered-off
property to generic CPUState"), all target CPUs have the
'start-powered-off' property.
This object_property_set_bool() call can not fail. Use _abort
to simplify.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/mips/cps.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index b6612c1762..4f12e23ab5 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -78,10 +78,9 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
CPUMIPSState *env = >env;
/* All VPs are halted on reset. Leave powering up to CPC. */
-if (!object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
- errp)) {
-return;
-}
+object_property_set_bool(OBJECT(cpu), "start-powered-off", true,
+ _abort);
+
/* All cores use the same clock tree */
qdev_connect_clock_in(DEVICE(cpu), "clk-in", s->clock);
--
2.41.0
[PATCH-for-9.0 06/11] hw/arm/bcm2836: Simplify use of 'reset-cbar' property
bcm2836_realize() is called by
- bcm2836_class_init() which sets:
bc->cpu_type = ARM_CPU_TYPE_NAME("cortex-a7")
- bcm2837_class_init() which sets:
bc->cpu_type = ARM_CPU_TYPE_NAME("cortex-a53")
Both Cortex-A7 / A53 have the ARM_FEATURE_CBAR set. If it isn't,
then this is a programming error: use _abort.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/arm/bcm2836.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 6986b71cb4..055c909e95 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -132,10 +132,8 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
s->cpu[n].core.mp_affinity = (bc->clusterid << 8) | n;
/* set periphbase/CBAR value for CPU-local registers */
-if (!object_property_set_int(OBJECT(>cpu[n].core), "reset-cbar",
- bc->peri_base, errp)) {
-return;
-}
+object_property_set_int(OBJECT(>cpu[n].core), "reset-cbar",
+bc->peri_base, _abort);
/* start powered off if not enabled */
if (!object_property_set_bool(OBJECT(>cpu[n].core),
--
2.41.0
[RFC PATCH-for-9.0 02/11] target/arm: Add target_aarch64_available() helper
We want to build HW models once, but don't want to
register types when all prerequisites are satisfied. Add
the target_aarch64_available() to know at runtime whether
TARGET_AARCH64 is built-in.
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/cpu-qom.h | 2 ++
target/arm/cpu.c | 9 +
2 files changed, 11 insertions(+)
diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index 02b914c876..bf6b3604ed 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -33,4 +33,6 @@ typedef struct AArch64CPUClass AArch64CPUClass;
DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU,
TYPE_AARCH64_CPU)
+bool target_aarch64_available(void);
+
#endif
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 25e9d2ae7b..1990c04089 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2548,3 +2548,12 @@ static void arm_cpu_register_types(void)
}
type_init(arm_cpu_register_types)
+
+bool target_aarch64_available(void)
+{
+#ifdef TARGET_AARCH64
+return true;
+#else
+return false;
+#endif
+}
--
2.41.0
[RFC PATCH-for-9.0 09/11] hw/arm/bcm2836: Allocate ARM CPU state with object_new()
The ARMCPU type is forward declared as a pointer to all hw/ files.
Its declaration is restricted to target/arm/ files. By using a
pointer in BCM283XState instead of embedding the whole CPU state,
we don't need to include "cpu.h" which is target-specific.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/arm/bcm2836.h | 4 ++--
hw/arm/bcm2836.c | 19 ++-
hw/arm/raspi.c | 2 +-
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
index 6f90cabfa3..784bab0aad 100644
--- a/include/hw/arm/bcm2836.h
+++ b/include/hw/arm/bcm2836.h
@@ -14,7 +14,7 @@
#include "hw/arm/bcm2835_peripherals.h"
#include "hw/intc/bcm2836_control.h"
-#include "target/arm/cpu.h"
+#include "target/arm/cpu-qom.h"
#include "qom/object.h"
#define TYPE_BCM283X "bcm283x"
@@ -38,7 +38,7 @@ struct BCM283XState {
uint32_t enabled_cpus;
struct {
-ARMCPU core;
+ARMCPU *core;
} cpu[BCM283X_NCPUS];
BCM2836ControlState control;
BCM2835PeripheralState peripherals;
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 8031a74600..4f5acee77e 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -39,8 +39,9 @@ static void bcm2836_init(Object *obj)
int n;
for (n = 0; n < bc->core_count; n++) {
-object_initialize_child(obj, "cpu[*]", >cpu[n].core,
-bc->cpu_type);
+s->cpu[n].core = ARM_CPU(object_new(bc->cpu_type));
+object_property_add_child(obj, "cpu[*]", OBJECT(s->cpu[n].core));
+qdev_realize_and_unref(DEVICE(s->cpu[n].core), NULL, _abort);
}
if (bc->core_count > 1) {
qdev_property_add_static(DEVICE(obj), _enabled_cores_property);
@@ -139,24 +140,24 @@ static void bcm2836_realize(DeviceState *dev, Error
**errp)
object_property_set_bool(OBJECT(>cpu[n].core), "start-powered-off",
n >= s->enabled_cpus, _abort);
-if (!qdev_realize(DEVICE(>cpu[n].core), NULL, errp)) {
+if (!qdev_realize(DEVICE(s->cpu[n].core), NULL, errp)) {
return;
}
/* Connect irq/fiq outputs from the interrupt controller. */
qdev_connect_gpio_out_named(DEVICE(>control), "irq", n,
-qdev_get_gpio_in(DEVICE(>cpu[n].core), ARM_CPU_IRQ));
+qdev_get_gpio_in(DEVICE(s->cpu[n].core), ARM_CPU_IRQ));
qdev_connect_gpio_out_named(DEVICE(>control), "fiq", n,
-qdev_get_gpio_in(DEVICE(>cpu[n].core), ARM_CPU_FIQ));
+qdev_get_gpio_in(DEVICE(s->cpu[n].core), ARM_CPU_FIQ));
/* Connect timers from the CPU to the interrupt controller */
-qdev_connect_gpio_out(DEVICE(>cpu[n].core), GTIMER_PHYS,
+qdev_connect_gpio_out(DEVICE(s->cpu[n].core), GTIMER_PHYS,
qdev_get_gpio_in_named(DEVICE(>control), "cntpnsirq", n));
-qdev_connect_gpio_out(DEVICE(>cpu[n].core), GTIMER_VIRT,
+qdev_connect_gpio_out(DEVICE(s->cpu[n].core), GTIMER_VIRT,
qdev_get_gpio_in_named(DEVICE(>control), "cntvirq", n));
-qdev_connect_gpio_out(DEVICE(>cpu[n].core), GTIMER_HYP,
+qdev_connect_gpio_out(DEVICE(s->cpu[n].core), GTIMER_HYP,
qdev_get_gpio_in_named(DEVICE(>control), "cnthpirq", n));
-qdev_connect_gpio_out(DEVICE(>cpu[n].core), GTIMER_SEC,
+qdev_connect_gpio_out(DEVICE(s->cpu[n].core), GTIMER_SEC,
qdev_get_gpio_in_named(DEVICE(>control), "cntpsirq", n));
}
}
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index cc4c4ec9bf..01c391b90a 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -252,7 +252,7 @@ static void setup_boot(MachineState *machine,
RaspiProcessorId processor_id,
s->binfo.firmware_loaded = true;
}
-arm_load_kernel(>soc.cpu[0].core, machine, >binfo);
+arm_load_kernel(s->soc.cpu[0].core, machine, >binfo);
}
static void raspi_machine_init(MachineState *machine)
--
2.41.0
[RFC PATCH-for-9.0 11/11] hw/intc/meson: Simplify how arm_gicv3_kvm.o objects are built
Use the target_aarch64_available() to build the ARM_GIC_KVM types
regardless the ARM/AARCH64 targets are selected, but restrict its
registration to TARGET_AARCH64 presence at runtime.
This will help to have a single binary running both ARM/Aarch64.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/intc/arm_gicv3_its_kvm.c | 1 +
hw/intc/arm_gicv3_kvm.c | 1 +
hw/intc/meson.build | 6 --
3 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c
index f7df602cff..b3063c4cd7 100644
--- a/hw/intc/arm_gicv3_its_kvm.c
+++ b/hw/intc/arm_gicv3_its_kvm.c
@@ -261,6 +261,7 @@ static const TypeInfo kvm_arm_its_info = {
.instance_size = sizeof(GICv3ITSState),
.class_init = kvm_arm_its_class_init,
.class_size = sizeof(KVMARMITSClass),
+.can_register = target_aarch64_available,
};
static void kvm_arm_its_register_types(void)
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 77eb37e131..33321dee5d 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -909,6 +909,7 @@ static const TypeInfo kvm_arm_gicv3_info = {
.instance_size = sizeof(GICv3State),
.class_init = kvm_arm_gicv3_class_init,
.class_size = sizeof(KVMARMGICv3Class),
+.can_register = target_aarch64_available,
};
static void kvm_arm_gicv3_register_types(void)
diff --git a/hw/intc/meson.build b/hw/intc/meson.build
index ed355941d1..d45eb76f36 100644
--- a/hw/intc/meson.build
+++ b/hw/intc/meson.build
@@ -40,8 +40,10 @@ endif
specific_ss.add(when: 'CONFIG_APIC', if_true: files('apic.c', 'apic_common.c'))
specific_ss.add(when: 'CONFIG_ARM_GIC', if_true:
files('arm_gicv3_cpuif_common.c'))
specific_ss.add(when: 'CONFIG_ARM_GICV3_TCG', if_true:
files('arm_gicv3_cpuif.c'))
-specific_ss.add(when: 'CONFIG_ARM_GIC_KVM', if_true: files('arm_gic_kvm.c'))
-specific_ss.add(when: ['CONFIG_ARM_GIC_KVM', 'TARGET_AARCH64'], if_true:
files('arm_gicv3_kvm.c', 'arm_gicv3_its_kvm.c'))
+specific_ss.add(when: 'CONFIG_ARM_GIC_KVM', if_true: files(
+ 'arm_gic_kvm.c',
+ 'arm_gicv3_kvm.c',
+ 'arm_gicv3_its_kvm.c'))
specific_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('armv7m_nvic.c'))
specific_ss.add(when: 'CONFIG_GRLIB', if_true: files('grlib_irqmp.c'))
specific_ss.add(when: 'CONFIG_IOAPIC', if_true: files('ioapic.c'))
--
2.41.0
[PATCH-for-9.0 08/11] hw/arm/bcm2836: Use ARM_CPU 'mp-affinity' property
The 'mp-affinity' property is present since commit 15a21fe028
("target-arm: Add mp-affinity property for ARM CPU class").
Use it and remove a /* TODO */ comment. Since all ARM CPUs
have this property, use _abort, because this call can
not fail.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/arm/bcm2836.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 198f9b5730..8031a74600 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -128,8 +128,8 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
qdev_get_gpio_in_named(DEVICE(>control), "gpu-fiq", 0));
for (n = 0; n < BCM283X_NCPUS; n++) {
-/* TODO: this should be converted to a property of ARM_CPU */
-s->cpu[n].core.mp_affinity = (bc->clusterid << 8) | n;
+object_property_set_int(OBJECT(>cpu[n].core), "mp-affinity",
+(bc->clusterid << 8) | n, _abort);
/* set periphbase/CBAR value for CPU-local registers */
object_property_set_int(OBJECT(>cpu[n].core), "reset-cbar",
--
2.41.0
[RFC PATCH-for-9.0 01/11] qom: Introduce the TypeInfo::can_register() handler
Add a helper to decide at runtime whether a type can
be registered to the QOM framework or not.
Signed-off-by: Philippe Mathieu-Daudé
---
include/qom/object.h | 4
qom/object.c | 3 +++
2 files changed, 7 insertions(+)
diff --git a/include/qom/object.h b/include/qom/object.h
index afccd24ca7..0d42fe17de 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -372,6 +372,8 @@ struct Object
* struct TypeInfo:
* @name: The name of the type.
* @parent: The name of the parent type.
+ * @can_register: This optional function is called before a type is registered.
+ * If it exists and returns false, the type is not registered.
* @instance_size: The size of the object (derivative of #Object). If
* @instance_size is 0, then the size of the object will be the size of the
* parent object.
@@ -414,6 +416,8 @@ struct TypeInfo
const char *name;
const char *parent;
+bool (*can_register)(void);
+
size_t instance_size;
size_t instance_align;
void (*instance_init)(Object *obj);
diff --git a/qom/object.c b/qom/object.c
index 95c0dc8285..f09b6b5a92 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -150,6 +150,9 @@ static TypeImpl *type_register_internal(const TypeInfo
*info)
TypeImpl *type_register(const TypeInfo *info)
{
assert(info->parent);
+if (info->can_register && !info->can_register()) {
+return NULL;
+}
return type_register_internal(info);
}
--
2.41.0
[RFC PATCH-for-9.0 10/11] hw/arm/raspi: Build bcm2836.o and raspi.o objects once
Use the target_aarch64_available() method to restrict
Aarch64 specific models. They will only be added at runtime
if TARGET_AARCH64 is built in.
The Raspberry Pi models can now be built once for all targets.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/arm/bcm2836.c | 5 +
hw/arm/raspi.c | 6 ++
hw/arm/meson.build | 6 --
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 4f5acee77e..ecf434c8ce 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -194,7 +194,6 @@ static void bcm2836_class_init(ObjectClass *oc, void *data)
dc->realize = bcm2836_realize;
};
-#ifdef TARGET_AARCH64
static void bcm2837_class_init(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
@@ -207,7 +206,6 @@ static void bcm2837_class_init(ObjectClass *oc, void *data)
bc->clusterid = 0x0;
dc->realize = bcm2836_realize;
};
-#endif
static const TypeInfo bcm283x_types[] = {
{
@@ -218,12 +216,11 @@ static const TypeInfo bcm283x_types[] = {
.name = TYPE_BCM2836,
.parent = TYPE_BCM283X,
.class_init = bcm2836_class_init,
-#ifdef TARGET_AARCH64
}, {
.name = TYPE_BCM2837,
.parent = TYPE_BCM283X,
.class_init = bcm2837_class_init,
-#endif
+.can_register = target_aarch64_available,
}, {
.name = TYPE_BCM283X,
.parent = TYPE_DEVICE,
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 01c391b90a..979937b9ac 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -349,7 +349,6 @@ static void raspi2b_machine_class_init(ObjectClass *oc,
void *data)
raspi_machine_class_common_init(mc, rmc->board_rev);
};
-#ifdef TARGET_AARCH64
static void raspi3ap_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -367,7 +366,6 @@ static void raspi3b_machine_class_init(ObjectClass *oc,
void *data)
rmc->board_rev = 0xa02082;
raspi_machine_class_common_init(mc, rmc->board_rev);
};
-#endif /* TARGET_AARCH64 */
static const TypeInfo raspi_machine_types[] = {
{
@@ -382,16 +380,16 @@ static const TypeInfo raspi_machine_types[] = {
.name = MACHINE_TYPE_NAME("raspi2b"),
.parent = TYPE_RASPI_MACHINE,
.class_init = raspi2b_machine_class_init,
-#ifdef TARGET_AARCH64
}, {
.name = MACHINE_TYPE_NAME("raspi3ap"),
.parent = TYPE_RASPI_MACHINE,
.class_init = raspi3ap_machine_class_init,
+.can_register = target_aarch64_available,
}, {
.name = MACHINE_TYPE_NAME("raspi3b"),
.parent = TYPE_RASPI_MACHINE,
.class_init = raspi3b_machine_class_init,
-#endif
+.can_register = target_aarch64_available,
}, {
.name = TYPE_RASPI_MACHINE,
.parent = TYPE_MACHINE,
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index 68245d3ad1..15d60685d0 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -38,7 +38,6 @@ arm_ss.add(when: 'CONFIG_STRONGARM', if_true:
files('strongarm.c'))
arm_ss.add(when: 'CONFIG_ALLWINNER_A10', if_true: files('allwinner-a10.c',
'cubieboard.c'))
arm_ss.add(when: 'CONFIG_ALLWINNER_H3', if_true: files('allwinner-h3.c',
'orangepi.c'))
arm_ss.add(when: 'CONFIG_ALLWINNER_R40', if_true: files('allwinner-r40.c',
'bananapi_m2u.c'))
-arm_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2836.c', 'raspi.c'))
arm_ss.add(when: 'CONFIG_STM32F100_SOC', if_true: files('stm32f100_soc.c'))
arm_ss.add(when: 'CONFIG_STM32F205_SOC', if_true: files('stm32f205_soc.c'))
arm_ss.add(when: 'CONFIG_STM32F405_SOC', if_true: files('stm32f405_soc.c'))
@@ -68,7 +67,10 @@ arm_ss.add(when: 'CONFIG_XEN', if_true: files('xen_arm.c'))
system_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c'))
system_ss.add(when: 'CONFIG_EXYNOS4', if_true: files('exynos4_boards.c'))
-system_ss.add(when: 'CONFIG_RASPI', if_true: files('bcm2835_peripherals.c'))
+system_ss.add(when: 'CONFIG_RASPI', if_true: files(
+ 'bcm2835_peripherals.c',
+ 'bcm2836.c',
+ 'raspi.c'))
system_ss.add(when: 'CONFIG_TOSA', if_true: files('tosa.c'))
hw_arch += {'arm': arm_ss}
--
2.41.0
[PATCH-for-9.0 03/11] target/arm: Declare ARM_CPU_TYPE_NAME/SUFFIX in 'cpu-qom.h'
Missed in commit 2d56be5a29 ("target: Declare
FOO_CPU_TYPE_NAME/SUFFIX in 'cpu-qom.h'"). See
it for more details.
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/cpu-qom.h | 3 +++
target/arm/cpu.h | 2 --
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index bf6b3604ed..be307037ff 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -33,6 +33,9 @@ typedef struct AArch64CPUClass AArch64CPUClass;
DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU,
TYPE_AARCH64_CPU)
+#define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
+#define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
+
bool target_aarch64_available(void);
#endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index a0282e0d28..d369275827 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2834,8 +2834,6 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync);
#define ARM_CPUID_TI915T 0x54029152
#define ARM_CPUID_TI925T 0x54029252
-#define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
-#define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
#define CPU_RESOLVING_TYPE TYPE_ARM_CPU
#define TYPE_ARM_HOST_CPU "host-" TYPE_ARM_CPU
--
2.41.0
[PATCH-for-9.0 07/11] hw/arm/bcm2836: Simplify access to 'start-powered-off' property
All ARM CPUs have the 'start-powered-off' property since commit
5de164304a ("arm: Allow secondary KVM CPUs to be booted via PSCI").
Note: since commit c1b701587e ("target/arm: Move start-powered-off
property to generic CPUState"), all CPUs for all targets have it.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/arm/bcm2836.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 055c909e95..198f9b5730 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -136,12 +136,8 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
bc->peri_base, _abort);
/* start powered off if not enabled */
-if (!object_property_set_bool(OBJECT(>cpu[n].core),
- "start-powered-off",
- n >= s->enabled_cpus,
- errp)) {
-return;
-}
+object_property_set_bool(OBJECT(>cpu[n].core), "start-powered-off",
+ n >= s->enabled_cpus, _abort);
if (!qdev_realize(DEVICE(>cpu[n].core), NULL, errp)) {
return;
--
2.41.0
[PATCH-for-9.0 00/11] hw/arm: Step toward building qemu-system-{arm, aarch64} altogether
Hi, This series is a step toward having a single qemu-system-aarch64 binary for both ARM and Aarch64 variants. First we add the TypeInfo::can_register() handler to QOM, to be able to decide at runtime if a type can be registered. We'll later use the target_aarch64_available() method to restrict some QOM types to the aarch64 build. Then few cleanups allow us to build the Raspi machines and its components as target-agnostic. To do that, instead of embedding a CPUState in its SoC container, we use a pointer to it. Since the type is forward-declared by "cpu-qom.h", we can use that in our hw/ headers. Then the correct CPU is instanciated by calling object_new() instead of object_initialize_child(). Finally objects are moved to meson system_ss[] source set to be built once. Does that look reasonable to keep merging TARGET_ARM/AARCH64? Thanks, Phil. Philippe Mathieu-Daudé (11): qom: Introduce the TypeInfo::can_register() handler target/arm: Add target_aarch64_available() helper target/arm: Declare ARM_CPU_TYPE_NAME/SUFFIX in 'cpu-qom.h' target/arm: Move ARM_CPU_IRQ/FIQ definitions to 'cpu-qom.h' target/arm: Move GTIMER definitions to 'cpu-defs.h' hw/arm/bcm2836: Simplify use of 'reset-cbar' property hw/arm/bcm2836: Simplify access to 'start-powered-off' property hw/arm/bcm2836: Use ARM_CPU 'mp-affinity' property hw/arm/bcm2836: Allocate ARM CPU state with object_new() hw/arm/raspi: Build bcm2836.o and raspi.o objects once hw/intc/meson: Simplify how arm_gicv3_kvm.o objects are built include/hw/arm/bcm2836.h| 4 ++-- include/qom/object.h| 4 target/arm/cpu-defs.h | 19 target/arm/cpu-qom.h| 11 ++ target/arm/cpu.h| 16 +- hw/arm/bcm2836.c| 43 - hw/arm/raspi.c | 8 +++ hw/intc/arm_gicv3_its_kvm.c | 1 + hw/intc/arm_gicv3_kvm.c | 1 + qom/object.c| 3 +++ target/arm/cpu.c| 9 hw/arm/meson.build | 6 -- hw/intc/meson.build | 6 -- 13 files changed, 80 insertions(+), 51 deletions(-) create mode 100644 target/arm/cpu-defs.h -- 2.41.0
[PATCH-for-9.0 05/11] target/arm: Move GTIMER definitions to 'cpu-defs.h'
To allow GTIMER_* definitions to be used by non-ARM specific
hardware models, move them to a new target agnostic "cpu-defs.h"
header.
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/cpu-defs.h | 19 +++
target/arm/cpu.h | 8 +---
hw/arm/bcm2836.c | 1 +
3 files changed, 21 insertions(+), 7 deletions(-)
create mode 100644 target/arm/cpu-defs.h
diff --git a/target/arm/cpu-defs.h b/target/arm/cpu-defs.h
new file mode 100644
index 00..1ad76aff14
--- /dev/null
+++ b/target/arm/cpu-defs.h
@@ -0,0 +1,19 @@
+/*
+ * ARM "target agnostic" CPU definitions
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef ARM_CPU_DEFS_H
+#define ARM_CPU_DEFS_H
+
+#define GTIMER_PHYS 0
+#define GTIMER_VIRT 1
+#define GTIMER_HYP 2
+#define GTIMER_SEC 3
+#define GTIMER_HYPVIRT 4
+#define NUM_GTIMERS 5
+
+#endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 124d829742..8107e4d446 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -24,6 +24,7 @@
#include "qemu/cpu-float.h"
#include "hw/registerfields.h"
#include "cpu-qom.h"
+#include "target/arm/cpu-defs.h"
#include "exec/cpu-defs.h"
#include "qapi/qapi-types-common.h"
@@ -154,13 +155,6 @@ typedef struct ARMGenericTimer {
uint64_t ctl; /* Timer Control register */
} ARMGenericTimer;
-#define GTIMER_PHYS 0
-#define GTIMER_VIRT 1
-#define GTIMER_HYP 2
-#define GTIMER_SEC 3
-#define GTIMER_HYPVIRT 4
-#define NUM_GTIMERS 5
-
#define VTCR_NSW (1u << 29)
#define VTCR_NSA (1u << 30)
#define VSTCR_SW VTCR_NSW
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 166dc896c0..6986b71cb4 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -15,6 +15,7 @@
#include "hw/arm/bcm2836.h"
#include "hw/arm/raspi_platform.h"
#include "hw/sysbus.h"
+#include "target/arm/cpu-defs.h"
struct BCM283XClass {
/*< private >*/
--
2.41.0
[PATCH-for-9.0 04/11] target/arm: Move ARM_CPU_IRQ/FIQ definitions to 'cpu-qom.h'
The ARM_CPU_IRQ/FIQ definitions are meant for the ARM CPU
QOM model. Move them to "cpu-qom.h" so any QOM code can
use them.
Signed-off-by: Philippe Mathieu-Daudé
---
Or do these definitions belong to cpu-defs.h?
---
target/arm/cpu-qom.h | 6 ++
target/arm/cpu.h | 6 --
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index be307037ff..38030450f7 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -36,6 +36,12 @@ DECLARE_CLASS_CHECKERS(AArch64CPUClass, AARCH64_CPU,
#define ARM_CPU_TYPE_SUFFIX "-" TYPE_ARM_CPU
#define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX)
+/* Meanings of the ARMCPU object's four inbound GPIO lines */
+#define ARM_CPU_IRQ 0
+#define ARM_CPU_FIQ 1
+#define ARM_CPU_VIRQ 2
+#define ARM_CPU_VFIQ 3
+
bool target_aarch64_available(void);
#endif
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index d369275827..124d829742 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -107,12 +107,6 @@ enum {
#define offsetofhigh32(S, M) (offsetof(S, M) + sizeof(uint32_t))
#endif
-/* Meanings of the ARMCPU object's four inbound GPIO lines */
-#define ARM_CPU_IRQ 0
-#define ARM_CPU_FIQ 1
-#define ARM_CPU_VIRQ 2
-#define ARM_CPU_VFIQ 3
-
/* ARM-specific extra insn start words:
* 1: Conditional execution bits
* 2: Partial exception syndrome for data aborts
--
2.41.0
Re: [PATCH] hw/net/rocker: avoid NULL pointer dereference in of_dpa_cmd_add_l2_flood
LORE has better view/threading for this one, https://lore.kernel.org/qemu-devel/20220624143912.1234427-1-mcasc...@redhat.com/ Which also links to https://gitlab.com/qemu-project/qemu/-/issues/1851 So basically, n/m. /mjt
Re: [PATCH 06/19] qapi/schema: adjust type narrowing for mypy's benefit
On Wed, Nov 22, 2023, 7:00 AM Markus Armbruster wrote: > John Snow writes: > > > On Tue, Nov 21, 2023, 9:09 AM Markus Armbruster > wrote: > > > >> John Snow writes: > >> > >> > We already take care to perform some type narrowing for arg_type and > >> > ret_type, but not in a way where mypy can utilize the result. A simple > >> > change to use a temporary variable helps the medicine go down. > >> > > >> > Signed-off-by: John Snow > >> > --- > >> > scripts/qapi/schema.py | 17 + > >> > 1 file changed, 9 insertions(+), 8 deletions(-) > >> > > >> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > >> > index 4600a566005..a1094283828 100644 > >> > --- a/scripts/qapi/schema.py > >> > +++ b/scripts/qapi/schema.py > >> > @@ -825,13 +825,14 @@ def __init__(self, name, info, doc, ifcond, > features, > >> > def check(self, schema): > >> > super().check(schema) > >> > if self._arg_type_name: > >> > -self.arg_type = schema.resolve_type( > >> > +arg_type = schema.resolve_type( > >> > self._arg_type_name, self.info, "command's 'data'") > >> > -if not isinstance(self.arg_type, QAPISchemaObjectType): > >> > +if not isinstance(arg_type, QAPISchemaObjectType): > >> > raise QAPISemError( > >> > self.info, > >> > "command's 'data' cannot take %s" > >> > -% self.arg_type.describe()) > >> > +% arg_type.describe()) > >> > +self.arg_type = arg_type > >> > if self.arg_type.variants and not self.boxed: > >> > raise QAPISemError( > >> > self.info, > > Same story as for QAPISchemaEvent.check() below. Correct? > Yep. > >> > @@ -848,8 +849,7 @@ def check(self, schema): > >> > if self.name not in > self.info.pragma.command_returns_exceptions: > >> > typ = self.ret_type > >> > if isinstance(typ, QAPISchemaArrayType): > >> > -typ = self.ret_type.element_type > >> > -assert typ > >> > +typ = typ.element_type > >> > > > > In this case, we've narrowed typ but not self.ret_type and mypy is not > sure > > they're synonymous here (lack of power in mypy's model, maybe?). Work in > > terms of the temporary type we've already narrowed so mypy knows we have > an > > element_type field. > > The conditional ensures @typ is QAPISchemaArrayType. > > In mypy's view, @typ is QAPISchemaArrayType, but self.ret_type is only > Optional[QAPISchemaType]. > > Therefore, it chokes on self.ret_type.element_type, but is happy with > typ.element_type. > > Correct? > I think so, yes. In this conditional block, we need to work in terms of typ, which has been narrowed. The broader type doesn't have .element_type. > Why delete the assertion? Oh! Hmm, should the deletion go into PATCH > 10? > Yeah, just a patch-splitting goof. I'll repair that. > >> if not isinstance(typ, QAPISchemaObjectType): > >> > raise QAPISemError( > >> > self.info, > >> > @@ -885,13 +885,14 @@ def __init__(self, name, info, doc, ifcond, > features, arg_type, boxed): > >> > def check(self, schema): > >> > super().check(schema) > >> > if self._arg_type_name: > >> > -self.arg_type = schema.resolve_type( > >> > +typ = schema.resolve_type( > >> > self._arg_type_name, self.info, "event's 'data'") > >> > -if not isinstance(self.arg_type, QAPISchemaObjectType): > >> > +if not isinstance(typ, QAPISchemaObjectType): > >> > raise QAPISemError( > >> > self.info, > >> > "event's 'data' cannot take %s" > >> > -% self.arg_type.describe()) > >> > +% typ.describe()) > >> > +self.arg_type = typ > >> > if self.arg_type.variants and not self.boxed: > >> > raise QAPISemError( > >> > self.info, > >> > >> Harmless enough. I can't quite see the mypy problem, though. Care to > >> elaborate a bit? > >> > > > > self.arg_type has a narrower type- or, it WILL at the end of this series > - > > so we need to narrow a temporary variable first before assigning it to > the > > object state. > > > > We already perform the necessary check/narrowing, so this is really just > > pointing out that it's a bad idea to assign the state before the type > > check. Now we type check before assigning state. > > After PATCH 16, .resolve_type() will return QAPISchemaType, and > self.arg_type will be Optional[QAPISchemaObjectType]. Correct? > Sounds right. Sometimes it's a little hard to see what the error is before the rest of the types go in, a hazard of needing all patches to bisect without regression. Do you want a more elaborate
[PATCH] hw/net/rocker: avoid NULL pointer dereference in of_dpa_cmd_add_l2_flood
Did this lost this CVE-2022-36648 fix?
https://lists.nongnu.org/archive/html/qemu-devel/2022-06/msg04469.html
rocker_tlv_parse_nested could return early because of no group ids in
the group_tlvs. In such case tlvs is NULL; tlvs[i + 1] in the next
for-loop will deref the NULL pointer.
Signed-off-by: Mauro Matteo Cascella
Reported-by:
---
hw/net/rocker/rocker_of_dpa.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
index b3b8c5bb6d..1611b79227 100644
--- a/hw/net/rocker/rocker_of_dpa.c
+++ b/hw/net/rocker/rocker_of_dpa.c
@@ -2039,6 +2039,11 @@ static int of_dpa_cmd_add_l2_flood(OfDpa *of_dpa,
OfDpaGroup *group,
rocker_tlv_parse_nested(tlvs, group->l2_flood.group_count,
group_tlvs[ROCKER_TLV_OF_DPA_GROUP_IDS]);
+if (!tlvs) {
+err = -ROCKER_EINVAL;
+goto err_out;
+}
+
for (i = 0; i < group->l2_flood.group_count; i++) {
group->l2_flood.group_ids[i] = rocker_tlv_get_le32(tlvs[i + 1]);
}
--
2.35.3
Re: [PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory
On 21/11/2023 22:10, Volodymyr Babchuk wrote:
From: Oleksandr Tyshchenko
Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to
inherit the owner of the directory.
Ah... so that's why the previous patch is there.
This is not the right way to fix it. The QEMU Xen support is *assuming*
that QEMU is either running in, or emulating, dom0. In the emulation
case this is probably fine, but the 'real Xen' case it should be using
the correct domid for node creation. I guess this could either be
supplied on the command line or discerned by reading the local domain
'domid' node.
Note that for other than Dom0 domain (non toolstack domain) the
"driver_domain" property should be set in domain config file for the
toolstack to create required directories in advance.
Signed-off-by: Oleksandr Tyshchenko
Signed-off-by: Volodymyr Babchuk
---
hw/xen/xen_pvdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index c5ad71e8dc..42bdd4f6c8 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -60,7 +60,8 @@ void xen_config_cleanup(void)
int xenstore_mkdir(char *path, int p)
{
-if (!qemu_xen_xs_create(xenstore, 0, 0, xen_domid, p, path)) {
+if (!qemu_xen_xs_create(xenstore, 0, XS_PRESERVE_OWNER,
+xen_domid, p, path)) {
xen_pv_printf(NULL, 0, "xs_mkdir %s: failed\n", path);
return -1;
}
Re: [PATCH v2 3/6] xen: xenstore: add possibility to preserve owner
On 21/11/2023 22:10, Volodymyr Babchuk wrote: Add option to preserve owner when creating an entry in Xen Store. This may be needed in cases when Qemu is working as device model in a *may* be needed? I don't undertstand why this patch is necessary and the commit comment isn't really helping me. domain that is Domain-0, e.g. in driver domain. "owner" parameter for qemu_xen_xs_create() function can have special value XS_PRESERVE_OWNER, which will make specific implementation to get original owner of an entry and pass it back to set_permissions() call. Please note, that XenStore inherits permissions, so even if entry is newly created by, it already has the owner set to match owner of entry at previous level. Signed-off-by: Volodymyr Babchuk
Re: [PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it
On 21/11/2023 22:10, Volodymyr Babchuk wrote: From: David Woodhouse This allows a XenDevice implementation to know whether it was created by QEMU, or merely discovered in XenStore after the toolstack created it. This will allow us to create frontend/backend nodes only when we should, rather than unconditionally attempting to overwrite them from a driver domain which doesn't have privileges to do so. As an added benefit, it also means we no longer have to call the xen_backend_set_device() function from the device models immediately after calling qdev_realize_and_unref(). Even though we could make the argument that it's safe to do so, and the pointer to the unreffed device *will* actually still be valid, it still made my skin itch to look at it. Signed-off-by: David Woodhouse --- hw/block/xen-block.c | 3 +-- hw/char/xen_console.c| 2 +- hw/net/xen_nic.c | 2 +- hw/xen/xen-bus.c | 4 include/hw/xen/xen-backend.h | 2 -- include/hw/xen/xen-bus.h | 2 ++ 6 files changed, 9 insertions(+), 6 deletions(-) Actually, I think you should probably update xen_backend_try_device_destroy() in this patch too. It currently uses xen_backend_list_find() to check whether the specified XenDevice has an associated XenBackendInstance. Paul
Re: [PATCH v2 2/6] xen: backends: touch some XenStore nodes only if device...
On 21/11/2023 22:10, Volodymyr Babchuk wrote: was created by QEMU Xen PV devices in QEMU can be created in two ways: either by QEMU itself, if they were passed via command line, or by Xen toolstack. In the latter case, QEMU scans XenStore entries and configures devices accordingly. In the second case we don't want QEMU to write/delete front-end entries for two reasons: it might have no access to those entries if it is running in un-privileged domain and it is just incorrect to overwrite entries already provided by Xen toolstack, because toolstack manages those nodes. For example, it might read backend- or frontend- state to be sure that they are both disconnected and it is safe to destroy a domain. This patch checks presence of xendev->backend to check if Xen PV device is acting as a backend (i.e. it was configured by Xen Technally *all* XenDevice objects are backends. toolstack) to decide if it should touch frontend entries in XenStore. Also, when we need to remove XenStore entries during device teardown only if they weren't created by Xen toolstack. If they were created by toolstack, then it is toolstack's job to do proper clean-up.
