Re: A question regarding TARGET_ALIGNED_ONLY flag
Thank you Richard.
On Sun, May 19, 2024 at 6:26 PM Richard Henderson <
richard.hender...@linaro.org> wrote:
> On 5/19/24 16:23, Michael Rolnik wrote:
> > Hi all,
> >
> > Previously there was *TARGET_ALIGNED_ONLY* option that caused all memory
> accessed to be
> > aligned, now it seems to be removed.
> > Is there a way to achieve memory access alignment with QEMU v9.0.0 when
> I am building a
> > custom target?
>
> Explicitly add MO_ALIGN to the MemOp argument of
> tcg_gen_qemu_{ld,st}_{i32,i64,i128}.
>
>
> r~
>
>
--
Best Regards,
Michael Rolnik
Re: [PATCH v3 5/5] tests/qtest: Add pnv-spi-seeprom qtest
On 5/15/24 19:41, Chalapathi V wrote:
In this commit Write a qtest pnv-spi-seeprom-test to check the
SPI transactions between spi controller and seeprom device.
Signed-off-by: Chalapathi V
---
tests/qtest/pnv-spi-seeprom-test.c | 129 +
tests/qtest/meson.build| 1 +
2 files changed, 130 insertions(+)
create mode 100644 tests/qtest/pnv-spi-seeprom-test.c
diff --git a/tests/qtest/pnv-spi-seeprom-test.c
b/tests/qtest/pnv-spi-seeprom-test.c
new file mode 100644
index 00..bfa57f3234
--- /dev/null
+++ b/tests/qtest/pnv-spi-seeprom-test.c
@@ -0,0 +1,129 @@
+/*
+ * QTest testcase for PowerNV 10 Seeprom Communications
+ *
+ * Copyright (c) 2024, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "qemu/bswap.h"
+#include "hw/ssi/pnv_spi_regs.h"
+
+#define P10_XSCOM_BASE 0x000603fcull
+#define SPIC2_XSCOM_BASE0xc0040
+
+/* To transmit READ opcode and address */
+#define READ_OP_TDR_DATA0x03000100
+/*
+ * N1 shift - tx 4 bytes (transmit opcode and address)
+ * N2 shift - tx and rx 8 bytes.
+ */
+#define READ_OP_COUNTER_CONFIG 0x20402b00
+/* SEQ_OP_SELECT_RESPONDER - N1 Shift - N2 Shift * 5 - SEQ_OP_STOP */
+#define READ_OP_SEQUENCER 0x1130404040404010
+
+/* To transmit WREN(Set Write Enable Latch in status0 register) opcode */
+#define WRITE_OP_WREN 0x0600
+/* To transmit WRITE opcode, address and data */
+#define WRITE_OP_TDR_DATA 0x0300010012345678
+/* N1 shift - tx 8 bytes (transmit opcode, address and data) */
+#define WRITE_OP_COUNTER_CONFIG 0x40002000
+/* SEQ_OP_SELECT_RESPONDER - N1 Shift - SEQ_OP_STOP */
+#define WRITE_OP_SEQUENCER 0x11301000
+
+static uint64_t pnv_xscom_addr(uint32_t pcba)
+{
+return P10_XSCOM_BASE | ((uint64_t) pcba << 3);
+}
I would prefer if the test used the pnv_xscom_addr() definition from
tests/qtest/pnv-xscom.h.
+static uint64_t pnv_spi_seeprom_xscom_addr(uint32_t reg)
+{
+return pnv_xscom_addr(SPIC2_XSCOM_BASE + reg);
+}
+
+static void pnv_spi_controller_xscom_write(QTestState *qts, uint32_t reg,
+uint64_t val)
+{
+qtest_writeq(qts, pnv_spi_seeprom_xscom_addr(reg), val);
+}
+
+static uint64_t pnv_spi_controller_xscom_read(QTestState *qts, uint32_t reg)
+{
+return qtest_readq(qts, pnv_spi_seeprom_xscom_addr(reg));
+}
+
+static void spi_seeprom_transaction(QTestState *qts)
+{
+/* SPI transactions to SEEPROM to read from SEEPROM image */
+pnv_spi_controller_xscom_write(qts, COUNTER_CONFIG_REG,
+READ_OP_COUNTER_CONFIG);
+pnv_spi_controller_xscom_write(qts, SEQUENCER_OPERATION_REG,
+READ_OP_SEQUENCER);
+pnv_spi_controller_xscom_write(qts, TRANSMIT_DATA_REG, READ_OP_TDR_DATA);
+pnv_spi_controller_xscom_write(qts, TRANSMIT_DATA_REG, 0);
+/* Read 5*8 bytes from SEEPROM at 0x100 */
+uint64_t rdr_val = pnv_spi_controller_xscom_read(qts, RECEIVE_DATA_REG);
+printf("RDR READ = 0x%lx\n", rdr_val);
+rdr_val = pnv_spi_controller_xscom_read(qts, RECEIVE_DATA_REG);
+rdr_val = pnv_spi_controller_xscom_read(qts, RECEIVE_DATA_REG);
+rdr_val = pnv_spi_controller_xscom_read(qts, RECEIVE_DATA_REG);
+rdr_val = pnv_spi_controller_xscom_read(qts, RECEIVE_DATA_REG);
+printf("RDR READ = 0x%lx\n", rdr_val);
+
+/* SPI transactions to SEEPROM to write to SEEPROM image */
+pnv_spi_controller_xscom_write(qts, COUNTER_CONFIG_REG,
+WRITE_OP_COUNTER_CONFIG);
+/* Set Write Enable Latch bit of status0 register */
+pnv_spi_controller_xscom_write(qts, SEQUENCER_OPERATION_REG,
+WRITE_OP_SEQUENCER);
+pnv_spi_controller_xscom_write(qts, TRANSMIT_DATA_REG, WRITE_OP_WREN);
+/* write 8 bytes to SEEPROM at 0x100 */
+pnv_spi_controller_xscom_write(qts, SEQUENCER_OPERATION_REG,
+WRITE_OP_SEQUENCER);
+pnv_spi_controller_xscom_write(qts, TRANSMIT_DATA_REG, WRITE_OP_TDR_DATA);
+}
+
+/* Find complete path of in_file in the current working directory */
+static void find_file(const char *in_file, char *in_path)
+{
+g_autofree char *cwd = g_get_current_dir();
+char *filepath = g_build_filename(cwd, in_file, NULL);
+if (!access(filepath, F_OK)) {
+strcpy(in_path, filepath);
+} else {
+strcpy(in_path, "");
+printf("File %s not found within %s\n", in_file, cwd);
+}
+}
+
+static void test_spi_seeprom(void)
+{
+QTestState *qts = NULL;
+char seepromfile[500];
+find_file("sbe_measurement_seeprom.bin.ecc", seepromfile);
hmm, could you generate the contents instead ?
+if (strcmp(seepromfile, "")) {
+printf("Starting QEMU with seeprom file.\n");
+qts = qtest_initf("-m 2G -machine powernv10 -smp 2,cores=2,"
+
Re: [PATCH v3 4/5] hw/ppc: SPI controller wiring to P10 chip
+ Phil
On 5/15/24 19:41, Chalapathi V wrote:
In this commit, create SPI controller on p10 chip and connect cs irq.
The QOM tree of spi controller and seeprom are.
/machine (powernv10-machine)
/chip[0] (power10_v2.0-pnv-chip)
/pib_spic[2] (pnv-spi-controller)
/pnv-spi-bus.2 (SSI)
/xscom-spi-controller-regs[0] (memory-region)
/machine (powernv10-machine)
/peripheral-anon (container)
/device[0] (25csm04)
/WP#[0] (irq)
/ssi-gpio-cs[0] (irq)
(qemu) qom-get /machine/peripheral-anon /device[76] "parent_bus"
"/machine/chip[0]/pib_spic[2]/pnv-spi-bus.2"
Signed-off-by: Chalapathi V
---
include/hw/ppc/pnv_chip.h | 3 +++
hw/ppc/pnv.c| 21 -
hw/ppc/pnv_spi_controller.c | 8
3 files changed, 31 insertions(+), 1 deletion(-)
diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
index 8589f3291e..d464858f79 100644
--- a/include/hw/ppc/pnv_chip.h
+++ b/include/hw/ppc/pnv_chip.h
@@ -6,6 +6,7 @@
#include "hw/ppc/pnv_core.h"
#include "hw/ppc/pnv_homer.h"
#include "hw/ppc/pnv_n1_chiplet.h"
+#include "hw/ssi/pnv_spi.h"
#include "hw/ppc/pnv_lpc.h"
#include "hw/ppc/pnv_occ.h"
#include "hw/ppc/pnv_psi.h"
@@ -118,6 +119,8 @@ struct Pnv10Chip {
PnvSBE sbe;
PnvHomer homer;
PnvN1Chiplet n1_chiplet;
+#define PNV10_CHIP_MAX_PIB_SPIC 6
+PnvSpiController pib_spic[PNV10_CHIP_MAX_PIB_SPIC];
uint32_t nr_quads;
PnvQuad *quads;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 6e3a5ccdec..6850592a85 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1829,6 +1829,11 @@ static void pnv_chip_power10_instance_init(Object *obj)
for (i = 0; i < pcc->i2c_num_engines; i++) {
object_initialize_child(obj, "i2c[*]", &chip10->i2c[i], TYPE_PNV_I2C);
}
+
+for (i = 0; i < PNV10_CHIP_MAX_PIB_SPIC ; i++) {
+object_initialize_child(obj, "pib_spic[*]", &chip10->pib_spic[i],
+TYPE_PNV_SPI_CONTROLLER);
+}
}
static void pnv_chip_power10_quad_realize(Pnv10Chip *chip10, Error **errp)
@@ -2043,7 +2048,21 @@ static void pnv_chip_power10_realize(DeviceState *dev,
Error **errp)
qdev_get_gpio_in(DEVICE(&chip10->psi),
PSIHB9_IRQ_SBE_I2C));
}
-
+/* PIB SPI Controller */
+for (i = 0; i < PNV10_CHIP_MAX_PIB_SPIC; i++) {
+object_property_set_int(OBJECT(&chip10->pib_spic[i]), "spic_num",
+i, &error_fatal);
+/* pib_spic[2] connected to 25csm04 which implements 1 byte transfer */
+object_property_set_int(OBJECT(&chip10->pib_spic[i]), "transfer_len",
+(i == 2) ? 1 : 4, &error_fatal);
+if (!sysbus_realize(SYS_BUS_DEVICE(OBJECT
+(&chip10->pib_spic[i])), errp)) {
+return;
+}
+pnv_xscom_add_subregion(chip, PNV10_XSCOM_PIB_SPIC_BASE +
+i * PNV10_XSCOM_PIB_SPIC_SIZE,
+&chip10->pib_spic[i].xscom_spic_regs);
+}
}
static void pnv_rainier_i2c_init(PnvMachineState *pnv)
diff --git a/hw/ppc/pnv_spi_controller.c b/hw/ppc/pnv_spi_controller.c
index e87f583074..3d47e932de 100644
--- a/hw/ppc/pnv_spi_controller.c
+++ b/hw/ppc/pnv_spi_controller.c
@@ -1067,9 +1067,17 @@ static void operation_sequencer(PnvSpiController *s)
static void do_reset(DeviceState *dev)
{
PnvSpiController *s = PNV_SPICONTROLLER(dev);
+DeviceState *ssi_dev;
trace_pnv_spi_reset();
+/* Connect cs irq */
+ssi_dev = ssi_get_cs(s->ssi_bus, 0);
+if (ssi_dev) {
+qemu_irq cs_line = qdev_get_gpio_in_named(ssi_dev, SSI_GPIO_CS, 0);
+qdev_connect_gpio_out_named(DEVICE(s), "cs", 0, cs_line);
+}
+
/* Reset all N1 and N2 counters, and other constants */
s->N2_bits = 0;
s->N2_bytes = 0;
Reviewed-by: Cédric Le Goater
Thanks,
C.
Re: [PATCH v3 3/5] hw/block: Add Microchip's 25CSM04 to m25p80
On 5/15/24 19:41, Chalapathi V wrote:
Add Microchip's 25CSM04 Serial EEPROM to m25p80. 25CSM04 provides 4 Mbits
of Serial EEPROM utilizing the Serial Peripheral Interface (SPI) compatible
bus. The device is organized as 524288 bytes of 8 bits each (512Kbyte) and
is optimized for use in consumer and industrial applications where reliable
and dependable nonvolatile memory storage is essential.
Signed-off-by: Chalapathi V
Reviewed-by: Cédric Le Goater
Thanks,
C.
---
hw/block/m25p80.c | 3 +++
hw/ppc/Kconfig| 1 +
2 files changed, 4 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8dec134832..824a6c5c60 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -357,6 +357,9 @@ static const FlashPartInfo known_devices[] = {
.sfdp_read = m25p80_sfdp_w25q512jv },
{ INFO("w25q01jvq", 0xef4021, 0, 64 << 10, 2048, ER_4K),
.sfdp_read = m25p80_sfdp_w25q01jvq },
+
+/* Microchip */
+{ INFO("25csm04", 0x29cc00, 0x100, 64 << 10, 8, 0) },
};
typedef enum {
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 6f9670b377..a93430b734 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -40,6 +40,7 @@ config POWERNV
select PCA9552
select PCA9554
select SSI
+select SSI_M25P80
config PPC405
bool
Re: [PATCH v3 1/5] ppc/pnv: Add SPI controller model
On 5/15/24 19:41, Chalapathi V wrote:
SPI controller device model supports a connection to a single SPI responder.
This provide access to SPI seeproms, TPM, flash device and an ADC controller.
All SPI function control is mapped into the SPI register space to enable full
control by firmware. In this commit SPI configuration component is modelled
which contains all SPI configuration and status registers as well as the hold
registers for data to be sent or having been received.
An existing QEMU SSI framework is used and SSI_BUS is created.
Signed-off-by: Chalapathi V
---
include/hw/ppc/pnv_xscom.h| 3 +
include/hw/ssi/pnv_spi.h | 44 +++
include/hw/ssi/pnv_spi_regs.h | 114 +
hw/ppc/pnv_spi_controller.c | 228 ++
The file names are not consistent.
Please rename hw/ppc/pnv_spi_controller.c to /hw/ssi/pnv_spi.c.
hw/ppc/Kconfig| 1 +
hw/ppc/meson.build| 1 +
hw/ppc/trace-events | 6 +
7 files changed, 397 insertions(+)
create mode 100644 include/hw/ssi/pnv_spi.h
create mode 100644 include/hw/ssi/pnv_spi_regs.h
create mode 100644 hw/ppc/pnv_spi_controller.c
diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
index 6209e18492..a77b97f9b1 100644
--- a/include/hw/ppc/pnv_xscom.h
+++ b/include/hw/ppc/pnv_xscom.h
@@ -194,6 +194,9 @@ struct PnvXScomInterfaceClass {
#define PNV10_XSCOM_PEC_PCI_BASE 0x8010800 /* index goes upwards ... */
#define PNV10_XSCOM_PEC_PCI_SIZE 0x200
+#define PNV10_XSCOM_PIB_SPIC_BASE 0xc
+#define PNV10_XSCOM_PIB_SPIC_SIZE 0x20
+
void pnv_xscom_init(PnvChip *chip, uint64_t size, hwaddr addr);
int pnv_dt_xscom(PnvChip *chip, void *fdt, int root_offset,
uint64_t xscom_base, uint64_t xscom_size,
diff --git a/include/hw/ssi/pnv_spi.h b/include/hw/ssi/pnv_spi.h
new file mode 100644
index 00..244ee1cfc0
--- /dev/null
+++ b/include/hw/ssi/pnv_spi.h
@@ -0,0 +1,44 @@
+/*
+ * QEMU PowerPC SPI Controller model
+ *
+ * Copyright (c) 2024, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * This model Supports a connection to a single SPI responder.
+ * Introduced for P10 to provide access to SPI seeproms, TPM, flash device
+ * and an ADC controller.
+ */
+#include "hw/ssi/ssi.h"
+
+#ifndef PPC_PNV_SPI_CONTROLLER_H
+#define PPC_PNV_SPI_CONTROLLER_H
+
+#define TYPE_PNV_SPI_CONTROLLER "pnv-spi-controller"
+#define PNV_SPICONTROLLER(obj) \
+OBJECT_CHECK(PnvSpiController, (obj), TYPE_PNV_SPI_CONTROLLER)
OBJECT_DECLARE_SIMPLE_TYPE(PnvSpiController, PNV_SPI_CONTROLLER)
which means PNV_SPICONTROLLER -> PNV_SPI_CONTROLLER
+
+#define SPI_CONTROLLER_REG_SIZE 8
please add a PNV_ prefix : PNV_SPI_CONTROLLER_REG_SIZE
+
+#define TYPE_PNV_SPI_BUS "pnv-spi-bus"
+typedef struct PnvSpiController {
+SysBusDevice parent_obj;
+
+SSIBus *ssi_bus;
+qemu_irq *cs_line;
+MemoryRegionxscom_spic_regs;
+/* SPI controller object number */
+uint32_tspic_num;
+
+/* SPI Controller registers */
+uint64_terror_reg;
+uint64_tcounter_config_reg;
+uint64_tconfig_reg1;
+uint64_tclock_config_reset_control;
+uint64_tmemory_mapping_reg;
+uint64_ttransmit_data_reg;
+uint64_treceive_data_reg;
+uint8_t sequencer_operation_reg[SPI_CONTROLLER_REG_SIZE];
+uint64_tstatus_reg;
I don't think the _reg suffix are needed. This is minor.
+} PnvSpiController;
+#endif /* PPC_PNV_SPI_CONTROLLER_H */
diff --git a/include/hw/ssi/pnv_spi_regs.h b/include/hw/ssi/pnv_spi_regs.h
new file mode 100644
index 00..6f613aca5e
--- /dev/null
+++ b/include/hw/ssi/pnv_spi_regs.h
@@ -0,0 +1,114 @@
+/*
+ * QEMU PowerPC SPI Controller model
+ *
+ * Copyright (c) 2023, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef SPI_CONTROLLER_REGS_H
+#define SPI_CONTROLLER_REGS_H
please add a PNV_ prefix
+
+/* Error Register */
+#define ERROR_REG 0x00
+
+/* counter_config_reg */
+#define COUNTER_CONFIG_REG 0x01
+#define COUNTER_CONFIG_REG_SHIFT_COUNT_N1 PPC_BITMASK(0, 7)
+#define COUNTER_CONFIG_REG_SHIFT_COUNT_N2 PPC_BITMASK(8, 15)
+#define COUNTER_CONFIG_REG_COUNT_COMPARE1 PPC_BITMASK(24, 31)
+#define COUNTER_CONFIG_REG_COUNT_COMPARE2 PPC_BITMASK(32, 39)
+#define COUNTER_CONFIG_REG_N1_COUNT_CONTROL PPC_BITMASK(48, 51)
+#define COUNTER_CONFIG_REG_N2_COUNT_CONTROL PPC_BITMASK(52, 55)
+
+/* config_reg */
+#define CONFIG_REG1 0x02
+
+/* clock_config_reset_control_ecc_enable_reg */
+#define CLOCK_CONFIG_REG0x03
+#define CLOCK_CONFIG_RESET_CONTROL_HARD_RESET 0x0084;
+#define CLOCK_CONFIG_REG_RESET_CONTROL PPC_BITMASK(24, 27)
+#define CLOCK_CONFIG_REG_ECC_CONTROLPPC_BITMASK(28, 30)
+
+/* memory_mapping_
RE: [PATCH] intel_iommu: Use the latest fault reasons defined by spec
> From: Duan, Zhenzhong > Sent: Monday, May 20, 2024 11:41 AM > > > > >-Original Message- > >From: Jason Wang > >Sent: Monday, May 20, 2024 8:44 AM > >To: Duan, Zhenzhong > >Cc: qemu-devel@nongnu.org; Liu, Yi L ; Peng, Chao P > >; Yu Zhang ; Michael > >S. Tsirkin ; Paolo Bonzini ; > >Richard Henderson ; Eduardo Habkost > >; Marcel Apfelbaum > >Subject: Re: [PATCH] intel_iommu: Use the latest fault reasons defined by > >spec > > > >On Fri, May 17, 2024 at 6:26 PM Zhenzhong Duan > > wrote: > >> > >> From: Yu Zhang > >> > >> Currently we use only VTD_FR_PASID_TABLE_INV as fault reason. > >> Update with more detailed fault reasons listed in VT-d spec 7.2.3. > >> > >> Signed-off-by: Yu Zhang > >> Signed-off-by: Zhenzhong Duan > >> --- > > > >I wonder if this could be noticed by the guest or not. If yes should > >we consider starting to add thing like version to vtd emulation code? > > Kernel only dumps the reason like below: > > DMAR: [DMA Write NO_PASID] Request device [20:00.0] fault addr 0x123460 > [fault reason 0x71] SM: Present bit in first-level paging entry is clear Yes, guest kernel would notice it as the fault would be injected to vm. > Maybe bump 1.0 -> 1.1? > My understanding version number is only informational and is far from > accurate to mark if a feature supported. Driver should check cap/ecap > bits instead. Should the version ID here be aligned with VT-d spec? If yes, it should be 3.0 as the scalable mode was introduced in spec 3.0. And the fault code was redefined together with the introduction of this translation mode. Below is the a snippet from the change log of VT-d spec. June 2018 3.0 • Removed all text related to Extended-Mode. • Added support for scalable-mode translation for DMA Remapping, that enables PASIDgranular first-level, second-level, nested and pass-through translation functions. • Widen invalidation queue descriptors and page request queue descriptors from 128 bits to 256 bits and redefined page-request and page-response descriptors. • Listed all fault conditions in a unified table and described DMA Remapping hardware behavior under each condition. Assigned new code for each fault condition in scalablemode operation. • Added support for Accessed/Dirty (A/D) bits in second-level translation. • Added support for submitting commands and receiving response from virtual DMA Remapping hardware. • Added a table on snooping behavior and memory type of hardware access to various remapping structures as appendix. • Move Page Request Overflow (PRO) fault reporting from Fault Status register (FSTS_REG) to Page Request Status register (PRS_REG). Regards. Yi Liu
Re: [PATCH v12 10/13] virtio-gpu: Move fence_poll timer to VirtIOGPUGL
On 2024/05/20 6:27, Dmitry Osipenko wrote: Move fence_poll timer to VirtIOGPUGL for consistency with cmdq_resume_bh that are used only by GL device. Signed-off-by: Dmitry Osipenko Thanks for refacotoring. Please move this before "[PATCH v12 01/13] virtio-gpu: Unrealize GL device" so that you don't have to rewrite code introduced by that patch.
Re: [PATCH v12 09/13] virtio-gpu: Handle resource blob commands
On 2024/05/20 6:27, Dmitry Osipenko wrote:
From: Antonio Caggiano
Support BLOB resources creation, mapping and unmapping by calling the
new stable virglrenderer 0.10 interface. Only enabled when available and
via the blob config. E.g. -device virtio-vga-gl,blob=true
Signed-off-by: Antonio Caggiano
Signed-off-by: Xenia Ragiadakou
Signed-off-by: Huang Rui
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 310 -
hw/display/virtio-gpu.c| 4 +-
include/hw/virtio/virtio-gpu.h | 2 +
3 files changed, 312 insertions(+), 4 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 7d4d2882a5af..63a5a983aad6 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -26,6 +26,18 @@
struct virtio_gpu_virgl_resource {
struct virtio_gpu_simple_resource base;
+MemoryRegion *mr;
+
+/*
+ * Used by virgl_cmd_resource_unref() to know whether async
+ * unmapping has been started. Blob can be both mapped/unmapped
+ * on unref and we shouldn't unmap blob that wasn't mapped in the
+ * first place because it's a error condition. This flag prevents
+ * performing step 3 of the async unmapping process described in the
+ * comment to virtio_gpu_virgl_async_unmap_resource_blob() if blob
+ * wasn't mapped in the first place on unref.
+ */
+bool async_unmap_in_progress;
I suggest adding a field that tells if mr is deleted to
virtio_gpu_virgl_hostmem_region instead to minimize the size of
virtio_gpu_virgl_resource.
};
static struct virtio_gpu_virgl_resource *
@@ -49,6 +61,128 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
}
#endif
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+struct virtio_gpu_virgl_hostmem_region {
+MemoryRegion mr;
+struct VirtIOGPU *g;
+struct virtio_gpu_virgl_resource *res;
+};
+
+static void virtio_gpu_virgl_resume_cmdq_bh(void *opaque)
+{
+VirtIOGPU *g = opaque;
+
+virtio_gpu_process_cmdq(g);
+}
+
+static void virtio_gpu_virgl_hostmem_region_free(void *obj)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+struct virtio_gpu_virgl_hostmem_region *vmr;
+VirtIOGPUBase *b;
+VirtIOGPUGL *gl;
+
+vmr = container_of(mr, struct virtio_gpu_virgl_hostmem_region, mr);
+vmr->res->mr = NULL;
+
+b = VIRTIO_GPU_BASE(vmr->g);
+b->renderer_blocked--;
+
+/*
+ * memory_region_unref() is executed from RCU thread context, while
+ * virglrenderer works only on the main-loop thread that's holding GL
+ * context.
+ */
+gl = VIRTIO_GPU_GL(vmr->g);
+qemu_bh_schedule(gl->cmdq_resume_bh);
+g_free(vmr);
+}
+
+static int
+virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
+ struct virtio_gpu_virgl_resource *res,
+ uint64_t offset)
+{
+struct virtio_gpu_virgl_hostmem_region *vmr;
+VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+MemoryRegion *mr;
+uint64_t size;
+void *data;
+int ret;
+
+if (!virtio_gpu_hostmem_enabled(b->conf)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: hostmem disabled\n", __func__);
+return -EOPNOTSUPP;
+}
+
+ret = virgl_renderer_resource_map(res->base.resource_id, &data, &size);
+if (ret) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl resource:
%s\n",
+ __func__, strerror(-ret));
+return ret;
+}
+
+vmr = g_new0(struct virtio_gpu_virgl_hostmem_region, 1);
+vmr->res = res;
+vmr->g = g;
+
+mr = &vmr->mr;
+memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
+memory_region_add_subregion(&b->hostmem, offset, mr);
+memory_region_set_enabled(mr, true);
+
+/*
+ * MR could outlive the resource if MR's reference is held outside of
+ * virtio-gpu. In order to prevent unmapping resource while MR is alive,
+ * and thus, making the data pointer invalid, we will block virtio-gpu
+ * command processing until MR is fully unreferenced and freed.
+ */
+OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
+
+res->mr = mr;
+
+return 0;
+}
+
+static int
+virtio_gpu_virgl_async_unmap_resource_blob(VirtIOGPU *g,
+ struct virtio_gpu_virgl_resource
*res)
+{
+VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+MemoryRegion *mr = res->mr;
+int ret;
+
+/*
+ * Perform async unmapping in 3 steps:
+ *
+ * 1. Begin async unmapping with memory_region_del_subregion()
+ *and suspend/block cmd processing.
+ * 2. Wait for res->mr to be freed and cmd processing resumed
+ *asynchronously by virtio_gpu_virgl_hostmem_region_free().
+ * 3. Finish the unmapping with final virgl_renderer_resource_unmap().
+ */
+if (mr) {
+/* render will be unblocked once MR is freed */
+b->renderer_blocked++;
+
+/* memory region owns self res->mr objec
RE: [PATCH] intel_iommu: Use the latest fault reasons defined by spec
>-Original Message- >From: Jason Wang >Sent: Monday, May 20, 2024 8:44 AM >To: Duan, Zhenzhong >Cc: qemu-devel@nongnu.org; Liu, Yi L ; Peng, Chao P >; Yu Zhang ; Michael >S. Tsirkin ; Paolo Bonzini ; >Richard Henderson ; Eduardo Habkost >; Marcel Apfelbaum >Subject: Re: [PATCH] intel_iommu: Use the latest fault reasons defined by >spec > >On Fri, May 17, 2024 at 6:26 PM Zhenzhong Duan > wrote: >> >> From: Yu Zhang >> >> Currently we use only VTD_FR_PASID_TABLE_INV as fault reason. >> Update with more detailed fault reasons listed in VT-d spec 7.2.3. >> >> Signed-off-by: Yu Zhang >> Signed-off-by: Zhenzhong Duan >> --- > >I wonder if this could be noticed by the guest or not. If yes should >we consider starting to add thing like version to vtd emulation code? Kernel only dumps the reason like below: DMAR: [DMA Write NO_PASID] Request device [20:00.0] fault addr 0x123460 [fault reason 0x71] SM: Present bit in first-level paging entry is clear Maybe bump 1.0 -> 1.1? My understanding version number is only informational and is far from accurate to mark if a feature supported. Driver should check cap/ecap bits instead. Thanks Zhenzhong
RE: [PATCH] intel_iommu: Use the latest fault reasons defined by spec
>-Original Message-
>From: Liu, Yi L
>Subject: RE: [PATCH] intel_iommu: Use the latest fault reasons defined by
>spec
>
>> From: CLEMENT MATHIEU--DRIF
>> Sent: Friday, May 17, 2024 9:13 PM
>>
>> Hi Zhenzhong
>>
>> On 17/05/2024 12:23, Zhenzhong Duan wrote:
>> > Caution: External email. Do not open attachments or click links, unless
>this email
>> comes from a known sender and you know the content is safe.
>> >
>> >
>> > From: Yu Zhang
>> >
>> > Currently we use only VTD_FR_PASID_TABLE_INV as fault reason.
>> > Update with more detailed fault reasons listed in VT-d spec 7.2.3.
>> >
>> > Signed-off-by: Yu Zhang
>> > Signed-off-by: Zhenzhong Duan
>> > ---
>> > hw/i386/intel_iommu_internal.h | 8 +++-
>> > hw/i386/intel_iommu.c | 25 -
>> > 2 files changed, 23 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/hw/i386/intel_iommu_internal.h
>b/hw/i386/intel_iommu_internal.h
>> > index f8cf99bddf..666e2cf2ce 100644
>> > --- a/hw/i386/intel_iommu_internal.h
>> > +++ b/hw/i386/intel_iommu_internal.h
>> > @@ -311,7 +311,13 @@ typedef enum VTDFaultReason {
>> > * request while disabled */
>> > VTD_FR_IR_SID_ERR = 0x26, /* Invalid Source-ID */
>> >
>> > -VTD_FR_PASID_TABLE_INV = 0x58, /*Invalid PASID table entry */
>> > +/* PASID directory entry access failure */
>> > +VTD_FR_PASID_DIR_ACCESS_ERR = 0x50,
>> > +/* The Present(P) field of pasid directory entry is 0 */
>> > +VTD_FR_PASID_DIR_ENTRY_P = 0x51,
>> > +VTD_FR_PASID_TABLE_ACCESS_ERR = 0x58, /* PASID table entry
>access failure */
>> > +VTD_FR_PASID_ENTRY_P = 0x59, /* The Present(P) field of pasidt-
>entry is 0 */
>> s/pasidt/pasid
>
>Per spec, it is pasid table entry. So Zhenzhong may need to use the same
>word
>With the line below. E.g. PASID Table entry.
Yes, will fix.
Thanks
Zhenzhong
>
>Regards,
>Yi Liu
>
>> > +VTD_FR_PASID_TABLE_ENTRY_INV = 0x5b, /*Invalid PASID table
>entry */
>> >
>> > /* Output address in the interrupt address range for scalable mode
>*/
>> > VTD_FR_SM_INTERRUPT_ADDR = 0x87,
>> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> > index cc8e59674e..0951ebb71d 100644
>> > --- a/hw/i386/intel_iommu.c
>> > +++ b/hw/i386/intel_iommu.c
>> > @@ -771,7 +771,7 @@ static int
>vtd_get_pdire_from_pdir_table(dma_addr_t
>> pasid_dir_base,
>> > addr = pasid_dir_base + index * entry_size;
>> > if (dma_memory_read(&address_space_memory, addr,
>> > pdire, entry_size, MEMTXATTRS_UNSPECIFIED)) {
>> > -return -VTD_FR_PASID_TABLE_INV;
>> > +return -VTD_FR_PASID_DIR_ACCESS_ERR;
>> > }
>> >
>> > pdire->val = le64_to_cpu(pdire->val);
>> > @@ -789,6 +789,7 @@ static int
>vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState
>> *s,
>> > dma_addr_t addr,
>> > VTDPASIDEntry *pe)
>> > {
>> > +uint8_t pgtt;
>> > uint32_t index;
>> > dma_addr_t entry_size;
>> > X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
>> > @@ -798,7 +799,7 @@ static int
>vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState
>> *s,
>> > addr = addr + index * entry_size;
>> > if (dma_memory_read(&address_space_memory, addr,
>> > pe, entry_size, MEMTXATTRS_UNSPECIFIED)) {
>> > -return -VTD_FR_PASID_TABLE_INV;
>> > +return -VTD_FR_PASID_TABLE_ACCESS_ERR;
>> > }
>> > for (size_t i = 0; i < ARRAY_SIZE(pe->val); i++) {
>> > pe->val[i] = le64_to_cpu(pe->val[i]);
>> > @@ -806,11 +807,13 @@ static int
>vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState
>> *s,
>> >
>> > /* Do translation type check */
>> > if (!vtd_pe_type_check(x86_iommu, pe)) {
>> > -return -VTD_FR_PASID_TABLE_INV;
>> > +return -VTD_FR_PASID_TABLE_ENTRY_INV;
>> > }
>> >
>> > -if (!vtd_is_level_supported(s, VTD_PE_GET_LEVEL(pe))) {
>> > -return -VTD_FR_PASID_TABLE_INV;
>> > +pgtt = VTD_PE_GET_TYPE(pe);
>> > +if (pgtt == VTD_SM_PASID_ENTRY_SLT &&
>> > +!vtd_is_level_supported(s, VTD_PE_GET_LEVEL(pe))) {
>> > +return -VTD_FR_PASID_TABLE_ENTRY_INV;
>> > }
>> >
>> > return 0;
>> > @@ -851,7 +854,7 @@ static int
>vtd_get_pe_from_pasid_table(IntelIOMMUState *s,
>> > }
>> >
>> > if (!vtd_pdire_present(&pdire)) {
>> > -return -VTD_FR_PASID_TABLE_INV;
>> > +return -VTD_FR_PASID_DIR_ENTRY_P;
>> > }
>> >
>> > ret = vtd_get_pe_from_pdire(s, pasid, &pdire, pe);
>> > @@ -860,7 +863,7 @@ static int
>vtd_get_pe_from_pasid_table(IntelIOMMUState *s,
>> > }
>> >
>> > if (!vtd_pe_present(pe)) {
>> > -return -VTD_FR_PASID_TABLE_INV;
>> > +return -VTD_FR_PASID_ENTRY_P;
>> > }
>> >
>> > return 0;
>> > @@ -913,7 +916,7 @@ static int
>vtd_ce_get_pasid_fpd(IntelIOMMUState *s,
>> > }
>> >
>> >
RE: [PATCH v3 08/16] aspeed/smc: support 64 bits dma dram address
Hi Cedric,
>
> Hello Jamin
>
> On 5/15/24 11:01, Jamin Lin wrote:
> > Hi Cedric,
> >
> > Sorry reply you late.
> >> Hello Jamin,
> >>
> >> To handle the DMA DRAM Side Address High register, we should
> >> reintroduce an "dram-base" property which I removed a while ago.
> Something like :
> >>
> >>
> >>
> >> diff --git a/include/hw/ssi/aspeed_smc.h
> >> b/include/hw/ssi/aspeed_smc.h index 7f32e43ff6f3..6d8ef6bc968f 100644
> >> --- a/include/hw/ssi/aspeed_smc.h
> >> +++ b/include/hw/ssi/aspeed_smc.h
> >> @@ -76,6 +76,7 @@ struct AspeedSMCState {
> >>AddressSpace flash_as;
> >>MemoryRegion *dram_mr;
> >>AddressSpace dram_as;
> >> +uint64_t dram_base;
> >>
> >>AddressSpace wdt2_as;
> >>MemoryRegion *wdt2_mr;
> >> diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c index
> >> 38858e4fdec1..3417949ad8a3 100644
> >> --- a/hw/arm/aspeed_ast27x0.c
> >> +++ b/hw/arm/aspeed_ast27x0.c
> >> @@ -500,6 +500,8 @@ static void
> >> aspeed_soc_ast2700_realize(DeviceState
> >> *dev, Error **errp)
> >>}
> >>
> >>/* FMC, The number of CS is set at the board level */
> >> +object_property_set_int(OBJECT(&s->fmc), "dram-base",
> >> +sc->memmap[ASPEED_DEV_SDRAM],
> >> + &error_abort);
> >>object_property_set_link(OBJECT(&s->fmc), "dram",
> >> OBJECT(s->dram_mr),
> >> &error_abort);
> >>if (!sysbus_realize(SYS_BUS_DEVICE(&s->fmc), errp)) { diff
> >> --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index
> >> 3fa783578e9e..29ebfc0fd8c8 100644
> >> --- a/hw/ssi/aspeed_smc.c
> >> +++ b/hw/ssi/aspeed_smc.c
> >> @@ -1372,6 +1372,7 @@ static const VMStateDescription
> >> vmstate_aspeed_smc = {
> >>
> >>static Property aspeed_smc_properties[] = {
> >>DEFINE_PROP_BOOL("inject-failure", AspeedSMCState,
> >> inject_failure, false),
> >> +DEFINE_PROP_UINT64("dram-base", AspeedSMCState, dram_base,
> 0),
> >>DEFINE_PROP_LINK("dram", AspeedSMCState, dram_mr,
> >> TYPE_MEMORY_REGION, MemoryRegion *),
> >>DEFINE_PROP_LINK("wdt2", AspeedSMCState, wdt2_mr,
> >>
> >>
> > I appreciate your kindly support and thanks for your suggestion.
> > Will add it.
>
> See my aspeed-9.1 branch, I did some changes, mostly in the last patch.
>
> * aspeed_smc_dma_len()
>
>- can use QEMU_ALIGN_UP(). simpler.
>
> * aspeed_smc_dma_rw():
>
>- dram_addr -> dma_dram_offset
>- There is no need to protect updates of the R_DMA_DRAM_ADDR_HIGH
> register with aspeed_smc_has_dma_dram_addr_high() since it is
> already protected with MMIO accesses. Skip the check and update
> always.
>
> * aspeed_smc_dma_dram_addr()
>
>- same as above.
>
> You can merge the changes in the respective patches if you agree.
>
> Still on the TODO list :
>
>- GIC review
>- aspeed/soc: fix incorrect dram size for AST2700
>
>
>
>
> Thanks,
>
> C.
>
I merged this commit into my code base and thanks for your kindly support.
https://github.com/legoater/qemu/commit/d1bc2c776422a9d0d6af2b4414fae83fde1832ba
About GIC settings, you can refer to our kernel DTS setting for detail.
https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/arch/arm64/boot/dts/aspeed/aspeed-g7.dtsi#L143-L164
Thanks-Jamin
>
>
> >
> >>
> >> With that, see below for more comments,
> >>
> >> On 4/16/24 11:18, Jamin Lin wrote:
> >>> AST2700 support the maximum dram size is 8GiB and has a "DMA DRAM
> >> Side
> >>> Address High Part(0x7C)"
> >>> register to support 64 bits dma dram address.
> >>> Add helper routines functions to compute the dma dram address, new
> >>> features and update trace-event to support 64 bits dram address.
> >>>
> >>> Signed-off-by: Troy Lee
> >>> Signed-off-by: Jamin Lin
> >>> ---
> >>>hw/ssi/aspeed_smc.c | 66
> >> +++--
> >>>hw/ssi/trace-events | 2 +-
> >>>2 files changed, 59 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index
> >>> 71abc7a2d8..a67cac3d0f 100644
> >>> --- a/hw/ssi/aspeed_smc.c
> >>> +++ b/hw/ssi/aspeed_smc.c
> >>> @@ -132,6 +132,9 @@
> >>>#define FMC_WDT2_CTRL_BOOT_SOURCE BIT(4) /* O:
> primary
> >> 1: alternate */
> >>>#define FMC_WDT2_CTRL_EN BIT(0)
> >>>
> >>> +/* DMA DRAM Side Address High Part (AST2700) */
> >>> +#define R_DMA_DRAM_ADDR_HIGH (0x7c / 4)
> >>> +
> >>>/* DMA Control/Status Register */
> >>>#define R_DMA_CTRL(0x80 / 4)
> >>>#define DMA_CTRL_REQUEST (1 << 31)
> >>> @@ -187,6 +190,7 @@
> >>> * 0x1FF: 32M bytes
> >>> */
> >>>#define DMA_DRAM_ADDR(asc, val) ((val) &
> (asc)->dma_dram_mask)
> >>> +#define DMA_DRAM_ADDR_HIGH(val) ((val) & 0xf)
> >>>#define DMA_FLASH_ADDR(asc, val) ((val) &
> (asc)->dma_flash_mask)
> >>>#define DMA_LENGTH(val) ((val) & 0x01FF)
> >>>
> >>> @@ -207,6 +211,7 @@ static
[PATCH 1/1] scsi-bus: Remove unused parameter state from scsi_dma_restart_cb
Signed-off-by: Ray Lee
---
hw/scsi/scsi-bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 9e40b0c920..7c3df9b31a 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -255,7 +255,7 @@ static void scsi_dma_restart_req(SCSIRequest *req, void
*opaque)
scsi_req_unref(req);
}
-static void scsi_dma_restart_cb(void *opaque, bool running, RunState state)
+static void scsi_dma_restart_cb(void *opaque, bool running)
{
SCSIDevice *s = opaque;
--
2.39.3
Re: [PATCH] intel_iommu: Use the latest fault reasons defined by spec
On Fri, May 17, 2024 at 6:26 PM Zhenzhong Duan wrote: > > From: Yu Zhang > > Currently we use only VTD_FR_PASID_TABLE_INV as fault reason. > Update with more detailed fault reasons listed in VT-d spec 7.2.3. > > Signed-off-by: Yu Zhang > Signed-off-by: Zhenzhong Duan > --- I wonder if this could be noticed by the guest or not. If yes should we consider starting to add thing like version to vtd emulation code? Thanks
[PATCH v12 07/13] virtio-gpu: Support blob scanout using dmabuf fd
From: Robert Beckett
Support displaying blob resources by handling SET_SCANOUT_BLOB
command.
Signed-by: Antonio Caggiano
Signed-off-by: Robert Beckett
Signed-off-by: Huang Rui
Reviewed-by: Antonio Caggiano
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 109 +
hw/display/virtio-gpu.c| 12 ++--
include/hw/virtio/virtio-gpu.h | 7 +++
meson.build| 1 +
4 files changed, 123 insertions(+), 6 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 612fa86e5f34..7d4d2882a5af 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -17,6 +17,8 @@
#include "trace.h"
#include "hw/virtio/virtio.h"
#include "hw/virtio/virtio-gpu.h"
+#include "hw/virtio/virtio-gpu-bswap.h"
+#include "hw/virtio/virtio-gpu-pixman.h"
#include "ui/egl-helpers.h"
@@ -78,6 +80,7 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
res->base.height = c2d.height;
res->base.format = c2d.format;
res->base.resource_id = c2d.resource_id;
+res->base.dmabuf_fd = -1;
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c2d.resource_id;
@@ -125,6 +128,7 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
res->base.height = c3d.height;
res->base.format = c3d.format;
res->base.resource_id = c3d.resource_id;
+res->base.dmabuf_fd = -1;
QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
args.handle = c3d.resource_id;
@@ -509,6 +513,106 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
g_free(resp);
}
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
+ struct virtio_gpu_ctrl_command *cmd)
+{
+struct virtio_gpu_framebuffer fb = { 0 };
+struct virgl_renderer_resource_info info;
+struct virtio_gpu_virgl_resource *res;
+struct virtio_gpu_set_scanout_blob ss;
+uint64_t fbend;
+
+VIRTIO_GPU_FILL_CMD(ss);
+virtio_gpu_scanout_blob_bswap(&ss);
+trace_virtio_gpu_cmd_set_scanout_blob(ss.scanout_id, ss.resource_id,
+ ss.r.width, ss.r.height, ss.r.x,
+ ss.r.y);
+
+if (ss.scanout_id >= g->parent_obj.conf.max_outputs) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
+ __func__, ss.scanout_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+return;
+}
+
+if (ss.resource_id == 0) {
+virtio_gpu_disable_scanout(g, ss.scanout_id);
+return;
+}
+
+if (ss.width < 16 ||
+ss.height < 16 ||
+ss.r.x + ss.r.width > ss.width ||
+ss.r.y + ss.r.height > ss.height) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for"
+ " resource %d, rect (%d,%d)+%d,%d, fb %d %d\n",
+ __func__, ss.scanout_id, ss.resource_id,
+ ss.r.x, ss.r.y, ss.r.width, ss.r.height,
+ ss.width, ss.height);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+return;
+}
+
+res = virtio_gpu_virgl_find_resource(g, ss.resource_id);
+if (!res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+if (virgl_renderer_resource_get_info(ss.resource_id, &info)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not have info %d\n",
+ __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+if (res->base.dmabuf_fd < 0) {
+res->base.dmabuf_fd = info.fd;
+}
+if (res->base.dmabuf_fd < 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource not backed by dmabuf
%d\n",
+ __func__, ss.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+fb.format = virtio_gpu_get_pixman_format(ss.format);
+if (!fb.format) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: pixel format not supported %d\n",
+ __func__, ss.format);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+return;
+}
+
+fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8);
+fb.width = ss.width;
+fb.height = ss.height;
+fb.stride = ss.strides[0];
+fb.offset = ss.offsets[0] + ss.r.x * fb.bytes_pp + ss.r.y * fb.stride;
+
+fbend = fb.offset;
+fbend += fb.stride * (ss.r.height - 1);
+fbend += fb.bytes_pp * ss.r.width;
+if (fbend > res->base.blob_size) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: fb end out of range\n",
+ __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+return;
+}
+
+
[PATCH v12 11/13] virtio-gpu: Move print_stats timer to VirtIOGPUGL
Move print_stats timer to VirtIOGPUGL for consistency with
cmdq_resume_bh and fence_poll that are used only by GL device.
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 12 +++-
include/hw/virtio/virtio-gpu.h | 2 +-
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index c8b25a0f5d7c..a41c4f8e1cef 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1069,6 +1069,7 @@ static struct virgl_renderer_callbacks virtio_gpu_3d_cbs
= {
static void virtio_gpu_print_stats(void *opaque)
{
VirtIOGPU *g = opaque;
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
if (g->stats.requests) {
fprintf(stderr, "stats: vq req %4d, %3d -- 3D %4d (%5d)\n",
@@ -1083,7 +1084,7 @@ static void virtio_gpu_print_stats(void *opaque)
} else {
fprintf(stderr, "stats: idle\r");
}
-timer_mod(g->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
+timer_mod(gl->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
}
static void virtio_gpu_fence_poll(void *opaque)
@@ -1146,9 +1147,10 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
virtio_gpu_fence_poll, g);
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
-g->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL,
- virtio_gpu_print_stats, g);
-timer_mod(g->print_stats, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) +
1000);
+gl->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+ virtio_gpu_print_stats, g);
+timer_mod(gl->print_stats,
+ qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 1000);
}
gl->cmdq_resume_bh = aio_bh_new(qemu_get_aio_context(),
@@ -1175,7 +1177,7 @@ void virtio_gpu_virgl_deinit(VirtIOGPU *g)
qemu_bh_delete(gl->cmdq_resume_bh);
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
-timer_free(g->print_stats);
+timer_free(gl->print_stats);
}
timer_free(gl->fence_poll);
virgl_renderer_cleanup(NULL);
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 529c34481158..aea559cdacc5 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -195,7 +195,6 @@ struct VirtIOGPU {
uint64_t hostmem;
bool processing_cmdq;
-QEMUTimer *print_stats;
uint32_t inflight;
struct {
@@ -233,6 +232,7 @@ struct VirtIOGPUGL {
QEMUBH *cmdq_resume_bh;
QEMUTimer *fence_poll;
+QEMUTimer *print_stats;
};
struct VhostUserGPU {
--
2.44.0
[PATCH v12 02/13] virtio-gpu: Handle virtio_gpu_virgl_init() failure
virtio_gpu_virgl_init() may fail, leading to a further Qemu crash
because Qemu assumes it never fails. Check virtio_gpu_virgl_init()
return code and don't execute virtio commands on error. Failed
virtio_gpu_virgl_init() will result in a timed out virtio commands
for a guest OS.
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c | 15 +--
hw/display/virtio-gpu-virgl.c | 3 +++
include/hw/virtio/virtio-gpu.h | 1 +
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 0c0a8d136954..b353c3193afa 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -29,9 +29,14 @@ static void virtio_gpu_gl_update_cursor_data(VirtIOGPU *g,
struct virtio_gpu_scanout *s,
uint32_t resource_id)
{
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
uint32_t width, height;
uint32_t pixels, *data;
+if (!gl->renderer_inited) {
+return;
+}
+
data = virgl_renderer_get_cursor_data(resource_id, &width, &height);
if (!data) {
return;
@@ -60,13 +65,18 @@ static void virtio_gpu_gl_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
VirtIOGPU *g = VIRTIO_GPU(vdev);
VirtIOGPUGL *gl = VIRTIO_GPU_GL(vdev);
struct virtio_gpu_ctrl_command *cmd;
+int ret;
-if (!virtio_queue_ready(vq)) {
+if (!virtio_queue_ready(vq) || gl->renderer_init_failed) {
return;
}
if (!gl->renderer_inited) {
-virtio_gpu_virgl_init(g);
+ret = virtio_gpu_virgl_init(g);
+if (ret) {
+gl->renderer_init_failed = true;
+return;
+}
gl->renderer_inited = true;
}
if (gl->renderer_reset) {
@@ -101,6 +111,7 @@ static void virtio_gpu_gl_reset(VirtIODevice *vdev)
if (gl->renderer_inited && !gl->renderer_reset) {
virtio_gpu_virgl_reset_scanout(g);
gl->renderer_reset = true;
+gl->renderer_init_failed = false;
}
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 6ba4c24fda1d..bfbc6553e879 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -668,6 +668,9 @@ int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
void virtio_gpu_virgl_deinit(VirtIOGPU *g)
{
+if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
+timer_free(g->print_stats);
+}
timer_free(g->fence_poll);
virgl_renderer_cleanup(NULL);
}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 8ece1ec2e98b..236daba24dd2 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -230,6 +230,7 @@ struct VirtIOGPUGL {
bool renderer_inited;
bool renderer_reset;
+bool renderer_init_failed;
};
struct VhostUserGPU {
--
2.44.0
[PATCH v12 13/13] virtio-gpu: Support Venus context
From: Antonio Caggiano
Request Venus when initializing VirGL and if venus=true flag is set for
virtio-gpu-gl device.
Signed-off-by: Antonio Caggiano
Signed-off-by: Huang Rui
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c | 2 ++
hw/display/virtio-gpu-virgl.c | 22 ++
hw/display/virtio-gpu.c| 13 +
include/hw/virtio/virtio-gpu.h | 3 +++
meson.build| 1 +
5 files changed, 37 insertions(+), 4 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index b8f395be8d2d..2078e74050bb 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -148,6 +148,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev,
Error **errp)
static Property virtio_gpu_gl_properties[] = {
DEFINE_PROP_BIT("stats", VirtIOGPU, parent_obj.conf.flags,
VIRTIO_GPU_FLAG_STATS_ENABLED, false),
+DEFINE_PROP_BIT("venus", VirtIOGPU, parent_obj.conf.flags,
+VIRTIO_GPU_FLAG_VENUS_ENABLED, false),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 70e2d28ba966..2e9862dd186a 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1130,6 +1130,11 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
flags |= VIRGL_RENDERER_D3D11_SHARE_TEXTURE;
}
#endif
+#ifdef VIRGL_RENDERER_VENUS
+if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+flags |= VIRGL_RENDERER_VENUS | VIRGL_RENDERER_RENDER_SERVER;
+}
+#endif
ret = virgl_renderer_init(g, flags, &virtio_gpu_3d_cbs);
if (ret != 0) {
@@ -1161,7 +1166,7 @@ static void virtio_gpu_virgl_add_capset(GArray
*capset_ids, uint32_t capset_id)
GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
{
-uint32_t capset2_max_ver, capset2_max_size;
+uint32_t capset_max_ver, capset_max_size;
GArray *capset_ids;
capset_ids = g_array_new(false, false, sizeof(uint32_t));
@@ -1170,12 +1175,21 @@ GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL);
virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
- &capset2_max_ver,
- &capset2_max_size);
-if (capset2_max_ver) {
+ &capset_max_ver,
+ &capset_max_size);
+if (capset_max_ver) {
virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2);
}
+if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VENUS,
+ &capset_max_ver,
+ &capset_max_size);
+if (capset_max_size) {
+virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VENUS);
+}
+}
+
return capset_ids;
}
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 052ab493a00b..0518bb858e88 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1491,6 +1491,19 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error
**errp)
#endif
}
+if (virtio_gpu_venus_enabled(g->parent_obj.conf)) {
+#ifdef HAVE_VIRGL_VENUS
+if (!virtio_gpu_blob_enabled(g->parent_obj.conf) ||
+!virtio_gpu_hostmem_enabled(g->parent_obj.conf)) {
+error_setg(errp, "venus requires enabled blob and hostmem
options");
+return;
+}
+#else
+error_setg(errp, "old virglrenderer, venus unsupported");
+return;
+#endif
+}
+
if (!virtio_gpu_base_device_realize(qdev,
virtio_gpu_handle_ctrl_cb,
virtio_gpu_handle_cursor_cb,
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 7e1fee836802..ec5d7517f141 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -99,6 +99,7 @@ enum virtio_gpu_base_conf_flags {
VIRTIO_GPU_FLAG_BLOB_ENABLED,
VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED,
VIRTIO_GPU_FLAG_RUTABAGA_ENABLED,
+VIRTIO_GPU_FLAG_VENUS_ENABLED,
};
#define virtio_gpu_virgl_enabled(_cfg) \
@@ -117,6 +118,8 @@ enum virtio_gpu_base_conf_flags {
(_cfg.flags & (1 << VIRTIO_GPU_FLAG_RUTABAGA_ENABLED))
#define virtio_gpu_hostmem_enabled(_cfg) \
(_cfg.hostmem > 0)
+#define virtio_gpu_venus_enabled(_cfg) \
+(_cfg.flags & (1 << VIRTIO_GPU_FLAG_VENUS_ENABLED))
struct virtio_gpu_base_conf {
uint32_t max_outputs;
diff --git a/meson.build b/meson.build
index 503a7736eda0..5a2b7b660c67 100644
--- a/meson.build
+++ b/meson.build
@@ -2305,6 +2305,7 @@ if virgl.version().version_compare('>=1.0.0')
config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', 1)
config_host_data.set('HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS', 1)
config_host_data.set('HAVE_VIRGL_RESOURCE_BLOB', 1)
+ config_host_d
[PATCH v12 08/13] virtio-gpu: Support suspension of commands processing
Check whether command processing has been finished; otherwise, stop
processing commands and retry the command again next time. This allows
us to support asynchronous execution of non-fenced commands needed for
unmapping host blobs safely.
Suggested-by: Akihiko Odaki
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 1e57a53d346c..6c8c7213bafa 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1054,6 +1054,11 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
/* process command */
vgc->process_cmd(g, cmd);
+/* command suspended */
+if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) {
+break;
+}
+
QTAILQ_REMOVE(&g->cmdq, cmd, next);
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
g->stats.requests++;
--
2.44.0
[PATCH v12 09/13] virtio-gpu: Handle resource blob commands
From: Antonio Caggiano
Support BLOB resources creation, mapping and unmapping by calling the
new stable virglrenderer 0.10 interface. Only enabled when available and
via the blob config. E.g. -device virtio-vga-gl,blob=true
Signed-off-by: Antonio Caggiano
Signed-off-by: Xenia Ragiadakou
Signed-off-by: Huang Rui
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 310 -
hw/display/virtio-gpu.c| 4 +-
include/hw/virtio/virtio-gpu.h | 2 +
3 files changed, 312 insertions(+), 4 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 7d4d2882a5af..63a5a983aad6 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -26,6 +26,18 @@
struct virtio_gpu_virgl_resource {
struct virtio_gpu_simple_resource base;
+MemoryRegion *mr;
+
+/*
+ * Used by virgl_cmd_resource_unref() to know whether async
+ * unmapping has been started. Blob can be both mapped/unmapped
+ * on unref and we shouldn't unmap blob that wasn't mapped in the
+ * first place because it's a error condition. This flag prevents
+ * performing step 3 of the async unmapping process described in the
+ * comment to virtio_gpu_virgl_async_unmap_resource_blob() if blob
+ * wasn't mapped in the first place on unref.
+ */
+bool async_unmap_in_progress;
};
static struct virtio_gpu_virgl_resource *
@@ -49,6 +61,128 @@ virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
}
#endif
+#ifdef HAVE_VIRGL_RESOURCE_BLOB
+struct virtio_gpu_virgl_hostmem_region {
+MemoryRegion mr;
+struct VirtIOGPU *g;
+struct virtio_gpu_virgl_resource *res;
+};
+
+static void virtio_gpu_virgl_resume_cmdq_bh(void *opaque)
+{
+VirtIOGPU *g = opaque;
+
+virtio_gpu_process_cmdq(g);
+}
+
+static void virtio_gpu_virgl_hostmem_region_free(void *obj)
+{
+MemoryRegion *mr = MEMORY_REGION(obj);
+struct virtio_gpu_virgl_hostmem_region *vmr;
+VirtIOGPUBase *b;
+VirtIOGPUGL *gl;
+
+vmr = container_of(mr, struct virtio_gpu_virgl_hostmem_region, mr);
+vmr->res->mr = NULL;
+
+b = VIRTIO_GPU_BASE(vmr->g);
+b->renderer_blocked--;
+
+/*
+ * memory_region_unref() is executed from RCU thread context, while
+ * virglrenderer works only on the main-loop thread that's holding GL
+ * context.
+ */
+gl = VIRTIO_GPU_GL(vmr->g);
+qemu_bh_schedule(gl->cmdq_resume_bh);
+g_free(vmr);
+}
+
+static int
+virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
+ struct virtio_gpu_virgl_resource *res,
+ uint64_t offset)
+{
+struct virtio_gpu_virgl_hostmem_region *vmr;
+VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+MemoryRegion *mr;
+uint64_t size;
+void *data;
+int ret;
+
+if (!virtio_gpu_hostmem_enabled(b->conf)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: hostmem disabled\n", __func__);
+return -EOPNOTSUPP;
+}
+
+ret = virgl_renderer_resource_map(res->base.resource_id, &data, &size);
+if (ret) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl resource:
%s\n",
+ __func__, strerror(-ret));
+return ret;
+}
+
+vmr = g_new0(struct virtio_gpu_virgl_hostmem_region, 1);
+vmr->res = res;
+vmr->g = g;
+
+mr = &vmr->mr;
+memory_region_init_ram_ptr(mr, OBJECT(mr), "blob", size, data);
+memory_region_add_subregion(&b->hostmem, offset, mr);
+memory_region_set_enabled(mr, true);
+
+/*
+ * MR could outlive the resource if MR's reference is held outside of
+ * virtio-gpu. In order to prevent unmapping resource while MR is alive,
+ * and thus, making the data pointer invalid, we will block virtio-gpu
+ * command processing until MR is fully unreferenced and freed.
+ */
+OBJECT(mr)->free = virtio_gpu_virgl_hostmem_region_free;
+
+res->mr = mr;
+
+return 0;
+}
+
+static int
+virtio_gpu_virgl_async_unmap_resource_blob(VirtIOGPU *g,
+ struct virtio_gpu_virgl_resource
*res)
+{
+VirtIOGPUBase *b = VIRTIO_GPU_BASE(g);
+MemoryRegion *mr = res->mr;
+int ret;
+
+/*
+ * Perform async unmapping in 3 steps:
+ *
+ * 1. Begin async unmapping with memory_region_del_subregion()
+ *and suspend/block cmd processing.
+ * 2. Wait for res->mr to be freed and cmd processing resumed
+ *asynchronously by virtio_gpu_virgl_hostmem_region_free().
+ * 3. Finish the unmapping with final virgl_renderer_resource_unmap().
+ */
+if (mr) {
+/* render will be unblocked once MR is freed */
+b->renderer_blocked++;
+
+/* memory region owns self res->mr object and frees it by itself */
+memory_region_set_enabled(mr, false);
+memory_region_del_subregion(&b->hostmem, mr);
+object_unparent(OBJECT(mr));
+} else {
+ret = virgl_renderer_res
[PATCH v12 06/13] virtio-gpu: Add virgl resource management
From: Huang Rui
In a preparation to adding host blobs support to virtio-gpu, add virgl
resource management that allows to retrieve resource based on its ID
and virgl resource wrapper on top of simple resource that will be contain
fields specific to virgl.
Signed-off-by: Huang Rui
Reviewed-by: Antonio Caggiano
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 76 +++
1 file changed, 76 insertions(+)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index fc667559cc41..612fa86e5f34 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -22,6 +22,23 @@
#include
+struct virtio_gpu_virgl_resource {
+struct virtio_gpu_simple_resource base;
+};
+
+static struct virtio_gpu_virgl_resource *
+virtio_gpu_virgl_find_resource(VirtIOGPU *g, uint32_t resource_id)
+{
+struct virtio_gpu_simple_resource *res;
+
+res = virtio_gpu_find_resource(g, resource_id);
+if (!res) {
+return NULL;
+}
+
+return container_of(res, struct virtio_gpu_virgl_resource, base);
+}
+
#if VIRGL_RENDERER_CALLBACKS_VERSION >= 4
static void *
virgl_get_egl_display(G_GNUC_UNUSED void *cookie)
@@ -35,11 +52,34 @@ static void virgl_cmd_create_resource_2d(VirtIOGPU *g,
{
struct virtio_gpu_resource_create_2d c2d;
struct virgl_renderer_resource_create_args args;
+struct virtio_gpu_virgl_resource *res;
VIRTIO_GPU_FILL_CMD(c2d);
trace_virtio_gpu_cmd_res_create_2d(c2d.resource_id, c2d.format,
c2d.width, c2d.height);
+if (c2d.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+ __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_virgl_find_resource(g, c2d.resource_id);
+if (res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+ __func__, c2d.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = g_new0(struct virtio_gpu_virgl_resource, 1);
+res->base.width = c2d.width;
+res->base.height = c2d.height;
+res->base.format = c2d.format;
+res->base.resource_id = c2d.resource_id;
+QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
args.handle = c2d.resource_id;
args.target = 2;
args.format = c2d.format;
@@ -59,11 +99,34 @@ static void virgl_cmd_create_resource_3d(VirtIOGPU *g,
{
struct virtio_gpu_resource_create_3d c3d;
struct virgl_renderer_resource_create_args args;
+struct virtio_gpu_virgl_resource *res;
VIRTIO_GPU_FILL_CMD(c3d);
trace_virtio_gpu_cmd_res_create_3d(c3d.resource_id, c3d.format,
c3d.width, c3d.height, c3d.depth);
+if (c3d.resource_id == 0) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource id 0 is not allowed\n",
+ __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = virtio_gpu_virgl_find_resource(g, c3d.resource_id);
+if (res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
+ __func__, c3d.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
+res = g_new0(struct virtio_gpu_virgl_resource, 1);
+res->base.width = c3d.width;
+res->base.height = c3d.height;
+res->base.format = c3d.format;
+res->base.resource_id = c3d.resource_id;
+QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
+
args.handle = c3d.resource_id;
args.target = c3d.target;
args.format = c3d.format;
@@ -82,12 +145,21 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
struct virtio_gpu_ctrl_command *cmd)
{
struct virtio_gpu_resource_unref unref;
+struct virtio_gpu_virgl_resource *res;
struct iovec *res_iovs = NULL;
int num_iovs = 0;
VIRTIO_GPU_FILL_CMD(unref);
trace_virtio_gpu_cmd_res_unref(unref.resource_id);
+res = virtio_gpu_virgl_find_resource(g, unref.resource_id);
+if (!res) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: resource does not exist %d\n",
+ __func__, unref.resource_id);
+cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
+return;
+}
+
virgl_renderer_resource_detach_iov(unref.resource_id,
&res_iovs,
&num_iovs);
@@ -95,6 +167,10 @@ static void virgl_cmd_resource_unref(VirtIOGPU *g,
virtio_gpu_cleanup_mapping_iov(g, res_iovs, num_iovs);
}
virgl_renderer_resource_unref(unref.resource_id);
+
+QTAILQ_REMOVE(&g->reslist, &res->base, next);
+
+g_free(res);
}
static void virgl_cmd_context_create(VirtIOGPU *g,
--
2.44.0
[PATCH v12 01/13] virtio-gpu: Unrealize GL device
Even though GL GPU doesn't support hotplugging today, free virgl
resources when GL device is unrealized. For consistency.
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c | 11 +++
hw/display/virtio-gpu-virgl.c | 6 ++
include/hw/virtio/virtio-gpu.h | 1 +
3 files changed, 18 insertions(+)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index e06be60dfbfc..0c0a8d136954 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -136,6 +136,16 @@ static Property virtio_gpu_gl_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static void virtio_gpu_gl_device_unrealize(DeviceState *qdev)
+{
+VirtIOGPU *g = VIRTIO_GPU(qdev);
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(qdev);
+
+if (gl->renderer_inited) {
+virtio_gpu_virgl_deinit(g);
+}
+}
+
static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -149,6 +159,7 @@ static void virtio_gpu_gl_class_init(ObjectClass *klass,
void *data)
vgc->update_cursor_data = virtio_gpu_gl_update_cursor_data;
vdc->realize = virtio_gpu_gl_device_realize;
+vdc->unrealize = virtio_gpu_gl_device_unrealize;
vdc->reset = virtio_gpu_gl_reset;
device_class_set_props(dc, virtio_gpu_gl_properties);
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 9f34d0e6619c..6ba4c24fda1d 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -665,3 +665,9 @@ int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
return capset2_max_ver ? 2 : 1;
}
+
+void virtio_gpu_virgl_deinit(VirtIOGPU *g)
+{
+timer_free(g->fence_poll);
+virgl_renderer_cleanup(NULL);
+}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 56d6e821bf04..8ece1ec2e98b 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -336,6 +336,7 @@ void virtio_gpu_virgl_fence_poll(VirtIOGPU *g);
void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
void virtio_gpu_virgl_reset(VirtIOGPU *g);
int virtio_gpu_virgl_init(VirtIOGPU *g);
+void virtio_gpu_virgl_deinit(VirtIOGPU *g);
int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
#endif
--
2.44.0
[PATCH v12 12/13] virtio-gpu: Register capsets dynamically
From: Pierre-Eric Pelloux-Prayer
virtio_gpu_virgl_get_num_capsets will return "num_capsets", but we can't
assume that capset_index 1 is always VIRGL2 once we'll support more capsets,
like Venus and DRM capsets. Register capsets dynamically to avoid that problem.
Signed-off-by: Pierre-Eric Pelloux-Prayer
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c | 6 --
hw/display/virtio-gpu-virgl.c | 33 +
include/hw/virtio/virtio-gpu.h | 4 +++-
3 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index 4d0a10070ab3..b8f395be8d2d 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -135,8 +135,8 @@ static void virtio_gpu_gl_device_realize(DeviceState *qdev,
Error **errp)
}
g->parent_obj.conf.flags |= (1 << VIRTIO_GPU_FLAG_VIRGL_ENABLED);
-VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
-virtio_gpu_virgl_get_num_capsets(g);
+g->capset_ids = virtio_gpu_virgl_get_capsets(g);
+VIRTIO_GPU_BASE(g)->virtio_config.num_capsets = g->capset_ids->len;
#ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
@@ -159,6 +159,8 @@ static void virtio_gpu_gl_device_unrealize(DeviceState
*qdev)
if (gl->renderer_inited) {
virtio_gpu_virgl_deinit(g);
}
+
+g_array_unref(g->capset_ids);
}
static void virtio_gpu_gl_class_init(ObjectClass *klass, void *data)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index a41c4f8e1cef..70e2d28ba966 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -623,19 +623,13 @@ static void virgl_cmd_get_capset_info(VirtIOGPU *g,
VIRTIO_GPU_FILL_CMD(info);
memset(&resp, 0, sizeof(resp));
-if (info.capset_index == 0) {
-resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL;
-virgl_renderer_get_cap_set(resp.capset_id,
- &resp.capset_max_version,
- &resp.capset_max_size);
-} else if (info.capset_index == 1) {
-resp.capset_id = VIRTIO_GPU_CAPSET_VIRGL2;
+
+if (info.capset_index < g->capset_ids->len) {
+resp.capset_id = g_array_index(g->capset_ids, uint32_t,
+ info.capset_index);
virgl_renderer_get_cap_set(resp.capset_id,
&resp.capset_max_version,
&resp.capset_max_size);
-} else {
-resp.capset_max_version = 0;
-resp.capset_max_size = 0;
}
resp.hdr.type = VIRTIO_GPU_RESP_OK_CAPSET_INFO;
virtio_gpu_ctrl_response(g, cmd, &resp.hdr, sizeof(resp));
@@ -1160,14 +1154,29 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
return 0;
}
-int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g)
+static void virtio_gpu_virgl_add_capset(GArray *capset_ids, uint32_t capset_id)
+{
+g_array_append_val(capset_ids, capset_id);
+}
+
+GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g)
{
uint32_t capset2_max_ver, capset2_max_size;
+GArray *capset_ids;
+
+capset_ids = g_array_new(false, false, sizeof(uint32_t));
+
+/* VIRGL is always supported. */
+virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL);
+
virgl_renderer_get_cap_set(VIRTIO_GPU_CAPSET_VIRGL2,
&capset2_max_ver,
&capset2_max_size);
+if (capset2_max_ver) {
+virtio_gpu_virgl_add_capset(capset_ids, VIRTIO_GPU_CAPSET_VIRGL2);
+}
-return capset2_max_ver ? 2 : 1;
+return capset_ids;
}
void virtio_gpu_virgl_deinit(VirtIOGPU *g)
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index aea559cdacc5..7e1fee836802 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -208,6 +208,8 @@ struct VirtIOGPU {
QTAILQ_HEAD(, VGPUDMABuf) bufs;
VGPUDMABuf *primary[VIRTIO_GPU_MAX_SCANOUTS];
} dmabuf;
+
+GArray *capset_ids;
};
struct VirtIOGPUClass {
@@ -347,6 +349,6 @@ void virtio_gpu_virgl_reset_scanout(VirtIOGPU *g);
void virtio_gpu_virgl_reset(VirtIOGPU *g);
int virtio_gpu_virgl_init(VirtIOGPU *g);
void virtio_gpu_virgl_deinit(VirtIOGPU *g);
-int virtio_gpu_virgl_get_num_capsets(VirtIOGPU *g);
+GArray *virtio_gpu_virgl_get_capsets(VirtIOGPU *g);
#endif
--
2.44.0
[PATCH v12 03/13] virtio-gpu: Use pkgconfig version to decide which virgl features are available
New virglrerenderer features were stabilized with release of v1.0.0.
Presence of symbols in virglrenderer.h doesn't guarantee ABI compatibility
with pre-release development versions of libvirglerender. Use virglrenderer
version to decide reliably which virgl features are available.
Signed-off-by: Dmitry Osipenko
---
meson.build | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/meson.build b/meson.build
index a9de71d45064..413ec5179145 100644
--- a/meson.build
+++ b/meson.build
@@ -2301,11 +2301,8 @@ config_host_data.set('CONFIG_PNG', png.found())
config_host_data.set('CONFIG_VNC', vnc.found())
config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
config_host_data.set('CONFIG_VNC_SASL', sasl.found())
-if virgl.found()
- config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT',
- cc.has_member('struct
virgl_renderer_resource_info_ext', 'd3d_tex2d',
- prefix: '#include ',
- dependencies: virgl))
+if virgl.version().version_compare('>=1.0.0')
+ config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', 1)
endif
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
--
2.44.0
[PATCH v12 10/13] virtio-gpu: Move fence_poll timer to VirtIOGPUGL
Move fence_poll timer to VirtIOGPUGL for consistency with cmdq_resume_bh
that are used only by GL device.
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-virgl.c | 9 +
include/hw/virtio/virtio-gpu.h | 2 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 63a5a983aad6..c8b25a0f5d7c 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -1089,11 +1089,12 @@ static void virtio_gpu_print_stats(void *opaque)
static void virtio_gpu_fence_poll(void *opaque)
{
VirtIOGPU *g = opaque;
+VirtIOGPUGL *gl = VIRTIO_GPU_GL(g);
virgl_renderer_poll();
virtio_gpu_process_cmdq(g);
if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) {
-timer_mod(g->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
+timer_mod(gl->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
}
}
@@ -1141,8 +1142,8 @@ int virtio_gpu_virgl_init(VirtIOGPU *g)
return ret;
}
-g->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL,
- virtio_gpu_fence_poll, g);
+gl->fence_poll = timer_new_ms(QEMU_CLOCK_VIRTUAL,
+ virtio_gpu_fence_poll, g);
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
g->print_stats = timer_new_ms(QEMU_CLOCK_VIRTUAL,
@@ -1176,6 +1177,6 @@ void virtio_gpu_virgl_deinit(VirtIOGPU *g)
if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
timer_free(g->print_stats);
}
-timer_free(g->fence_poll);
+timer_free(gl->fence_poll);
virgl_renderer_cleanup(NULL);
}
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index f3c8014acc80..529c34481158 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -195,7 +195,6 @@ struct VirtIOGPU {
uint64_t hostmem;
bool processing_cmdq;
-QEMUTimer *fence_poll;
QEMUTimer *print_stats;
uint32_t inflight;
@@ -233,6 +232,7 @@ struct VirtIOGPUGL {
bool renderer_init_failed;
QEMUBH *cmdq_resume_bh;
+QEMUTimer *fence_poll;
};
struct VhostUserGPU {
--
2.44.0
[PATCH v12 04/13] virtio-gpu: Support context-init feature with virglrenderer
From: Huang Rui
Patch "virtio-gpu: CONTEXT_INIT feature" has added the context_init
feature flags. Expose this feature and support creating virglrenderer
context with flags using context_id if libvirglrenderer is new enough.
Originally-by: Antonio Caggiano
Signed-off-by: Huang Rui
Reviewed-by: Antonio Caggiano
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu-gl.c| 4
hw/display/virtio-gpu-virgl.c | 20 ++--
meson.build | 1 +
3 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/hw/display/virtio-gpu-gl.c b/hw/display/virtio-gpu-gl.c
index b353c3193afa..4d0a10070ab3 100644
--- a/hw/display/virtio-gpu-gl.c
+++ b/hw/display/virtio-gpu-gl.c
@@ -138,6 +138,10 @@ static void virtio_gpu_gl_device_realize(DeviceState
*qdev, Error **errp)
VIRTIO_GPU_BASE(g)->virtio_config.num_capsets =
virtio_gpu_virgl_get_num_capsets(g);
+#ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
+g->parent_obj.conf.flags |= 1 << VIRTIO_GPU_FLAG_CONTEXT_INIT_ENABLED;
+#endif
+
virtio_gpu_device_realize(qdev, errp);
}
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index bfbc6553e879..fc667559cc41 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -106,8 +106,24 @@ static void virgl_cmd_context_create(VirtIOGPU *g,
trace_virtio_gpu_cmd_ctx_create(cc.hdr.ctx_id,
cc.debug_name);
-virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen,
- cc.debug_name);
+if (cc.context_init) {
+if (!virtio_gpu_context_init_enabled(g->parent_obj.conf)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: context_init disabled",
+ __func__);
+cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC;
+return;
+}
+
+#ifdef HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS
+virgl_renderer_context_create_with_flags(cc.hdr.ctx_id,
+ cc.context_init,
+ cc.nlen,
+ cc.debug_name);
+return;
+#endif
+}
+
+virgl_renderer_context_create(cc.hdr.ctx_id, cc.nlen, cc.debug_name);
}
static void virgl_cmd_context_destroy(VirtIOGPU *g,
diff --git a/meson.build b/meson.build
index 413ec5179145..ba0f067484ca 100644
--- a/meson.build
+++ b/meson.build
@@ -2303,6 +2303,7 @@ config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
config_host_data.set('CONFIG_VNC_SASL', sasl.found())
if virgl.version().version_compare('>=1.0.0')
config_host_data.set('HAVE_VIRGL_D3D_INFO_EXT', 1)
+ config_host_data.set('HAVE_VIRGL_CONTEXT_CREATE_WITH_FLAGS', 1)
endif
config_host_data.set('CONFIG_VIRTFS', have_virtfs)
config_host_data.set('CONFIG_VTE', vte.found())
--
2.44.0
[PATCH v12 00/13] Support blob memory and venus on qemu
Hello, This series enables Vulkan Venus context support on virtio-gpu. All virglrender and almost all Linux kernel prerequisite changes needed by Venus are already in upstream. For kernel there is a pending KVM patchset that fixes mapping of compound pages needed for DRM drivers using TTM [1], othewrwise hostmem blob mapping will fail with a KVM error from Qemu. [1] https://lore.kernel.org/kvm/20240229025759.1187910-1-steve...@google.com/ You'll need to use recent Mesa version containing patch that removes dependency on cross-device feature from Venus that isn't supported by Qemu [2]. [2] https://gitlab.freedesktop.org/mesa/mesa/-/commit/087e9a96d13155e26987befae78b6ccbb7ae242b Example Qemu cmdline that enables Venus: qemu-system-x86_64 -device virtio-vga-gl,hostmem=4G,blob=true,venus=true \ -machine q35,accel=kvm,memory-backend=mem1 \ -object memory-backend-memfd,id=mem1,size=8G -m 8G Changes from V11 to V12 - Fixed virgl_cmd_resource_create_blob() error handling. Now it doesn't corrupt resource list and releases resource properly on error. Thanks to Akihiko Odaki for spotting the bug. - Added new patch that handles virtio_gpu_virgl_init() failure gracefully, fixing Qemu crash. Besides fixing the crash, it allows to implement a cleaner virtio_gpu_virgl_deinit(). - virtio_gpu_virgl_deinit() now assumes that previously virgl was initialized successfully when it was inited at all. Suggested by Akihiko Odaki. - Fixed missed freeing of print_stats timer in virtio_gpu_virgl_deinit() - Added back blob unmapping or RESOURCE_UNREF that was requested by Akihiko Odaki. Added comment to the code explaining how async unmapping works. Added back `res->async_unmap_in_progress` flag and added comment telling why it's needed. - Moved cmdq_resume_bh to VirtIOGPUGL and made coding style changes suggested by Akihiko Odaki. - Added patches that move fence_poll and print_stats timers to VirtIOGPUGL for consistency with cmdq_resume_bh. Changes from V10 to V11 - Replaced cmd_resume bool in struct ctrl_command with "cmd->finished + !VIRTIO_GPU_FLAG_FENCE" checking as was requested by Akihiko Odaki. - Reworked virgl_cmd_resource_unmap/unref_blob() to avoid re-adding the 'async_unmap_in_progress' flag that was dropped in v9: 1. virgl_cmd_resource_[un]map_blob() now doesn't check itself whether resource was previously mapped and lets virglrenderer to do the checking. 2. error returned by virgl_renderer_resource_unmap() is now handled and reported properly, previously the error wasn't checked. The virgl_renderer_resource_unmap() fails if resource wasn't mapped. 3. virgl_cmd_resource_unref_blob() now doesn't allow to unref resource that is mapped, it's a error condition if guest didn't unmap resource before doing the unref. Previously unref was implicitly unmapping resource. Changes from V9 to V10 - Dropped 'async_unmap_in_progress' variable and switched to use aio_bh_new() isntead of oneshot variant in the "blob commands" patch. - Further improved error messages by printing error code when actual error occurrs and using ERR_UNSPEC instead of ERR_ENOMEM when we don't really know if it was ENOMEM for sure. - Added vdc->unrealize for the virtio GL device and freed virgl data. - Dropped UUID and doc/migration patches. UUID feature isn't needed anymore, instead we changed Mesa Venus driver to not require UUID. - Renamed virtio-gpu-gl "vulkan" property name back to "venus". Changes from V8 to V9 - Added resuming of cmdq processing when hostmem MR is freed, as was suggested by Akihiko Odaki. - Added more error messages, suggested by Akihiko Odaki - Dropped superfluous 'res->async_unmap_completed', suggested by Akihiko Odaki. - Kept using cmd->suspended flag. Akihiko Odaki suggested to make virtio_gpu_virgl_process_cmd() return false if cmd processing is suspended, but it's not easy to implement due to ubiquitous VIRTIO_GPU_FILL_CMD() macros that returns void, requiring to change all the virtio-gpu processing code. - Added back virtio_gpu_virgl_resource as was requested by Akihiko Odaki, though I'm not convinced it's really needed. - Switched to use GArray, renamed capset2_max_ver/size vars and moved "vulkan" property definition to the virtio-gpu-gl device in the Venus patch, like was suggested by Akihiko Odaki. - Moved UUID to virtio_gpu_virgl_resource and dropped UUID save/restore since it will require bumping VM version and virgl device isn't miratable anyways. - Fixed exposing UUID feature with Rutabaga - Dropped linux-headers update patch because headers were already updated in Qemu/staging. - Added patch that updates virtio migration doc with a note about virtio-gpu migration specifics, suggested by Akihiko Odaki. - Addressed coding style issue noticed by Akihiko Odaki Changes from V7 to V8 - Supported suspension of virtio-gpu commands processing and made u
[PATCH v12 05/13] virtio-gpu: Don't require udmabuf when blobs and virgl are enabled
The udmabuf usage is mandatory when virgl is disabled and blobs feature
enabled in the Qemu machine configuration. If virgl and blobs are enabled,
then udmabuf requirement is optional. Since udmabuf isn't widely supported
by a popular Linux distros today, let's relax the udmabuf requirement for
blobs=on,virgl=on. Now, a full-featured virtio-gpu acceleration is
available to Qemu users without a need to have udmabuf available in the
system.
Reviewed-by: Antonio Caggiano
Signed-off-by: Huang Rui
Reviewed-by: Antonio Caggiano
Reviewed-by: Marc-André Lureau
Signed-off-by: Dmitry Osipenko
---
hw/display/virtio-gpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index ae831b6b3e3e..dac272ecadb1 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1472,6 +1472,7 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error
**errp)
if (virtio_gpu_blob_enabled(g->parent_obj.conf)) {
if (!virtio_gpu_rutabaga_enabled(g->parent_obj.conf) &&
+!virtio_gpu_virgl_enabled(g->parent_obj.conf) &&
!virtio_gpu_have_udmabuf()) {
error_setg(errp, "need rutabaga or udmabuf for blob resources");
return;
--
2.44.0
[PATCH V9 8/8] docs/specs/acpi_hw_reduced_hotplug: Add the CPU Hotplug Event Bit
GED interface is used by many hotplug events like memory hotplug, NVDIMM hotplug and non-hotplug events like system power down event. Each of these can be selected using a bit in the 32 bit GED IO interface. A bit has been reserved for the CPU hotplug event. Signed-off-by: Salil Mehta Reviewed-by: Gavin Shan --- docs/specs/acpi_hw_reduced_hotplug.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/specs/acpi_hw_reduced_hotplug.rst b/docs/specs/acpi_hw_reduced_hotplug.rst index 0bd3f9399f..3acd6fcd8b 100644 --- a/docs/specs/acpi_hw_reduced_hotplug.rst +++ b/docs/specs/acpi_hw_reduced_hotplug.rst @@ -64,7 +64,8 @@ GED IO interface (4 byte access) 0: Memory hotplug event 1: System power down event 2: NVDIMM hotplug event -3-31: Reserved + 3: CPU hotplug event +4-31: Reserved **write_access:** -- 2.34.1
[PATCH V9 4/8] hw/acpi: Update GED _EVT method AML with CPU scan
OSPM evaluates _EVT method to map the event. The CPU hotplug event eventually
results in start of the CPU scan. Scan figures out the CPU and the kind of
event(plug/unplug) and notifies it back to the guest. Update the GED AML _EVT
method with the call to \\_SB.CPUS.CSCN
Also, macro CPU_SCAN_METHOD might be referred in other places like during GED
intialization so it makes sense to have its definition placed in some common
header file like cpu_hotplug.h. But doing this can cause compilation break
because of the conflicting macro definitions present in cpu.c and cpu_hotplug.c
and because both these files get compiled due to historic reasons of x86 world
i.e. decision to use legacy(GPE.2)/modern(GED) CPU hotplug interface happens
during runtime [1]. To mitigate above, for now, declare a new common macro
ACPI_CPU_SCAN_METHOD for CPU scan method instead.
(This needs a separate discussion later on for clean-up)
Reference:
[1]
https://lore.kernel.org/qemu-devel/1463496205-251412-24-git-send-email-imamm...@redhat.com/
Co-developed-by: Keqian Zhu
Signed-off-by: Keqian Zhu
Signed-off-by: Salil Mehta
Reviewed-by: Jonathan Cameron
Reviewed-by: Gavin Shan
Tested-by: Vishnu Pajjuri
Tested-by: Xianglai Li
Tested-by: Miguel Luis
Reviewed-by: Shaoqin Huang
---
hw/acpi/cpu.c | 2 +-
hw/acpi/generic_event_device.c | 4
include/hw/acpi/cpu_hotplug.h | 2 ++
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 473b37ba88..af2b6655d2 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -327,7 +327,7 @@ const VMStateDescription vmstate_cpu_hotplug = {
#define CPUHP_RES_DEVICE "PRES"
#define CPU_LOCK "CPLK"
#define CPU_STS_METHOD"CSTA"
-#define CPU_SCAN_METHOD "CSCN"
+#define CPU_SCAN_METHOD ACPI_CPU_SCAN_METHOD
#define CPU_NOTIFY_METHOD "CTFY"
#define CPU_EJECT_METHOD "CEJ0"
#define CPU_OST_METHOD"COST"
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 54d3b4bf9d..63226b0040 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -109,6 +109,10 @@ void build_ged_aml(Aml *table, const char *name,
HotplugHandler *hotplug_dev,
aml_append(if_ctx, aml_call0(MEMORY_DEVICES_CONTAINER "."
MEMORY_SLOT_SCAN_METHOD));
break;
+case ACPI_GED_CPU_HOTPLUG_EVT:
+aml_append(if_ctx, aml_call0(ACPI_CPU_CONTAINER "."
+ ACPI_CPU_SCAN_METHOD));
+break;
case ACPI_GED_PWR_DOWN_EVT:
aml_append(if_ctx,
aml_notify(aml_name(ACPI_POWER_BUTTON_DEVICE),
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 48b291e45e..ef631750b4 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -20,6 +20,8 @@
#include "hw/acpi/cpu.h"
#define ACPI_CPU_HOTPLUG_REG_LEN 12
+#define ACPI_CPU_SCAN_METHOD "CSCN"
+#define ACPI_CPU_CONTAINER "\\_SB.CPUS"
typedef struct AcpiCpuHotplug {
Object *device;
--
2.34.1
[PATCH V9 7/8] gdbstub: Add helper function to unregister GDB register space
Add common function to help unregister the GDB register space. This shall be
done in context to the CPU unrealization.
Signed-off-by: Salil Mehta
Tested-by: Vishnu Pajjuri
Reviewed-by: Gavin Shan
Tested-by: Xianglai Li
Tested-by: Miguel Luis
Reviewed-by: Shaoqin Huang
Reviewed-by: Vishnu Pajjuri
---
gdbstub/gdbstub.c | 13 +
hw/core/cpu-common.c | 1 -
include/exec/gdbstub.h | 6 ++
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index b3574997ea..1949b09240 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -617,6 +617,19 @@ void gdb_register_coprocessor(CPUState *cpu,
}
}
+void gdb_unregister_coprocessor_all(CPUState *cpu)
+{
+/*
+ * Safe to nuke everything. GDBRegisterState::xml is static const char so
+ * it won't be freed
+ */
+g_array_free(cpu->gdb_regs, true);
+
+cpu->gdb_regs = NULL;
+cpu->gdb_num_regs = 0;
+cpu->gdb_num_g_regs = 0;
+}
+
static void gdb_process_breakpoint_remove_all(GDBProcess *p)
{
CPUState *cpu = gdb_get_first_cpu_in_process(p);
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 0f0a247f56..e5140b4bc1 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -274,7 +274,6 @@ static void cpu_common_finalize(Object *obj)
{
CPUState *cpu = CPU(obj);
-g_array_free(cpu->gdb_regs, TRUE);
qemu_lockcnt_destroy(&cpu->in_ioctl_lock);
qemu_mutex_destroy(&cpu->work_mutex);
}
diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index eb14b91139..249d4d4bc8 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -49,6 +49,12 @@ void gdb_register_coprocessor(CPUState *cpu,
gdb_get_reg_cb get_reg, gdb_set_reg_cb set_reg,
const GDBFeature *feature, int g_pos);
+/**
+ * gdb_unregister_coprocessor_all() - unregisters supplemental set of registers
+ * @cpu - the CPU associated with registers
+ */
+void gdb_unregister_coprocessor_all(CPUState *cpu);
+
/**
* gdbserver_start: start the gdb server
* @port_or_device: connection spec for gdb
--
2.34.1
[PATCH V9 6/8] physmem: Add helper function to destroy CPU AddressSpace
Virtual CPU Hot-unplug leads to unrealization of a CPU object. This also
involves destruction of the CPU AddressSpace. Add common function to help
destroy the CPU AddressSpace.
Signed-off-by: Salil Mehta
Tested-by: Vishnu Pajjuri
Reviewed-by: Gavin Shan
Tested-by: Xianglai Li
Tested-by: Miguel Luis
Reviewed-by: Shaoqin Huang
---
include/exec/cpu-common.h | 8
include/hw/core/cpu.h | 1 +
system/physmem.c | 29 +
3 files changed, 38 insertions(+)
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 815342d043..240ee04369 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -129,6 +129,14 @@ size_t qemu_ram_pagesize_largest(void);
*/
void cpu_address_space_init(CPUState *cpu, int asidx,
const char *prefix, MemoryRegion *mr);
+/**
+ * cpu_address_space_destroy:
+ * @cpu: CPU for which address space needs to be destroyed
+ * @asidx: integer index of this address space
+ *
+ * Note that with KVM only one address space is supported.
+ */
+void cpu_address_space_destroy(CPUState *cpu, int asidx);
void cpu_physical_memory_rw(hwaddr addr, void *buf,
hwaddr len, bool is_write);
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index bb398e8237..60b160d0b4 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -486,6 +486,7 @@ struct CPUState {
QSIMPLEQ_HEAD(, qemu_work_item) work_list;
struct CPUAddressSpace *cpu_ases;
+int cpu_ases_count;
int num_ases;
AddressSpace *as;
MemoryRegion *memory;
diff --git a/system/physmem.c b/system/physmem.c
index 342b7a8fd4..146f17826a 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -763,6 +763,7 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
if (!cpu->cpu_ases) {
cpu->cpu_ases = g_new0(CPUAddressSpace, cpu->num_ases);
+cpu->cpu_ases_count = cpu->num_ases;
}
newas = &cpu->cpu_ases[asidx];
@@ -776,6 +777,34 @@ void cpu_address_space_init(CPUState *cpu, int asidx,
}
}
+void cpu_address_space_destroy(CPUState *cpu, int asidx)
+{
+CPUAddressSpace *cpuas;
+
+assert(cpu->cpu_ases);
+assert(asidx >= 0 && asidx < cpu->num_ases);
+/* KVM cannot currently support multiple address spaces. */
+assert(asidx == 0 || !kvm_enabled());
+
+cpuas = &cpu->cpu_ases[asidx];
+if (tcg_enabled()) {
+memory_listener_unregister(&cpuas->tcg_as_listener);
+}
+
+address_space_destroy(cpuas->as);
+g_free_rcu(cpuas->as, rcu);
+
+if (asidx == 0) {
+/* reset the convenience alias for address space 0 */
+cpu->as = NULL;
+}
+
+if (--cpu->cpu_ases_count == 0) {
+g_free(cpu->cpu_ases);
+cpu->cpu_ases = NULL;
+}
+}
+
AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
{
/* Return the AddressSpace corresponding to the specified index */
--
2.34.1
[PATCH V9 5/8] hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
CPUs Control device(\\_SB.PCI0) register interface for the x86 arch is IO port
based and existing CPUs AML code assumes _CRS objects would evaluate to a system
resource which describes IO Port address. But on ARM arch CPUs control
device(\\_SB.PRES) register interface is memory-mapped hence _CRS object should
evaluate to system resource which describes memory-mapped base address. Update
build CPUs AML function to accept both IO/MEMORY region spaces and accordingly
update the _CRS object.
On x86, CPU Hotplug uses Generic ACPI GPE Block Bit 2 (GPE.2) event handler to
notify OSPM about any CPU hot(un)plug events. Latest CPU Hotplug is based on
ACPI Generic Event Device framework and uses ACPI GED device for the same. Not
all architectures support GPE based CPU Hotplug event handler. Hence, make AML
for GPE.2 event handler conditional.
Co-developed-by: Keqian Zhu
Signed-off-by: Keqian Zhu
Signed-off-by: Salil Mehta
Reviewed-by: Gavin Shan
Tested-by: Vishnu Pajjuri
Reviewed-by: Jonathan Cameron
Tested-by: Xianglai Li
Tested-by: Miguel Luis
Reviewed-by: Shaoqin Huang
---
hw/acpi/cpu.c | 23 ---
hw/i386/acpi-build.c | 3 ++-
include/hw/acpi/cpu.h | 5 +++--
3 files changed, 21 insertions(+), 10 deletions(-)
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index af2b6655d2..4c63514b16 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -343,9 +343,10 @@ const VMStateDescription vmstate_cpu_hotplug = {
#define CPU_FW_EJECT_EVENT "CEJF"
void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
-build_madt_cpu_fn build_madt_cpu, hwaddr io_base,
+build_madt_cpu_fn build_madt_cpu, hwaddr base_addr,
const char *res_root,
-const char *event_handler_method)
+const char *event_handler_method,
+AmlRegionSpace rs)
{
Aml *ifctx;
Aml *field;
@@ -370,13 +371,19 @@ void build_cpus_aml(Aml *table, MachineState *machine,
CPUHotplugFeatures opts,
aml_append(cpu_ctrl_dev, aml_mutex(CPU_LOCK, 0));
crs = aml_resource_template();
-aml_append(crs, aml_io(AML_DECODE16, io_base, io_base, 1,
+if (rs == AML_SYSTEM_IO) {
+aml_append(crs, aml_io(AML_DECODE16, base_addr, base_addr, 1,
ACPI_CPU_HOTPLUG_REG_LEN));
+} else {
+aml_append(crs, aml_memory32_fixed(base_addr,
+ ACPI_CPU_HOTPLUG_REG_LEN, AML_READ_WRITE));
+}
+
aml_append(cpu_ctrl_dev, aml_name_decl("_CRS", crs));
/* declare CPU hotplug MMIO region with related access fields */
aml_append(cpu_ctrl_dev,
-aml_operation_region("PRST", AML_SYSTEM_IO, aml_int(io_base),
+aml_operation_region("PRST", rs, aml_int(base_addr),
ACPI_CPU_HOTPLUG_REG_LEN));
field = aml_field("PRST", AML_BYTE_ACC, AML_NOLOCK,
@@ -700,9 +707,11 @@ void build_cpus_aml(Aml *table, MachineState *machine,
CPUHotplugFeatures opts,
aml_append(sb_scope, cpus_dev);
aml_append(table, sb_scope);
-method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
-aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
-aml_append(table, method);
+if (event_handler_method) {
+method = aml_method(event_handler_method, 0, AML_NOTSERIALIZED);
+aml_append(method, aml_call0("\\_SB.CPUS." CPU_SCAN_METHOD));
+aml_append(table, method);
+}
g_free(cphp_res_path);
}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 53f804ac16..b73b136605 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1537,7 +1537,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
.fw_unplugs_cpu = pm->smi_on_cpu_unplug,
};
build_cpus_aml(dsdt, machine, opts, pc_madt_cpu_entry,
- pm->cpu_hp_io_base, "\\_SB.PCI0", "\\_GPE._E02");
+ pm->cpu_hp_io_base, "\\_SB.PCI0", "\\_GPE._E02",
+ AML_SYSTEM_IO);
}
if (pcms->memhp_io_base && nr_mem) {
diff --git a/include/hw/acpi/cpu.h b/include/hw/acpi/cpu.h
index e6e1a9ef59..48cded697c 100644
--- a/include/hw/acpi/cpu.h
+++ b/include/hw/acpi/cpu.h
@@ -61,9 +61,10 @@ typedef void (*build_madt_cpu_fn)(int uid, const
CPUArchIdList *apic_ids,
GArray *entry, bool force_enabled);
void build_cpus_aml(Aml *table, MachineState *machine, CPUHotplugFeatures opts,
-build_madt_cpu_fn build_madt_cpu, hwaddr io_base,
+build_madt_cpu_fn build_madt_cpu, hwaddr base_addr,
const char *res_root,
-const char *event_handler_method);
+const char *event_handler_method,
+AmlRegionSpace rs);
void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list);
-
[PATCH V9 3/8] hw/acpi: Update ACPI GED framework to support vCPU Hotplug
ACPI GED (as described in the ACPI 6.4 spec) uses an interrupt listed in the
_CRS object of GED to intimate OSPM about an event. Later then demultiplexes the
notified event by evaluating ACPI _EVT method to know the type of event. Use
ACPI GED to also notify the guest kernel about any CPU hot(un)plug events.
ACPI CPU hotplug related initialization should only happen if ACPI_CPU_HOTPLUG
support has been enabled for particular architecture. Add cpu_hotplug_hw_init()
stub to avoid compilation break.
Co-developed-by: Keqian Zhu
Signed-off-by: Keqian Zhu
Signed-off-by: Salil Mehta
Reviewed-by: Jonathan Cameron
Reviewed-by: Gavin Shan
Reviewed-by: David Hildenbrand
Reviewed-by: Shaoqin Huang
Tested-by: Vishnu Pajjuri
Tested-by: Xianglai Li
Tested-by: Miguel Luis
Reviewed-by: Vishnu Pajjuri
---
hw/acpi/acpi-cpu-hotplug-stub.c| 6 ++
hw/acpi/cpu.c | 6 +-
hw/acpi/generic_event_device.c | 17 +
include/hw/acpi/generic_event_device.h | 4
4 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/acpi-cpu-hotplug-stub.c b/hw/acpi/acpi-cpu-hotplug-stub.c
index 3fc4b14c26..c6c61bb9cd 100644
--- a/hw/acpi/acpi-cpu-hotplug-stub.c
+++ b/hw/acpi/acpi-cpu-hotplug-stub.c
@@ -19,6 +19,12 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent,
Object *owner,
return;
}
+void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
+ CPUHotplugState *state, hwaddr base_addr)
+{
+return;
+}
+
void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
{
return;
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 69aaa563db..473b37ba88 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -221,7 +221,11 @@ void cpu_hotplug_hw_init(MemoryRegion *as, Object *owner,
const CPUArchIdList *id_list;
int i;
-assert(mc->possible_cpu_arch_ids);
+/* hotplug might not be available for all types like x86/microvm etc. */
+if (!mc->possible_cpu_arch_ids) {
+return;
+}
+
id_list = mc->possible_cpu_arch_ids(machine);
state->dev_count = id_list->len;
state->devs = g_new0(typeof(*state->devs), state->dev_count);
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 2d6e91b124..54d3b4bf9d 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -12,6 +12,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/acpi/acpi.h"
+#include "hw/acpi/cpu.h"
#include "hw/acpi/generic_event_device.h"
#include "hw/irq.h"
#include "hw/mem/pc-dimm.h"
@@ -25,6 +26,7 @@ static const uint32_t ged_supported_events[] = {
ACPI_GED_MEM_HOTPLUG_EVT,
ACPI_GED_PWR_DOWN_EVT,
ACPI_GED_NVDIMM_HOTPLUG_EVT,
+ACPI_GED_CPU_HOTPLUG_EVT,
};
/*
@@ -234,6 +236,8 @@ static void acpi_ged_device_plug_cb(HotplugHandler
*hotplug_dev,
} else {
acpi_memory_plug_cb(hotplug_dev, &s->memhp_state, dev, errp);
}
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+acpi_cpu_plug_cb(hotplug_dev, &s->cpuhp_state, dev, errp);
} else {
error_setg(errp, "virt: device plug request for unsupported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -248,6 +252,8 @@ static void acpi_ged_unplug_request_cb(HotplugHandler
*hotplug_dev,
if ((object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) &&
!(object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM {
acpi_memory_unplug_request_cb(hotplug_dev, &s->memhp_state, dev, errp);
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+acpi_cpu_unplug_request_cb(hotplug_dev, &s->cpuhp_state, dev, errp);
} else {
error_setg(errp, "acpi: device unplug request for unsupported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -261,6 +267,8 @@ static void acpi_ged_unplug_cb(HotplugHandler *hotplug_dev,
if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
acpi_memory_unplug_cb(&s->memhp_state, dev, errp);
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
+acpi_cpu_unplug_cb(&s->cpuhp_state, dev, errp);
} else {
error_setg(errp, "acpi: device unplug for unsupported device"
" type: %s", object_get_typename(OBJECT(dev)));
@@ -272,6 +280,7 @@ static void acpi_ged_ospm_status(AcpiDeviceIf *adev,
ACPIOSTInfoList ***list)
AcpiGedState *s = ACPI_GED(adev);
acpi_memory_ospm_status(&s->memhp_state, list);
+acpi_cpu_ospm_status(&s->cpuhp_state, list);
}
static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
@@ -286,6 +295,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev,
AcpiEventStatusBits ev)
sel = ACPI_GED_PWR_DOWN_EVT;
} else if (ev & ACPI_NVDIMM_HOTPLUG_STATUS) {
sel = ACPI_GED_NVDIMM_HOTPLUG_EVT;
+} else if (ev & ACPI_CPU_HOTPLUG_STATUS) {
+sel = ACPI_GED_CPU
[PATCH V9 0/8] Add architecture agnostic code to support vCPU Hotplug
Virtual CPU hotplug support is being added across various architectures[1][3].
This series adds various code bits common across all architectures:
1. vCPU creation and Parking code refactor [Patch 1]
2. Update ACPI GED framework to support vCPU Hotplug [Patch 2,3]
3. ACPI CPUs AML code change [Patch 4,5]
4. Helper functions to support unrealization of CPU objects [Patch 6,7]
5. Docs [Patch 8]
Repository:
[*] https://github.com/salil-mehta/qemu.git
virt-cpuhp-armv8/rfc-v3.arch.agnostic.v9
NOTE: For ARM, above will work in combination of the architecture specific part
based on
RFC V2 [1]. This architecture specific patch-set RFC V3 shall be floated soon
and is present
at below location
[*] https://github.com/salil-mehta/qemu/tree/virt-cpuhp-armv8/rfc-v3-rc1
Revision History:
Patch-set V8 -> V9
1. Addressed Vishnu Pajjuri's (Ampere) comments
- Added kvm_fd to trace in kvm_create_vcpu
- Some clean ups: arch vcpu-id and sbd variable
- Added the missed initialization of cpu->gdb_num_regs
2. Addressed the commnet from Zhao Liu (Intel)
- Make initialization of CPU Hotplug state conditional
(possible_cpu_arch_ids!=NULL)
Link:
https://lore.kernel.org/qemu-devel/2024031202.12992-1-salil.me...@huawei.com/
Patch-set V7 -> V8
1. Rebased and Fixed the conflicts
Patch-set V6 -> V7
1. Addressed Alex Bennée's comments
- Updated the docs
2. Addressed Igor Mammedov's comments
- Merged patches [Patch V6 3/9] & [Patch V6 7/9] with [Patch V6 4/9]
- Updated commit-log of [Patch V6 1/9] and [Patch V6 5/9]
3. Added Shaoqin Huang's Reviewed-by tags for whole series.
Link:
https://lore.kernel.org/qemu-devel/20231013105129.25648-1-salil.me...@huawei.com/
Patch-set V5 -> V6
1. Addressed Gavin Shan's comments
- Fixed the assert() ranges of address spaces
- Rebased the patch-set to latest changes in the qemu.git
- Added Reviewed-by tags for patches {8,9}
2. Addressed Jonathan Cameron's comments
- Updated commit-log for [Patch V5 1/9] with mention of trace events
- Added Reviewed-by tags for patches {1,5}
3. Added Tested-by tags from Xianglai Li
4. Fixed checkpatch.pl error "Qemu -> QEMU" in [Patch V5 1/9]
Link:
https://lore.kernel.org/qemu-devel/20231011194355.15628-1-salil.me...@huawei.com/
Patch-set V4 -> V5
1. Addressed Gavin Shan's comments
- Fixed the trace events print string for kvm_{create,get,park,destroy}_vcpu
- Added Reviewed-by tag for patch {1}
2. Added Shaoqin Huang's Reviewed-by tags for Patches {2,3}
3. Added Tested-by Tag from Vishnu Pajjuri to the patch-set
4. Dropped the ARM specific [Patch V4 10/10]
Link:
https://lore.kernel.org/qemu-devel/20231009203601.17584-1-salil.me...@huawei.com/
Patch-set V3 -> V4
1. Addressed David Hilderbrand's comments
- Fixed the wrong doc comment of kvm_park_vcpu API prototype
- Added Reviewed-by tags for patches {2,4}
Link:
https://lore.kernel.org/qemu-devel/20231009112812.10612-1-salil.me...@huawei.com/
Patch-set V2 -> V3
1. Addressed Jonathan Cameron's comments
- Fixed 'vcpu-id' type wrongly changed from 'unsigned long' to 'integer'
- Removed unnecessary use of variable 'vcpu_id' in kvm_park_vcpu
- Updated [Patch V2 3/10] commit-log with details of ACPI_CPU_SCAN_METHOD
macro
- Updated [Patch V2 5/10] commit-log with details of conditional event
handler method
- Added Reviewed-by tags for patches {2,3,4,6,7}
2. Addressed Gavin Shan's comments
- Remove unnecessary use of variable 'vcpu_id' in kvm_par_vcpu
- Fixed return value in kvm_get_vcpu from -1 to -ENOENT
- Reset the value of 'gdb_num_g_regs' in gdb_unregister_coprocessor_all
- Fixed the kvm_{create,park}_vcpu prototypes docs
- Added Reviewed-by tags for patches {2,3,4,5,6,7,9,10}
3. Addressed one earlier missed comment by Alex Bennée in RFC V1
- Added traces instead of DPRINTF in the newly added and some existing
functions
Link:
https://lore.kernel.org/qemu-devel/20230930001933.2660-1-salil.me...@huawei.com/
Patch-set V1 -> V2
1. Addressed Alex Bennée's comments
- Refactored the kvm_create_vcpu logic to get rid of goto
- Added the docs for kvm_{create,park}_vcpu prototypes
- Splitted the gdbstub and AddressSpace destruction change into separate
patches
- Added Reviewed-by tags for patches {2,10}
Link:
https://lore.kernel.org/qemu-devel/20230929124304.13672-1-salil.me...@huawei.com/
References:
[1]
https://lore.kernel.org/qemu-devel/20230926100436.28284-1-salil.me...@huawei.com/
[2] https://lore.kernel.org/all/20230913163823.7880-1-james.mo...@arm.com/
[3]
https://lore.kernel.org/qemu-devel/cover.1695697701.git.lixiang...@loongson.cn/
Salil Mehta (8):
accel/kvm: Extract common KVM vCPU {creation,parking} code
hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header file
hw/acpi: Update ACPI GED framework to support vCPU Hotplug
hw/acpi: Update GED _EVT method AML with CPU scan
hw/acpi: Update CPUs AML with cpu-(ctrl)dev change
physmem: Add helper function to destroy CP
[PATCH V9 2/8] hw/acpi: Move CPU ctrl-dev MMIO region len macro to common header file
CPU ctrl-dev MMIO region length could be used in ACPI GED and various other
architecture specific places. Move ACPI_CPU_HOTPLUG_REG_LEN macro to more
appropriate common header file.
Signed-off-by: Salil Mehta
Reviewed-by: Alex Bennée
Reviewed-by: Jonathan Cameron
Reviewed-by: Gavin Shan
Reviewed-by: David Hildenbrand
Reviewed-by: Shaoqin Huang
Tested-by: Vishnu Pajjuri
Tested-by: Xianglai Li
Tested-by: Miguel Luis
---
hw/acpi/cpu.c | 2 +-
include/hw/acpi/cpu_hotplug.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
index 2d81c1e790..69aaa563db 100644
--- a/hw/acpi/cpu.c
+++ b/hw/acpi/cpu.c
@@ -1,13 +1,13 @@
#include "qemu/osdep.h"
#include "migration/vmstate.h"
#include "hw/acpi/cpu.h"
+#include "hw/acpi/cpu_hotplug.h"
#include "hw/core/cpu.h"
#include "qapi/error.h"
#include "qapi/qapi-events-acpi.h"
#include "trace.h"
#include "sysemu/numa.h"
-#define ACPI_CPU_HOTPLUG_REG_LEN 12
#define ACPI_CPU_SELECTOR_OFFSET_WR 0
#define ACPI_CPU_FLAGS_OFFSET_RW 4
#define ACPI_CPU_CMD_OFFSET_WR 5
diff --git a/include/hw/acpi/cpu_hotplug.h b/include/hw/acpi/cpu_hotplug.h
index 3b932a..48b291e45e 100644
--- a/include/hw/acpi/cpu_hotplug.h
+++ b/include/hw/acpi/cpu_hotplug.h
@@ -19,6 +19,8 @@
#include "hw/hotplug.h"
#include "hw/acpi/cpu.h"
+#define ACPI_CPU_HOTPLUG_REG_LEN 12
+
typedef struct AcpiCpuHotplug {
Object *device;
MemoryRegion io;
--
2.34.1
[PATCH V9 1/8] accel/kvm: Extract common KVM vCPU {creation, parking} code
KVM vCPU creation is done once during the vCPU realization when Qemu vCPU thread
is spawned. This is common to all the architectures as of now.
Hot-unplug of vCPU results in destruction of the vCPU object in QOM but the
corresponding KVM vCPU object in the Host KVM is not destroyed as KVM doesn't
support vCPU removal. Therefore, its representative KVM vCPU object/context in
Qemu is parked.
Refactor architecture common logic so that some APIs could be reused by vCPU
Hotplug code of some architectures likes ARM, Loongson etc. Update new/old APIs
with trace events instead of DPRINTF. No functional change is intended here.
Signed-off-by: Salil Mehta
Reviewed-by: Gavin Shan
Tested-by: Vishnu Pajjuri
Reviewed-by: Jonathan Cameron
Tested-by: Xianglai Li
Tested-by: Miguel Luis
Reviewed-by: Shaoqin Huang
Reviewed-by: Vishnu Pajjuri
---
accel/kvm/kvm-all.c| 64 --
accel/kvm/kvm-cpus.h | 14 +
accel/kvm/trace-events | 5 +++-
3 files changed, 67 insertions(+), 16 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index c0be9f5eed..9cd7d69bde 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -128,6 +128,7 @@ static QemuMutex kml_slots_lock;
#define kvm_slots_unlock() qemu_mutex_unlock(&kml_slots_lock)
static void kvm_slot_init_dirty_bitmap(KVMSlot *mem);
+static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id);
static inline void kvm_resample_fd_remove(int gsi)
{
@@ -340,14 +341,53 @@ err:
return ret;
}
+void kvm_park_vcpu(CPUState *cpu)
+{
+struct KVMParkedVcpu *vcpu;
+
+trace_kvm_park_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu));
+
+vcpu = g_malloc0(sizeof(*vcpu));
+vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
+vcpu->kvm_fd = cpu->kvm_fd;
+QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
+}
+
+int kvm_create_vcpu(CPUState *cpu)
+{
+unsigned long vcpu_id = kvm_arch_vcpu_id(cpu);
+KVMState *s = kvm_state;
+int kvm_fd;
+
+/* check if the KVM vCPU already exist but is parked */
+kvm_fd = kvm_get_vcpu(s, vcpu_id);
+if (kvm_fd < 0) {
+/* vCPU not parked: create a new KVM vCPU */
+kvm_fd = kvm_vm_ioctl(s, KVM_CREATE_VCPU, vcpu_id);
+if (kvm_fd < 0) {
+error_report("KVM_CREATE_VCPU IOCTL failed for vCPU %lu", vcpu_id);
+return kvm_fd;
+}
+}
+
+trace_kvm_create_vcpu(cpu->cpu_index, vcpu_id, kvm_fd);
+
+cpu->kvm_fd = kvm_fd;
+cpu->kvm_state = s;
+cpu->vcpu_dirty = true;
+cpu->dirty_pages = 0;
+cpu->throttle_us_per_full = 0;
+
+return 0;
+}
+
static int do_kvm_destroy_vcpu(CPUState *cpu)
{
KVMState *s = kvm_state;
long mmap_size;
-struct KVMParkedVcpu *vcpu = NULL;
int ret = 0;
-trace_kvm_destroy_vcpu();
+trace_kvm_destroy_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu));
ret = kvm_arch_destroy_vcpu(cpu);
if (ret < 0) {
@@ -373,10 +413,7 @@ static int do_kvm_destroy_vcpu(CPUState *cpu)
}
}
-vcpu = g_malloc0(sizeof(*vcpu));
-vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
-vcpu->kvm_fd = cpu->kvm_fd;
-QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
+kvm_park_vcpu(cpu);
err:
return ret;
}
@@ -397,6 +434,8 @@ static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id)
if (cpu->vcpu_id == vcpu_id) {
int kvm_fd;
+trace_kvm_get_vcpu(vcpu_id);
+
QLIST_REMOVE(cpu, node);
kvm_fd = cpu->kvm_fd;
g_free(cpu);
@@ -404,7 +443,7 @@ static int kvm_get_vcpu(KVMState *s, unsigned long vcpu_id)
}
}
-return kvm_vm_ioctl(s, KVM_CREATE_VCPU, (void *)vcpu_id);
+return -ENOENT;
}
int kvm_init_vcpu(CPUState *cpu, Error **errp)
@@ -415,19 +454,14 @@ int kvm_init_vcpu(CPUState *cpu, Error **errp)
trace_kvm_init_vcpu(cpu->cpu_index, kvm_arch_vcpu_id(cpu));
-ret = kvm_get_vcpu(s, kvm_arch_vcpu_id(cpu));
+ret = kvm_create_vcpu(cpu);
if (ret < 0) {
-error_setg_errno(errp, -ret, "kvm_init_vcpu: kvm_get_vcpu failed
(%lu)",
+error_setg_errno(errp, -ret,
+ "kvm_init_vcpu: kvm_create_vcpu failed (%lu)",
kvm_arch_vcpu_id(cpu));
goto err;
}
-cpu->kvm_fd = ret;
-cpu->kvm_state = s;
-cpu->vcpu_dirty = true;
-cpu->dirty_pages = 0;
-cpu->throttle_us_per_full = 0;
-
mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
if (mmap_size < 0) {
ret = mmap_size;
diff --git a/accel/kvm/kvm-cpus.h b/accel/kvm/kvm-cpus.h
index ca40add32c..ba5bea5bca 100644
--- a/accel/kvm/kvm-cpus.h
+++ b/accel/kvm/kvm-cpus.h
@@ -22,5 +22,19 @@ bool kvm_supports_guest_debug(void);
int kvm_insert_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len);
int kvm_remove_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len);
void kvm_remove_all_breakpoints(CPUState *cpu);
+/**
+ * kvm_create_vcpu - Gets a park
Re: A question regarding TARGET_ALIGNED_ONLY flag
On 5/19/24 16:23, Michael Rolnik wrote:
Hi all,
Previously there was *TARGET_ALIGNED_ONLY* option that caused all memory accessed to be
aligned, now it seems to be removed.
Is there a way to achieve memory access alignment with QEMU v9.0.0 when I am building a
custom target?
Explicitly add MO_ALIGN to the MemOp argument of
tcg_gen_qemu_{ld,st}_{i32,i64,i128}.
r~
A question regarding TARGET_ALIGNED_ONLY flag
Hi all, Previously there was *TARGET_ALIGNED_ONLY* option that caused all memory accessed to be aligned, now it seems to be removed. Is there a way to achieve memory access alignment with QEMU v9.0.0 when I am building a custom target? -- Best Regards, Michael Rolnik
[PATCH v3 0/3] Initial support for One-Time Programmable Memory (OTP) in BCM2835
All BCM2835 boards have on-board OTP memory with 66 32-bit rows. Usually, its contents are accessible via mailbox commands. [Changes in v3] - Forgot to replace constant with macro in one particular spot. [Changes in v2] - Replace read/write with get/set in bcm2835_otp.c. - Use impl instead of valid in bcm2835_otp.c. - Replace all constant values with macros defined in bcm2835_otp.h. - Change memory region size of OTP device to 0x80. - After further testing on a real Raspberry Pi 3, I noticed a few things contrary to my initial assumptions: -- The customer OTP lock bit is bit 6 of row 32, NOT bit 30 of row 30. This is currently undocumented to my knowledge. -- The above lock indeed applies to the private key as well. Rayhan Faizel (3): hw/nvram: Add BCM2835 OTP device hw/arm: Connect OTP device to BCM2835 hw/misc: Implement mailbox properties for customer OTP and device specific private keys hw/arm/bcm2835_peripherals.c | 15 ++- hw/misc/bcm2835_property.c | 87 + hw/nvram/bcm2835_otp.c | 187 +++ hw/nvram/meson.build | 1 + include/hw/arm/bcm2835_peripherals.h | 3 +- include/hw/arm/raspberrypi-fw-defs.h | 2 + include/hw/misc/bcm2835_property.h | 2 + include/hw/nvram/bcm2835_otp.h | 68 ++ 8 files changed, 363 insertions(+), 2 deletions(-) create mode 100644 hw/nvram/bcm2835_otp.c create mode 100644 include/hw/nvram/bcm2835_otp.h -- 2.34.1
[PATCH v3 2/3] hw/arm: Connect OTP device to BCM2835
Replace stubbed OTP memory region with the new OTP device.
Signed-off-by: Rayhan Faizel
---
hw/arm/bcm2835_peripherals.c | 13 -
include/hw/arm/bcm2835_peripherals.h | 3 ++-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 1695d8b453..7d735bb56c 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -116,6 +116,10 @@ static void raspi_peripherals_base_init(Object *obj)
object_property_add_const_link(OBJECT(&s->fb), "dma-mr",
OBJECT(&s->gpu_bus_mr));
+/* OTP */
+object_initialize_child(obj, "bcm2835-otp", &s->otp,
+TYPE_BCM2835_OTP);
+
/* Property channel */
object_initialize_child(obj, "property", &s->property,
TYPE_BCM2835_PROPERTY);
@@ -374,6 +378,14 @@ void bcm_soc_peripherals_common_realize(DeviceState *dev,
Error **errp)
sysbus_connect_irq(SYS_BUS_DEVICE(&s->fb), 0,
qdev_get_gpio_in(DEVICE(&s->mboxes), MBOX_CHAN_FB));
+/* OTP */
+if (!sysbus_realize(SYS_BUS_DEVICE(&s->otp), errp)) {
+return;
+}
+
+memory_region_add_subregion(&s->peri_mr, OTP_OFFSET,
+sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->otp), 0));
+
/* Property channel */
if (!sysbus_realize(SYS_BUS_DEVICE(&s->property), errp)) {
return;
@@ -500,7 +512,6 @@ void bcm_soc_peripherals_common_realize(DeviceState *dev,
Error **errp)
create_unimp(s, &s->i2s, "bcm2835-i2s", I2S_OFFSET, 0x100);
create_unimp(s, &s->smi, "bcm2835-smi", SMI_OFFSET, 0x100);
create_unimp(s, &s->bscsl, "bcm2835-spis", BSC_SL_OFFSET, 0x100);
-create_unimp(s, &s->otp, "bcm2835-otp", OTP_OFFSET, 0x80);
create_unimp(s, &s->dbus, "bcm2835-dbus", DBUS_OFFSET, 0x8000);
create_unimp(s, &s->ave0, "bcm2835-ave0", AVE0_OFFSET, 0x8000);
create_unimp(s, &s->v3d, "bcm2835-v3d", V3D_OFFSET, 0x1000);
diff --git a/include/hw/arm/bcm2835_peripherals.h
b/include/hw/arm/bcm2835_peripherals.h
index 636203baa5..1eeaeec9e0 100644
--- a/include/hw/arm/bcm2835_peripherals.h
+++ b/include/hw/arm/bcm2835_peripherals.h
@@ -33,6 +33,7 @@
#include "hw/usb/hcd-dwc2.h"
#include "hw/ssi/bcm2835_spi.h"
#include "hw/i2c/bcm2835_i2c.h"
+#include "hw/nvram/bcm2835_otp.h"
#include "hw/misc/unimp.h"
#include "qom/object.h"
@@ -71,7 +72,7 @@ struct BCMSocPeripheralBaseState {
BCM2835SPIState spi[1];
BCM2835I2CState i2c[3];
OrIRQState orgated_i2c_irq;
-UnimplementedDeviceState otp;
+BCM2835OTPState otp;
UnimplementedDeviceState dbus;
UnimplementedDeviceState ave0;
UnimplementedDeviceState v3d;
--
2.34.1
[PATCH v3 1/3] hw/nvram: Add BCM2835 OTP device
The OTP device registers are currently stubbed. For now, the device
houses the OTP rows which will be accessed directly by other peripherals.
Signed-off-by: Rayhan Faizel
---
hw/nvram/bcm2835_otp.c | 187 +
hw/nvram/meson.build | 1 +
include/hw/nvram/bcm2835_otp.h | 68
3 files changed, 256 insertions(+)
create mode 100644 hw/nvram/bcm2835_otp.c
create mode 100644 include/hw/nvram/bcm2835_otp.h
diff --git a/hw/nvram/bcm2835_otp.c b/hw/nvram/bcm2835_otp.c
new file mode 100644
index 00..c4aed28472
--- /dev/null
+++ b/hw/nvram/bcm2835_otp.c
@@ -0,0 +1,187 @@
+/*
+ * BCM2835 One-Time Programmable (OTP) Memory
+ *
+ * The OTP implementation is mostly a stub except for the OTP rows
+ * which are accessed directly by other peripherals such as the mailbox.
+ *
+ * The OTP registers are unimplemented due to lack of documentation.
+ *
+ * Copyright (c) 2024 Rayhan Faizel
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/nvram/bcm2835_otp.h"
+#include "migration/vmstate.h"
+
+/* OTP rows are 1-indexed */
+uint32_t bcm2835_otp_get_row(BCM2835OTPState *s, unsigned int row)
+{
+assert(row <= BCM2835_OTP_ROW_COUNT && row >= 1);
+
+return s->otp_rows[row - 1];
+}
+
+void bcm2835_otp_set_row(BCM2835OTPState *s, unsigned int row,
+ uint32_t value)
+{
+assert(row <= BCM2835_OTP_ROW_COUNT && row >= 1);
+
+/* Real OTP rows work as e-fuses */
+s->otp_rows[row - 1] |= value;
+}
+
+static uint64_t bcm2835_otp_read(void *opaque, hwaddr addr, unsigned size)
+{
+switch (addr) {
+case BCM2835_OTP_BOOTMODE_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_BOOTMODE_REG\n");
+break;
+case BCM2835_OTP_CONFIG_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_CONFIG_REG\n");
+break;
+case BCM2835_OTP_CTRL_LO_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_CTRL_LO_REG\n");
+break;
+case BCM2835_OTP_CTRL_HI_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_CTRL_HI_REG\n");
+break;
+case BCM2835_OTP_STATUS_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_STATUS_REG\n");
+break;
+case BCM2835_OTP_BITSEL_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_BITSEL_REG\n");
+break;
+case BCM2835_OTP_DATA_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_DATA_REG\n");
+break;
+case BCM2835_OTP_ADDR_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_ADDR_REG\n");
+break;
+case BCM2835_OTP_WRITE_DATA_READ_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_WRITE_DATA_READ_REG\n");
+break;
+case BCM2835_OTP_INIT_STATUS_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_INIT_STATUS_REG\n");
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: Bad offset 0x%" HWADDR_PRIx "\n", __func__, addr);
+}
+
+return 0;
+}
+
+static void bcm2835_otp_write(void *opaque, hwaddr addr,
+ uint64_t value, unsigned int size)
+{
+switch (addr) {
+case BCM2835_OTP_BOOTMODE_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_BOOTMODE_REG\n");
+break;
+case BCM2835_OTP_CONFIG_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_CONFIG_REG\n");
+break;
+case BCM2835_OTP_CTRL_LO_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_CTRL_LO_REG\n");
+break;
+case BCM2835_OTP_CTRL_HI_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_CTRL_HI_REG\n");
+break;
+case BCM2835_OTP_STATUS_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_STATUS_REG\n");
+break;
+case BCM2835_OTP_BITSEL_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_BITSEL_REG\n");
+break;
+case BCM2835_OTP_DATA_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_DATA_REG\n");
+break;
+case BCM2835_OTP_ADDR_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_ADDR_REG\n");
+break;
+case BCM2835_OTP_WRITE_DATA_READ_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_WRITE_DATA_READ_REG\n");
+break;
+case BCM2835_OTP_INIT_STATUS_REG:
+qemu_log_mask(LOG_UNIMP,
+ "bcm2835_otp: BCM2835_OTP_INIT_STATUS_REG\n");
+
[PATCH v3 3/3] hw/misc: Implement mailbox properties for customer OTP and device specific private keys
Four mailbox properties are implemented as follows:
1. Customer OTP: GET_CUSTOMER_OTP and SET_CUSTOMER_OTP
2. Device-specific private key: GET_PRIVATE_KEY and
SET_PRIVATE_KEY.
The customer OTP is located in the rows 36-43. The device-specific private key
is located in the rows 56-63.
The customer OTP can be locked with the magic numbers 0x 0xaffe
when running the SET_CUSTOMER_OTP mailbox command. Bit 6 of row 32 indicates
this lock, which is undocumented. The lock also applies to the device-specific
private key.
Signed-off-by: Rayhan Faizel
---
hw/arm/bcm2835_peripherals.c | 2 +
hw/misc/bcm2835_property.c | 87
include/hw/arm/raspberrypi-fw-defs.h | 2 +
include/hw/misc/bcm2835_property.h | 2 +
4 files changed, 93 insertions(+)
diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 7d735bb56c..ac153a96b9 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -132,6 +132,8 @@ static void raspi_peripherals_base_init(Object *obj)
OBJECT(&s->fb));
object_property_add_const_link(OBJECT(&s->property), "dma-mr",
OBJECT(&s->gpu_bus_mr));
+object_property_add_const_link(OBJECT(&s->property), "otp",
+ OBJECT(&s->otp));
/* Extended Mass Media Controller */
object_initialize_child(obj, "sdhci", &s->sdhci, TYPE_SYSBUS_SDHCI);
diff --git a/hw/misc/bcm2835_property.c b/hw/misc/bcm2835_property.c
index bdd9a6bbce..63de3db621 100644
--- a/hw/misc/bcm2835_property.c
+++ b/hw/misc/bcm2835_property.c
@@ -32,6 +32,7 @@ static void bcm2835_property_mbox_push(BCM2835PropertyState
*s, uint32_t value)
uint32_t tmp;
int n;
uint32_t offset, length, color;
+uint32_t start_num, number, otp_row;
/*
* Copy the current state of the framebuffer config; we will update
@@ -322,6 +323,89 @@ static void
bcm2835_property_mbox_push(BCM2835PropertyState *s, uint32_t value)
0);
resplen = VCHI_BUSADDR_SIZE;
break;
+
+/* Customer OTP */
+
+case RPI_FWREQ_GET_CUSTOMER_OTP:
+start_num = ldl_le_phys(&s->dma_as, value + 12);
+number = ldl_le_phys(&s->dma_as, value + 16);
+
+resplen = 8 + 4 * number;
+
+for (n = start_num; n < start_num + number &&
+ n < BCM2835_OTP_CUSTOMER_OTP_LEN; n++) {
+otp_row = bcm2835_otp_get_row(s->otp,
+ BCM2835_OTP_CUSTOMER_OTP + n);
+stl_le_phys(&s->dma_as,
+value + 20 + ((n - start_num) << 2), otp_row);
+}
+break;
+case RPI_FWREQ_SET_CUSTOMER_OTP:
+start_num = ldl_le_phys(&s->dma_as, value + 12);
+number = ldl_le_phys(&s->dma_as, value + 16);
+
+resplen = 4;
+
+/* Magic numbers to permanently lock customer OTP */
+if (start_num == BCM2835_OTP_LOCK_NUM1 &&
+number == BCM2835_OTP_LOCK_NUM2) {
+bcm2835_otp_set_row(s->otp,
+BCM2835_OTP_ROW_32,
+BCM2835_OTP_ROW_32_LOCK);
+break;
+}
+
+/* If row 32 has the lock bit, don't allow further writes */
+if (bcm2835_otp_get_row(s->otp, BCM2835_OTP_ROW_32) &
+BCM2835_OTP_ROW_32_LOCK) {
+break;
+}
+
+for (n = start_num; n < start_num + number &&
+ n < BCM2835_OTP_CUSTOMER_OTP_LEN; n++) {
+otp_row = ldl_le_phys(&s->dma_as,
+ value + 20 + ((n - start_num) << 2));
+bcm2835_otp_set_row(s->otp,
+BCM2835_OTP_CUSTOMER_OTP + n, otp_row);
+}
+break;
+
+/* Device-specific private key */
+
+case RPI_FWREQ_GET_PRIVATE_KEY:
+start_num = ldl_le_phys(&s->dma_as, value + 12);
+number = ldl_le_phys(&s->dma_as, value + 16);
+
+resplen = 8 + 4 * number;
+
+for (n = start_num; n < start_num + number &&
+ n < BCM2835_OTP_PRIVATE_KEY_LEN; n++) {
+otp_row = bcm2835_otp_get_row(s->otp,
+ BCM2835_OTP_PRIVATE_KEY + n);
+stl_le_phys(&s->dma_as,
+value + 20 + ((n - start_num) << 2), otp_row);
+}
+break;
+case RPI_FWREQ_SET_PRIVATE_KEY:
+start_num = ldl_le_phys(&s->dma_as, value + 12);
+number = ldl_le_phys(&s->dma_as, value + 16);
+
+resplen = 4;
+
+/* If row 32 has the lock bit, don't allow further writes */
+if (bcm2835_otp_get_row(s->otp, BCM2835_OTP_ROW_32) &
+
Re: [PATCH] hw/riscv/virt: Add hotplugging and virtio-md-pci support
On 5/18/24 16:50, David Hildenbrand wrote:
Hi,
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4fdb66052587..16c2bdbfe6b6 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -53,6 +53,8 @@
#include "hw/pci-host/gpex.h"
#include "hw/display/ramfb.h"
#include "hw/acpi/aml-build.h"
+#include "hw/mem/memory-device.h"
+#include "hw/virtio/virtio-mem-pci.h"
#include "qapi/qapi-visit-common.h"
#include "hw/virtio/virtio-iommu.h"
@@ -1407,6 +1409,7 @@ static void virt_machine_init(MachineState *machine)
DeviceState *mmio_irqchip, *virtio_irqchip, *pcie_irqchip;
int i, base_hartid, hart_count;
int socket_count = riscv_socket_count(machine);
+ hwaddr device_memory_base, device_memory_size;
/* Check socket count limit */
if (VIRT_SOCKETS_MAX < socket_count) {
@@ -1553,6 +1556,25 @@ static void virt_machine_init(MachineState *machine)
memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base,
mask_rom);
+ device_memory_base = ROUND_UP(s->memmap[VIRT_DRAM].base +
machine->ram_size,
+ GiB);
+ device_memory_size = machine->maxram_size - machine->ram_size;
+
+ if (riscv_is_32bit(&s->soc[0])) {
+ hwaddr memtop = device_memory_base + ROUND_UP(device_memory_size, GiB);
+
+ if (memtop > UINT32_MAX) {
+ error_report("Memory exceeds 32-bit limit by %lu bytes",
+ memtop - UINT32_MAX);
+ exit(EXIT_FAILURE);
+ }
+ }
+
+ if (device_memory_size > 0) {
+ machine_memory_devices_init(machine, device_memory_base,
+ device_memory_size);
+ }
+
I think we need a design discussion before proceeding here. You're allocating
all
available memory as a memory device area, but in theory we might also support
pc-dimm hotplugs (which would be the equivalent of adding physical RAM dimms to
the board.) in the future too. If you're not familiar with this feature you can
check it out the docs in [1].
Note that DIMMs are memory devices as well. You can plug into the memory device
area both, ACPI-based memory devices (DIMM, NVDIMM) or virtio-based memory
devices (virtio-mem, virtio-pmem).
As an example, the 'virt' ARM board (hw/arm/virt.c) reserves a space for this
type of hotplug by checking how much 'ram_slots' we're allocating for it:
device_memory_size = ms->maxram_size - ms->ram_size + ms->ram_slots * GiB;
Note that we increased the region size to be able to fit most requests even if
alignment of memory devices is weird. See below.
In sane setups, this is usually not required (adding a single additional GB for
some flexiility might be good enough).
Other boards do the same with ms->ram_slots. We should consider doing it as
well,
now, even if we're not up to the point of supporting pc-dimm hotplug, to avoid
having to change the memory layout later in the road and breaking existing
setups.
If we want to copy the ARM board, ram_slots is capped to ACPI_MAX_RAM_SLOTS
(256).
Each RAM slot is considered to be a 1GiB dimm, i.e. we would reserve 256GiB for
them.
This only reserves some *additional* space to fixup weird alignment of memory
devices. *not* the actual space for these devices.
We don't consider each DIMM to be 1 GiB in size, but add an additional 1 GiB in
case we have to align DIMMs in physical address space.
I *think* this dates back to old x86 handling where we aligned the address of
each DIMM to be at a 1 GiB boundary. So if you would have plugged two 128 MiB
DIMMs, you'd have required more than 256 MiB of space in the area after
aligning inside the memory device area.
Thanks for the explanation. I missed the part where the ram_slots were being
used just to solve potential alignment issues and pc-dimms could occupy the same
space being allocated via machine_memory_devices_init().
This patch isn't far off then. If we take care to avoid plugging unaligned
memory
we might not even need this spare area.
Note: I do not have the visibility of discussions on the memory management
space,
and I might be missing details such as "we don't care about pc-dimm hotplug
anymore, it's legacy, we're going to support only virtio-md-pci from now on". We
had a situation like that with virtio-balloon and virtio-mem in the past, and
I'm
not sure if this might fall in the same category.
Not sure if I got your comment right, but virtio-mem was never supposed to be a
virtio-balloon replacement (especially of the free-page-reporting and memory
stats part).
I was trying to refer to a situation we faced 3+ years ago in the powerpc
machines,
where we were trying to add virtio-mem support there given that virtio-mem
is/was
been seen (as far as I can remember anyways) as a more robust solution than
virtio-balloon + DIMM hotplug for guest memory management from the host point of
view.
I'm probably misrepresenting the whole situation though, it has been
Re: [PATCH] intel_iommu: Use the latest fault reasons defined by spec
> From: CLEMENT MATHIEU--DRIF
> Sent: Friday, May 17, 2024 9:13 PM
>
> Hi Zhenzhong
>
> On 17/05/2024 12:23, Zhenzhong Duan wrote:
> > Caution: External email. Do not open attachments or click links, unless
> > this email
> comes from a known sender and you know the content is safe.
> >
> >
> > From: Yu Zhang
> >
> > Currently we use only VTD_FR_PASID_TABLE_INV as fault reason.
> > Update with more detailed fault reasons listed in VT-d spec 7.2.3.
> >
> > Signed-off-by: Yu Zhang
> > Signed-off-by: Zhenzhong Duan
> > ---
> > hw/i386/intel_iommu_internal.h | 8 +++-
> > hw/i386/intel_iommu.c | 25 -
> > 2 files changed, 23 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> > index f8cf99bddf..666e2cf2ce 100644
> > --- a/hw/i386/intel_iommu_internal.h
> > +++ b/hw/i386/intel_iommu_internal.h
> > @@ -311,7 +311,13 @@ typedef enum VTDFaultReason {
> > * request while disabled */
> > VTD_FR_IR_SID_ERR = 0x26, /* Invalid Source-ID */
> >
> > -VTD_FR_PASID_TABLE_INV = 0x58, /*Invalid PASID table entry */
> > +/* PASID directory entry access failure */
> > +VTD_FR_PASID_DIR_ACCESS_ERR = 0x50,
> > +/* The Present(P) field of pasid directory entry is 0 */
> > +VTD_FR_PASID_DIR_ENTRY_P = 0x51,
> > +VTD_FR_PASID_TABLE_ACCESS_ERR = 0x58, /* PASID table entry access
> > failure */
> > +VTD_FR_PASID_ENTRY_P = 0x59, /* The Present(P) field of pasidt-entry
> > is 0 */
> s/pasidt/pasid
Per spec, it is pasid table entry. So Zhenzhong may need to use the same word
With the line below. E.g. PASID Table entry.
Regards,
Yi Liu
Ok fine
regards,
>cmd
> > +VTD_FR_PASID_TABLE_ENTRY_INV = 0x5b, /*Invalid PASID table entry */
> >
> > /* Output address in the interrupt address range for scalable mode */
> > VTD_FR_SM_INTERRUPT_ADDR = 0x87,
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index cc8e59674e..0951ebb71d 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -771,7 +771,7 @@ static int vtd_get_pdire_from_pdir_table(dma_addr_t
> pasid_dir_base,
> > addr = pasid_dir_base + index * entry_size;
> > if (dma_memory_read(&address_space_memory, addr,
> > pdire, entry_size, MEMTXATTRS_UNSPECIFIED)) {
> > -return -VTD_FR_PASID_TABLE_INV;
> > +return -VTD_FR_PASID_DIR_ACCESS_ERR;
> > }
> >
> > pdire->val = le64_to_cpu(pdire->val);
> > @@ -789,6 +789,7 @@ static int
> > vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState
> *s,
> > dma_addr_t addr,
> > VTDPASIDEntry *pe)
> > {
> > +uint8_t pgtt;
> > uint32_t index;
> > dma_addr_t entry_size;
> > X86IOMMUState *x86_iommu = X86_IOMMU_DEVICE(s);
> > @@ -798,7 +799,7 @@ static int
> > vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState
> *s,
> > addr = addr + index * entry_size;
> > if (dma_memory_read(&address_space_memory, addr,
> > pe, entry_size, MEMTXATTRS_UNSPECIFIED)) {
> > -return -VTD_FR_PASID_TABLE_INV;
> > +return -VTD_FR_PASID_TABLE_ACCESS_ERR;
> > }
> > for (size_t i = 0; i < ARRAY_SIZE(pe->val); i++) {
> > pe->val[i] = le64_to_cpu(pe->val[i]);
> > @@ -806,11 +807,13 @@ static int
> > vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState
> *s,
> >
> > /* Do translation type check */
> > if (!vtd_pe_type_check(x86_iommu, pe)) {
> > -return -VTD_FR_PASID_TABLE_INV;
> > +return -VTD_FR_PASID_TABLE_ENTRY_INV;
> > }
> >
> > -if (!vtd_is_level_supported(s, VTD_PE_GET_LEVEL(pe))) {
> > -return -VTD_FR_PASID_TABLE_INV;
> > +pgtt = VTD_PE_GET_TYPE(pe);
> > +if (pgtt == VTD_SM_PASID_ENTRY_SLT &&
> > +!vtd_is_level_supported(s, VTD_PE_GET_LEVEL(pe))) {
> > +return -VTD_FR_PASID_TABLE_ENTRY_INV;
> > }
> >
> > return 0;
> > @@ -851,7 +854,7 @@ static int vtd_get_pe_from_pasid_table(IntelIOMMUState
> > *s,
> > }
> >
> > if (!vtd_pdire_present(&pdire)) {
> > -return -VTD_FR_PASID_TABLE_INV;
> > +return -VTD_FR_PASID_DIR_ENTRY_P;
> > }
> >
> > ret = vtd_get_pe_from_pdire(s, pasid, &pdire, pe);
> > @@ -860,7 +863,7 @@ static int vtd_get_pe_from_pasid_table(IntelIOMMUState
> > *s,
> > }
> >
> > if (!vtd_pe_present(pe)) {
> > -return -VTD_FR_PASID_TABLE_INV;
> > +return -VTD_FR_PASID_ENTRY_P;
> > }
> >
> > return 0;
> > @@ -913,7 +916,7 @@ static int vtd_ce_get_pasid_fpd(IntelIOMMUState *s,
> > }
> >
> > if (!vtd_pdire_present(&pdire)) {
> > -return -VTD_FR_PASID_TABLE_INV;
> > +return -VTD_FR_PASID_DIR_ENTRY_P;
> > }
> >
> > /*
> > @@ -1770,7 +1773,11 @@ static const bool vtd_qualified_faults[] = {
> >
