Re: [PATCH v2 3/3] hw/riscv/virt: Add ACPI GED and PC-DIMM MHP support
Daniel Henrique Barboza writes:
> On 5/21/24 07:56, Björn Töpel wrote:
>> From: Björn Töpel
>>
>> Add ACPI GED for the RISC-V "virt" machine, and wire up PC-DIMM memory
>> hotplugging support. Heavily based/copied from hw/arm/virt.c.
>>
>> Signed-off-by: Björn Töpel
>> ---
>> hw/riscv/Kconfig | 3 ++
>> hw/riscv/virt-acpi-build.c | 16 ++
>> hw/riscv/virt.c| 104 -
>> include/hw/riscv/virt.h| 6 ++-
>> 4 files changed, 126 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
>> index 08f82dbb681a..bebe412f2107 100644
>> --- a/hw/riscv/Kconfig
>> +++ b/hw/riscv/Kconfig
>> @@ -56,6 +56,9 @@ config RISCV_VIRT
>> select PLATFORM_BUS
>> select ACPI
>> select ACPI_PCI
>> +select MEM_DEVICE
>> +select DIMM
>> +select ACPI_HW_REDUCED
>> select VIRTIO_MEM_SUPPORTED
>> select VIRTIO_PMEM_SUPPORTED
>>
>> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
>> index 6dc3baa9ec86..61813abdef3f 100644
>> --- a/hw/riscv/virt-acpi-build.c
>> +++ b/hw/riscv/virt-acpi-build.c
>> @@ -27,6 +27,8 @@
>> #include "hw/acpi/acpi-defs.h"
>> #include "hw/acpi/acpi.h"
>> #include "hw/acpi/aml-build.h"
>> +#include "hw/acpi/memory_hotplug.h"
>> +#include "hw/acpi/generic_event_device.h"
>> #include "hw/acpi/pci.h"
>> #include "hw/acpi/utils.h"
>> #include "hw/intc/riscv_aclint.h"
>> @@ -432,6 +434,20 @@ static void build_dsdt(GArray *table_data,
>> acpi_dsdt_add_gpex_host(scope, PCIE_IRQ + VIRT_IRQCHIP_NUM_SOURCES
>> * 2);
>> }
>>
>> +if (s->acpi_dev) {
>> +uint32_t event = object_property_get_uint(OBJECT(s->acpi_dev),
>> + "ged-event",
>> _abort);
>> +
>> +build_ged_aml(scope, "\\_SB."GED_DEVICE,
>> HOTPLUG_HANDLER(s->acpi_dev),
>> + GED_IRQ, AML_SYSTEM_MEMORY,
>> memmap[VIRT_ACPI_GED].base);
>> +
>> +if (event & ACPI_GED_MEM_HOTPLUG_EVT) {
>> +build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
>> + AML_SYSTEM_MEMORY,
>> + memmap[VIRT_PCDIMM_ACPI].base);
>> +}
>> +}
>> +
>> aml_append(dsdt, scope);
>>
>> /* copy AML table into ACPI tables blob and patch header there */
>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>> index 443902f919d2..2e35890187f2 100644
>> --- a/hw/riscv/virt.c
>> +++ b/hw/riscv/virt.c
>> @@ -53,10 +53,13 @@
>> #include "hw/pci-host/gpex.h"
>> #include "hw/display/ramfb.h"
>> #include "hw/acpi/aml-build.h"
>> +#include "hw/acpi/generic_event_device.h"
>> +#include "hw/acpi/memory_hotplug.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"
>> +#include "hw/mem/pc-dimm.h"
>>
>> /* KVM AIA only supports APLIC MSI. APLIC Wired is always emulated by
>> QEMU. */
>> static bool virt_use_kvm_aia(RISCVVirtState *s)
>> @@ -84,6 +87,8 @@ static const MemMapEntry virt_memmap[] = {
>> [VIRT_UART0] ={ 0x1000, 0x100 },
>> [VIRT_VIRTIO] = { 0x10001000,0x1000 },
>> [VIRT_FW_CFG] = { 0x1010, 0x18 },
>> +[VIRT_PCDIMM_ACPI] = { 0x1020, MEMORY_HOTPLUG_IO_LEN },
>> +[VIRT_ACPI_GED] = { 0x1021, ACPI_GED_EVT_SEL_LEN },
>> [VIRT_FLASH] ={ 0x2000, 0x400 },
>> [VIRT_IMSIC_M] = { 0x2400, VIRT_IMSIC_MAX_SIZE },
>> [VIRT_IMSIC_S] = { 0x2800, VIRT_IMSIC_MAX_SIZE },
>> @@ -1400,6 +1405,28 @@ static void virt_machine_done(Notifier *notifier,
>> void *data)
>> }
>> }
>>
>> +static DeviceState *create_acpi_ged(RISCVVirtState *s)
>> +{
>> +DeviceState *dev;
>> +MachineState *ms = MACHINE(s);
>> +uint32_t event = 0;
>> +
>> +if (ms->ram_slots) {
>> +event |= ACPI_GED_MEM_HOTPLUG_EVT;
>> +}
>> +
>> +dev = qdev_new(TYPE_
Re: [PATCH v2 1/3] hw/riscv/virt: Add memory hotplugging and virtio-md-pci support
David Hildenbrand writes:
> On 24.05.24 15:14, Daniel Henrique Barboza wrote:
>>
>>
>> On 5/21/24 07:56, Björn Töpel wrote:
>>> From: Björn Töpel
>>>
>>> Virtio-based memory devices (virtio-mem/virtio-pmem) allows for
>>> dynamic resizing of virtual machine memory, and requires proper
>>> hotplugging (add/remove) support to work.
>>>
>>> Add device memory support for RISC-V "virt" machine, and enable
>>> virtio-md-pci with the corresponding missing hotplugging callbacks.
>>>
>>> Signed-off-by: Björn Töpel
>>> ---
>>>hw/riscv/Kconfig | 2 +
>>>hw/riscv/virt.c| 83 +-
>>>hw/virtio/virtio-mem.c | 5 ++-
>>>3 files changed, 87 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
>>> index a2030e3a6ff0..08f82dbb681a 100644
>>> --- a/hw/riscv/Kconfig
>>> +++ b/hw/riscv/Kconfig
>>> @@ -56,6 +56,8 @@ config RISCV_VIRT
>>>select PLATFORM_BUS
>>>select ACPI
>>>select ACPI_PCI
>>> +select VIRTIO_MEM_SUPPORTED
>>> +select VIRTIO_PMEM_SUPPORTED
>>>
>>>config SHAKTI_C
>>>bool
>>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>>> index 4fdb66052587..443902f919d2 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) {
>>> @@ -1420,6 +1423,12 @@ static void virt_machine_init(MachineState *machine)
>>>exit(1);
>>>}
>>>
>>> +if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
>>> +error_report("unsupported amount of memory slots: %"PRIu64,
>>> + machine->ram_slots);
>>> +exit(EXIT_FAILURE);
>>> +}
>>> +
>>>/* Initialize sockets */
>>>mmio_irqchip = virtio_irqchip = pcie_irqchip = NULL;
>>>for (i = 0; i < socket_count; i++) {
>>> @@ -1553,6 +1562,37 @@ static void virt_machine_init(MachineState *machine)
>>>memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base,
>>>mask_rom);
>>>
>>> +/* device memory */
>>> +device_memory_base = ROUND_UP(s->memmap[VIRT_DRAM].base +
>>> machine->ram_size,
>>> + GiB);
>>> +device_memory_size = machine->maxram_size - machine->ram_size;
>>> +if (device_memory_size > 0) {
>>> +/*
>>> + * Each DIMM is aligned based on the backend's alignment value.
>>> + * Assume max 1G hugepage alignment per slot.
>>> + */
>>> +device_memory_size += machine->ram_slots * GiB;
>>
>> We don't need to align to 1GiB. This calc can use 2MiB instead (or 4MiB if
>> we're
>> running 32 bits).
>>
>>> +
>>> +if (riscv_is_32bit(>soc[0])) {
>>> +hwaddr memtop = device_memory_base +
>>> ROUND_UP(device_memory_size,
>>> + GiB);
>>
>> Same here - alignment is 2/4 MiB.
>>
>>> +
>>> +if (memtop > UINT32_MAX) {
>>> +error_report("memory exceeds 32-bit limit by %lu bytes",
>>> + memtop - UINT32_MAX);
>>> +exit(EXIT_FAILURE);
>>> +}
>>> +}
>>> +
>>> +if (device_memory_base + device_memory_size < device_memory_size) {
>>> +
Re: [PATCH v2 1/3] hw/riscv/virt: Add memory hotplugging and virtio-md-pci support
Daniel Henrique Barboza writes:
> On 5/21/24 07:56, Björn Töpel wrote:
>> From: Björn Töpel
>>
>> Virtio-based memory devices (virtio-mem/virtio-pmem) allows for
>> dynamic resizing of virtual machine memory, and requires proper
>> hotplugging (add/remove) support to work.
>>
>> Add device memory support for RISC-V "virt" machine, and enable
>> virtio-md-pci with the corresponding missing hotplugging callbacks.
>>
>> Signed-off-by: Björn Töpel
>> ---
>> hw/riscv/Kconfig | 2 +
>> hw/riscv/virt.c| 83 +-
>> hw/virtio/virtio-mem.c | 5 ++-
>> 3 files changed, 87 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
>> index a2030e3a6ff0..08f82dbb681a 100644
>> --- a/hw/riscv/Kconfig
>> +++ b/hw/riscv/Kconfig
>> @@ -56,6 +56,8 @@ config RISCV_VIRT
>> select PLATFORM_BUS
>> select ACPI
>> select ACPI_PCI
>> +select VIRTIO_MEM_SUPPORTED
>> +select VIRTIO_PMEM_SUPPORTED
>>
>> config SHAKTI_C
>> bool
>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>> index 4fdb66052587..443902f919d2 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) {
>> @@ -1420,6 +1423,12 @@ static void virt_machine_init(MachineState *machine)
>> exit(1);
>> }
>>
>> +if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
>> +error_report("unsupported amount of memory slots: %"PRIu64,
>> + machine->ram_slots);
>
> Let's also add the maximum amount allowed in this message, e.g. this error:
>
> $ (...) -m 2G,slots=512,maxmem=8G
> qemu-system-riscv64: unsupported amount of memory slots: 512
>
> could be something like:
>
> qemu-system-riscv64: unsupported amount of memory slots (512), maximum
> amount: 256
>
>
> LGTM otherwise. Thanks,
Thanks for getting back!
Sure! I'll fix this in the next rev.
Björn
[PATCH v2 3/3] hw/riscv/virt: Add ACPI GED and PC-DIMM MHP support
From: Björn Töpel
Add ACPI GED for the RISC-V "virt" machine, and wire up PC-DIMM memory
hotplugging support. Heavily based/copied from hw/arm/virt.c.
Signed-off-by: Björn Töpel
---
hw/riscv/Kconfig | 3 ++
hw/riscv/virt-acpi-build.c | 16 ++
hw/riscv/virt.c| 104 -
include/hw/riscv/virt.h| 6 ++-
4 files changed, 126 insertions(+), 3 deletions(-)
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index 08f82dbb681a..bebe412f2107 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -56,6 +56,9 @@ config RISCV_VIRT
select PLATFORM_BUS
select ACPI
select ACPI_PCI
+select MEM_DEVICE
+select DIMM
+select ACPI_HW_REDUCED
select VIRTIO_MEM_SUPPORTED
select VIRTIO_PMEM_SUPPORTED
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 6dc3baa9ec86..61813abdef3f 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -27,6 +27,8 @@
#include "hw/acpi/acpi-defs.h"
#include "hw/acpi/acpi.h"
#include "hw/acpi/aml-build.h"
+#include "hw/acpi/memory_hotplug.h"
+#include "hw/acpi/generic_event_device.h"
#include "hw/acpi/pci.h"
#include "hw/acpi/utils.h"
#include "hw/intc/riscv_aclint.h"
@@ -432,6 +434,20 @@ static void build_dsdt(GArray *table_data,
acpi_dsdt_add_gpex_host(scope, PCIE_IRQ + VIRT_IRQCHIP_NUM_SOURCES *
2);
}
+if (s->acpi_dev) {
+uint32_t event = object_property_get_uint(OBJECT(s->acpi_dev),
+ "ged-event", _abort);
+
+build_ged_aml(scope, "\\_SB."GED_DEVICE, HOTPLUG_HANDLER(s->acpi_dev),
+ GED_IRQ, AML_SYSTEM_MEMORY, memmap[VIRT_ACPI_GED].base);
+
+if (event & ACPI_GED_MEM_HOTPLUG_EVT) {
+build_memory_hotplug_aml(scope, ms->ram_slots, "\\_SB", NULL,
+ AML_SYSTEM_MEMORY,
+ memmap[VIRT_PCDIMM_ACPI].base);
+}
+}
+
aml_append(dsdt, scope);
/* copy AML table into ACPI tables blob and patch header there */
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 443902f919d2..2e35890187f2 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -53,10 +53,13 @@
#include "hw/pci-host/gpex.h"
#include "hw/display/ramfb.h"
#include "hw/acpi/aml-build.h"
+#include "hw/acpi/generic_event_device.h"
+#include "hw/acpi/memory_hotplug.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"
+#include "hw/mem/pc-dimm.h"
/* KVM AIA only supports APLIC MSI. APLIC Wired is always emulated by QEMU. */
static bool virt_use_kvm_aia(RISCVVirtState *s)
@@ -84,6 +87,8 @@ static const MemMapEntry virt_memmap[] = {
[VIRT_UART0] ={ 0x1000, 0x100 },
[VIRT_VIRTIO] = { 0x10001000,0x1000 },
[VIRT_FW_CFG] = { 0x1010, 0x18 },
+[VIRT_PCDIMM_ACPI] = { 0x1020, MEMORY_HOTPLUG_IO_LEN },
+[VIRT_ACPI_GED] = { 0x1021, ACPI_GED_EVT_SEL_LEN },
[VIRT_FLASH] ={ 0x2000, 0x400 },
[VIRT_IMSIC_M] = { 0x2400, VIRT_IMSIC_MAX_SIZE },
[VIRT_IMSIC_S] = { 0x2800, VIRT_IMSIC_MAX_SIZE },
@@ -1400,6 +1405,28 @@ static void virt_machine_done(Notifier *notifier, void
*data)
}
}
+static DeviceState *create_acpi_ged(RISCVVirtState *s)
+{
+DeviceState *dev;
+MachineState *ms = MACHINE(s);
+uint32_t event = 0;
+
+if (ms->ram_slots) {
+event |= ACPI_GED_MEM_HOTPLUG_EVT;
+}
+
+dev = qdev_new(TYPE_ACPI_GED);
+qdev_prop_set_uint32(dev, "ged-event", event);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
+
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, s->memmap[VIRT_ACPI_GED].base);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 1, s->memmap[VIRT_PCDIMM_ACPI].base);
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, qdev_get_gpio_in(s->irqchip[0],
+GED_IRQ));
+
+return dev;
+}
+
static void virt_machine_init(MachineState *machine)
{
const MemMapEntry *memmap = virt_memmap;
@@ -1612,6 +1639,10 @@ static void virt_machine_init(MachineState *machine)
gpex_pcie_init(system_memory, pcie_irqchip, s);
+if (virt_is_acpi_enabled(s)) {
+s->acpi_dev = create_acpi_ged(s);
+}
+
create_platform_bus(s, mmio_irqchip);
serial_mm_init(system_memory, memmap[VIRT_UART0].base,
@@ -1752,6 +1783,7 @@ static HotplugHandler
*virt_machine_get_hotplug_handler(MachineState *machine,
MachineClass *mc = MACHINE_GET_CLASS(machine);
if (device_is_dynamic_sysbus(mc, d
[PATCH v2 2/3] hw/riscv/virt-acpi-build: Expose device memory in ACPI SRAT
From: Björn Töpel
Now that device memory is supported by RISC-V 'virt', expose that
region in the ACPI SRAT (System/Static Resource Affinity Table).
ACPI SRAT is used by, e.g., the virtio-mem Linux kernel driver [1].
Link: https://virtio-mem.gitlab.io/user-guide/user-guide-linux.html # [1]
Signed-off-by: Björn Töpel
---
hw/riscv/virt-acpi-build.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 0925528160f8..6dc3baa9ec86 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -610,6 +610,13 @@ build_srat(GArray *table_data, BIOSLinker *linker,
RISCVVirtState *vms)
}
}
+if (ms->device_memory) {
+build_srat_memory(table_data, ms->device_memory->base,
+ memory_region_size(>device_memory->mr),
+ ms->numa_state->num_nodes - 1,
+ MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
+}
+
acpi_table_end(linker, );
}
--
2.40.1
[PATCH v2 1/3] hw/riscv/virt: Add memory hotplugging and virtio-md-pci support
From: Björn Töpel
Virtio-based memory devices (virtio-mem/virtio-pmem) allows for
dynamic resizing of virtual machine memory, and requires proper
hotplugging (add/remove) support to work.
Add device memory support for RISC-V "virt" machine, and enable
virtio-md-pci with the corresponding missing hotplugging callbacks.
Signed-off-by: Björn Töpel
---
hw/riscv/Kconfig | 2 +
hw/riscv/virt.c| 83 +-
hw/virtio/virtio-mem.c | 5 ++-
3 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index a2030e3a6ff0..08f82dbb681a 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -56,6 +56,8 @@ config RISCV_VIRT
select PLATFORM_BUS
select ACPI
select ACPI_PCI
+select VIRTIO_MEM_SUPPORTED
+select VIRTIO_PMEM_SUPPORTED
config SHAKTI_C
bool
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 4fdb66052587..443902f919d2 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) {
@@ -1420,6 +1423,12 @@ static void virt_machine_init(MachineState *machine)
exit(1);
}
+if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
+error_report("unsupported amount of memory slots: %"PRIu64,
+ machine->ram_slots);
+exit(EXIT_FAILURE);
+}
+
/* Initialize sockets */
mmio_irqchip = virtio_irqchip = pcie_irqchip = NULL;
for (i = 0; i < socket_count; i++) {
@@ -1553,6 +1562,37 @@ static void virt_machine_init(MachineState *machine)
memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base,
mask_rom);
+/* device memory */
+device_memory_base = ROUND_UP(s->memmap[VIRT_DRAM].base +
machine->ram_size,
+ GiB);
+device_memory_size = machine->maxram_size - machine->ram_size;
+if (device_memory_size > 0) {
+/*
+ * Each DIMM is aligned based on the backend's alignment value.
+ * Assume max 1G hugepage alignment per slot.
+ */
+device_memory_size += machine->ram_slots * GiB;
+
+if (riscv_is_32bit(>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_base + device_memory_size < device_memory_size) {
+error_report("unsupported amount of device memory");
+exit(EXIT_FAILURE);
+}
+
+machine_memory_devices_init(machine, device_memory_base,
+device_memory_size);
+}
+
/*
* Init fw_cfg. Must be done before riscv_load_fdt, otherwise the
* device tree cannot be altered and we get FDT_ERR_NOSPACE.
@@ -1712,12 +1752,21 @@ static HotplugHandler
*virt_machine_get_hotplug_handler(MachineState *machine,
MachineClass *mc = MACHINE_GET_CLASS(machine);
if (device_is_dynamic_sysbus(mc, dev) ||
-object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI) ||
+object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
return HOTPLUG_HANDLER(machine);
}
return NULL;
}
+static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
+DeviceState *dev, Error **errp)
+{
+if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
+virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
+}
+}
+
static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
@@ -1735,6 +1784,35 @@ static void virt_machine_device_plug_cb(HotplugHandler
*hotplug_dev,
if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
create_fdt_virtio_iommu(s, pci_get_bdf(PCI_DEVICE(dev)));
}
+
+if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
+
[PATCH v2 0/3] RISC-V virt MHP support
From: Björn Töpel The RISC-V "virt" machine is currently missing memory hotplugging support (MHP). This series adds the missing virtio-md, and PC-DIMM support. I've include both the ACPI MHP/GED/PC-DIMM, and virtio-md in the same series. The two patches (virtio-md) are probably less controversial, and can be pulled separately from the last patch. The first patch includes MHP that works with DT. The second patch is basic device memory ACPI SRAT plumbing. The third patch includes ACPI GED, and ACPI MHP/PC-DIMM support. Changelog v1->v2: * Cap the maximum number of supported ram slots (Daniel) * Split the ACPI parts out from virtio-md patch (Daniel) * Allow for max 1G PC-DIMM alignment per ram slot (Daniel) * Add missing defined(__riscv__) for virtio-mem code * Include PC-DIMM MHP supportin the series * Misc code structure changes Cheers, Björn Björn Töpel (3): hw/riscv/virt: Add memory hotplugging and virtio-md-pci support hw/riscv/virt-acpi-build: Expose device memory in ACPI SRAT hw/riscv/virt: Add ACPI GED and PC-DIMM MHP support hw/riscv/Kconfig | 5 + hw/riscv/virt-acpi-build.c | 23 + hw/riscv/virt.c| 183 - hw/virtio/virtio-mem.c | 5 +- include/hw/riscv/virt.h| 6 +- 5 files changed, 218 insertions(+), 4 deletions(-) base-commit: 01782d6b294f95bcde334386f0aaac593cd28c0d -- 2.40.1
Re: [PATCH] hw/riscv/virt: Add hotplugging and virtio-md-pci support
Daniel Henrique Barboza writes:
> On 5/20/24 15:51, Björn Töpel wrote:
>> Daniel/David,
>>
>> Daniel Henrique Barboza writes:
>>
>>> 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(>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
Re: [PATCH] hw/riscv/virt: Add hotplugging and virtio-md-pci support
Daniel/David,
Daniel Henrique Barboza writes:
> 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(>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.
I'm a bit lost here, so please bare with me. We don't require the 1 GiB
alignment on RV AFAIU. I'm having a hard time figuring out what missing
in my patch.
[...]
>>> I see that David Hildenbrand is also CCed in the patch so he'll let us know
>>> if
>>> I'm out of line with what I'm asking.
>>
>> Supporting PC-DIMMs might be required at some point when dealing with OSes
>> that don't support virtio-mem and friends.
...and also for testing the PC-DIMM ACPI patching path.
Re: [PATCH] hw/riscv/virt: Add hotplugging and virtio-md-pci support
Daniel,
Thanks for taking a look!
Daniel Henrique Barboza writes:
> Hi Björj,
>
> On 5/14/24 08:06, Björn Töpel wrote:
>> From: Björn Töpel
>>
>> Virtio-based memory devices allows for dynamic resizing of virtual
>> machine memory, and requires proper hotplugging (add/remove) support
>> to work.
>>
>> Enable virtio-md-pci with the corresponding missing hotplugging
>> callbacks for the RISC-V "virt" machine.
>>
>> Signed-off-by: Björn Töpel
>> ---
>> This is basic support for MHP that works with DT. There some minor
>> ACPI SRAT plumbing in there as well. Ideally we'd like proper ACPI MHP
>> support as well. I have a branch [1], where I've applied this patch,
>> plus ACPI GED/PC-DIMM MHP support on top of Sunil's QEMU branch
>> (contains some ACPI DSDT additions) [2], for the curious/brave ones.
>> However, the ACPI MHP support this is not testable on upstream Linux
>> yet (ACPI AIA support, and ACPI NUMA SRAT series are ongoing).
>>
>> I'll follow-up with proper ACPI GED/PC-DIMM MHP patches, once the
>> dependencies land (Linux kernel and QEMU).
>>
>> I'll post the Linux MHP/virtio-mem v2 patches later this week!
>>
>>
>> Cheers,
>> Björn
>>
>> [1] https://github.com/bjoto/qemu/commits/virtio-mem-pc-dimm-mhp-acpi/
>> [2]
>> https://lore.kernel.org/linux-riscv/20240501121742.1215792-1-suni...@ventanamicro.com/
>> ---
>> hw/riscv/Kconfig | 2 ++
>> hw/riscv/virt-acpi-build.c | 7 +
>> hw/riscv/virt.c| 64 +-
>> hw/virtio/virtio-mem.c | 2 +-
>> 4 files changed, 73 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
>> index a2030e3a6ff0..08f82dbb681a 100644
>> --- a/hw/riscv/Kconfig
>> +++ b/hw/riscv/Kconfig
>> @@ -56,6 +56,8 @@ config RISCV_VIRT
>> select PLATFORM_BUS
>> select ACPI
>> select ACPI_PCI
>> +select VIRTIO_MEM_SUPPORTED
>> +select VIRTIO_PMEM_SUPPORTED
>>
>> config SHAKTI_C
>> bool
>> diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
>> index 0925528160f8..6dc3baa9ec86 100644
>> --- a/hw/riscv/virt-acpi-build.c
>> +++ b/hw/riscv/virt-acpi-build.c
>> @@ -610,6 +610,13 @@ build_srat(GArray *table_data, BIOSLinker *linker,
>> RISCVVirtState *vms)
>> }
>> }
>>
>> +if (ms->device_memory) {
>> +build_srat_memory(table_data, ms->device_memory->base,
>> + memory_region_size(>device_memory->mr),
>> + ms->numa_state->num_nodes - 1,
>> + MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
>> +}
>> +
>> acpi_table_end(linker, );
>
> When the time comes I believe we'll want this chunk in a separated ACPI patch.
Hmm, I first thought about adding this to the ACPI MHP series, but then
realized that virtio-mem relies on SRAT for ACPI boots (again -- RISC-V
Linux does not support that upstream yet...).
Do you mean that you'd prefer this chunk in a separate patch?
Björn
[PATCH] hw/riscv/virt: Add hotplugging and virtio-md-pci support
From: Björn Töpel
Virtio-based memory devices allows for dynamic resizing of virtual
machine memory, and requires proper hotplugging (add/remove) support
to work.
Enable virtio-md-pci with the corresponding missing hotplugging
callbacks for the RISC-V "virt" machine.
Signed-off-by: Björn Töpel
---
This is basic support for MHP that works with DT. There some minor
ACPI SRAT plumbing in there as well. Ideally we'd like proper ACPI MHP
support as well. I have a branch [1], where I've applied this patch,
plus ACPI GED/PC-DIMM MHP support on top of Sunil's QEMU branch
(contains some ACPI DSDT additions) [2], for the curious/brave ones.
However, the ACPI MHP support this is not testable on upstream Linux
yet (ACPI AIA support, and ACPI NUMA SRAT series are ongoing).
I'll follow-up with proper ACPI GED/PC-DIMM MHP patches, once the
dependencies land (Linux kernel and QEMU).
I'll post the Linux MHP/virtio-mem v2 patches later this week!
Cheers,
Björn
[1] https://github.com/bjoto/qemu/commits/virtio-mem-pc-dimm-mhp-acpi/
[2]
https://lore.kernel.org/linux-riscv/20240501121742.1215792-1-suni...@ventanamicro.com/
---
hw/riscv/Kconfig | 2 ++
hw/riscv/virt-acpi-build.c | 7 +
hw/riscv/virt.c| 64 +-
hw/virtio/virtio-mem.c | 2 +-
4 files changed, 73 insertions(+), 2 deletions(-)
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index a2030e3a6ff0..08f82dbb681a 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -56,6 +56,8 @@ config RISCV_VIRT
select PLATFORM_BUS
select ACPI
select ACPI_PCI
+select VIRTIO_MEM_SUPPORTED
+select VIRTIO_PMEM_SUPPORTED
config SHAKTI_C
bool
diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c
index 0925528160f8..6dc3baa9ec86 100644
--- a/hw/riscv/virt-acpi-build.c
+++ b/hw/riscv/virt-acpi-build.c
@@ -610,6 +610,13 @@ build_srat(GArray *table_data, BIOSLinker *linker,
RISCVVirtState *vms)
}
}
+if (ms->device_memory) {
+build_srat_memory(table_data, ms->device_memory->base,
+ memory_region_size(>device_memory->mr),
+ ms->numa_state->num_nodes - 1,
+ MEM_AFFINITY_HOTPLUGGABLE | MEM_AFFINITY_ENABLED);
+}
+
acpi_table_end(linker, );
}
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(>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);
+}
+
/*
* Init fw_cfg. Must be done before riscv_load_fdt, otherwise the
* device tree cannot be altered and we get FDT_ERR_NOSPACE.
@@ -1712,12 +1734,21 @@ static HotplugHandler
*virt_machine_get_hotplug_handler(MachineState *machine,
MachineClass *mc = MACHINE_GET_CLASS(machine);
if (device_is_dynamic_sysbus(mc, dev) ||
-object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI) ||
+object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
return HOTPLUG_HANDLER(machine);
}
return NULL;
}
+static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
+DeviceState *dev, Error **errp)
+{
+if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
+virtio_md_pci_pre
Re: qemu riscv, thead c906, Linux boot regression
Daniel Henrique Barboza writes:
> On 1/24/24 16:26, Björn Töpel wrote:
>> Daniel Henrique Barboza writes:
>>
>>> On 1/24/24 09:49, Björn Töpel wrote:
>>>> Hi!
>>>>
>>>> I bumped the RISC-V Linux kernel CI to use qemu 8.2.0, and realized that
>>>> thead c906 didn't boot anymore. Bisection points to commit d6a427e2c0b2
>>>> ("target/riscv/cpu.c: restrict 'marchid' value")
>>>>
>>>> Reverting that commit, or the hack below solves the boot issue:
>>>>
>>>> --8<--
>>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>>> index 8cbfc7e781ad..e18596c8a55a 100644
>>>> --- a/target/riscv/cpu.c
>>>> +++ b/target/riscv/cpu.c
>>>> @@ -505,6 +505,9 @@ static void rv64_thead_c906_cpu_init(Object *obj)
>>>>cpu->cfg.ext_xtheadsync = true;
>>>>
>>>>cpu->cfg.mvendorid = THEAD_VENDOR_ID;
>>>> +cpu->cfg.marchid = ((QEMU_VERSION_MAJOR << 16) |
>>>> +(QEMU_VERSION_MINOR << 8) |
>>>> +(QEMU_VERSION_MICRO));
>>>>#ifndef CONFIG_USER_ONLY
>>>>set_satp_mode_max_supported(cpu, VM_1_10_SV39);
>>>>#endif
>>>> --8<--
>>>>
>>>> I'm unsure what the correct qemu way of adding a default value is,
>>>> or if c906 should have a proper marchid.
>>>
>>> In case you need to set a 'marchid' different than zero for c906, this hack
>>> would
>>> be a proper fix. As mentioned in the commit msg of the patch you mentioned:
>>>
>>> "Named CPUs should set 'marchid' to a meaningful value instead, and generic
>>>CPUs can set to any valid value."
>>>
>>> That means that any specific marchid value that the CPU uses must to be set
>>> in its own cpu_init() function.
>>
>> Got it. Thanks, Daniel!
>>
>> For completeness (since it came up on the weekly PW call); Conor pointed
>> out that zero *is* indeed the right marchid for c906, and in fact, the
>> non-zero marchid pre commit d6a427e2c0b2 was incorrect.
>>
>> Post commit d6a427e2c0b2, the correct alternative is picked up, and
>> ERRATA_THEAD_PBMT (using non-standard memory type bits in
>> page-table-entries) kicks in. AFAIU, that's not implemented by qemu's
>> c906 support, which then traps.
>
>
> This looks like a very good reason to actually push what you called 'hack' as
> a fix. Yeah, in theory that commit did nothing wrong, but the side effect
> (missing support for non-standard memory type bits) is kind of a QEMU problem.
For me, it'd be weird to add the hack (setting marchid to non-zero).
Claiming that it's a "thead-c906 emulation" in qemu, but w/o the proper
page-bit support. That's just cpu rv64 plus some extra instructions --
not the c906.
Björn
Re: qemu riscv, thead c906, Linux boot regression
Daniel Henrique Barboza writes:
> On 1/24/24 09:49, Björn Töpel wrote:
>> Hi!
>>
>> I bumped the RISC-V Linux kernel CI to use qemu 8.2.0, and realized that
>> thead c906 didn't boot anymore. Bisection points to commit d6a427e2c0b2
>> ("target/riscv/cpu.c: restrict 'marchid' value")
>>
>> Reverting that commit, or the hack below solves the boot issue:
>>
>> --8<--
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 8cbfc7e781ad..e18596c8a55a 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -505,6 +505,9 @@ static void rv64_thead_c906_cpu_init(Object *obj)
>> cpu->cfg.ext_xtheadsync = true;
>>
>> cpu->cfg.mvendorid = THEAD_VENDOR_ID;
>> +cpu->cfg.marchid = ((QEMU_VERSION_MAJOR << 16) |
>> +(QEMU_VERSION_MINOR << 8) |
>> +(QEMU_VERSION_MICRO));
>> #ifndef CONFIG_USER_ONLY
>> set_satp_mode_max_supported(cpu, VM_1_10_SV39);
>> #endif
>> --8<--
>>
>> I'm unsure what the correct qemu way of adding a default value is,
>> or if c906 should have a proper marchid.
>
> In case you need to set a 'marchid' different than zero for c906, this hack
> would
> be a proper fix. As mentioned in the commit msg of the patch you mentioned:
>
> "Named CPUs should set 'marchid' to a meaningful value instead, and generic
> CPUs can set to any valid value."
>
> That means that any specific marchid value that the CPU uses must to be set
> in its own cpu_init() function.
Got it. Thanks, Daniel!
For completeness (since it came up on the weekly PW call); Conor pointed
out that zero *is* indeed the right marchid for c906, and in fact, the
non-zero marchid pre commit d6a427e2c0b2 was incorrect.
Post commit d6a427e2c0b2, the correct alternative is picked up, and
ERRATA_THEAD_PBMT (using non-standard memory type bits in
page-table-entries) kicks in. AFAIU, that's not implemented by qemu's
c906 support, which then traps.
That's the theory. Maybe Christoph knows if the non-standard bits are
implemented or not?
Regardless; I removed booting Qemu T-head c906 from the CI, and the
build/boot passes nicely ;-) [1]
Björn
[1]
https://github.com/linux-riscv/linux-riscv/actions/runs/7641764759/job/20819801235?pr=447
Re: qemu riscv, thead c906, Linux boot regression
Conor Dooley writes:
> On Wed, Jan 24, 2024 at 01:49:51PM +0100, Björn Töpel wrote:
>> Hi!
>>
>> I bumped the RISC-V Linux kernel CI to use qemu 8.2.0, and realized that
>> thead c906 didn't boot anymore. Bisection points to commit d6a427e2c0b2
>> ("target/riscv/cpu.c: restrict 'marchid' value")
>>
>> Reverting that commit, or the hack below solves the boot issue:
>>
>> --8<--
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 8cbfc7e781ad..e18596c8a55a 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -505,6 +505,9 @@ static void rv64_thead_c906_cpu_init(Object *obj)
>> cpu->cfg.ext_xtheadsync = true;
>>
>> cpu->cfg.mvendorid = THEAD_VENDOR_ID;
>> +cpu->cfg.marchid = ((QEMU_VERSION_MAJOR << 16) |
>> +(QEMU_VERSION_MINOR << 8) |
>> +(QEMU_VERSION_MICRO));
>> #ifndef CONFIG_USER_ONLY
>> set_satp_mode_max_supported(cpu, VM_1_10_SV39);
>> #endif
>> --8<--
>>
>> I'm unsure what the correct qemu way of adding a default value is,
>> or if c906 should have a proper marchid.
>
> The "correct" marchid/mimpid values for the c906 are zero.
Ok! Thanks for clearing that up for me.
> I haven't looked into the code at all, so I am "assuming" that it is
> being zero intialised at present. Linux applies the errata fixups for
> the c906 when archid and impid are both zero - so your patch will avoid
> these fixups being applied.
I'm also assuming 0, -- will double-check. Hmm, that means that the
*previous* marchid was incorrect (pre d6a427e2c0b2).
> Do you think that perhaps the emulation in QEMU does not support what
> the kernel uses once then errata fixups are enabled?
Did a quick look at the c906 "in_asm,int" logs:
| 0x80201040: 1273 sfence.vma zero,zero
| 0x80201044: 18051073 csrrw zero,satp,a0
|
| riscv_cpu_do_interrupt: hart:0, async:0, cause:000c,
epc:0x80201048, tval:0x80201048, desc=exec_page_fault
| riscv_cpu_do_interrupt: hart:0, async:0, cause:000c,
epc:0x80001048, tval:0x80001048, desc=exec_page_fault
| ...cont forever
So it looks like we're tripping over the page tables, when we're turning
on paging.
Hmm, maybe it's not qemu, but the c906 that has been broken for a while?
I'll disable it temporarily from CI anyhow, and will continue digging.
Thanks for the pointers/clarifications, Conor!
Björn
qemu riscv, thead c906, Linux boot regression
Hi!
I bumped the RISC-V Linux kernel CI to use qemu 8.2.0, and realized that
thead c906 didn't boot anymore. Bisection points to commit d6a427e2c0b2
("target/riscv/cpu.c: restrict 'marchid' value")
Reverting that commit, or the hack below solves the boot issue:
--8<--
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8cbfc7e781ad..e18596c8a55a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -505,6 +505,9 @@ static void rv64_thead_c906_cpu_init(Object *obj)
cpu->cfg.ext_xtheadsync = true;
cpu->cfg.mvendorid = THEAD_VENDOR_ID;
+cpu->cfg.marchid = ((QEMU_VERSION_MAJOR << 16) |
+(QEMU_VERSION_MINOR << 8) |
+(QEMU_VERSION_MICRO));
#ifndef CONFIG_USER_ONLY
set_satp_mode_max_supported(cpu, VM_1_10_SV39);
#endif
--8<--
I'm unsure what the correct qemu way of adding a default value is,
or if c906 should have a proper marchid.
Maybe Christoph or Zhiwei can answer?
qemu command-line:
qemu-system-riscv64 -nodefaults -nographic -machine virt,acpi=off \
-cpu thead-c906 ...
Thanks,
Björn
[PATCH v2] riscv, qemu_fw_cfg: Add support for RISC-V architecture
From: Björn Töpel Qemu fw_cfg support was missing for RISC-V, which made it hard to do proper vmcore dumps from qemu. Add the missing RISC-V arch-defines. You can now do vmcore dumps from qemu. Add "-device vmcoreinfo" to the qemu command-line. From the qemu monitor: (qemu) dump-guest-memory vmcore The vmcore can now be used, e.g., with the "crash" utility. Acked-by: "Michael S. Tsirkin" Acked-by: Alistair Francis Tested-by: Clément Léger Signed-off-by: Björn Töpel --- v2: Fixed typo (Clément) ...and collected A-b/T-bs --- drivers/firmware/Kconfig | 2 +- drivers/firmware/qemu_fw_cfg.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index b59e3041fd62..f05ff56629b3 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -155,7 +155,7 @@ config RASPBERRYPI_FIRMWARE config FW_CFG_SYSFS tristate "QEMU fw_cfg device support in sysfs" - depends on SYSFS && (ARM || ARM64 || PARISC || PPC_PMAC || SPARC || X86) + depends on SYSFS && (ARM || ARM64 || PARISC || PPC_PMAC || RISCV || SPARC || X86) depends on HAS_IOPORT_MAP default n help diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c index a69399a6b7c0..1448f61173b3 100644 --- a/drivers/firmware/qemu_fw_cfg.c +++ b/drivers/firmware/qemu_fw_cfg.c @@ -211,7 +211,7 @@ static void fw_cfg_io_cleanup(void) /* arch-specific ctrl & data register offsets are not available in ACPI, DT */ #if !(defined(FW_CFG_CTRL_OFF) && defined(FW_CFG_DATA_OFF)) -# if (defined(CONFIG_ARM) || defined(CONFIG_ARM64)) +# if (defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_RISCV)) # define FW_CFG_CTRL_OFF 0x08 # define FW_CFG_DATA_OFF 0x00 # define FW_CFG_DMA_OFF 0x10 base-commit: 1c8b86a3799f7e5be903c3f49fcdaee29fd385b5 -- 2.39.2
[PATCH] riscv, qemu_fw_cfg: Add support for RISC-V architecture
From: Björn Töpel Qemu fw_cfg support was missing for RISC-V, which made it hard to do proper vmcore dumps from qemu. Add the missing RISC-V arch-defines. You can now do vmcore dumps from qemu. Add "-device vmcoreinfo" to the qemu command-line. From the qemu montior: (qemu) dump-guest-memory vmcore The vmcore can now be used, e.g., with the "crash" utility. Signed-off-by: Björn Töpel --- drivers/firmware/Kconfig | 2 +- drivers/firmware/qemu_fw_cfg.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index b59e3041fd62..f05ff56629b3 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -155,7 +155,7 @@ config RASPBERRYPI_FIRMWARE config FW_CFG_SYSFS tristate "QEMU fw_cfg device support in sysfs" - depends on SYSFS && (ARM || ARM64 || PARISC || PPC_PMAC || SPARC || X86) + depends on SYSFS && (ARM || ARM64 || PARISC || PPC_PMAC || RISCV || SPARC || X86) depends on HAS_IOPORT_MAP default n help diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c index a69399a6b7c0..1448f61173b3 100644 --- a/drivers/firmware/qemu_fw_cfg.c +++ b/drivers/firmware/qemu_fw_cfg.c @@ -211,7 +211,7 @@ static void fw_cfg_io_cleanup(void) /* arch-specific ctrl & data register offsets are not available in ACPI, DT */ #if !(defined(FW_CFG_CTRL_OFF) && defined(FW_CFG_DATA_OFF)) -# if (defined(CONFIG_ARM) || defined(CONFIG_ARM64)) +# if (defined(CONFIG_ARM) || defined(CONFIG_ARM64) || defined(CONFIG_RISCV)) # define FW_CFG_CTRL_OFF 0x08 # define FW_CFG_DATA_OFF 0x00 # define FW_CFG_DMA_OFF 0x10 base-commit: 1c8b86a3799f7e5be903c3f49fcdaee29fd385b5 -- 2.39.2
[PATCH] optionrom: Turn off -fcf-protection
Ubuntu GCC enables -fcf-protection globally, which is not supported for x86 16-bit (realmode). This causes the build to fail. This commit turns off that option. Signed-off-by: Björn Töpel --- pc-bios/optionrom/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile index 5d55d25acca2..c5f5fa02ef06 100644 --- a/pc-bios/optionrom/Makefile +++ b/pc-bios/optionrom/Makefile @@ -21,6 +21,7 @@ override CFLAGS += $(filter -W%, $(QEMU_CFLAGS)) override CFLAGS += $(CFLAGS_NOPIE) -ffreestanding -I$(TOPSRC_DIR)/include override CFLAGS += $(call cc-option, -fno-stack-protector) override CFLAGS += $(call cc-option, -m16) +override CFLAGS += $(call cc-option, -fcf-protection=none) ifeq ($(filter -m16, $(CFLAGS)),) # Attempt to work around compilers that lack -m16 (GCC <= 4.8, clang <= ??) -- 2.32.0
