Re: [PATCH 2/2] hw/isa/vt82c686.c: Embed i8259 irq in device state instead of allocating
Am 3. Juli 2024 00:09:45 UTC schrieb BALATON Zoltan :
>On Tue, 2 Jul 2024, Bernhard Beschow wrote:
>> Am 2. Juli 2024 18:42:23 UTC schrieb Bernhard Beschow :
>>> Am 1. Juli 2024 12:58:15 UTC schrieb Peter Maydell
>>> :
On Sat, 29 Jun 2024 at 21:01, BALATON Zoltan wrote:
>
> To avoid a warning about unfreed qemu_irq embed the i8259 irq in the
> device state instead of allocating it.
>
> Signed-off-by: BALATON Zoltan
> ---
> hw/isa/vt82c686.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 8582ac0322..834051abeb 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -592,6 +592,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(ViaISAState, VIA_ISA)
>
> struct ViaISAState {
> PCIDevice dev;
> +
> +IRQState i8259_irq;
> qemu_irq cpu_intr;
> qemu_irq *isa_irqs_in;
> uint16_t irq_state[ISA_NUM_IRQS];
> @@ -715,13 +717,12 @@ static void via_isa_realize(PCIDevice *d, Error
> **errp)
> ViaISAState *s = VIA_ISA(d);
> DeviceState *dev = DEVICE(d);
> PCIBus *pci_bus = pci_get_bus(d);
> -qemu_irq *isa_irq;
> ISABus *isa_bus;
> int i;
>
> qdev_init_gpio_out(dev, &s->cpu_intr, 1);
> qdev_init_gpio_in_named(dev, via_isa_pirq, "pirq", PCI_NUM_PINS);
> -isa_irq = qemu_allocate_irqs(via_isa_request_i8259_irq, s, 1);
> +qemu_init_irq(&s->i8259_irq, via_isa_request_i8259_irq, s, 0);
> isa_bus = isa_bus_new(dev, pci_address_space(d),
> pci_address_space_io(d),
>errp);
So if I understand correctly, this IRQ line isn't visible
from outside this chip,
>>>
>>> Actally it is, in the form of the INTR pin. Assuming similar naming
>
>The INTR pin corresponds to qemu_irq cpu_intr not the i8259_irq.
>
>>> conventions in vt82xx and piix, one can confirm this by consulting the
>>> piix4 datasheet, "Figure 5. Interrupt Controller Block Diagram". Moreover,
>>> the pegasos2 schematics (linked in the QEMU documentation) suggest that
>>> this pin is actually used there, although not modeled in QEMU.
>>
>> Well, QEMU does actually wire the intr pin in the pegasos2 board code,
>> except that it isn't a named gpio like in piix4. If we allow this pin to
>
>I could make that named to make it clearer, now it's the only output gpio so
>did not name it as usually devices that only have one output don't use named
>gpios for that.
>
>> be wired before the south bridge's realize we might be able to eliminate the
>> "intermediate irq forwarder" as Phil used to name it, resulting in less and
>> more efficient code. This solution would basically follow the pattern I
>> outlined under below link.
>
>I think the problem here is that i8259 does not provide an output gpio for
>this interrupt that the VT82xx could pass on but instead i8259_init() needs a
>qemu_irq to be passed rhat the i8259 model will set. This seems to be a legacy
>init function so the fix may be to Qdev-ify i8259 and add an output irq to it
>then its users could instantiate and connect its IRQs as usual and we don't
>need to create a qemu_irq to pass it to i8259_init().
I've implemented the approach avoiding the intermediate IRQ forwarders here:
https://github.com/shentok/qemu/commits/upstream/vt82c686-irq/ . I'd send this
series to the list as soon as I resolve some email authentication issues.
Best regards,
Bernhard
>
>Regards,
>BALATON Zoltan
>
>> Best regards,
>> Bernhard
>>
>>> Compare this to how the "intr" pin is exposed by the piix4 device model and
>>> wired in the Malta board.
>>>
we're just trying to wire together
two internal components of the chip? If so, I agree that
this seems a better way than creating a named GPIO that
we then have to document as a "not really an external
connection, don't try to use this" line. (We've done that
before I think in other devices, and it works but it's
a bit odd-looking.)
That said, I do notice that the via_isa_request_i8259_irq()
function doesn't do anything except pass the level onto
another qemu_irq, so I think the theoretical ideal would be
if we could arrange to plumb things directly through rather
than needing this extra qemu_irq and function. There's
probably a reason (order of device creation/connection?)
that doesn't work though.
>>>
>>> I think there could be a general pattern of device creation/connection
>>> which I've outlined here:
>>> https://lore.kernel.org/qemu-devel/0ffb5fd2-08ce-4cec-9001-e7ac24407...@gmail.com/
>>>
>>> Best regards,
>>> Bernhard
>>>
-- PMM
>>
>>
Re: [PATCH ats_vtd v5 04/22] intel_iommu: do not consider wait_desc as an invalid descriptor
On 2024/7/2 23:29, CLEMENT MATHIEU--DRIF wrote:
On 02/07/2024 15:33, Yi Liu 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.
On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote:
From: Clément Mathieu--Drif
Signed-off-by: Clément Mathieu--Drif
Reviewed-by: Zhenzhong Duan
---
hw/i386/intel_iommu.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 98996ededc..71cebe2fd3 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3500,6 +3500,11 @@ static bool
vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
} else if (inv_desc->lo & VTD_INV_DESC_WAIT_IF) {
/* Interrupt flag */
vtd_generate_completion_event(s);
+ } else if (inv_desc->lo & VTD_INV_DESC_WAIT_FN) {
+ /*
+ * SW = 0, IF = 0, FN = 1
+ * Nothing to do as we process the events sequentially
+ */
This code looks a bit weird. SW field does not co-exist with IF. But
either
SW or IF can co-exist with FN flag. Is it? Have you already seen a wait
descriptor that only has FN flag set but no SW nor IF flag?
Yes, my test suite triggers that condition
I see. Spec indeed has such usage. Please add a comment for it.
Since it does not need a response, so QEMU can just bypass it. Also
please adjust the subject a bit. It's misleading. Perhaps
"intel_iommu: Bypass barrier wait descriptor"
Spec CH 7.10
a. Submit Invalidation Wait Descriptor (inv_wait_dsc) with Fence flag
(FN=1) Set to Invalidation
Queue. This ensures that all requests submitted to the Invalidation Queue
ahead of this wait
descriptor are processed and completed by remapping hardware before
processing requests
after the Invalidation Wait Descriptor. It is not required to specify SW
flag (or IF flag) in this
descriptor or for software to wait on its completion, as its function is to
only act as a barrier.
} else {
error_report_once("%s: invalid wait desc: hi=%"PRIx64",
lo=%"PRIx64
" (unknown type)", __func__, inv_desc->hi,
--
Regards,
Yi Liu
--
Regards,
Yi Liu
Re: [PATCH 1/3] util/cpuinfo-riscv: Support host/cpuinfo.h for riscv
On 3/7/24 01:04, Richard Henderson wrote: On 7/2/24 12:55, Philippe Mathieu-Daudé wrote: On 27/6/24 20:03, Richard Henderson wrote: ... + info |= !got_sigill * CPUINFO_ZBA; A bit too optimized to my taste, 'if (sigill) info|=ZBA' would be simpler to follow. I switched to "info |= got_sigill ? 0 : CPUINFO_ZBA". Thanks :)
Re: [PATCH 1/3] hw/intc/loongson_ipi_common: Add loongson ipi common class
在2024年7月3日七月 下午2:40，maobibo写道：
[...]
Hi Bobo,
>
> MMIO is loongson ipi specific, it is not necessary to put into common
> function. Functions loongson_ipi_core_readl/loongson_ipi_core_writel can
> be exported in header file include/hw/intc/loongson_ipi_common.h, or get
> MemoryRegionOps of first memoryregion of loongson_ipi instance.
>
> There is pseudo code:
Thanks for your demonstration. I'm still not quite convinced it's worthy to
split
but I'm not going to block you if we don't have other oppositions.
Do you mind to finish conversion of loongson_ipi as well? Since you are
drafting the
design.
>
> static void loongson_ipi_realize(DeviceState *dev, Error **errp)
> {
> LoongarchIPIState *s = LOONGARCH_IPI(dev);
> LoongarchIPIClass *lic = LOONGARCH_IPI_GET_CLASS(s);
> Error *local_err = NULL;
>
> lic->parent_realize(dev, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
>
>
> *do mmio specific implematation in loongson ipi itself*
> }
>
> static void loongson_ipi_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> LoongsonIPICommonClass *licc = LOONGSON_IPI_COMMON_CLASS(klass);
> LoongarchIPIClass *lic = LOONGARCH_IPI_CLASS(klass);
>
> device_class_set_parent_realize(dc, loongson_ipi_realize,
> &lic->parent_realize);
> licc->get_iocsr_as = get_iocsr_as;
> }
>
>>
>> If current implementation is hindering your future plan can you elaborate so
>> we
>> can work on a resolution.
>>
>> I'm happy to help with devlopment and testing.
>>
>>> 3. Interace cpu_by_arch_id is added, by default generic function
>>> cpu_by_arch_id() is used to search vcpu from physical cpuid, it is
>>> generic searching method. Different machine may define another search
>>> method such binary searching method.
>>
>> If you are going to implement some faster searching algorithm why don't we
>> make it generic for all architectures?
> It depends on the detailed physical id layout, is physical id is growing
> up with logic cpu id or irrelative with logic cpu id? Different
> architecture has different logic definition about physical id.
For x86' APIC id and RISC-V's hardid they are all somehow linear.
I'd suggest you to post a RFC patch regarding better algorithm.
Thanks
- Jiaxun
>
> Regards
> Bibo Mao
>
>>
>> Thanks
>> - Jiaxun
>>
>>>
>>> Signed-off-by: Bibo Mao
>>> ---
>>> hw/intc/loongson_ipi_common.c | 394 ++
>>> include/hw/intc/loongson_ipi_common.h | 71 +
>>> 2 files changed, 465 insertions(+)
>>> create mode 100644 hw/intc/loongson_ipi_common.c
>>> create mode 100644 include/hw/intc/loongson_ipi_common.h
>>>
>>> diff --git a/hw/intc/loongson_ipi_common.c
>>> b/hw/intc/loongson_ipi_common.c
>>> new file mode 100644
>>> index 00..f462f24f32
>>> --- /dev/null
>>> +++ b/hw/intc/loongson_ipi_common.c
>>> @@ -0,0 +1,394 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>>> +/*
>>> + * Loongson ipi interrupt support
>>> + *
>>> + * Copyright (C) 2021 Loongson Technology Corporation Limited
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "hw/boards.h"
>>> +#include "hw/sysbus.h"
>>> +#include "hw/intc/loongson_ipi_common.h"
>>> +#include "hw/irq.h"
>>> +#include "hw/qdev-properties.h"
>>> +#include "qapi/error.h"
>>> +#include "qemu/log.h"
>>> +#include "exec/address-spaces.h"
>>> +#include "exec/memory.h"
>>> +#include "migration/vmstate.h"
>>> +#include "trace.h"
>>> +
>>> +static MemTxResult loongson_ipi_core_readl(void *opaque, hwaddr addr,
>>> + uint64_t *data,
>>> + unsigned size, MemTxAttrs
>>> attrs)
>>> +{
>>> +IPICore *s = opaque;
>>> +uint64_t ret = 0;
>>> +int index = 0;
>>> +
>>> +addr &= 0xff;
>>> +switch (addr) {
>>> +case CORE_STATUS_OFF:
>>> +ret = s->status;
>>> +break;
>>> +case CORE_EN_OFF:
>>> +ret = s->en;
>>> +break;
>>> +case CORE_SET_OFF:
>>> +ret = 0;
>>> +break;
>>> +case CORE_CLEAR_OFF:
>>> +ret = 0;
>>> +break;
>>> +case CORE_BUF_20 ... CORE_BUF_38 + 4:
>>> +index = (addr - CORE_BUF_20) >> 2;
>>> +ret = s->buf[index];
>>> +break;
>>> +default:
>>> +qemu_log_mask(LOG_UNIMP, "invalid read: %x", (uint32_t)addr);
>>> +break;
>>> +}
>>> +
>>> +trace_loongson_ipi_read(size, (uint64_t)addr, ret);
>>> +*data = ret;
>>> +return MEMTX_OK;
>>> +}
>>> +
>>> +static MemTxResult loongson_ipi_iocsr_readl(void *opaque, hwaddr addr,
>>> +uint64_t *data,
>>> +unsigned size, MemTxAttrs
>>> attrs)
>>> +{
>>> +LoongsonIPICommonState *ipi = opaque;
>>> +IPICore *s;
>>> +
>>> +if (attrs.requester_id >= ipi->num_cpu) {
>>> +return MEMT
Re: [PATCH v43 2/2] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
On 18:10 Tue 02 Jul , Philippe Mathieu-Daudé wrote:
> "General command" (GEN_CMD, CMD56) is described as:
>
> GEN_CMD is the same as the single block read or write
> commands (CMD24 or CMD17). The difference is that [...]
> the data block is not a memory payload data but has a
> vendor specific format and meaning.
>
> Thus this block must not be stored overwriting data block
> on underlying storage drive. Keep it in a dedicated
> 'vendor_data[]' array.
>
> Signed-off-by: Philippe Mathieu-Daudé
> Tested-by: Cédric Le Goater
> ---
> v43: Do not re-use VMSTATE_UNUSED_V (danpb)
> ---
> hw/sd/sd.c | 16 +---
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 808dc1cea6..418ccb14a4 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -153,6 +153,8 @@ struct SDState {
> uint32_t data_offset;
> size_t data_size;
> uint8_t data[512];
> +uint8_t vendor_data[512];
> +
> qemu_irq readonly_cb;
> qemu_irq inserted_cb;
> QEMUTimer *ocr_power_timer;
> @@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
> sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
> sd->wp_group_bits = sect;
> sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
> +memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
> memset(sd->function_group, 0, sizeof(sd->function_group));
> sd->erase_start = INVALID_ADDRESS;
> sd->erase_end = INVALID_ADDRESS;
> @@ -835,6 +838,7 @@ static const VMStateDescription sd_vmstate = {
> VMSTATE_UINT32(data_offset, SDState),
> VMSTATE_UINT8_ARRAY(data, SDState, 512),
> VMSTATE_UNUSED_V(1, 512),
> +VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
Don't you need to bump the VMState version then?
> VMSTATE_BOOL(enable, SDState),
> VMSTATE_END_OF_LIST()
> },
> @@ -2187,9 +2191,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
> break;
>
> case 56: /* CMD56: GEN_CMD */
> -sd->data[sd->data_offset ++] = value;
> -if (sd->data_offset >= sd->blk_len) {
> -APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
> +sd->vendor_data[sd->data_offset ++] = value;
> +if (sd->data_offset >= sizeof(sd->vendor_data)) {
> sd->state = sd_transfer_state;
> }
> break;
> @@ -2261,12 +2264,11 @@ uint8_t sd_read_byte(SDState *sd)
> break;
>
> case 56: /* CMD56: GEN_CMD */
> -if (sd->data_offset == 0)
> -APP_READ_BLOCK(sd->data_start, sd->blk_len);
> -ret = sd->data[sd->data_offset ++];
> +ret = sd->vendor_data[sd->data_offset ++];
>
> -if (sd->data_offset >= sd->blk_len)
> +if (sd->data_offset >= sizeof(sd->vendor_data)) {
> sd->state = sd_transfer_state;
> +}
> break;
>
> default:
> --
> 2.41.0
>
--
Re: [PATCH 1/3] hw/intc/loongson_ipi_common: Add loongson ipi common class
On 2024/7/3 下午3:33, Jiaxun Yang wrote:
在2024年7月3日七月 下午2:40，maobibo写道：
[...]
Hi Bobo,
MMIO is loongson ipi specific, it is not necessary to put into common
function. Functions loongson_ipi_core_readl/loongson_ipi_core_writel can
be exported in header file include/hw/intc/loongson_ipi_common.h, or get
MemoryRegionOps of first memoryregion of loongson_ipi instance.
There is pseudo code:
Thanks for your demonstration. I'm still not quite convinced it's worthy to
split
but I'm not going to block you if we don't have other oppositions.
Do you mind to finish conversion of loongson_ipi as well? Since you are
drafting the
design.
It depends on schedule, I hope Loongarch IPI can merge before qemu is
frozen. However I will try to write loongson_ipi but it can just pass to
compile, no test. If it takes too long time to merge both Loongarch IPI
and loongson ipi, I will only provide support for Loongarch IPI :)
static void loongson_ipi_realize(DeviceState *dev, Error **errp)
{
LoongarchIPIState *s = LOONGARCH_IPI(dev);
LoongarchIPIClass *lic = LOONGARCH_IPI_GET_CLASS(s);
Error *local_err = NULL;
lic->parent_realize(dev, &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
*do mmio specific implematation in loongson ipi itself*
}
static void loongson_ipi_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
LoongsonIPICommonClass *licc = LOONGSON_IPI_COMMON_CLASS(klass);
LoongarchIPIClass *lic = LOONGARCH_IPI_CLASS(klass);
device_class_set_parent_realize(dc, loongson_ipi_realize,
&lic->parent_realize);
licc->get_iocsr_as = get_iocsr_as;
}
If current implementation is hindering your future plan can you elaborate so we
can work on a resolution.
I'm happy to help with devlopment and testing.
3. Interace cpu_by_arch_id is added, by default generic function
cpu_by_arch_id() is used to search vcpu from physical cpuid, it is
generic searching method. Different machine may define another search
method such binary searching method.
If you are going to implement some faster searching algorithm why don't we
make it generic for all architectures?
It depends on the detailed physical id layout, is physical id is growing
up with logic cpu id or irrelative with logic cpu id? Different
architecture has different logic definition about physical id.
For x86' APIC id and RISC-V's hardid they are all somehow linear.
I'd suggest you to post a RFC patch regarding better algorithm.
Thanks for your suggestion, currently there is no such plan, it is above
my ability :(
Regards
Bibo Mao
Thanks
- Jiaxun
Regards
Bibo Mao
Thanks
- Jiaxun
Signed-off-by: Bibo Mao
---
hw/intc/loongson_ipi_common.c | 394 ++
include/hw/intc/loongson_ipi_common.h | 71 +
2 files changed, 465 insertions(+)
create mode 100644 hw/intc/loongson_ipi_common.c
create mode 100644 include/hw/intc/loongson_ipi_common.h
diff --git a/hw/intc/loongson_ipi_common.c
b/hw/intc/loongson_ipi_common.c
new file mode 100644
index 00..f462f24f32
--- /dev/null
+++ b/hw/intc/loongson_ipi_common.c
@@ -0,0 +1,394 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Loongson ipi interrupt support
+ *
+ * Copyright (C) 2021 Loongson Technology Corporation Limited
+ */
+
+#include "qemu/osdep.h"
+#include "hw/boards.h"
+#include "hw/sysbus.h"
+#include "hw/intc/loongson_ipi_common.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "exec/address-spaces.h"
+#include "exec/memory.h"
+#include "migration/vmstate.h"
+#include "trace.h"
+
+static MemTxResult loongson_ipi_core_readl(void *opaque, hwaddr addr,
+ uint64_t *data,
+ unsigned size, MemTxAttrs
attrs)
+{
+IPICore *s = opaque;
+uint64_t ret = 0;
+int index = 0;
+
+addr &= 0xff;
+switch (addr) {
+case CORE_STATUS_OFF:
+ret = s->status;
+break;
+case CORE_EN_OFF:
+ret = s->en;
+break;
+case CORE_SET_OFF:
+ret = 0;
+break;
+case CORE_CLEAR_OFF:
+ret = 0;
+break;
+case CORE_BUF_20 ... CORE_BUF_38 + 4:
+index = (addr - CORE_BUF_20) >> 2;
+ret = s->buf[index];
+break;
+default:
+qemu_log_mask(LOG_UNIMP, "invalid read: %x", (uint32_t)addr);
+break;
+}
+
+trace_loongson_ipi_read(size, (uint64_t)addr, ret);
+*data = ret;
+return MEMTX_OK;
+}
+
+static MemTxResult loongson_ipi_iocsr_readl(void *opaque, hwaddr addr,
+uint64_t *data,
+unsigned size, MemTxAttrs
attrs)
+{
+LoongsonIPICommonState *ipi = opaque;
+IPICore *s;
+
+if (attrs.requester_id >= ipi->num_cpu) {
+return M
Re: [RFC v3 1/2] target/loongarch: Add loongson binary translation feature
在2024年7月1日七月 下午2:57，Jiaxun Yang写道：
> 在2024年5月30日五月 上午7:49，Bibo Mao写道：
>> Loongson Binary Translation (LBT) is used to accelerate binary
>> translation, which contains 4 scratch registers (scr0 to scr3), x86/ARM
>> eflags (eflags) and x87 fpu stack pointer (ftop).
>>
>> Now LBT feature is added in kvm mode, not supported in TCG mode since
>> it is not emulated. Feature variable lbt is added with OnOffAuto type,
>> If lbt feature is not supported with KVM host, it reports error if there
>> is lbt=on command line.
>>
>> If there is no any command line about lbt parameter, it checks whether
>> KVM host supports lbt feature and set the corresponding value in cpucfg.
>>
>> Signed-off-by: Bibo Mao
> Hi Bibo,
>
> I was going across recent LoongArch changes and this comes into my attention:
>
>> ---
>> target/loongarch/cpu.c| 53 +++
>> target/loongarch/cpu.h| 6 +++
>> target/loongarch/kvm/kvm.c| 26 +
>> target/loongarch/kvm/kvm_loongarch.h | 16
>> target/loongarch/loongarch-qmp-cmds.c | 2 +-
>> 5 files changed, 102 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
>> index b5c1ec94af..14265b6667 100644
>> --- a/target/loongarch/cpu.c
>> +++ b/target/loongarch/cpu.c
>> @@ -571,6 +571,30 @@ static void loongarch_cpu_disas_set_info(CPUState
>> *s, disassemble_info *info)
>> info->print_insn = print_insn_loongarch;
>> }
>>
>> +static void loongarch_cpu_check_lbt(CPUState *cs, Error **errp)
>> +{
>> +CPULoongArchState *env = cpu_env(cs);
>> +LoongArchCPU *cpu = LOONGARCH_CPU(cs);
>> +bool kvm_supported;
>> +
>> +kvm_supported = kvm_feature_supported(cs, LOONGARCH_FEATURE_LBT);
>
> IMHO if there is no global states that should be saved/restored VM wise,
> this should be handled at per CPU level, preferably with CPUCFG flags hint.
>
> We should minimize non-privilege KVM feature bits to prevent hindering
> asymmetry ISA system.
+ Huacai for further discussion
Hi Bibo, Huacai,
I investigated the topic further and went through the thread on kernel side.
I think Huacai and me are all on the same page that we should unify the
interface for per-CPU
level feature probing and setting interface. Huacai purposed converting all
features to VM feature
but I still believe CPUCFG is the best interface.
To probe LBT before actual vcpu creation, we can borrow the approach used by
other architectures
(kvm_arm_create_scratch_host_vcpu() & kvm_riscv_create_scratch_vcpu()).
Kernel will reject setting unknown CPUCFG bits with -EINVAL, so to probe LBT we
just need to perform
KVM_SET_REGS to scratch vcpu with LBT set to see if it's valid for kernel.
There is no need for any other
probing interface.
I do think scratch CPU interface is also necessary if we are going to implement
cpu = host.
Huacai, would you agree with me?
Thanks
- Jiaxun
>
> Thanks
> - Jiaxun
>
> --
> - Jiaxun
--
- Jiaxun
Re: [PATCH 1/2] Python: bump minimum sphinx version to 3.4.3
On 7/2/24 21:59, John Snow wrote:
With RHEL 8 support retired (It's been two years today since RHEL 9
came out), our very oldest build platform version of Sphinx is now
3.4.3; and keeping backwards compatibility for versions as old as v1.6
when using domain extensions is a lot of work we don't need to do.
Technically that's unrelated: thanks to your venv work, :) builds on
RHEL 8 / CentOS Stream 8 do not pick the platform Sphinx, because it
runs under Python 3.6. Therefore the version included in RHEL 8 does
not matter for picking the minimum supported Sphinx version.
Debian 11: v3.4.3 (QEMU support ends 2024-07-xx)
Nice. :)
diff --git a/pythondeps.toml b/pythondeps.toml
index 9c16602d303..bc656376caa 100644
--- a/pythondeps.toml
+++ b/pythondeps.toml
@@ -23,7 +23,7 @@ meson = { accepted = ">=0.63.0", installed = "1.2.3", canary =
"meson" }
[docs]
# Please keep the installed versions in sync with docs/requirements.txt
-sphinx = { accepted = ">=1.6", installed = "5.3.0", canary = "sphinx-build" }
+sphinx = { accepted = ">=3.4.3", installed = "5.3.0", canary = "sphinx-build" }
sphinx_rtd_theme = { accepted = ">=0.5", installed = "1.1.1" }
[avocado]
Acked-by: Paolo Bonzini
Paolo
Re: [PATCH v2 00/22] qga: clean up command source locations and conditionals
On Wed, Jul 03, 2024 at 10:15:44AM +0400, Marc-André Lureau wrote: > Hi Daniel > > On Tue, Jul 2, 2024 at 10:00 PM Daniel P. Berrangé > wrote: > > > > Ping: for any review comments from QGA maintainers ? > > Maybe you could resend for patchew to correctly handle the series. I don't want to spam the list again just because patchew didn't handle it. If anyone can't just pull the messages from mail though, they are also at https://gitlab.com/berrange/qemu/-/tags/qga-conditions-v2 With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v43 2/2] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
On 3/7/24 09:39, Luc Michel wrote:
On 18:10 Tue 02 Jul , Philippe Mathieu-Daudé wrote:
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
Tested-by: Cédric Le Goater
---
v43: Do not re-use VMSTATE_UNUSED_V (danpb)
---
hw/sd/sd.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 808dc1cea6..418ccb14a4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -153,6 +153,8 @@ struct SDState {
uint32_t data_offset;
size_t data_size;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -835,6 +838,7 @@ static const VMStateDescription sd_vmstate = {
VMSTATE_UINT32(data_offset, SDState),
VMSTATE_UINT8_ARRAY(data, SDState, 512),
VMSTATE_UNUSED_V(1, 512),
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
Don't you need to bump the VMState version then?
Indeed. I'll add a subsection which is the recommended way:
https://www.qemu.org/docs/master/devel/migration/main.html#subsections
The most common structure change is adding new data, e.g. when
adding a newer form of device, or adding that state that you
previously forgot to migrate. This is best solved using a subsection.
Thanks,
Phil.
[PATCH v2 2/5] hw/net:ftgmac100: support 64 bits dma dram address for AST2700
ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35)
And the base address of dram is "0x4 " which
is 64bits address.
It have "Normal Priority Transmit Ring Base Address Register High(0x17C)",
"High Priority Transmit Ring Base Address Register High(0x184)" and
"Receive Ring Base Address Register High(0x18C)" to save the high part physical
address of descriptor manager.
Ex: TX descriptor manager address [34:0]
The "Normal Priority Transmit Ring Base Address Register High(0x17C)"
bits [2:0] which corresponds the bits [34:32] of the 64 bits address of
the TX ring buffer address.
The "Normal Priority Transmit Ring Base Address Register(0x20)" bits [31:0]
which corresponds the bits [31:0] of the 64 bits address
of the TX ring buffer address.
Besides, it have "TXDES 2" and "RXDES 2"
to save the high part physical address of packet buffer.
Ex: TX packet buffer address [34:0]
The "TXDES 2" bits [18:16] which corresponds the bits [34:32]
of the 64 bits address of the TX packet buffer address
and "TXDES 3" bits [31:0] which corresponds the bits [31:0]
of the 64 bits address of the TX packet buffer address.
Update TX/RX ring and descriptor data type to uint64_t
and supports TX/RX ring, descriptor and packet buffers
64 bits address for all ASPEED SOCs models.
Incrementing the version of vmstate to 2.
Introduce a new class(ftgmac100_high),
class attribute and memop handlers,
the realize handler instantiate a new memory
region and map it on top of the current one to
read/write FTGMAC100_*_HIGH regs.
Signed-off-by: Jamin Lin
---
hw/net/ftgmac100.c | 156 -
include/hw/net/ftgmac100.h | 24 --
2 files changed, 151 insertions(+), 29 deletions(-)
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 4e88430b2f..3d13f54efc 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -56,6 +56,16 @@
#define FTGMAC100_PHYDATA 0x64
#define FTGMAC100_FCR 0x68
+/*
+ * FTGMAC100 High registers
+ *
+ * values below are offset by - FTGMAC100_HIGH_OFFSET from datasheet
+ * because its memory region is start at FTGMAC100_HIGH_OFFSET
+ */
+#define FTGMAC100_NPTXR_BADR_HIGH (0x17C - FTGMAC100_HIGH_OFFSET)
+#define FTGMAC100_HPTXR_BADR_HIGH (0x184 - FTGMAC100_HIGH_OFFSET)
+#define FTGMAC100_RXR_BADR_HIGH (0x18C - FTGMAC100_HIGH_OFFSET)
+
/*
* Interrupt status register & interrupt enable register
*/
@@ -165,6 +175,8 @@
#define FTGMAC100_TXDES1_TX2FIC (1 << 30)
#define FTGMAC100_TXDES1_TXIC(1 << 31)
+#define FTGMAC100_TXDES2_TXBUF_BADR_HI(x) (((x) >> 16) & 0x7)
+
/*
* Receive descriptor
*/
@@ -198,13 +210,15 @@
#define FTGMAC100_RXDES1_UDP_CHKSUM_ERR (1 << 26)
#define FTGMAC100_RXDES1_IP_CHKSUM_ERR (1 << 27)
+#define FTGMAC100_RXDES2_RXBUF_BADR_HI(x) (((x) >> 16) & 0x7)
+
/*
* Receive and transmit Buffer Descriptor
*/
typedef struct {
uint32_tdes0;
uint32_tdes1;
-uint32_tdes2;/* not used by HW */
+uint32_tdes2;/* used by HW high address */
uint32_tdes3;
} FTGMAC100Desc;
@@ -515,12 +529,14 @@ out:
return frame_size;
}
-static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t tx_ring,
-uint32_t tx_descriptor)
+static void ftgmac100_do_tx(FTGMAC100State *s, uint64_t tx_ring,
+uint64_t tx_descriptor)
{
+FTGMAC100Class *fc = FTGMAC100_GET_CLASS(s);
int frame_size = 0;
uint8_t *ptr = s->frame;
-uint32_t addr = tx_descriptor;
+uint64_t addr = tx_descriptor;
+uint64_t buf_addr = 0;
uint32_t flags = 0;
while (1) {
@@ -559,7 +575,12 @@ static void ftgmac100_do_tx(FTGMAC100State *s, uint32_t
tx_ring,
len = sizeof(s->frame) - frame_size;
}
-if (dma_memory_read(&address_space_memory, bd.des3,
+buf_addr = bd.des3;
+if (fc->is_dma64) {
+buf_addr = deposit64(buf_addr, 32, 32,
+ FTGMAC100_TXDES2_TXBUF_BADR_HI(bd.des2));
+}
+if (dma_memory_read(&address_space_memory, buf_addr,
ptr, len, MEMTXATTRS_UNSPECIFIED)) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to read packet @
0x%x\n",
__func__, bd.des3);
@@ -726,9 +747,9 @@ static uint64_t ftgmac100_read(void *opaque, hwaddr addr,
unsigned size)
case FTGMAC100_MATH1:
return s->math[1];
case FTGMAC100_RXR_BADR:
-return s->rx_ring;
+return extract64(s->rx_ring, 0, 32);
case FTGMAC100_NPTXR_BADR:
-return s->tx_ring;
+return extract64(s->tx_ring, 0, 32);
case FTGMAC100_ITC:
return s->itc;
case FTGMAC100_DBLAC:
@@ -799,9 +820,8 @@ static void ftgmac100_write(void *opaque, hwaddr addr,
HWADDR_PRIx "\n", __func__, value);
return;
}
-
-s->rx_ring = value;
-s->rx_descr
[PATCH v2 3/5] aspeed/soc: update to ftgmac100_high model for AST2700
ASPEED AST2700 SOC is a 64 bits quad core CPUs (Cortex-a35)
And the base address of dram is "0x4 " which
is 64bits address.
Update its network model to ftgmac100_high to support
64bits dram address DMA.
Signed-off-by: Jamin Lin
---
hw/arm/aspeed_ast27x0.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 18e6a8b10c..04604a4bef 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -332,7 +332,7 @@ static void aspeed_soc_ast2700_init(Object *obj)
for (i = 0; i < sc->macs_num; i++) {
object_initialize_child(obj, "ftgmac100[*]", &s->ftgmac100[i],
-TYPE_FTGMAC100);
+TYPE_FTGMAC100_HIGH);
object_initialize_child(obj, "mii[*]", &s->mii[i], TYPE_ASPEED_MII);
}
@@ -552,6 +552,7 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev,
Error **errp)
return;
}
+/* Net */
for (i = 0; i < sc->macs_num; i++) {
object_property_set_bool(OBJECT(&s->ftgmac100[i]), "aspeed", true,
&error_abort);
--
2.34.1
[PATCH v2 0/5] support AST2700 network
change from v1: - ftgmac100 - fix coding style - support 64 bits dma dram address for AST2700 change from v2: - ftgmac100: update memory region size to 0x200. - ftgmac100: introduce a new class(ftgmac100_high), class attribute and memop handlers, for FTGMAC100_*_HIGH regs read/write. - aspeed_ast27x0: update network model to ftgmac100_high to support 64 bits dram address DMA. - m25p80: support quad mode for w25q01jvq Jamin Lin (5): hw/net:ftgmac100: update memory region size to 0x200 hw/net:ftgmac100: support 64 bits dma dram address for AST2700 aspeed/soc: update to ftgmac100_high model for AST2700 hw/block: m25p80: support quad mode for w25q01jvq test/avocado/machine_aspeed.py: update to test network for AST2700 hw/arm/aspeed_ast27x0.c | 3 +- hw/block/m25p80.c | 16 hw/net/ftgmac100.c | 158 +++- include/hw/net/ftgmac100.h | 24 +++-- tests/avocado/machine_aspeed.py | 13 +-- 5 files changed, 178 insertions(+), 36 deletions(-) -- 2.34.1
[PATCH v2 5/5] test/avocado/machine_aspeed.py: update to test network for AST2700
Update a test case to test network connection via ssh and
changes to test Aspeed OpenBMC SDK v09.02 for AST2700.
ASPEED fixed TX mask issue from linux/drivers/ftgmac100.c.
It is required to use ASPEED SDK image since v09.02
for AST2700 QEMU network testing.
A test image is downloaded from the ASPEED Forked OpenBMC GitHub
release repository :
https://github.com/AspeedTech-BMC/openbmc/releases/
Test command:
```
cd build
pyvenv/bin/avocado run
../qemu/tests/avocado/machine_aspeed.py:AST2x00MachineSDK.test_aarch64_ast2700_evb_sdk_v09_02
```
Signed-off-by: Jamin Lin
---
tests/avocado/machine_aspeed.py | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
index 3a20644fb2..855da805ae 100644
--- a/tests/avocado/machine_aspeed.py
+++ b/tests/avocado/machine_aspeed.py
@@ -313,14 +313,14 @@ def do_test_arm_aspeed_sdk_start(self, image):
def do_test_aarch64_aspeed_sdk_start(self, image):
self.vm.set_console()
-self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw')
+self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
+ '-net', 'nic', '-net',
'user,hostfwd=:127.0.0.1:0-:22')
self.vm.launch()
self.wait_for_console_pattern('U-Boot 2023.10')
self.wait_for_console_pattern('## Loading kernel from FIT Image')
self.wait_for_console_pattern('Starting kernel ...')
-self.wait_for_console_pattern("systemd[1]: Hostname set to")
@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on
GitLab')
@@ -387,15 +387,15 @@ def test_arm_ast2600_evb_sdk(self):
year = time.strftime("%Y")
self.ssh_command_output_contains('/sbin/hwclock -f /dev/rtc1', year);
-def test_aarch64_ast2700_evb_sdk_v09_01(self):
+def test_aarch64_ast2700_evb_sdk_v09_02(self):
"""
:avocado: tags=arch:aarch64
:avocado: tags=machine:ast2700-evb
"""
image_url = ('https://github.com/AspeedTech-BMC/openbmc/releases/'
- 'download/v09.01/ast2700-default-obmc.tar.gz')
-image_hash =
'b1cc0fd73c7650d34c9c8459a243f52a91e9e27144b8608b2645ab19461d1e07'
+ 'download/v09.02/ast2700-default-obmc.tar.gz')
+image_hash =
'ac969c2602f4e6bdb69562ff466b89ae3fe1d86e1f6797bb7969d787f82116a7'
image_path = self.fetch_asset(image_url, asset_hash=image_hash,
algorithm='sha256')
archive.extract(image_path, self.workdir)
@@ -436,4 +436,5 @@ def test_aarch64_ast2700_evb_sdk_v09_01(self):
self.vm.add_args('-smp', str(num_cpu))
self.do_test_aarch64_aspeed_sdk_start(image_dir + 'image-bmc')
-
+self.wait_for_console_pattern('nodistro.0 ast2700-default ttyS12')
+self.ssh_connect('root', '0penBmc', False)
--
2.34.1
Re: [PATCH v2 00/22] qga: clean up command source locations and conditionals
Hi On Wed, Jul 3, 2024 at 12:06 PM Daniel P. Berrangé wrote: > > On Wed, Jul 03, 2024 at 10:15:44AM +0400, Marc-André Lureau wrote: > > Hi Daniel > > > > On Tue, Jul 2, 2024 at 10:00 PM Daniel P. Berrangé > > wrote: > > > > > > Ping: for any review comments from QGA maintainers ? > > > > Maybe you could resend for patchew to correctly handle the series. > > I don't want to spam the list again just because patchew didn't > handle it. If anyone can't just pull the messages from mail > though, they are also at > https://gitlab.com/berrange/qemu/-/tags/qga-conditions-v2 patchew also handles R-b/A-b/T-b tags, mail ref, versioning etc..
[PATCH v2 1/5] hw/net:ftgmac100: update memory region size to 0x200
According to the datasheet of ASPEED SOCs, one MAC controller owns 128KB of register space for AST2500. However, one MAC controller only owns 64KB of register space for AST2600 and AST2700. It set the memory region size 128KB and it occupied another controllers Address Spaces. Currently, the ftgmac100 model use 0x100 register space. To support DMA 64 bits dram address and new future mode(ftgmac100_high) which have "Normal Priority Transmit Ring Base Address Register High(0x17C)", "High Priority Transmit Ring Base Address Register High(0x184)" and "Receive Ring Base Address Register High(0x18C)" to save the high part physical address of descriptor manager. Update memory region size to 0x200. Signed-off-by: Jamin Lin --- hw/net/ftgmac100.c | 2 +- include/hw/net/ftgmac100.h | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c index 25e4c0cd5b..4e88430b2f 100644 --- a/hw/net/ftgmac100.c +++ b/hw/net/ftgmac100.c @@ -1108,7 +1108,7 @@ static void ftgmac100_realize(DeviceState *dev, Error **errp) } memory_region_init_io(&s->iomem, OBJECT(dev), &ftgmac100_ops, s, - TYPE_FTGMAC100, 0x2000); + TYPE_FTGMAC100, FTGMAC100_NR_REGS); sysbus_init_mmio(sbd, &s->iomem); sysbus_init_irq(sbd, &s->irq); qemu_macaddr_default_if_unset(&s->conf.macaddr); diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h index 765d1538a4..5a970676da 100644 --- a/include/hw/net/ftgmac100.h +++ b/include/hw/net/ftgmac100.h @@ -14,6 +14,8 @@ #define TYPE_FTGMAC100 "ftgmac100" OBJECT_DECLARE_SIMPLE_TYPE(FTGMAC100State, FTGMAC100) +#define FTGMAC100_NR_REGS 0x200 + #include "hw/sysbus.h" #include "net/net.h" -- 2.34.1
[PATCH v2 4/5] hw/block: m25p80: support quad mode for w25q01jvq
According to the w25q01jv datasheet at page 16,
it is required to set QE bit in "Status Register 2".
Besides, users are able to utilize "Write Status Register 1(0x01)"
command to set QE bit in "Status Register 2" and
utilize "Read Status Register 2(0x35)" command to get the QE bit status.
To support quad mode for w25q01jvq, update collecting data needed
2 bytes for WRSR command in decode_new_cmd function and
verify QE bit at the second byte of collecting data bit 2
in complete_collecting_data.
Update RDCR_EQIO command to set bit 2 of return data
if quad mode enable in decode_new_cmd.
Signed-off-by: Troy Lee
Signed-off-by: Jamin Lin
---
hw/block/m25p80.c | 16
1 file changed, 16 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8dec134832..9e99107b42 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -416,6 +416,7 @@ typedef enum {
/*
* Micron: 0x35 - enable QPI
* Spansion: 0x35 - read control register
+ * Winbond: 0x35 - quad enable
*/
RDCR_EQIO = 0x35,
RSTQIO = 0xf5,
@@ -798,6 +799,11 @@ static void complete_collecting_data(Flash *s)
s->four_bytes_address_mode = extract32(s->data[1], 5, 1);
}
break;
+case MAN_WINBOND:
+if (s->len > 1) {
+s->quad_enable = !!(s->data[1] & 0x02);
+}
+break;
default:
break;
}
@@ -1254,6 +1260,10 @@ static void decode_new_cmd(Flash *s, uint32_t value)
s->needed_bytes = 2;
s->state = STATE_COLLECTING_VAR_LEN_DATA;
break;
+case MAN_WINBOND:
+s->needed_bytes = 2;
+s->state = STATE_COLLECTING_VAR_LEN_DATA;
+break;
default:
s->needed_bytes = 1;
s->state = STATE_COLLECTING_DATA;
@@ -1431,6 +1441,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case MAN_MACRONIX:
s->quad_enable = true;
break;
+case MAN_WINBOND:
+s->data[0] = (!!s->quad_enable) << 1;
+s->pos = 0;
+s->len = 1;
+s->state = STATE_READING_DATA;
+break;
default:
break;
}
--
2.34.1
[RFC PATCH v4 0/2] Support RISC-V CSR read/write in Qtest environment
These patches add functionality for unit testing RISC-V-specific registers. The first patch adds a Qtest backend, and the second implements a simple test. --- v4: - Change wrapper to direct call --- Ivan Klokov (2): target/riscv: Add RISC-V CSR qtest support tests/qtest: QTest example for RISC-V CSR register target/riscv/cpu.c | 17 +++ target/riscv/cpu.h | 3 ++ target/riscv/csr.c | 53 +- tests/qtest/libqtest.c | 27 +++ tests/qtest/libqtest.h | 14 ++ tests/qtest/meson.build | 2 + tests/qtest/riscv-csr-test.c | 86 7 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 tests/qtest/riscv-csr-test.c -- 2.34.1
[RFC PATCH v4 1/2] target/riscv: Add RISC-V CSR qtest support
The RISC-V architecture supports the creation of custom
CSR-mapped devices. It would be convenient to test them in the same way
as MMIO-mapped devices. To do this, a new call has been added
to read/write CSR registers.
Signed-off-by: Ivan Klokov
---
target/riscv/cpu.c | 17 ++
target/riscv/cpu.h | 3 +++
target/riscv/csr.c | 53 +-
tests/qtest/libqtest.c | 27 +
tests/qtest/libqtest.h | 14 +++
5 files changed, 113 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 69a08e8c2c..7c15860414 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1149,6 +1149,18 @@ void riscv_cpu_finalize_features(RISCVCPU *cpu, Error
**errp)
}
}
+#ifndef CONFIG_USER_ONLY
+static void riscv_cpu_register_csr_qtest_callback(void)
+{
+static gsize reinit_done;
+if (g_once_init_enter(&reinit_done)) {
+qtest_set_command_cb(csr_qtest_callback);
+
+g_once_init_leave(&reinit_done, 1);
+}
+}
+#endif
+
static void riscv_cpu_realize(DeviceState *dev, Error **errp)
{
CPUState *cs = CPU(dev);
@@ -1175,6 +1187,11 @@ static void riscv_cpu_realize(DeviceState *dev, Error
**errp)
riscv_cpu_register_gdb_regs_for_features(cs);
+#ifndef CONFIG_USER_ONLY
+/* register callback for csr qtests */
+riscv_cpu_register_csr_qtest_callback();
+#endif
+
#ifndef CONFIG_USER_ONLY
if (cpu->cfg.debug) {
riscv_trigger_realize(&cpu->env);
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6fe0d712b4..6d4bbec53c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -32,6 +32,8 @@
#include "cpu_cfg.h"
#include "qapi/qapi-types-common.h"
#include "cpu-qom.h"
+#include "qemu/cutils.h"
+#include "sysemu/qtest.h"
typedef struct CPUArchState CPURISCVState;
@@ -813,6 +815,7 @@ bool riscv_cpu_accelerator_compatible(RISCVCPU *cpu);
/* CSR function table */
extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
+bool csr_qtest_callback(CharBackend *chr, gchar **words);
extern const bool valid_vm_1_10_32[], valid_vm_1_10_64[];
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 58ef7079dc..f4f5128c9c 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -29,7 +29,7 @@
#include "sysemu/cpu-timers.h"
#include "qemu/guest-random.h"
#include "qapi/error.h"
-
+#include "tests/qtest/libqtest.h"
/* CSR function table public API */
void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops)
@@ -4549,6 +4549,57 @@ static RISCVException write_jvt(CPURISCVState *env, int
csrno,
return RISCV_EXCP_NONE;
}
+#if !defined(CONFIG_USER_ONLY)
+static uint64_t csr_call(char *cmd, uint64_t cpu_num, int csrno,
+uint64_t *val)
+{
+RISCVCPU *cpu = RISCV_CPU(cpu_by_arch_id(cpu_num));
+CPURISCVState *env = &cpu->env;
+
+int ret = RISCV_EXCP_NONE;
+if (strcmp(cmd, "get_csr") == 0) {
+ret = riscv_csrrw(env, csrno, (target_ulong *)val, 0, 0);
+
+} else if (strcmp(cmd, "set_csr") == 0) {
+ret = riscv_csrrw(env, csrno, NULL, *(target_ulong *)val,
MAKE_64BIT_MASK(0, TARGET_LONG_BITS));
+}
+
+if (ret == RISCV_EXCP_NONE) {
+ret = 0;
+} else {
+g_assert_not_reached();
+}
+
+return ret;
+}
+
+bool csr_qtest_callback(CharBackend *chr, gchar **words)
+{
+if (strcmp(words[0], "csr") == 0) {
+
+uint64_t res, cpu;
+
+uint64_t val;
+int rc, csr;
+
+rc = qemu_strtou64(words[2], NULL, 0, &cpu);
+g_assert(rc == 0);
+rc = qemu_strtoi(words[3], NULL, 0, &csr);
+g_assert(rc == 0);
+rc = qemu_strtou64(words[4], NULL, 0, &val);
+g_assert(rc == 0);
+res = csr_call(words[1], cpu, csr, &val);
+
+qtest_send_prefix(chr);
+qtest_sendf(chr, "OK %"PRIx64" "TARGET_FMT_lx"\n", res,
(target_ulong)val);
+
+return true;
+}
+
+return false;
+}
+#endif
+
/*
* Control and Status Register function table
* riscv_csr_operations::predicate() must be provided for an implemented CSR
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index c7f6897d78..f8c3ff15a9 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1205,6 +1205,33 @@ uint64_t qtest_rtas_call(QTestState *s, const char *name,
return 0;
}
+static void qtest_rsp_csr(QTestState *s, uint64_t *val)
+{
+gchar **args;
+uint64_t ret;
+int rc;
+
+args = qtest_rsp_args(s, 3);
+
+rc = qemu_strtou64(args[1], NULL, 16, &ret);
+g_assert(rc == 0);
+rc = qemu_strtou64(args[2], NULL, 16, val);
+g_assert(rc == 0);
+
+g_strfreev(args);
+}
+
+uint64_t qtest_csr_call(QTestState *s, const char *name,
+ uint64_t cpu, int csr,
+ uint64_t *val)
+{
+qtest_sendf(s, "csr %s 0x%"PRIx64" %d 0x%"PRIx64"\n",
+name, cpu, csr, *val);
+
+qtest_rsp_csr(s, val);
+return 0;
+}
+
[RFC PATCH v4 2/2] tests/qtest: QTest example for RISC-V CSR register
Added demo for reading CSR register from qtest environment.
Signed-off-by: Ivan Klokov
---
tests/qtest/meson.build | 2 +
tests/qtest/riscv-csr-test.c | 86
2 files changed, 88 insertions(+)
create mode 100644 tests/qtest/riscv-csr-test.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 12792948ff..45d651da99 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -259,6 +259,8 @@ qtests_s390x = \
qtests_riscv32 = \
(config_all_devices.has_key('CONFIG_SIFIVE_E_AON') ?
['sifive-e-aon-watchdog-test'] : [])
+qtests_riscv32 += ['riscv-csr-test']
+
qos_test_ss = ss.source_set()
qos_test_ss.add(
'ac97-test.c',
diff --git a/tests/qtest/riscv-csr-test.c b/tests/qtest/riscv-csr-test.c
new file mode 100644
index 00..e9af9ca724
--- /dev/null
+++ b/tests/qtest/riscv-csr-test.c
@@ -0,0 +1,86 @@
+/*
+ * QTest testcase for RISC-V CSRs
+ *
+ * Copyright (c) 2024 Syntacore.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+#include "libqtest.h"
+
+static uint64_t qcsr_call(QTestState *qts, const char *name, uint64_t cpu,
+ int csrno, uint64_t *val)
+{
+uint64_t res = 0;
+
+res = qtest_csr_call(qts, name, cpu, csrno, val);
+
+return res;
+}
+
+static int qcsr_get_csr(QTestState *qts, uint64_t cpu,
+int csrno, uint64_t *val)
+{
+int res;
+
+res = qcsr_call(qts, "get_csr", cpu, csrno, val);
+
+return res;
+}
+
+static int qcsr_set_csr(QTestState *qts, uint64_t cpu,
+int csrno, uint64_t *val)
+{
+int res;
+
+res = qcsr_call(qts, "set_csr", cpu, csrno, val);
+
+return res;
+}
+
+static void run_test_csr(void)
+{
+
+uint64_t res;
+uint64_t val = 0;
+
+res = qcsr_call(global_qtest, "get_csr", 0, 0xf11, &val);
+
+g_assert_cmpint(res, ==, 0);
+g_assert_cmpint(val, ==, 0x100);
+
+val = 0xff;
+res = qcsr_call(global_qtest, "set_csr", 0, 0x342, &val);
+
+g_assert_cmpint(res, ==, 0);
+
+val = 0;
+res = qcsr_call(global_qtest, "get_csr", 0, 0x342, &val);
+
+g_assert_cmpint(res, ==, 0);
+g_assert_cmpint(val, ==, 0xff);
+
+qtest_quit(global_qtest);
+}
+
+int main(int argc, char **argv)
+{
+g_test_init(&argc, &argv, NULL);
+
+qtest_add_func("/cpu/csr", run_test_csr);
+
+qtest_start("-machine virt -cpu any,mvendorid=0x100");
+
+return g_test_run();
+
+}
--
2.34.1
Re: [PULL 3/5] hw/core: allow parameter=1 for SMP topology on any machine
Hi Michael,
This patch fixes a regression that was introduced in QEMU 9.0,
reported by yet another user at:
https://gitlab.com/qemu-project/qemu/-/issues/2420
Could you pull this patch into stable-9.0. If you think testing
is important for stable, the following patch adds further unit
testing coverage too.
Daniel
On Fri, May 17, 2024 at 05:02:25PM +0200, Philippe Mathieu-Daudé wrote:
> From: Daniel P. Berrangé
>
> This effectively reverts
>
> commit 54c4ea8f3ae614054079395842128a856a73dbf9
> Author: Zhao Liu
> Date: Sat Mar 9 00:01:37 2024 +0800
>
> hw/core/machine-smp: Deprecate unsupported "parameter=1" SMP
> configurations
>
> but is not done as a 'git revert' since the part of the changes to the
> file hw/core/machine-smp.c which add 'has_XXX' checks remain desirable.
> Furthermore, we have to tweak the subsequently added unit test to
> account for differing warning message.
>
> The rationale for the original deprecation was:
>
> "Currently, it was allowed for users to specify the unsupported
>topology parameter as "1". For example, x86 PC machine doesn't
>support drawer/book/cluster topology levels, but user could specify
>"-smp drawers=1,books=1,clusters=1".
>
>This is meaningless and confusing, so that the support for this kind
>of configurations is marked deprecated since 9.0."
>
> There are varying POVs on the topic of 'unsupported' topology levels.
>
> It is common to say that on a system without hyperthreading, that there
> is always 1 thread. Likewise when new CPUs introduced a concept of
> multiple "dies', it was reasonable to say that all historical CPUs
> before that implicitly had 1 'die'. Likewise for the more recently
> introduced 'modules' and 'clusters' parameter'. From this POV, it is
> valid to set 'parameter=1' on the -smp command line for any machine,
> only a value > 1 is strictly an error condition.
>
> It doesn't cause any functional difficulty for QEMU, because internally
> the QEMU code is itself assuming that all "unsupported" parameters
> implicitly have a value of '1'.
>
> At the libvirt level, we've allowed applications to set 'parameter=1'
> when configuring a guest, and pass that through to QEMU.
>
> Deprecating this creates extra difficulty for because there's no info
> exposed from QEMU about which machine types "support" which parameters.
> Thus, libvirt can't know whether it is valid to pass 'parameter=1' for
> a given machine type, or whether it will trigger deprecation messages.
>
> Since there's no apparent functional benefit to deleting this deprecated
> behaviour from QEMU, and it creates problems for consumers of QEMU,
> remove this deprecation.
>
> Signed-off-by: Daniel P. Berrangé
> Reviewed-by: Zhao Liu
> Reviewed-by: Ján Tomko
> Message-ID: <20240513123358.612355-2-berra...@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/core/machine-smp.c | 84 -
> tests/unit/test-smp-parse.c | 8 ++--
> 2 files changed, 31 insertions(+), 61 deletions(-)
>
> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> index 2b93fa99c9..5d8d7edcbd 100644
> --- a/hw/core/machine-smp.c
> +++ b/hw/core/machine-smp.c
> @@ -118,76 +118,46 @@ void machine_parse_smp_config(MachineState *ms,
> }
>
> /*
> - * If not supported by the machine, a topology parameter must be
> - * omitted.
> + * If not supported by the machine, a topology parameter must
> + * not be set to a value greater than 1.
> */
> -if (!mc->smp_props.modules_supported && config->has_modules) {
> -if (config->modules > 1) {
> -error_setg(errp, "modules not supported by this "
> - "machine's CPU topology");
> -return;
> -} else {
> -/* Here modules only equals 1 since we've checked zero case. */
> -warn_report("Deprecated CPU topology (considered invalid): "
> -"Unsupported modules parameter mustn't be "
> -"specified as 1");
> -}
> +if (!mc->smp_props.modules_supported &&
> +config->has_modules && config->modules > 1) {
> +error_setg(errp,
> + "modules > 1 not supported by this machine's CPU
> topology");
> +return;
> }
> modules = modules > 0 ? modules : 1;
>
> -if (!mc->smp_props.clusters_supported && config->has_clusters) {
> -if (config->clusters > 1) {
> -error_setg(errp, "clusters not supported by this "
> - "machine's CPU topology");
> -return;
> -} else {
> -/* Here clusters only equals 1 since we've checked zero case. */
> -warn_report("Deprecated CPU topology (considered invalid): "
> -"Unsupported clusters parameter mustn't be "
> -"specified as 1");
> -}
> +if (!mc->smp_props.clusters_supported &&
> +
Re: [PATCH v2 05/22] qga: move linux disk/cpu stats command impls to commands-linux.c
On 13/6/24 17:01, Daniel P. Berrangé wrote:
The qmp_guest_{diskstats,cpustats} command impls in
commands-posix.c are surrounded by '#ifdef __linux__' so should
instead live in commands-linux.c
This also removes a "#ifdef CONFIG_LINUX" that was nested inside
a "#ifdef __linux__".
Signed-off-by: Daniel P. Berrangé
---
qga/commands-linux.c | 195 ++
qga/commands-posix.c | 199 ---
2 files changed, 195 insertions(+), 199 deletions(-)
Easy to review using 'git-diff --color-moved=dimmed-zebra'.
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 06/22] qga: move linux memory block command impls to commands-linux.c
On 13/6/24 17:43, Daniel P. Berrangé wrote:
The qmp_guest_{set,get}_{memory_blocks,block_info} command impls in
commands-posix.c are surrounded by '#ifdef __linux__' so should
instead live in commands-linux.c
This also removes a "#ifdef CONFIG_LINUX" that was nested inside
a "#ifdef __linux__".
Signed-off-by: Daniel P. Berrangé
---
qga/commands-linux.c | 308 ++
qga/commands-posix.c | 311 +--
2 files changed, 309 insertions(+), 310 deletions(-)
Reviewed using 'git-diff --color-moved=dimmed-zebra'.
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH ats_vtd v5 04/22] intel_iommu: do not consider wait_desc as an invalid descriptor
On 03/07/2024 09:29, Yi Liu wrote:
On 2024/7/2 23:29, CLEMENT MATHIEU--DRIF wrote:
On 02/07/2024 15:33, Yi Liu 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.
On 2024/7/2 13:52, CLEMENT MATHIEU--DRIF wrote:
From: Clément Mathieu--Drif
Signed-off-by: Clément Mathieu--Drif
Reviewed-by: Zhenzhong Duan
---
hw/i386/intel_iommu.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 98996ededc..71cebe2fd3 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -3500,6 +3500,11 @@ static bool
vtd_process_wait_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
} else if (inv_desc->lo & VTD_INV_DESC_WAIT_IF) {
/* Interrupt flag */
vtd_generate_completion_event(s);
+ } else if (inv_desc->lo & VTD_INV_DESC_WAIT_FN) {
+ /*
+ * SW = 0, IF = 0, FN = 1
+ * Nothing to do as we process the events sequentially
+ */
This code looks a bit weird. SW field does not co-exist with IF. But
either
SW or IF can co-exist with FN flag. Is it? Have you already seen a wait
descriptor that only has FN flag set but no SW nor IF flag?
Yes, my test suite triggers that condition
I see. Spec indeed has such usage. Please add a comment for it.
Since it does not need a response, so QEMU can just bypass it. Also
please adjust the subject a bit. It's misleading. Perhaps
"intel_iommu: Bypass barrier wait descriptor"
Fine, will do
Spec CH 7.10
a. Submit Invalidation Wait Descriptor (inv_wait_dsc) with Fence flag
(FN=1) Set to Invalidation
Queue. This ensures that all requests submitted to the Invalidation
Queue ahead of this wait
descriptor are processed and completed by remapping hardware before
processing requests
after the Invalidation Wait Descriptor. It is not required to specify
SW flag (or IF flag) in this
descriptor or for software to wait on its completion, as its function
is to only act as a barrier.
} else {
error_report_once("%s: invalid wait desc: hi=%"PRIx64",
lo=%"PRIx64
" (unknown type)", __func__,
inv_desc->hi,
--
Regards,
Yi Liu
Re: [PATCH v2 08/22] qga: conditionalize schema for commands unsupported on Windows
On 13/6/24 17:43, Daniel P. Berrangé wrote:
Rather than creating stubs for every command that just return
QERR_UNSUPPORTED, use 'if' conditions in the QAPI schema to
fully exclude generation of the commands on Windows.
The command will be rejected at QMP dispatch time instead,
avoiding reimplementing rejection by blocking the stub commands.
This changes the error message for affected commands from
{"class": "CommandNotFound", "desc": "Command FOO has been disabled"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
This also fixes an accidental inconsistency where some commands
(guest-get-diskstats & guest-get-cpustats) are implemented as
stubs, yet not added to the blockedrpc list. Those change their
error message from
{"class": "GenericError, "desc": "this feature or command is not currently
supported"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
The final additional benefit is that the QGA protocol reference
now documents what conditions enable use of the command.
Signed-off-by: Daniel P. Berrangé
---
qga/commands-posix.c | 2 +-
qga/commands-win32.c | 56 +---
qga/qapi-schema.json | 45 +++
3 files changed, 32 insertions(+), 71 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 09/22] qga: conditionalize schema for commands unsupported on non-Linux POSIX
On 13/6/24 17:43, Daniel P. Berrangé wrote:
Rather than creating stubs for every command that just return
QERR_UNSUPPORTED, use 'if' conditions in the QAPI schema to
fully exclude generation of the commands on non-Linux POSIX
platforms
The command will be rejected at QMP dispatch time instead,
avoiding reimplementing rejection by blocking the stub commands.
This changes the error message for affected commands from
{"class": "CommandNotFound", "desc": "Command FOO has been disabled"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
This has the additional benefit that the QGA protocol reference
now documents what conditions enable use of the command.
Signed-off-by: Daniel P. Berrangé
---
qga/commands-posix.c | 66
qga/qapi-schema.json | 30 +++-
2 files changed, 17 insertions(+), 79 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 10/22] qga: conditionalize schema for commands requiring getifaddrs
On 13/6/24 17:43, Daniel P. Berrangé wrote:
Rather than creating stubs for every comamnd that just return
QERR_UNSUPPORTED, use 'if' conditions in the QAPI schema to
fully exclude generation of the network interface command on
POSIX platforms lacking getifaddrs().
The command will be rejected at QMP dispatch time instead,
avoiding reimplementing rejection by blocking the stub commands.
This changes the error message for affected commands from
{"class": "CommandNotFound", "desc": "Command FOO has been disabled"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
This has the additional benefit that the QGA protocol reference
now documents what conditions enable use of the command.
Signed-off-by: Daniel P. Berrangé
---
qga/commands-posix.c | 13 -
qga/qapi-schema.json | 15 ++-
2 files changed, 10 insertions(+), 18 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 1/1] memory tier: consolidate the initialization of memory tiers
Hi Jonathan,
I appreciate your feedback and valuable suggestions. Replies inlined.
July 2, 2024 at 6:25 AM, "Jonathan Cameron" wrote:
>
> On Fri, 28 Jun 2024 06:09:23 +
>
> "Ho-Ren (Jack) Chuang" wrote:
>
> >
> > If we simply move the set_node_memory_tier() from memory_tier_init()
> >
> > to late_initcall(), it will result in HMAT not registering
> >
> > the mt_adistance_algorithm callback function, because
> >
> > set_node_memory_tier() is not performed during the memory tiering
> >
> > initialization phase, leading to a lack of correct default_dram
> >
> > information.
> >
> >
> >
> > Therefore, we introduced a nodemask to pass the information of the
> >
> > default DRAM nodes. The reason for not choosing to reuse
> >
> > default_dram_type->nodes is that it is not clean enough. So in the end,
> >
> > we use a __initdata variable, which is a variable that is released once
> >
> > initialization is complete, including both CPU and memory nodes for HMAT
> >
> > to iterate through.
> >
> >
> >
> > Besides, since default_dram_type may be checked/used during the
> >
> > initialization process of HMAT and drivers, it is better to keep the
> >
> > allocation of default_dram_type in memory_tier_init().
> >
> >
> >
> > Signed-off-by: Ho-Ren (Jack) Chuang
> >
> > Suggested-by: Jonathan Cameron
> >
> > ---
> >
> > drivers/acpi/numa/hmat.c | 5 +--
> >
> > include/linux/memory-tiers.h | 2 ++
> >
> > mm/memory-tiers.c | 59 +++-
> >
> > 3 files changed, 28 insertions(+), 38 deletions(-)
> >
> >
> >
> > diff --git a/drivers/acpi/numa/hmat.c b/drivers/acpi/numa/hmat.c
> >
> > index 2c8ccc91ebe6..a2f9e7a4b479 100644
> >
> > --- a/drivers/acpi/numa/hmat.c
> >
> > +++ b/drivers/acpi/numa/hmat.c
> >
> > @@ -940,10 +940,7 @@ static int hmat_set_default_dram_perf(void)
> >
> > struct memory_target *target;
> >
> > struct access_coordinate *attrs;
> >
> >
> >
> > - if (!default_dram_type)
> >
> > - return -EIO;
> >
> > -
> >
> > - for_each_node_mask(nid, default_dram_type->nodes) {
> >
> > + for_each_node_mask(nid, default_dram_nodes) {
> >
>
> As below. Do we care if the combination of RAM + CPU wasn't true
>
> earlier and is true by this point? If not, why not just
>
> compute the node mask in here and not store it.
>
It makes sense to me. I think we can move the computation to here
and remove the global node mask.
> >
> > pxm = node_to_pxm(nid);
> >
> > target = find_mem_target(pxm);
> >
> > if (!target)
> >
> > diff --git a/include/linux/memory-tiers.h b/include/linux/memory-tiers.h
> >
> > index 0d70788558f4..fa61ad9c4d75 100644
> >
> > --- a/include/linux/memory-tiers.h
> >
> > +++ b/include/linux/memory-tiers.h
> >
> > @@ -38,6 +38,7 @@ struct access_coordinate;
> >
> > #ifdef CONFIG_NUMA
> >
> > extern bool numa_demotion_enabled;
> >
> > extern struct memory_dev_type *default_dram_type;
> >
> > +extern nodemask_t default_dram_nodes __initdata;
> >
> > struct memory_dev_type *alloc_memory_type(int adistance);
> >
> > void put_memory_type(struct memory_dev_type *memtype);
> >
> > void init_node_memory_type(int node, struct memory_dev_type *default_type);
> >
> > @@ -76,6 +77,7 @@ static inline bool node_is_toptier(int node)
> >
> >
> >
> > #define numa_demotion_enabled false
> >
> > #define default_dram_type NULL
> >
> > +#define default_dram_nodes NODE_MASK_NONE
> >
> > /*
> >
> > * CONFIG_NUMA implementation returns non NULL error.
> >
> > */
> >
> > diff --git a/mm/memory-tiers.c b/mm/memory-tiers.c
> >
> > index 6632102bd5c9..a19a90c3ad36 100644
> >
> > --- a/mm/memory-tiers.c
> >
> > +++ b/mm/memory-tiers.c
> >
> > @@ -43,6 +43,7 @@ static LIST_HEAD(memory_tiers);
> >
> > static LIST_HEAD(default_memory_types);
> >
> > static struct node_memory_type_map node_memory_types[MAX_NUMNODES];
> >
> > struct memory_dev_type *default_dram_type;
> >
> > +nodemask_t default_dram_nodes __initdata = NODE_MASK_NONE;
> >
> >
> >
> > static const struct bus_type memory_tier_subsys = {
> >
> > .name = "memory_tiering",
> >
> > @@ -671,28 +672,38 @@ EXPORT_SYMBOL_GPL(mt_put_memory_types);
> >
> >
> >
> > /*
> >
> > * This is invoked via `late_initcall()` to initialize memory tiers for
> >
> > - * CPU-less memory nodes after driver initialization, which is
> >
> > - * expected to provide `adistance` algorithms.
> >
> > + * memory nodes, both with and without CPUs. After the initialization of
> >
> > + * firmware and devices, adistance algorithms are expected to be provided.
> >
> > */
> >
> > static int __init memory_tier_late_init(void)
> >
> > {
> >
> > int nid;
> >
> > + struct memory_tier *memtier;
> >
> >
> >
> > + get_online_mems();
> >
> > guard(mutex)(&memory_tier_lock);
> >
> > + /*
> >
> > + * Look at all the existing and uninitialized N_MEMORY nodes and
> >
> > + * add them to default
Re: [PATCH v2 12/22] qga: conditionalize schema for commands only supported on Windows
On 13/6/24 17:43, Daniel P. Berrangé wrote:
Rather than creating stubs for every command that just return
QERR_UNSUPPORTED, use 'if' conditions in the QAPI schema to
fully exclude generation of the commands on non-Windows.
The command will be rejected at QMP dispatch time instead,
avoiding reimplementing rejection by blocking the stub commands.
This changes the error message for affected commands from
{"class": "CommandNotFound", "desc": "Command FOO has been disabled"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
This has the additional benefit that the QGA protocol reference
now documents what conditions enable use of the command.
Signed-off-by: Daniel P. Berrangé
---
qga/commands-posix.c | 9 -
qga/qapi-schema.json | 15 ++-
2 files changed, 10 insertions(+), 14 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 14/22] qga: conditionalize schema for commands requiring fstrim
On 13/6/24 17:43, Daniel P. Berrangé wrote:
Rather than creating stubs for every command that just return
QERR_UNSUPPORTED, use 'if' conditions in the QAPI schema to
fully exclude generation of the filesystem trimming commands
on POSIX platforms lacking required APIs.
The command will be rejected at QMP dispatch time instead,
avoiding reimplementing rejection by blocking the stub commands.
This changes the error message for affected commands from
{"class": "CommandNotFound", "desc": "Command FOO has been disabled"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
This has the additional benefit that the QGA protocol reference
now documents what conditions enable use of the command.
Signed-off-by: Daniel P. Berrangé
---
qga/commands-posix.c | 13 -
qga/qapi-schema.json | 9 ++---
2 files changed, 6 insertions(+), 16 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 13/22] qga: conditionalize schema for commands requiring fsfreeze
On 13/6/24 17:43, Daniel P. Berrangé wrote:
Rather than creating stubs for every command that just return
QERR_UNSUPPORTED, use 'if' conditions in the schema to fully
exclude generation of the filesystem freezing commands on POSIX
platforms lacking the required APIs.
The command will be rejected at QMP dispatch time instead,
avoiding reimplementing rejection by blocking the stub commands.
This changes the error message for affected commands from
{"class": "CommandNotFound", "desc": "Command FOO has been disabled"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
This has the additional benefit that the QGA protocol reference
now documents what conditions enable use of the command.
Signed-off-by: Daniel P. Berrangé
---
qga/commands-posix.c | 47
qga/qapi-schema.json | 15 +-
2 files changed, 10 insertions(+), 52 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 15/22] qga: conditionalize schema for commands requiring libudev
On 13/6/24 17:43, Daniel P. Berrangé wrote:
Rather than creating stubs for every command that just return
QERR_UNSUPPORTED, use 'if' conditions in the schema to fully
exclude generation of the filesystem trimming commands on POSIX
platforms lacking required APIs.
The command will be rejected at QMP dispatch time instead,
avoiding reimplementing rejection by blocking the stub commands.
This changes the error message for affected commands from
{"class": "CommandNotFound", "desc": "Command FOO has been disabled"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
This has the additional benefit that the QGA protocol reference
now documents what conditions enable use of the command.
Signed-off-by: Daniel P. Berrangé
---
qga/commands-linux.c | 8
qga/qapi-schema.json | 8
2 files changed, 4 insertions(+), 12 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 16/22] qga: conditionalize schema for commands requiring utmpx
On 13/6/24 17:44, Daniel P. Berrangé wrote:
Rather than creating stubs for every command that just return
QERR_UNSUPPORTED, use 'if' conditions in the QAPI schema to
fully exclude generation of the get-users command on POSIX
platforms lacking required APIs.
The command will be rejected at QMP dispatch time instead,
avoiding reimplementing rejection by blocking the stub commands.
This changes the error message for affected commands from
{"class": "CommandNotFound", "desc": "Command FOO has been disabled"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
This has the additional benefit that the QGA protocol reference
now documents what conditions enable use of the command.
Signed-off-by: Daniel P. Berrangé
---
qga/commands-posix.c | 10 +-
qga/qapi-schema.json | 6 --
2 files changed, 5 insertions(+), 11 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PULL 3/5] hw/core: allow parameter=1 for SMP topology on any machine
03.07.2024 11:21, Daniel P. Berrangé wrote: Hi Michael, This patch fixes a regression that was introduced in QEMU 9.0, reported by yet another user at: https://gitlab.com/qemu-project/qemu/-/issues/2420 Aha. Could you pull this patch into stable-9.0. If you think testing is important for stable, the following patch adds further unit testing coverage too. Sure. I think I considered applying it, but for some reason (which I don't recall anymore) rejected that thought. This change sort of clashes with 8ec0a46347987c7 "hw/core/machine: Support modules in -smp" though, -- I'm fixing the context. Yes, I'll pick up the test as well. Thanks, /mjt -- GPG Key transition (from rsa2048 to rsa4096) since 2024-04-24. New key: rsa4096/61AD3D98ECDF2C8E 9D8B E14E 3F2A 9DD7 9199 28F1 61AD 3D98 ECDF 2C8E Old key: rsa2048/457CE0A0804465C5 6EE1 95D1 886E 8FFB 810D 4324 457C E0A0 8044 65C5 Transition statement: http://www.corpit.ru/mjt/gpg-transition-2024.txt
Re: [PATCH v2 17/22] qga: conditionalize schema for commands not supported on other UNIX
On 13/6/24 17:44, Daniel P. Berrangé wrote:
Rather than creating stubs for every command that just return
QERR_UNSUPPORTED, use 'if' conditions in the QAPI schema to
fully exclude generation of the commands on other UNIX.
The command will be rejected at QMP dispatch time instead,
avoiding reimplementing rejection by blocking the stub commands.
This changes the error message for affected commands from
{"class": "CommandNotFound", "desc": "Command FOO has been disabled"}
to
{"class": "CommandNotFound", "desc": "The command FOO has not been found"}
This has the additional benefit that the QGA protocol reference
now documents what conditions enable use of the command.
Signed-off-by: Daniel P. Berrangé
---
meson.build | 1 +
qga/commands-posix.c | 8
qga/qapi-schema.json | 3 ++-
3 files changed, 3 insertions(+), 9 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 19/22] qga: move declare of QGAConfig struct to top of file
On 13/6/24 17:44, Daniel P. Berrangé wrote:
It is referenced by QGAState already, and it is clearer to declare all
data types at the top of the file, rather than have them mixed with
code later.
Signed-off-by: Daniel P. Berrangé
---
qga/main.c | 44 ++--
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/qga/main.c b/qga/main.c
index 17b6ce18ac..647d27037c 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -70,6 +70,28 @@ typedef struct GAPersistentState {
typedef struct GAConfig GAConfig;
Matter of style, personally I'd squash within the
typedef.
+struct GAConfig {
+char *channel_path;
+char *method;
+char *log_filepath;
+char *pid_filepath;
+#ifdef CONFIG_FSFREEZE
+char *fsfreeze_hook;
+#endif
+char *state_dir;
+#ifdef _WIN32
+const char *service;
+#endif
+gchar *bliststr; /* blockedrpcs may point to this string */
+gchar *aliststr; /* allowedrpcs may point to this string */
+GList *blockedrpcs;
+GList *allowedrpcs;
+int daemonize;
+GLogLevelFlags log_level;
+int dumpconf;
+bool retry_path;
+};
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 20/22] qga: remove pointless 'blockrpcs_key' variable
On 13/6/24 17:44, Daniel P. Berrangé wrote: This variable was used to support back compat for the old config file key name, and became redundant after the following change: commit a7a2d636ae4549ef0551134d4bf8e084a14431c4 Author: Philippe Mathieu-Daudé Date: Thu May 30 08:36:43 2024 +0200 qga: Remove deprecated 'blacklist' argument / config key Signed-off-by: Daniel P. Berrangé --- qga/main.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 21/22] qga: allow configuration file path via the cli
On 13/6/24 17:44, Daniel P. Berrangé wrote:
Allowing the user to set the QGA_CONF environment variable to change
the default configuration file path is very unusual practice, made
more obscure since this ability is not documented.
This introduces the more normal '-c PATH' / '--config=PATH' command
line argument approach. This requires that we parse the comamnd line
twice, since we want the command line arguments to take priority over
the configuration file settings in general.
Signed-off-by: Daniel P. Berrangé
---
docs/interop/qemu-ga.rst | 5 +
qga/main.c | 35 +++
2 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/docs/interop/qemu-ga.rst b/docs/interop/qemu-ga.rst
index 72fb75a6f5..e42b370319 100644
--- a/docs/interop/qemu-ga.rst
+++ b/docs/interop/qemu-ga.rst
@@ -33,6 +33,11 @@ Options
.. program:: qemu-ga
+.. option:: -c, --config=PATH
+
+ Configuration file path (the default is |CONFDIR|\ ``/qemu-ga.conf``,
+ unless overriden by the QGA_CONF environment variable)
+
.. option:: -m, --method=METHOD
Transport method: one of ``unix-listen``, ``virtio-serial``, or
diff --git a/qga/main.c b/qga/main.c
index 6ff022a85d..f68a32bf7b 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1018,15 +1018,14 @@ static GList *split_list(const gchar *str, const gchar
*delim)
return list;
}
-static void config_load(GAConfig *config)
+static void config_load(GAConfig *config, const char *confpath, bool required)
{
GError *gerr = NULL;
GKeyFile *keyfile;
-g_autofree char *conf = g_strdup(g_getenv("QGA_CONF")) ?:
get_relocated_path(QGA_CONF_DEFAULT);
/* read system config */
keyfile = g_key_file_new();
-if (!g_key_file_load_from_file(keyfile, conf, 0, &gerr)) {
+if (!g_key_file_load_from_file(keyfile, confpath, 0, &gerr)) {
goto end;
}
if (g_key_file_has_key(keyfile, "general", "daemon", NULL)) {
@@ -1092,10 +1091,10 @@ static void config_load(GAConfig *config)
end:
g_key_file_free(keyfile);
-if (gerr &&
-!(gerr->domain == G_FILE_ERROR && gerr->code == G_FILE_ERROR_NOENT)) {
+if (gerr && (required ||
+ !(gerr->domain == G_FILE_ERROR && gerr->code ==
G_FILE_ERROR_NOENT))) {
g_critical("error loading configuration from path: %s, %s",
- conf, gerr->message);
+ confpath, gerr->message);
exit(EXIT_FAILURE);
}
g_clear_error(&gerr);
@@ -1167,12 +1166,13 @@ static void config_dump(GAConfig *config)
static void config_parse(GAConfig *config, int argc, char **argv)
{
-const char *sopt = "hVvdm:p:l:f:F::b:a:s:t:Dr";
+const char *sopt = "hVvdc:m:p:l:f:F::b:a:s:t:Dr";
int opt_ind = 0, ch;
bool block_rpcs = false, allow_rpcs = false;
const struct option lopt[] = {
{ "help", 0, NULL, 'h' },
{ "version", 0, NULL, 'V' },
+{ "config", 1, NULL, 'c' },
{ "dump-conf", 0, NULL, 'D' },
{ "logfile", 1, NULL, 'l' },
{ "pidfile", 1, NULL, 'f' },
@@ -1192,6 +1192,26 @@ static void config_parse(GAConfig *config, int argc,
char **argv)
{ "retry-path", 0, NULL, 'r' },
{ NULL, 0, NULL, 0 }
};
+g_autofree char *confpath = g_strdup(g_getenv("QGA_CONF")) ?:
+get_relocated_path(QGA_CONF_DEFAULT);
+bool confrequired = false;
+
+while ((ch = getopt_long(argc, argv, sopt, lopt, NULL)) != -1) {
+switch (ch) {
+case 'c':
+g_free(confpath);
+confpath = g_strdup(optarg);
+confrequired = true;
+break;
+default:
+break;
+}
+}
+
+config_load(config, confpath, confrequired);
+
+/* Reset for second pass */
+optind = 1;
while ((ch = getopt_long(argc, argv, sopt, lopt, &opt_ind)) != -1) {
switch (ch) {
@@ -1582,7 +1602,6 @@ int main(int argc, char **argv)
qga_qmp_init_marshal(&ga_commands);
init_dfl_pathnames();
-config_load(config);
config_parse(config, argc, argv);
if (config->pid_filepath == NULL) {
This looks like a trivial (correct) CLI change, but
I don't feel confident anymore reviewing this area,
so will let others have a look.
Regards,
Phil.
Re: [PATCH v2 02/22] qga: move linux vcpu command impls to commands-linux.c
On 13/6/24 17:01, Daniel P. Berrangé wrote: The qmp_guest_set_vcpus and qmp_guest_get_vcpus command impls in commands-posix.c are surrounded by '#ifdef __linux__' so should instead live in commands-linux.c Reviewed-by: Manos Pitsidianakis Signed-off-by: Daniel P. Berrangé --- qga/commands-linux.c | 141 +++ qga/commands-posix.c | 139 -- 2 files changed, 141 insertions(+), 139 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 03/22] qga: move linux suspend command impls to commands-linux.c
On 13/6/24 17:01, Daniel P. Berrangé wrote:
The qmp_guest_suspend_{disk,ram,hybrid} command impls in
commands-posix.c are surrounded by '#ifdef __linux__' so should
instead live in commands-linux.c
Reviewed-by: Manos Pitsidianakis
Signed-off-by: Daniel P. Berrangé
---
qga/commands-linux.c | 265 +++
qga/commands-posix.c | 265 ---
2 files changed, 265 insertions(+), 265 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 04/22] qga: move linux fs/disk command impls to commands-linux.c
On 13/6/24 17:01, Daniel P. Berrangé wrote:
The qmp_guest_{fstrim, get_fsinfo, get_disks} command impls in
commands-posix.c are surrounded by '#ifdef __linux__' so should
instead live in commands-linux.c
Reviewed-by: Manos Pitsidianakis
Signed-off-by: Daniel P. Berrangé
---
qga/commands-linux.c | 904 ++
qga/commands-posix.c | 909 ---
2 files changed, 904 insertions(+), 909 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 1/3] util/cpuinfo-riscv: Support host/cpuinfo.h for riscv
On 2024/6/28 2:03, Richard Henderson wrote:
Move detection code out of tcg, similar to other hosts.
Signed-off-by: Richard Henderson
Reviewed-by: LIU Zhiwei
Zhiwei
---
host/include/riscv/host/cpuinfo.h | 23 +
tcg/riscv/tcg-target.h| 46 -
util/cpuinfo-riscv.c | 85 +++
tcg/riscv/tcg-target.c.inc| 84 +++---
util/meson.build | 2 +
5 files changed, 139 insertions(+), 101 deletions(-)
create mode 100644 host/include/riscv/host/cpuinfo.h
create mode 100644 util/cpuinfo-riscv.c
diff --git a/host/include/riscv/host/cpuinfo.h
b/host/include/riscv/host/cpuinfo.h
new file mode 100644
index 00..2b00660e36
--- /dev/null
+++ b/host/include/riscv/host/cpuinfo.h
@@ -0,0 +1,23 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Host specific cpu identification for RISC-V.
+ */
+
+#ifndef HOST_CPUINFO_H
+#define HOST_CPUINFO_H
+
+#define CPUINFO_ALWAYS (1u << 0) /* so cpuinfo is nonzero */
+#define CPUINFO_ZBA (1u << 1)
+#define CPUINFO_ZBB (1u << 2)
+#define CPUINFO_ZICOND (1u << 3)
+
+/* Initialized with a constructor. */
+extern unsigned cpuinfo;
+
+/*
+ * We cannot rely on constructor ordering, so other constructors must
+ * use the function interface rather than the variable above.
+ */
+unsigned cpuinfo_init(void);
+
+#endif /* HOST_CPUINFO_H */
diff --git a/tcg/riscv/tcg-target.h b/tcg/riscv/tcg-target.h
index 2c1b680b93..1a347eaf6e 100644
--- a/tcg/riscv/tcg-target.h
+++ b/tcg/riscv/tcg-target.h
@@ -25,6 +25,8 @@
#ifndef RISCV_TCG_TARGET_H
#define RISCV_TCG_TARGET_H
+#include "host/cpuinfo.h"
+
#define TCG_TARGET_INSN_UNIT_SIZE 4
#define TCG_TARGET_NB_REGS 32
#define MAX_CODE_GEN_BUFFER_SIZE ((size_t)-1)
@@ -80,18 +82,12 @@ typedef enum {
#define TCG_TARGET_CALL_ARG_I128TCG_CALL_ARG_NORMAL
#define TCG_TARGET_CALL_RET_I128TCG_CALL_RET_NORMAL
-#if defined(__riscv_arch_test) && defined(__riscv_zbb)
-# define have_zbb true
-#else
-extern bool have_zbb;
-#endif
-
/* optional instructions */
#define TCG_TARGET_HAS_negsetcond_i32 1
#define TCG_TARGET_HAS_div_i32 1
#define TCG_TARGET_HAS_rem_i32 1
#define TCG_TARGET_HAS_div2_i32 0
-#define TCG_TARGET_HAS_rot_i32 have_zbb
+#define TCG_TARGET_HAS_rot_i32 (cpuinfo & CPUINFO_ZBB)
#define TCG_TARGET_HAS_deposit_i32 0
#define TCG_TARGET_HAS_extract_i32 0
#define TCG_TARGET_HAS_sextract_i32 0
@@ -106,17 +102,17 @@ extern bool have_zbb;
#define TCG_TARGET_HAS_ext16s_i32 1
#define TCG_TARGET_HAS_ext8u_i321
#define TCG_TARGET_HAS_ext16u_i32 1
-#define TCG_TARGET_HAS_bswap16_i32 have_zbb
-#define TCG_TARGET_HAS_bswap32_i32 have_zbb
+#define TCG_TARGET_HAS_bswap16_i32 (cpuinfo & CPUINFO_ZBB)
+#define TCG_TARGET_HAS_bswap32_i32 (cpuinfo & CPUINFO_ZBB)
#define TCG_TARGET_HAS_not_i32 1
-#define TCG_TARGET_HAS_andc_i32 have_zbb
-#define TCG_TARGET_HAS_orc_i32 have_zbb
-#define TCG_TARGET_HAS_eqv_i32 have_zbb
+#define TCG_TARGET_HAS_andc_i32 (cpuinfo & CPUINFO_ZBB)
+#define TCG_TARGET_HAS_orc_i32 (cpuinfo & CPUINFO_ZBB)
+#define TCG_TARGET_HAS_eqv_i32 (cpuinfo & CPUINFO_ZBB)
#define TCG_TARGET_HAS_nand_i32 0
#define TCG_TARGET_HAS_nor_i32 0
-#define TCG_TARGET_HAS_clz_i32 have_zbb
-#define TCG_TARGET_HAS_ctz_i32 have_zbb
-#define TCG_TARGET_HAS_ctpop_i32have_zbb
+#define TCG_TARGET_HAS_clz_i32 (cpuinfo & CPUINFO_ZBB)
+#define TCG_TARGET_HAS_ctz_i32 (cpuinfo & CPUINFO_ZBB)
+#define TCG_TARGET_HAS_ctpop_i32(cpuinfo & CPUINFO_ZBB)
#define TCG_TARGET_HAS_brcond2 1
#define TCG_TARGET_HAS_setcond2 1
#define TCG_TARGET_HAS_qemu_st8_i32 0
@@ -125,7 +121,7 @@ extern bool have_zbb;
#define TCG_TARGET_HAS_div_i64 1
#define TCG_TARGET_HAS_rem_i64 1
#define TCG_TARGET_HAS_div2_i64 0
-#define TCG_TARGET_HAS_rot_i64 have_zbb
+#define TCG_TARGET_HAS_rot_i64 (cpuinfo & CPUINFO_ZBB)
#define TCG_TARGET_HAS_deposit_i64 0
#define TCG_TARGET_HAS_extract_i64 0
#define TCG_TARGET_HAS_sextract_i64 0
@@ -137,18 +133,18 @@ extern bool have_zbb;
#define TCG_TARGET_HAS_ext8u_i641
#define TCG_TARGET_HAS_ext16u_i64 1
#define TCG_TARGET_HAS_ext32u_i64 1
-#define TCG_TARGET_HAS_bswap16_i64 have_zbb
-#define TCG_TARGET_HAS_bswap32_i64 have_zbb
-#define TCG_TARGET_HAS_bswap64_i64 have_zbb
+#define TCG_TARGET_HAS_bswap16_i64 (cpuinfo & CPUINFO_ZBB)
+#define TCG_TARGET_HAS_bswap32_i64 (cpuinfo & CPUINFO_ZBB)
+#define TCG_TARGET_HAS_bswap64_i64 (cpuinfo & CPUINFO_ZBB)
#define TCG_TARGET_HAS_not_i64 1
-#define TCG_TARGET_HAS_andc_i64 have_zbb
-#define TCG_TARGET
[PATCH v44 1/3] hw/sd/sdcard: Use spec v3.01 by default
Recent SDHCI expect cards to support the v3.01 spec
to negociate lower I/O voltage. Select it by default.
Versioned machine types with a version of 9.0 or
earlier retain the old default (spec v2.00).
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Cédric Le Goater
---
v43: update versioned machines (danpb)
---
hw/core/machine.c | 1 +
hw/sd/sd.c| 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 655d75c21f..4377f943d5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -38,6 +38,7 @@ GlobalProperty hw_compat_9_0[] = {
{"arm-cpu", "backcompat-cntfrq", "true" },
{"scsi-disk-base", "migrate-emulated-scsi-request", "false" },
{"vfio-pci", "skip-vsc-check", "false" },
+{"sd-card", "spec_version", "2" },
};
const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b158402704..808dc1cea6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2471,7 +2471,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
- spec_version, SD_PHY_SPECv2_00_VERS),
+ spec_version, SD_PHY_SPECv3_01_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
/* We do not model the chip select pin, so allow the board to select
* whether card should be in SSI or MMC/SD mode. It is also up to the
--
2.41.0
[PATCH v44 0/3] hw/sd/sdcard: Cleanups before adding eMMC support
(patches from v42 already reviewed not reposted) - Addressed review comments from Daniel & Luc wrt migration - Remove old comment Philippe Mathieu-Daudé (3): hw/sd/sdcard: Use spec v3.01 by default hw/sd/sdcard: Do not store vendor data on block drive (CMD56) hw/sd/sdcard: Remove leftover comment about removed 'spi' Property hw/core/machine.c | 1 + hw/sd/sd.c| 32 2 files changed, 21 insertions(+), 12 deletions(-) -- 2.41.0
[PATCH v44 3/3] hw/sd/sdcard: Remove leftover comment about removed 'spi' Property
Commit c3287c0f70 ("hw/sd: Introduce a "sd-card" SPI variant
model") removed the 'spi' property. Remove the comment left
over.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 4
1 file changed, 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 000b923c73..904da440ba 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2485,10 +2485,6 @@ static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
spec_version, SD_PHY_SPECv3_01_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
-/* We do not model the chip select pin, so allow the board to select
- * whether card should be in SSI or MMC/SD mode. It is also up to the
- * board to ensure that ssi transfers only occur when the chip select
- * is asserted. */
DEFINE_PROP_END_OF_LIST()
};
--
2.41.0
[PATCH v44 2/3] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
---
v43: Do not re-use VMSTATE_UNUSED_V (danpb)
v44: Use subsection (Luc)
---
hw/sd/sd.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 808dc1cea6..000b923c73 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -153,6 +153,8 @@ struct SDState {
uint32_t data_offset;
size_t data_size;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -793,6 +796,16 @@ static const VMStateDescription sd_ocr_vmstate = {
},
};
+static const VMStateDescription sd_vendordata_vmstate = {
+.name = "sd-card/vendor_data-state",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
+VMSTATE_END_OF_LIST()
+},
+};
+
static int sd_vmstate_pre_load(void *opaque)
{
SDState *sd = opaque;
@@ -840,6 +853,7 @@ static const VMStateDescription sd_vmstate = {
},
.subsections = (const VMStateDescription * const []) {
&sd_ocr_vmstate,
+&sd_vendordata_vmstate,
NULL
},
};
@@ -2187,9 +2201,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd->vendor_data[sd->data_offset++] = value;
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
}
break;
@@ -2261,12 +2274,11 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
+ret = sd->vendor_data[sd->data_offset++];
-if (sd->data_offset >= sd->blk_len)
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
+}
break;
default:
--
2.41.0
Re: [PATCH v2 1/1] memory tier: consolidate the initialization of memory tiers
Jonathan Cameron writes:
> On Fri, 28 Jun 2024 06:09:23 +
> "Ho-Ren (Jack) Chuang" wrote:
[snip]
>> @@ -875,8 +886,7 @@ static int __meminit memtier_hotplug_callback(struct
>> notifier_block *self,
>>
>> static int __init memory_tier_init(void)
>> {
>> -int ret, node;
>> -struct memory_tier *memtier;
>> +int ret;
>>
>> ret = subsys_virtual_register(&memory_tier_subsys, NULL);
>> if (ret)
>> @@ -887,7 +897,8 @@ static int __init memory_tier_init(void)
>> GFP_KERNEL);
>> WARN_ON(!node_demotion);
>> #endif
>> -mutex_lock(&memory_tier_lock);
>> +
>> +guard(mutex)(&memory_tier_lock);
>
> If this was safe to do without the rest of the change (I think so)
> then better to pull that out as a trivial precursor so less noise
> in here.
>
>> /*
>> * For now we can have 4 faster memory tiers with smaller adistance
>> * than default DRAM tier.
>> @@ -897,29 +908,9 @@ static int __init memory_tier_init(void)
>> if (IS_ERR(default_dram_type))
>> panic("%s() failed to allocate default DRAM tier\n", __func__);
>>
>> -/*
>> - * Look at all the existing N_MEMORY nodes and add them to
>> - * default memory tier or to a tier if we already have memory
>> - * types assigned.
>> - */
>> -for_each_node_state(node, N_MEMORY) {
>> -if (!node_state(node, N_CPU))
>> -/*
>> - * Defer memory tier initialization on
>> - * CPUless numa nodes. These will be initialized
>> - * after firmware and devices are initialized.
>> - */
>> -continue;
>> -
>> -memtier = set_node_memory_tier(node);
>> -if (IS_ERR(memtier))
>> -/*
>> - * Continue with memtiers we are able to setup
>> - */
>> -break;
>> -}
>> -establish_demotion_targets();
>> -mutex_unlock(&memory_tier_lock);
>> +/* Record nodes with memory and CPU to set default DRAM performance. */
>> +nodes_and(default_dram_nodes, node_states[N_MEMORY],
>> + node_states[N_CPU]);
>
> There are systems where (for various esoteric reasons, such as describing an
> association with some other memory that isn't DRAM where the granularity
> doesn't match) the CPU nodes contain no DRAM but rather it's one node away.
> Handling that can be a job for another day though.
>
> Why does this need to be computed here? Why not do it in
> hmat_set_default_dram_perf? Doesn't seem to be used anywhere else.
IMO, which node is default dram node is a general concept instead of
HMAT specific. So, I think that it's better to decide that in the
general code (memory-tiers.c).
>>
>> hotplug_memory_notifier(memtier_hotplug_callback, MEMTIER_HOTPLUG_PRI);
>> return 0;
--
Best Regards,
Huang, Ying
[PATCH] hw/ufs: Fix mcq register range determination logic
The function ufs_is_mcq_reg() only evaluated the range of the
mcq_op_reg offset, which is defined as a constant.
Therefore, it was possible for ufs_is_mcq_reg() to return true
despite ufs device is configured to not support the mcq.
This could cause ufs_mmio_read()/ufs_mmio_write() to overflow the
buffer. So fix it.
Fixes: 5c079578d2e4 ("hw/ufs: Add support MCQ of UFSHCI 4.0")
Signed-off-by: Jeuk Kim
---
hw/ufs/ufs.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index 683fff5840..cf0edd281c 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -57,7 +57,13 @@ static inline uint64_t ufs_reg_size(UfsHc *u)
static inline bool ufs_is_mcq_reg(UfsHc *u, uint64_t addr, unsigned size)
{
-uint64_t mcq_reg_addr = ufs_mcq_reg_addr(u, 0);
+uint64_t mcq_reg_addr;
+
+if (!u->params.mcq) {
+return false;
+}
+
+mcq_reg_addr = ufs_mcq_reg_addr(u, 0);
return (addr >= mcq_reg_addr &&
addr + size <= mcq_reg_addr + sizeof(u->mcq_reg));
}
--
2.34.1
Re: [PATCH v44 2/3] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
On 3/7/24 10:51, Philippe Mathieu-Daudé wrote:
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
---
v43: Do not re-use VMSTATE_UNUSED_V (danpb)
v44: Use subsection (Luc)
---
hw/sd/sd.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 808dc1cea6..000b923c73 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -153,6 +153,8 @@ struct SDState {
uint32_t data_offset;
size_t data_size;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -793,6 +796,16 @@ static const VMStateDescription sd_ocr_vmstate = {
},
};
+static const VMStateDescription sd_vendordata_vmstate = {
+.name = "sd-card/vendor_data-state",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
+VMSTATE_END_OF_LIST()
+},
+};
+
static int sd_vmstate_pre_load(void *opaque)
{
SDState *sd = opaque;
@@ -840,6 +853,7 @@ static const VMStateDescription sd_vmstate = {
},
.subsections = (const VMStateDescription * const []) {
&sd_ocr_vmstate,
+&sd_vendordata_vmstate,
NULL
},
};
Sigh, forgot to squash:
-- >8 --
@@ -919,3 +918,0 @@ static void sd_blk_write(SDState *sd, uint64_t addr,
uint32_t len)
-#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len)
-#define APP_WRITE_BLOCK(a, len)
-
---
@@ -2187,9 +2201,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd->vendor_data[sd->data_offset++] = value;
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
}
break;
@@ -2261,12 +2274,11 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
+ret = sd->vendor_data[sd->data_offset++];
-if (sd->data_offset >= sd->blk_len)
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
+}
break;
default:
[PATCH v45 1/3] hw/sd/sdcard: Remove leftover comment about removed 'spi' Property
Commit c3287c0f70 ("hw/sd: Introduce a "sd-card" SPI variant
model") removed the 'spi' property. Remove the comment left
over.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/sd/sd.c | 4
1 file changed, 4 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b158402704..53767beaf8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2473,10 +2473,6 @@ static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
spec_version, SD_PHY_SPECv2_00_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
-/* We do not model the chip select pin, so allow the board to select
- * whether card should be in SSI or MMC/SD mode. It is also up to the
- * board to ensure that ssi transfers only occur when the chip select
- * is asserted. */
DEFINE_PROP_END_OF_LIST()
};
--
2.41.0
[PATCH v45 2/3] hw/sd/sdcard: Use spec v3.01 by default
Recent SDHCI expect cards to support the v3.01 spec
to negociate lower I/O voltage. Select it by default.
Versioned machine types with a version of 9.0 or
earlier retain the old default (spec v2.00).
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Cédric Le Goater
---
v43: update versioned machines (danpb)
---
hw/core/machine.c | 1 +
hw/sd/sd.c| 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 655d75c21f..4377f943d5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -38,6 +38,7 @@ GlobalProperty hw_compat_9_0[] = {
{"arm-cpu", "backcompat-cntfrq", "true" },
{"scsi-disk-base", "migrate-emulated-scsi-request", "false" },
{"vfio-pci", "skip-vsc-check", "false" },
+{"sd-card", "spec_version", "2" },
};
const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 53767beaf8..a08a452d81 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2471,7 +2471,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
static Property sd_properties[] = {
DEFINE_PROP_UINT8("spec_version", SDState,
- spec_version, SD_PHY_SPECv2_00_VERS),
+ spec_version, SD_PHY_SPECv3_01_VERS),
DEFINE_PROP_DRIVE("drive", SDState, blk),
DEFINE_PROP_END_OF_LIST()
};
--
2.41.0
[PATCH v45 3/3] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)
"General command" (GEN_CMD, CMD56) is described as:
GEN_CMD is the same as the single block read or write
commands (CMD24 or CMD17). The difference is that [...]
the data block is not a memory payload data but has a
vendor specific format and meaning.
Thus this block must not be stored overwriting data block
on underlying storage drive. Keep it in a dedicated
'vendor_data[]' array.
Signed-off-by: Philippe Mathieu-Daudé
---
v43: Do not re-use VMSTATE_UNUSED_V (danpb)
v44: Use subsection (Luc)
v45: Remove APP_READ_BLOCK/APP_WRITE_BLOCK macros
---
hw/sd/sd.c | 29 +++--
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a08a452d81..5d58937dd4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -153,6 +153,8 @@ struct SDState {
uint32_t data_offset;
size_t data_size;
uint8_t data[512];
+uint8_t vendor_data[512];
+
qemu_irq readonly_cb;
qemu_irq inserted_cb;
QEMUTimer *ocr_power_timer;
@@ -719,6 +721,7 @@ static void sd_reset(DeviceState *dev)
sd->wp_switch = sd->blk ? !blk_is_writable(sd->blk) : false;
sd->wp_group_bits = sect;
sd->wp_group_bmap = bitmap_new(sd->wp_group_bits);
+memset(sd->vendor_data, 0xec, sizeof(sd->vendor_data));
memset(sd->function_group, 0, sizeof(sd->function_group));
sd->erase_start = INVALID_ADDRESS;
sd->erase_end = INVALID_ADDRESS;
@@ -793,6 +796,16 @@ static const VMStateDescription sd_ocr_vmstate = {
},
};
+static const VMStateDescription sd_vendordata_vmstate = {
+.name = "sd-card/vendor_data-state",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT8_ARRAY(vendor_data, SDState, 512),
+VMSTATE_END_OF_LIST()
+},
+};
+
static int sd_vmstate_pre_load(void *opaque)
{
SDState *sd = opaque;
@@ -840,6 +853,7 @@ static const VMStateDescription sd_vmstate = {
},
.subsections = (const VMStateDescription * const []) {
&sd_ocr_vmstate,
+&sd_vendordata_vmstate,
NULL
},
};
@@ -902,9 +916,6 @@ static void sd_blk_write(SDState *sd, uint64_t addr,
uint32_t len)
}
}
-#define APP_READ_BLOCK(a, len) memset(sd->data, 0xec, len)
-#define APP_WRITE_BLOCK(a, len)
-
static void sd_erase(SDState *sd)
{
uint64_t erase_start = sd->erase_start;
@@ -2187,9 +2198,8 @@ void sd_write_byte(SDState *sd, uint8_t value)
break;
case 56: /* CMD56: GEN_CMD */
-sd->data[sd->data_offset ++] = value;
-if (sd->data_offset >= sd->blk_len) {
-APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
+sd->vendor_data[sd->data_offset++] = value;
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
}
break;
@@ -2261,12 +2271,11 @@ uint8_t sd_read_byte(SDState *sd)
break;
case 56: /* CMD56: GEN_CMD */
-if (sd->data_offset == 0)
-APP_READ_BLOCK(sd->data_start, sd->blk_len);
-ret = sd->data[sd->data_offset ++];
+ret = sd->vendor_data[sd->data_offset++];
-if (sd->data_offset >= sd->blk_len)
+if (sd->data_offset >= sizeof(sd->vendor_data)) {
sd->state = sd_transfer_state;
+}
break;
default:
--
2.41.0
[PATCH v45 0/3] hw/sd/sdcard: Cleanups before adding eMMC support
(patches from v42 already reviewed not reposted) - Addressed review comments from Daniel & Luc wrt migration - Remove old comment Philippe Mathieu-Daudé (3): hw/sd/sdcard: Remove leftover comment about removed 'spi' Property hw/sd/sdcard: Use spec v3.01 by default hw/sd/sdcard: Do not store vendor data on block drive (CMD56) hw/core/machine.c | 1 + hw/sd/sd.c| 35 --- 2 files changed, 21 insertions(+), 15 deletions(-) -- 2.41.0
Re: [PATCH 0/2] i386/sev: Two trivial improvements to sev_get_capabilities()
On 6/24/24 10:52, Michal Privoznik wrote: > I've noticed that recent QEMU + libvirt (current HEADs, roughly) behave > a bit different than expected. The problem is in recent change to > 'query-sev-capabilities' command (well, sev_get_capabilities() in fact) > which libvirt uses (see patch 2/2). The first one is trivial. > > Michal Privoznik (2): > i386/sev: Fix error message in sev_get_capabilities() > i386/sev: Fallback to the default SEV device if none provided in > sev_get_capabilities() > > target/i386/sev.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > Polite ping. Patch 2/2 is not reviewed and it causes problems to libvirt. Michal
[PATCH] hw: Fix crash that happens when introspecting scsi-block on older machine types
"make check SPEED=slow" is currently failing the device-introspect-test on
older machine types since introspecting "scsi-block" is causing an abort:
$ ./qemu-system-x86_64 -M pc-q35-8.0 -monitor stdio
QEMU 9.0.50 monitor - type 'help' for more information
(qemu) device_add scsi-block,help
Unexpected error in object_property_find_err() at
../../devel/qemu/qom/object.c:1357:
can't apply global scsi-disk-base.migrate-emulated-scsi-request=false:
Property 'scsi-block.migrate-emulated-scsi-request' not found
Aborted (core dumped)
The problem is that the compat code tries to change the
"migrate-emulated-scsi-request" property for all devices that are
derived from "scsi-block", but the property has only been added
to "scsi-hd" and "scsi-cd" via the DEFINE_SCSI_DISK_PROPERTIES macro.
Thus let's fix the problem by only changing the property on the devices
that really have this property.
Fixes: b4912afa5f ("scsi-disk: Fix crash for VM configured with USB CDROM after
live migration")
Signed-off-by: Thomas Huth
---
hw/core/machine.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 655d75c21f..60858a8565 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -36,7 +36,8 @@
GlobalProperty hw_compat_9_0[] = {
{"arm-cpu", "backcompat-cntfrq", "true" },
-{"scsi-disk-base", "migrate-emulated-scsi-request", "false" },
+{"scsi-hd", "migrate-emulated-scsi-request", "false" },
+{"scsi-cd", "migrate-emulated-scsi-request", "false" },
{"vfio-pci", "skip-vsc-check", "false" },
};
const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
--
2.45.2
Re: [PATCH v2 1/5] hw/net:ftgmac100: update memory region size to 0x200
On 7/3/24 10:16 AM, Jamin Lin wrote:
According to the datasheet of ASPEED SOCs,
one MAC controller owns 128KB of register space for AST2500.
However, one MAC controller only owns 64KB of register space for AST2600
and AST2700.
It set the memory region size 128KB and it occupied another
controllers Address Spaces.
Currently, the ftgmac100 model use 0x100 register space.
To support DMA 64 bits dram address and new future mode(ftgmac100_high) which
have "Normal Priority Transmit Ring Base Address Register High(0x17C)",
"High Priority Transmit Ring Base Address Register High(0x184)" and
"Receive Ring Base Address Register High(0x18C)" to save the high part physical
address of descriptor manager.
Update memory region size to 0x200.
Signed-off-by: Jamin Lin
---
hw/net/ftgmac100.c | 2 +-
include/hw/net/ftgmac100.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c
index 25e4c0cd5b..4e88430b2f 100644
--- a/hw/net/ftgmac100.c
+++ b/hw/net/ftgmac100.c
@@ -1108,7 +1108,7 @@ static void ftgmac100_realize(DeviceState *dev, Error
**errp)
}
memory_region_init_io(&s->iomem, OBJECT(dev), &ftgmac100_ops, s,
- TYPE_FTGMAC100, 0x2000);
+ TYPE_FTGMAC100, FTGMAC100_NR_REGS);
sysbus_init_mmio(sbd, &s->iomem);
sysbus_init_irq(sbd, &s->irq);
qemu_macaddr_default_if_unset(&s->conf.macaddr);
diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h
index 765d1538a4..5a970676da 100644
--- a/include/hw/net/ftgmac100.h
+++ b/include/hw/net/ftgmac100.h
@@ -14,6 +14,8 @@
#define TYPE_FTGMAC100 "ftgmac100"
OBJECT_DECLARE_SIMPLE_TYPE(FTGMAC100State, FTGMAC100)
+#define FTGMAC100_NR_REGS 0x200
Since this value will size a memory region, I think the define name should
be changed to FTGMAC100_{MEM,REGION,MMIO}_SIZE. What ever you prefer.
Thanks,
C.
+
#include "hw/sysbus.h"
#include "net/net.h"
Re: [PATCH v2 4/5] hw/block: m25p80: support quad mode for w25q01jvq
On 7/3/24 10:16 AM, Jamin Lin wrote:
According to the w25q01jv datasheet at page 16,
it is required to set QE bit in "Status Register 2".
Besides, users are able to utilize "Write Status Register 1(0x01)"
command to set QE bit in "Status Register 2" and
utilize "Read Status Register 2(0x35)" command to get the QE bit status.
To support quad mode for w25q01jvq, update collecting data needed
2 bytes for WRSR command in decode_new_cmd function and
verify QE bit at the second byte of collecting data bit 2
in complete_collecting_data.
Update RDCR_EQIO command to set bit 2 of return data
if quad mode enable in decode_new_cmd.
Signed-off-by: Troy Lee
Signed-off-by: Jamin Lin
Reviewed-by: Cédric Le Goater
Thanks,
C.
---
hw/block/m25p80.c | 16
1 file changed, 16 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8dec134832..9e99107b42 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -416,6 +416,7 @@ typedef enum {
/*
* Micron: 0x35 - enable QPI
* Spansion: 0x35 - read control register
+ * Winbond: 0x35 - quad enable
*/
RDCR_EQIO = 0x35,
RSTQIO = 0xf5,
@@ -798,6 +799,11 @@ static void complete_collecting_data(Flash *s)
s->four_bytes_address_mode = extract32(s->data[1], 5, 1);
}
break;
+case MAN_WINBOND:
+if (s->len > 1) {
+s->quad_enable = !!(s->data[1] & 0x02);
+}
+break;
default:
break;
}
@@ -1254,6 +1260,10 @@ static void decode_new_cmd(Flash *s, uint32_t value)
s->needed_bytes = 2;
s->state = STATE_COLLECTING_VAR_LEN_DATA;
break;
+case MAN_WINBOND:
+s->needed_bytes = 2;
+s->state = STATE_COLLECTING_VAR_LEN_DATA;
+break;
default:
s->needed_bytes = 1;
s->state = STATE_COLLECTING_DATA;
@@ -1431,6 +1441,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
case MAN_MACRONIX:
s->quad_enable = true;
break;
+case MAN_WINBOND:
+s->data[0] = (!!s->quad_enable) << 1;
+s->pos = 0;
+s->len = 1;
+s->state = STATE_READING_DATA;
+break;
default:
break;
}
Re: [PATCH v2 5/5] test/avocado/machine_aspeed.py: update to test network for AST2700
On 7/3/24 10:16 AM, Jamin Lin wrote:
Update a test case to test network connection via ssh and
changes to test Aspeed OpenBMC SDK v09.02 for AST2700.
ASPEED fixed TX mask issue from linux/drivers/ftgmac100.c.
It is required to use ASPEED SDK image since v09.02
for AST2700 QEMU network testing.
A test image is downloaded from the ASPEED Forked OpenBMC GitHub
release repository :
https://github.com/AspeedTech-BMC/openbmc/releases/
Test command:
```
cd build
pyvenv/bin/avocado run
../qemu/tests/avocado/machine_aspeed.py:AST2x00MachineSDK.test_aarch64_ast2700_evb_sdk_v09_02
```
Signed-off-by: Jamin Lin
Could you please split the patch ? The change of SDK should be
a standalone patch.
Thanks,
C.
---
tests/avocado/machine_aspeed.py | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/tests/avocado/machine_aspeed.py b/tests/avocado/machine_aspeed.py
index 3a20644fb2..855da805ae 100644
--- a/tests/avocado/machine_aspeed.py
+++ b/tests/avocado/machine_aspeed.py
@@ -313,14 +313,14 @@ def do_test_arm_aspeed_sdk_start(self, image):
def do_test_aarch64_aspeed_sdk_start(self, image):
self.vm.set_console()
-self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw')
+self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
+ '-net', 'nic', '-net',
'user,hostfwd=:127.0.0.1:0-:22')
self.vm.launch()
self.wait_for_console_pattern('U-Boot 2023.10')
self.wait_for_console_pattern('## Loading kernel from FIT Image')
self.wait_for_console_pattern('Starting kernel ...')
-self.wait_for_console_pattern("systemd[1]: Hostname set to")
@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on GitLab')
@@ -387,15 +387,15 @@ def test_arm_ast2600_evb_sdk(self):
year = time.strftime("%Y")
self.ssh_command_output_contains('/sbin/hwclock -f /dev/rtc1', year);
-def test_aarch64_ast2700_evb_sdk_v09_01(self):
+def test_aarch64_ast2700_evb_sdk_v09_02(self):
"""
:avocado: tags=arch:aarch64
:avocado: tags=machine:ast2700-evb
"""
image_url = ('https://github.com/AspeedTech-BMC/openbmc/releases/'
- 'download/v09.01/ast2700-default-obmc.tar.gz')
-image_hash =
'b1cc0fd73c7650d34c9c8459a243f52a91e9e27144b8608b2645ab19461d1e07'
+ 'download/v09.02/ast2700-default-obmc.tar.gz')
+image_hash =
'ac969c2602f4e6bdb69562ff466b89ae3fe1d86e1f6797bb7969d787f82116a7'
image_path = self.fetch_asset(image_url, asset_hash=image_hash,
algorithm='sha256')
archive.extract(image_path, self.workdir)
@@ -436,4 +436,5 @@ def test_aarch64_ast2700_evb_sdk_v09_01(self):
self.vm.add_args('-smp', str(num_cpu))
self.do_test_aarch64_aspeed_sdk_start(image_dir + 'image-bmc')
-
+self.wait_for_console_pattern('nodistro.0 ast2700-default ttyS12')
+self.ssh_connect('root', '0penBmc', False)
[PATCH v8 1/3] hw/pci: Add all Data Object Types defined in PCIe r6.0
Add all of the defined protocols/features from the PCIe-SIG r6.0 "Table 6-32 PCI-SIG defined Data Object Types (Vendor ID = 0001h)" table. Signed-off-by: Alistair Francis Reviewed-by: Jonathan Cameron Reviewed-by: Wilfred Mallawa --- include/hw/pci/pcie_doe.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/hw/pci/pcie_doe.h b/include/hw/pci/pcie_doe.h index 87dc17dcef..15d94661f9 100644 --- a/include/hw/pci/pcie_doe.h +++ b/include/hw/pci/pcie_doe.h @@ -46,6 +46,8 @@ REG32(PCI_DOE_CAP_STATUS, 0) /* PCI-SIG defined Data Object Types - r6.0 Table 6-32 */ #define PCI_SIG_DOE_DISCOVERY 0x00 +#define PCI_SIG_DOE_CMA 0x01 +#define PCI_SIG_DOE_SECURED_CMA 0x02 #define PCI_DOE_DW_SIZE_MAX (1 << 18) #define PCI_DOE_PROTOCOL_NUM_MAX256 -- 2.45.2
[PATCH v8 2/3] backends: Initial support for SPDM socket support
From: Huai-Cheng Kuo SPDM enables authentication, attestation and key exchange to assist in providing infrastructure security enablement. It's a standard published by the DMTF [1]. SPDM supports multiple transports, including PCIe DOE and MCTP. This patch adds support to QEMU to connect to an external SPDM instance. SPDM support can be added to any QEMU device by exposing a TCP socket to a SPDM server. The server can then implement the SPDM decoding/encoding support, generally using libspdm [2]. This is similar to how the current TPM implementation works and means that the heavy lifting of setting up certificate chains, capabilities, measurements and complex crypto can be done outside QEMU by a well supported and tested library. 1: https://www.dmtf.org/standards/SPDM 2: https://github.com/DMTF/libspdm Signed-off-by: Huai-Cheng Kuo Signed-off-by: Chris Browy Co-developed-by: Jonathan Cameron Signed-off-by: Jonathan Cameron [ Changes by WM - Bug fixes from testing ] Signed-off-by: Wilfred Mallawa [ Changes by AF: - Convert to be more QEMU-ified - Move to backends as it isn't PCIe specific ] Signed-off-by: Alistair Francis --- MAINTAINERS | 6 + include/sysemu/spdm-socket.h | 74 backends/spdm-socket.c | 216 +++ backends/Kconfig | 4 + backends/meson.build | 2 + 5 files changed, 302 insertions(+) create mode 100644 include/sysemu/spdm-socket.h create mode 100644 backends/spdm-socket.c diff --git a/MAINTAINERS b/MAINTAINERS index 6725913c8b..c76a0cfe12 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3397,6 +3397,12 @@ F: tests/qtest/*tpm* F: docs/specs/tpm.rst T: git https://github.com/stefanberger/qemu-tpm.git tpm-next +SPDM +M: Alistair Francis +S: Maintained +F: backends/spdm-socket.c +F: include/sysemu/spdm-socket.h + Checkpatch S: Odd Fixes F: scripts/checkpatch.pl diff --git a/include/sysemu/spdm-socket.h b/include/sysemu/spdm-socket.h new file mode 100644 index 00..5d8bd9aa4e --- /dev/null +++ b/include/sysemu/spdm-socket.h @@ -0,0 +1,74 @@ +/* + * QEMU SPDM socket support + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef SPDM_REQUESTER_H +#define SPDM_REQUESTER_H + +/** + * spdm_socket_connect: connect to an external SPDM socket + * @port: port to connect to + * @errp: error object handle + * + * This will connect to an external SPDM socket server. On error + * it will return -1 and errp will be set. On success this function + * will return the socket number. + */ +int spdm_socket_connect(uint16_t port, Error **errp); + +/** + * spdm_socket_rsp: send and receive a message to a SPDM server + * @socket: socket returned from spdm_socket_connect() + * @transport_type: SPDM_SOCKET_TRANSPORT_TYPE_* macro + * @req: request buffer + * @req_len: request buffer length + * @rsp: response buffer + * @rsp_len: response buffer length + * + * Send platform data to a SPDM server on socket and then receive + * a response. + */ +uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type, + void *req, uint32_t req_len, + void *rsp, uint32_t rsp_len); + +/** + * spdm_socket_close: send a shutdown command to the server + * @socket: socket returned from spdm_socket_connect() + * @transport_type: SPDM_SOCKET_TRANSPORT_TYPE_* macro + * + * This will issue a shutdown command to the server. + */ +void spdm_socket_close(const int socket, uint32_t transport_type); + +#define SPDM_SOCKET_COMMAND_NORMAL0x0001 +#define SPDM_SOCKET_COMMAND_OOB_ENCAP_KEY_UPDATE 0x8001 +#define SPDM_SOCKET_COMMAND_CONTINUE 0xFFFD +#define SPDM_SOCKET_COMMAND_SHUTDOWN 0xFFFE +#define SPDM_SOCKET_COMMAND_UNKOWN0x +#define SPDM_SOCKET_COMMAND_TEST 0xDEAD + +#define SPDM_SOCKET_TRANSPORT_TYPE_MCTP 0x01 +#define SPDM_SOCKET_TRANSPORT_TYPE_PCI_D
[PATCH v8 3/3] hw/nvme: Add SPDM over DOE support
From: Wilfred Mallawa Setup Data Object Exchange (DOE) as an extended capability for the NVME controller and connect SPDM to it (CMA) to it. Signed-off-by: Wilfred Mallawa Signed-off-by: Alistair Francis Reviewed-by: Jonathan Cameron Acked-by: Klaus Jensen --- docs/specs/index.rst| 1 + docs/specs/spdm.rst | 134 include/hw/pci/pci_device.h | 7 ++ include/hw/pci/pcie_doe.h | 3 + hw/nvme/ctrl.c | 62 + 5 files changed, 207 insertions(+) create mode 100644 docs/specs/spdm.rst diff --git a/docs/specs/index.rst b/docs/specs/index.rst index 1484e3e760..e2d907959a 100644 --- a/docs/specs/index.rst +++ b/docs/specs/index.rst @@ -29,6 +29,7 @@ guest hardware that is specific to QEMU. edu ivshmem-spec pvpanic + spdm standard-vga virt-ctlr vmcoreinfo diff --git a/docs/specs/spdm.rst b/docs/specs/spdm.rst new file mode 100644 index 00..f7de080ff0 --- /dev/null +++ b/docs/specs/spdm.rst @@ -0,0 +1,134 @@ +== +QEMU Security Protocols and Data Models (SPDM) Support +== + +SPDM enables authentication, attestation and key exchange to assist in +providing infrastructure security enablement. It's a standard published +by the `DMTF`_. + +QEMU supports connecting to a SPDM responder implementation. This allows an +external application to emulate the SPDM responder logic for an SPDM device. + +Setting up a SPDM server + + +When using QEMU with SPDM devices QEMU will connect to a server which +implements the SPDM functionality. + +SPDM-Utils +-- + +You can use `SPDM Utils`_ to emulate a responder. This is the simplest method. + +SPDM-Utils is a Linux applications to manage, test and develop devices +supporting DMTF Security Protocol and Data Model (SPDM). It is written in Rust +and utilises libspdm. + +To use SPDM-Utils you will need to do the following steps. Details are included +in the SPDM-Utils README. + + 1. `Build libspdm`_ + 2. `Build SPDM Utils`_ + 3. `Run it as a server`_ + +spdm-emu + + +You can use `spdm emu`_ to model the +SPDM responder. + +.. code-block:: shell + +$ cd spdm-emu +$ git submodule init; git submodule update --recursive +$ mkdir build; cd build +$ cmake -DARCH=x64 -DTOOLCHAIN=GCC -DTARGET=Debug -DCRYPTO=openssl .. +$ make -j32 +$ make copy_sample_key # Build certificates, required for SPDM authentication. + +It is worth noting that the certificates should be in compliance with +PCIe r6.1 sec 6.31.3. This means you will need to add the following to +openssl.cnf + +.. code-block:: + +subjectAltName = otherName:2.23.147;UTF8:Vendor=1b36:Device=0010:CC=010802:REV=02:SSVID=1af4:SSID=1100 +2.23.147 = ASN1:OID:2.23.147 + +and then manually regenerate some certificates with: + +.. code-block:: shell + +$ openssl req -nodes -newkey ec:param.pem -keyout end_responder.key \ +-out end_responder.req -sha384 -batch \ +-subj "/CN=DMTF libspdm ECP384 responder cert" + +$ openssl x509 -req -in end_responder.req -out end_responder.cert \ +-CA inter.cert -CAkey inter.key -sha384 -days 3650 -set_serial 3 \ +-extensions v3_end -extfile ../openssl.cnf + +$ openssl asn1parse -in end_responder.cert -out end_responder.cert.der + +$ cat ca.cert.der inter.cert.der end_responder.cert.der > bundle_responder.certchain.der + +You can use SPDM-Utils instead as it will generate the correct certificates +automatically. + +The responder can then be launched with + +.. code-block:: shell + +$ cd bin +$ ./spdm_responder_emu --trans PCI_DOE + +Connecting an SPDM NVMe device +== + +Once a SPDM server is running we can start QEMU and connect to the server. + +For an NVMe device first let's setup a block we can use + +.. code-block:: shell + +$ cd qemu-spdm/linux/image +$ dd if=/dev/zero of=blknvme bs=1M count=2096 # 2GB NNMe Drive + +Then you can add this to your QEMU command line: + +.. code-block:: shell + +-drive file=blknvme,if=none,id=mynvme,format=raw \ +-device nvme,drive=mynvme,serial=deadbeef,spdm_port=2323 + +At which point QEMU will try to connect to the SPDM server. + +Note that if using x64-64 you will want to use the q35 machine instead +of the default. So the entire QEMU command might look like this + +.. code-block:: shell + +qemu-system-x86_64 -M q35 \ +--kernel bzImage \ +-drive file=rootfs.ext2,if=virtio,format=raw \ +-append "root=/dev/vda console=ttyS0" \ +-net none -nographic \ +-drive file=blknvme,if=none,id=mynvme,format=raw \ +-device nvme,drive=mynvme,serial=deadbeef,spdm_port=2323 + +.. _DMTF: + https://www.dmtf.org/standards/SPDM + +.. _SPDM Utils: + https://github.com/westerndigitalcorporation/spdm-utils + +.. _spdm emu: + https://github.com/
[PATCH v8 0/3] Initial support for SPDM Responders
The Security Protocol and Data Model (SPDM) Specification defines messages, data objects, and sequences for performing message exchanges over a variety of transport and physical media. - https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf SPDM currently supports PCIe DOE and MCTP transports, but it can be extended to support others in the future. This series adds support to QEMU to connect to an external SPDM instance. SPDM support can be added to any QEMU device by exposing a TCP socket to a SPDM server. The server can then implement the SPDM decoding/encoding support, generally using libspdm [1]. This is similar to how the current TPM implementation works and means that the heavy lifting of setting up certificate chains, capabilities, measurements and complex crypto can be done outside QEMU by a well supported and tested library. This series implements socket support and exposes SPDM for a NVMe device. 1: https://github.com/DMTF/libspdm v8: - Fixup i386 failures (thanks to Wilfred) - Passes CI on GitLab: https://gitlab.com/alistair23/qemu/-/tree/mainline/alistair/spdm-socket.next?ref_type=heads v7: - Fixup checkpatch failures - Fixup test failures - Rename port name to be clearer v6: - Add documentation to public functions - Rename socket variable to spdm_socket - Don't override errp - Correctly return false from nvme_init_pci() on error v5: - Update MAINTAINERS v4: - Rebase v3: - Spelling fixes - Support for SPDM-Utils v2: - Add cover letter - A few code fixes based on comments - Document SPDM-Utils - A few tweaks and clarifications to the documentation Alistair Francis (1): hw/pci: Add all Data Object Types defined in PCIe r6.0 Huai-Cheng Kuo (1): backends: Initial support for SPDM socket support Wilfred Mallawa (1): hw/nvme: Add SPDM over DOE support MAINTAINERS | 6 + docs/specs/index.rst | 1 + docs/specs/spdm.rst | 134 ++ include/hw/pci/pci_device.h | 7 ++ include/hw/pci/pcie_doe.h| 5 + include/sysemu/spdm-socket.h | 74 backends/spdm-socket.c | 216 +++ hw/nvme/ctrl.c | 62 ++ backends/Kconfig | 4 + backends/meson.build | 2 + 10 files changed, 511 insertions(+) create mode 100644 docs/specs/spdm.rst create mode 100644 include/sysemu/spdm-socket.h create mode 100644 backends/spdm-socket.c -- 2.45.2
RE: [PATCH v2 5/5] test/avocado/machine_aspeed.py: update to test network for AST2700
> -Original Message-
> From: Cédric Le Goater
> Sent: Wednesday, July 3, 2024 5:18 PM
> To: Jamin Lin ; Peter Maydell
> ; Steven Lee ; Troy
> Lee ; Andrew Jeffery ;
> Joel Stanley ; Alistair Francis ;
> Kevin
> Wolf ; Hanna Reitz ; Jason Wang
> ; Cleber Rosa ; Philippe
> Mathieu-Daudé ; Wainer dos Santos Moschetta
> ; Beraldo Leal ; open list:ASPEED
> BMCs ; open list:All patches CC here
> ; open list:Block layer core
>
> Cc: Troy Lee ; Yunlin Tang
>
> Subject: Re: [PATCH v2 5/5] test/avocado/machine_aspeed.py: update to test
> network for AST2700
>
> On 7/3/24 10:16 AM, Jamin Lin wrote:
> > Update a test case to test network connection via ssh and changes to
> > test Aspeed OpenBMC SDK v09.02 for AST2700.
> >
> > ASPEED fixed TX mask issue from linux/drivers/ftgmac100.c.
> > It is required to use ASPEED SDK image since v09.02 for AST2700 QEMU
> > network testing.
> >
> > A test image is downloaded from the ASPEED Forked OpenBMC GitHub
> > release repository :
> > https://github.com/AspeedTech-BMC/openbmc/releases/
> >
> > Test command:
> > ```
> > cd build
> > pyvenv/bin/avocado run
> > ../qemu/tests/avocado/machine_aspeed.py:AST2x00MachineSDK.test_aarch
> 64
> > _ast2700_evb_sdk_v09_02
> > ```
> >
> > Signed-off-by: Jamin Lin
>
> Could you please split the patch ? The change of SDK should be a standalone
> patch.
>
Will fix
Thanks-Jamin
>
> Thanks,
>
> C.
>
>
> > ---
> > tests/avocado/machine_aspeed.py | 13 +++--
> > 1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/tests/avocado/machine_aspeed.py
> > b/tests/avocado/machine_aspeed.py index 3a20644fb2..855da805ae 100644
> > --- a/tests/avocado/machine_aspeed.py
> > +++ b/tests/avocado/machine_aspeed.py
> > @@ -313,14 +313,14 @@ def do_test_arm_aspeed_sdk_start(self, image):
> >
> > def do_test_aarch64_aspeed_sdk_start(self, image):
> > self.vm.set_console()
> > -self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw')
> > +self.vm.add_args('-drive', 'file=' + image + ',if=mtd,format=raw',
> > + '-net', 'nic', '-net',
> > + 'user,hostfwd=:127.0.0.1:0-:22')
> >
> > self.vm.launch()
> >
> > self.wait_for_console_pattern('U-Boot 2023.10')
> > self.wait_for_console_pattern('## Loading kernel from FIT
> Image')
> > self.wait_for_console_pattern('Starting kernel ...')
> > -self.wait_for_console_pattern("systemd[1]: Hostname set to")
> >
> > @skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is
> > unstable on GitLab')
> >
> > @@ -387,15 +387,15 @@ def test_arm_ast2600_evb_sdk(self):
> > year = time.strftime("%Y")
> > self.ssh_command_output_contains('/sbin/hwclock -f
> > /dev/rtc1', year);
> >
> > -def test_aarch64_ast2700_evb_sdk_v09_01(self):
> > +def test_aarch64_ast2700_evb_sdk_v09_02(self):
> > """
> > :avocado: tags=arch:aarch64
> > :avocado: tags=machine:ast2700-evb
> > """
> >
> > image_url =
> ('https://github.com/AspeedTech-BMC/openbmc/releases/'
> > - 'download/v09.01/ast2700-default-obmc.tar.gz')
> > -image_hash =
> 'b1cc0fd73c7650d34c9c8459a243f52a91e9e27144b8608b2645ab19461d1e07'
> > + 'download/v09.02/ast2700-default-obmc.tar.gz')
> > +image_hash =
> 'ac969c2602f4e6bdb69562ff466b89ae3fe1d86e1f6797bb7969d787f82116a7'
> > image_path = self.fetch_asset(image_url,
> asset_hash=image_hash,
> > algorithm='sha256')
> > archive.extract(image_path, self.workdir) @@ -436,4 +436,5
> > @@ def test_aarch64_ast2700_evb_sdk_v09_01(self):
> >
> > self.vm.add_args('-smp', str(num_cpu))
> > self.do_test_aarch64_aspeed_sdk_start(image_dir +
> > 'image-bmc')
> > -
> > +self.wait_for_console_pattern('nodistro.0 ast2700-default ttyS12')
> > +self.ssh_connect('root', '0penBmc', False)
RE: [PATCH v2 1/5] hw/net:ftgmac100: update memory region size to 0x200
Hi Cedric,
> Subject: Re: [PATCH v2 1/5] hw/net:ftgmac100: update memory region size to
> 0x200
>
> On 7/3/24 10:16 AM, Jamin Lin wrote:
> > According to the datasheet of ASPEED SOCs, one MAC controller owns
> > 128KB of register space for AST2500.
> >
> > However, one MAC controller only owns 64KB of register space for
> > AST2600 and AST2700.
> >
> > It set the memory region size 128KB and it occupied another
> > controllers Address Spaces.
> >
> > Currently, the ftgmac100 model use 0x100 register space.
> > To support DMA 64 bits dram address and new future
> > mode(ftgmac100_high) which have "Normal Priority Transmit Ring Base
> > Address Register High(0x17C)", "High Priority Transmit Ring Base
> > Address Register High(0x184)" and "Receive Ring Base Address Register
> > High(0x18C)" to save the high part physical address of descriptor manager.
> >
> > Update memory region size to 0x200.
> >
> > Signed-off-by: Jamin Lin
> > ---
> > hw/net/ftgmac100.c | 2 +-
> > include/hw/net/ftgmac100.h | 2 ++
> > 2 files changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/net/ftgmac100.c b/hw/net/ftgmac100.c index
> > 25e4c0cd5b..4e88430b2f 100644
> > --- a/hw/net/ftgmac100.c
> > +++ b/hw/net/ftgmac100.c
> > @@ -1108,7 +1108,7 @@ static void ftgmac100_realize(DeviceState *dev,
> Error **errp)
> > }
> >
> > memory_region_init_io(&s->iomem, OBJECT(dev), &ftgmac100_ops,
> s,
> > - TYPE_FTGMAC100, 0x2000);
> > + TYPE_FTGMAC100,
> FTGMAC100_NR_REGS);
> > sysbus_init_mmio(sbd, &s->iomem);
> > sysbus_init_irq(sbd, &s->irq);
> > qemu_macaddr_default_if_unset(&s->conf.macaddr);
> > diff --git a/include/hw/net/ftgmac100.h b/include/hw/net/ftgmac100.h
> > index 765d1538a4..5a970676da 100644
> > --- a/include/hw/net/ftgmac100.h
> > +++ b/include/hw/net/ftgmac100.h
> > @@ -14,6 +14,8 @@
> > #define TYPE_FTGMAC100 "ftgmac100"
> > OBJECT_DECLARE_SIMPLE_TYPE(FTGMAC100State, FTGMAC100)
> >
> > +#define FTGMAC100_NR_REGS 0x200
>
> Since this value will size a memory region, I think the define name should be
> changed to FTGMAC100_{MEM,REGION,MMIO}_SIZE. What ever you prefer.
>
Will fix
Thanks-Jamin
>
> Thanks,
>
> C.
>
>
>
> > +
> > #include "hw/sysbus.h"
> > #include "net/net.h"
> >
Re: [PATCH] hw: Fix crash that happens when introspecting scsi-block on older machine types
On 3/7/24 11:09, Thomas Huth wrote:
"make check SPEED=slow" is currently failing the device-introspect-test on
older machine types since introspecting "scsi-block" is causing an abort:
$ ./qemu-system-x86_64 -M pc-q35-8.0 -monitor stdio
QEMU 9.0.50 monitor - type 'help' for more information
(qemu) device_add scsi-block,help
Unexpected error in object_property_find_err() at
../../devel/qemu/qom/object.c:1357:
can't apply global scsi-disk-base.migrate-emulated-scsi-request=false:
Property 'scsi-block.migrate-emulated-scsi-request' not found
Aborted (core dumped)
The problem is that the compat code tries to change the
"migrate-emulated-scsi-request" property for all devices that are
derived from "scsi-block", but the property has only been added
to "scsi-hd" and "scsi-cd" via the DEFINE_SCSI_DISK_PROPERTIES macro.
Thus let's fix the problem by only changing the property on the devices
that really have this property.
Fixes: b4912afa5f ("scsi-disk: Fix crash for VM configured with USB CDROM after live
migration")
(FTR this commit was not reviewed).
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Thomas Huth
---
hw/core/machine.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Re: [PATCH] hw: Fix crash that happens when introspecting scsi-block on older machine types
On Wed, Jul 3, 2024 at 5:09 PM Thomas Huth wrote:
> "make check SPEED=slow" is currently failing the device-introspect-test on
> older machine types since introspecting "scsi-block" is causing an abort:
>
> $ ./qemu-system-x86_64 -M pc-q35-8.0 -monitor stdio
> QEMU 9.0.50 monitor - type 'help' for more information
> (qemu) device_add scsi-block,help
> Unexpected error in object_property_find_err() at
> ../../devel/qemu/qom/object.c:1357:
> can't apply global scsi-disk-base.migrate-emulated-scsi-request=false:
> Property 'scsi-block.migrate-emulated-scsi-request' not found
> Aborted (core dumped)
>
> The problem is that the compat code tries to change the
> "migrate-emulated-scsi-request" property for all devices that are
> derived from "scsi-block", but the property has only been added
> to "scsi-hd" and "scsi-cd" via the DEFINE_SCSI_DISK_PROPERTIES macro.
>
> Thus let's fix the problem by only changing the property on the devices
> that really have this property.
>
Thanks for the fix.
> Fixes: b4912afa5f ("scsi-disk: Fix crash for VM configured with USB CDROM
> after live migration")
> Signed-off-by: Thomas Huth
> ---
> hw/core/machine.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 655d75c21f..60858a8565 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -36,7 +36,8 @@
>
> GlobalProperty hw_compat_9_0[] = {
> {"arm-cpu", "backcompat-cntfrq", "true" },
> -{"scsi-disk-base", "migrate-emulated-scsi-request", "false" },
> +{"scsi-hd", "migrate-emulated-scsi-request", "false" },
> +{"scsi-cd", "migrate-emulated-scsi-request", "false" },
> {"vfio-pci", "skip-vsc-check", "false" },
> };
> const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
> --
> 2.45.2
>
Acked-by: Hyman Huang
--
Best regards
Re: [RFC v3 1/2] target/loongarch: Add loongson binary translation feature
On Wed, Jul 3, 2024 at 3:51 PM Jiaxun Yang wrote:
>
>
>
> 在2024年7月1日七月 下午2:57，Jiaxun Yang写道：
> > 在2024年5月30日五月 上午7:49，Bibo Mao写道：
> >> Loongson Binary Translation (LBT) is used to accelerate binary
> >> translation, which contains 4 scratch registers (scr0 to scr3), x86/ARM
> >> eflags (eflags) and x87 fpu stack pointer (ftop).
> >>
> >> Now LBT feature is added in kvm mode, not supported in TCG mode since
> >> it is not emulated. Feature variable lbt is added with OnOffAuto type,
> >> If lbt feature is not supported with KVM host, it reports error if there
> >> is lbt=on command line.
> >>
> >> If there is no any command line about lbt parameter, it checks whether
> >> KVM host supports lbt feature and set the corresponding value in cpucfg.
> >>
> >> Signed-off-by: Bibo Mao
> > Hi Bibo,
> >
> > I was going across recent LoongArch changes and this comes into my
> > attention:
> >
> >> ---
> >> target/loongarch/cpu.c| 53 +++
> >> target/loongarch/cpu.h| 6 +++
> >> target/loongarch/kvm/kvm.c| 26 +
> >> target/loongarch/kvm/kvm_loongarch.h | 16
> >> target/loongarch/loongarch-qmp-cmds.c | 2 +-
> >> 5 files changed, 102 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
> >> index b5c1ec94af..14265b6667 100644
> >> --- a/target/loongarch/cpu.c
> >> +++ b/target/loongarch/cpu.c
> >> @@ -571,6 +571,30 @@ static void loongarch_cpu_disas_set_info(CPUState
> >> *s, disassemble_info *info)
> >> info->print_insn = print_insn_loongarch;
> >> }
> >>
> >> +static void loongarch_cpu_check_lbt(CPUState *cs, Error **errp)
> >> +{
> >> +CPULoongArchState *env = cpu_env(cs);
> >> +LoongArchCPU *cpu = LOONGARCH_CPU(cs);
> >> +bool kvm_supported;
> >> +
> >> +kvm_supported = kvm_feature_supported(cs, LOONGARCH_FEATURE_LBT);
> >
> > IMHO if there is no global states that should be saved/restored VM wise,
> > this should be handled at per CPU level, preferably with CPUCFG flags hint.
> >
> > We should minimize non-privilege KVM feature bits to prevent hindering
> > asymmetry ISA system.
>
> + Huacai for further discussion
>
> Hi Bibo, Huacai,
>
> I investigated the topic further and went through the thread on kernel side.
>
> I think Huacai and me are all on the same page that we should unify the
> interface for per-CPU
> level feature probing and setting interface. Huacai purposed converting all
> features to VM feature
> but I still believe CPUCFG is the best interface.
>
> To probe LBT before actual vcpu creation, we can borrow the approach used by
> other architectures
> (kvm_arm_create_scratch_host_vcpu() & kvm_riscv_create_scratch_vcpu()).
>
> Kernel will reject setting unknown CPUCFG bits with -EINVAL, so to probe LBT
> we just need to perform
> KVM_SET_REGS to scratch vcpu with LBT set to see if it's valid for kernel.
> There is no need for any other
> probing interface.
>
> I do think scratch CPU interface is also necessary if we are going to
> implement cpu = host.
>
> Huacai, would you agree with me?
For me the important thing is consistency, all vm-features or all
vcpu-features are both accepted.
Huacai
>
> Thanks
> - Jiaxun
>
> >
> > Thanks
> > - Jiaxun
> >
> > --
> > - Jiaxun
>
> --
> - Jiaxun
[PATCH 1/2] qdev: change qdev_prop_set_globals() to use Object*
Next patch will add Accel globals support and will use
qdev_prop_set_globals().
At this moment this function is hardwired to be used with DeviceState.
dev->hotplugged is checked to determine if object_apply_global_props()
will receive a NULL or an &error_fatal errp.
Change qdev_prop_set_globals() to receive an Object and an errp. The
logic using dev->hotplugged is moved to the caller, device_post_init().
Signed-off-by: Daniel Henrique Barboza
---
hw/core/qdev-properties.c| 5 ++---
hw/core/qdev.c | 2 +-
include/hw/qdev-properties.h | 2 +-
3 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 86a583574d..4867d7dd9e 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -920,10 +920,9 @@ int qdev_prop_check_globals(void)
return ret;
}
-void qdev_prop_set_globals(DeviceState *dev)
+void qdev_prop_set_globals(Object *obj, Error **errp)
{
-object_apply_global_props(OBJECT(dev), global_props(),
- dev->hotplugged ? NULL : &error_fatal);
+object_apply_global_props(obj, global_props(), errp);
}
/* --- 64bit unsigned int 'size' type --- */
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f3a996f57d..923f9ca74c 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -673,7 +673,7 @@ static void device_post_init(Object *obj)
* precedence.
*/
object_apply_compat_props(obj);
-qdev_prop_set_globals(DEVICE(obj));
+qdev_prop_set_globals(obj, DEVICE(obj)->hotplugged ? NULL : &error_fatal);
}
/* Unlink device from bus and free the structure. */
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 09aa04ca1e..a1737bf4d8 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -210,7 +210,7 @@ void qdev_prop_register_global(GlobalProperty *prop);
const GlobalProperty *qdev_find_global_prop(Object *obj,
const char *name);
int qdev_prop_check_globals(void);
-void qdev_prop_set_globals(DeviceState *dev);
+void qdev_prop_set_globals(Object *obj, Error **errp);
void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj,
const char *name, const char *value);
--
2.45.2
[PATCH 0/2] qdev,accel-system: allow Accel type globals
Hi, This is another approach of the problem we tried to fix with [1]. It was suggested by Paolo during the review. In the current handling of '-accel' only the first instance is parsed. All other instances (aside from a 'helper' command that triggers the help text and exits) is ignored. So this command line: qemu-system-riscv64 -accel kvm -accel kvm,riscv-aia=hwaccel Won't change 'riscv-aia' to 'hwaccel'. This is affecting at least one use case we have with libvirt and RISC-V: we can't set 'riscv-aia' by appending '-accel kvm,riscv-aia=val' via in the domain XML. libvirt will add a leading '-accel kvm' in the regular command line and ignore the second. We'll add official libvirt support for this KVM property in the near future (we're still discussing if this prop should be a bool instead), but for now a QEMU side change would unlock this RISC-V KVM use case in libvirt. With this series we'll be able to set 'riscv-aia' using globals as follows: qemu-system-riscv64 -accel kvm -global kvm-accel.riscv-aia=hwaccel This change benefit all archs (not just RISC-V) and will allow globals to be used with all accelerators, not just KVM. [1] https://lore.kernel.org/qemu-devel/20240701133038.1489043-1-dbarb...@ventanamicro.com/ Daniel Henrique Barboza (2): qdev: change qdev_prop_set_globals() to use Object* qdev, accel-system: add support to Accel globals accel/accel-system.c | 3 +++ hw/core/qdev-properties.c| 39 +++- hw/core/qdev.c | 2 +- include/hw/qdev-properties.h | 2 +- 4 files changed, 30 insertions(+), 16 deletions(-) -- 2.45.2
[PATCH 2/2] qdev, accel-system: add support to Accel globals
We're not honouring KVM options that are provided by any -accel option
aside from the first. In this example:
qemu-system-riscv64 -accel kvm,riscv-aia=emul (...) \
-accel kvm,riscv-aia=hwaccel
'riscv-aia' will be set to 'emul', ignoring the last occurrence of the
option that set 'riscv-aia' to 'hwaccel'.
A failed attempt to solve this issue by setting 'merge_lists' can be
found in [1]. A different approach was suggested in [2]: enable global
properties for accelerators. This allows an user to override any accel
setting by using '-global' and we won't need to change how '-accel' opts
are handled.
This is done by calling qdev_prop_set_globals() in accel_init_machine().
As done in device_post_init(), user's global props take precedence over
compat props so qdev_prop_set_globals() is called after
object_set_accelerator_compat_props().
qdev_prop_check_globals() is then changed to recognize TYPE_ACCEL
globals as valid.
Re-using the fore-mentioned example we'll be able to set
riscv-aia=hwaccel by doing the following:
qemu-system-riscv64 -accel kvm,riscv-aia=emul (...) \
-global kvm-accel.riscv-aia=hwaccel
[1]
https://lore.kernel.org/qemu-devel/20240701133038.1489043-1-dbarb...@ventanamicro.com/
[2]
https://lore.kernel.org/qemu-devel/CABgObfbVJkCdpHV2uA7LGTbYEaoaizrbeG0vNo_T1ua5b=j...@mail.gmail.com/
Reported-by: Andrew Jones
Suggested-by: Paolo Bonzini
Signed-off-by: Daniel Henrique Barboza
---
accel/accel-system.c | 3 +++
hw/core/qdev-properties.c | 34 +++---
2 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/accel/accel-system.c b/accel/accel-system.c
index f6c947dd82..40c73ec62f 100644
--- a/accel/accel-system.c
+++ b/accel/accel-system.c
@@ -29,6 +29,8 @@
#include "sysemu/cpus.h"
#include "qemu/error-report.h"
#include "accel-system.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
int accel_init_machine(AccelState *accel, MachineState *ms)
{
@@ -43,6 +45,7 @@ int accel_init_machine(AccelState *accel, MachineState *ms)
object_unref(OBJECT(accel));
} else {
object_set_accelerator_compat_props(acc->compat_props);
+qdev_prop_set_globals(OBJECT(accel), &error_fatal);
}
return ret;
}
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 4867d7dd9e..462a8f96e0 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -10,6 +10,7 @@
#include "qemu/cutils.h"
#include "qdev-prop-internal.h"
#include "qom/qom-qobject.h"
+#include "qemu/accel.h"
void qdev_prop_set_after_realize(DeviceState *dev, const char *name,
Error **errp)
@@ -894,27 +895,38 @@ int qdev_prop_check_globals(void)
for (i = 0; i < global_props()->len; i++) {
GlobalProperty *prop;
-ObjectClass *oc;
+ObjectClass *globalc, *oc;
DeviceClass *dc;
prop = g_ptr_array_index(global_props(), i);
if (prop->used) {
continue;
}
-oc = object_class_by_name(prop->driver);
-oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
-if (!oc) {
-warn_report("global %s.%s has invalid class name",
-prop->driver, prop->property);
-ret = 1;
+globalc = object_class_by_name(prop->driver);
+oc = object_class_dynamic_cast(globalc, TYPE_DEVICE);
+if (oc) {
+dc = DEVICE_CLASS(oc);
+if (!dc->hotpluggable && !prop->used) {
+warn_report("global %s.%s=%s not used",
+prop->driver, prop->property, prop->value);
+ret = 1;
+}
continue;
}
-dc = DEVICE_CLASS(oc);
-if (!dc->hotpluggable && !prop->used) {
+
+/*
+ * At this point is either an accelerator global that is
+ * unused or an invalid global. Both set ret = 1.
+ */
+ret = 1;
+
+oc = object_class_dynamic_cast(globalc, TYPE_ACCEL);
+if (oc) {
warn_report("global %s.%s=%s not used",
prop->driver, prop->property, prop->value);
-ret = 1;
-continue;
+} else {
+warn_report("global %s.%s has invalid class name",
+prop->driver, prop->property);
}
}
return ret;
--
2.45.2
Re: [PATCH 1/4] python: linter changes for pylint 3.x
John Snow writes: > New bleeding edge versions, new nits to iron out. This addresses the > 'check-python-tox' optional GitLab test, while 'check-python-minreqs' > saw no regressions, since it's frozen on an older version of pylint. > > Fixes: > qemu/machine/machine.py:345:52: E0606: Possibly using variable 'sock' before > assignment (possibly-used-before-assignment) > qemu/utils/qemu_ga_client.py:168:4: R1711: Useless return at end of function > or method (useless-return) > > Signed-off-by: John Snow Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH 2/4] python: Do not use pylint 3.2.4 with python 3.8
John Snow writes: > There is a bug in this version, > see: https://github.com/pylint-dev/pylint/issues/9751 > > Signed-off-by: John Snow Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
[PATCH] qapi/qom: Document feature unstable of @x-vfio-user-server
Commit 8f9a9259d32c added ObjectType member @x-vfio-user-server with feature unstable, but neglected to explain why it is unstable. Do that now. Fixes: 8f9a9259d32c (vfio-user: define vfio-user-server object) Cc: Elena Ufimtseva Cc: John G Johnson Cc: Jagannathan Raman Cc: qemu-sta...@nongnu.org Signed-off-by: Markus Armbruster --- qapi/qom.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qapi/qom.json b/qapi/qom.json index 8bd299265e..3c0f8c633d 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -1012,7 +1012,8 @@ # # Features: # -# @unstable: Member @x-remote-object is experimental. +# @unstable: Members @x-remote-object and @x-vfio-user-server are +# experimental. # # Since: 6.0 ## -- 2.45.0
Re: [RFC v3 1/2] target/loongarch: Add loongson binary translation feature
On 2024/7/3 下午5:43, Huacai Chen wrote:
On Wed, Jul 3, 2024 at 3:51 PM Jiaxun Yang wrote:
在2024年7月1日七月 下午2:57，Jiaxun Yang写道：
在2024年5月30日五月 上午7:49，Bibo Mao写道：
Loongson Binary Translation (LBT) is used to accelerate binary
translation, which contains 4 scratch registers (scr0 to scr3), x86/ARM
eflags (eflags) and x87 fpu stack pointer (ftop).
Now LBT feature is added in kvm mode, not supported in TCG mode since
it is not emulated. Feature variable lbt is added with OnOffAuto type,
If lbt feature is not supported with KVM host, it reports error if there
is lbt=on command line.
If there is no any command line about lbt parameter, it checks whether
KVM host supports lbt feature and set the corresponding value in cpucfg.
Signed-off-by: Bibo Mao
Hi Bibo,
I was going across recent LoongArch changes and this comes into my attention:
---
target/loongarch/cpu.c| 53 +++
target/loongarch/cpu.h| 6 +++
target/loongarch/kvm/kvm.c| 26 +
target/loongarch/kvm/kvm_loongarch.h | 16
target/loongarch/loongarch-qmp-cmds.c | 2 +-
5 files changed, 102 insertions(+), 1 deletion(-)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index b5c1ec94af..14265b6667 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -571,6 +571,30 @@ static void loongarch_cpu_disas_set_info(CPUState
*s, disassemble_info *info)
info->print_insn = print_insn_loongarch;
}
+static void loongarch_cpu_check_lbt(CPUState *cs, Error **errp)
+{
+CPULoongArchState *env = cpu_env(cs);
+LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+bool kvm_supported;
+
+kvm_supported = kvm_feature_supported(cs, LOONGARCH_FEATURE_LBT);
IMHO if there is no global states that should be saved/restored VM wise,
this should be handled at per CPU level, preferably with CPUCFG flags hint.
We should minimize non-privilege KVM feature bits to prevent hindering
asymmetry ISA system.
+ Huacai for further discussion
Hi Bibo, Huacai,
I investigated the topic further and went through the thread on kernel side.
I think Huacai and me are all on the same page that we should unify the
interface for per-CPU
level feature probing and setting interface. Huacai purposed converting all
features to VM feature
but I still believe CPUCFG is the best interface.
To probe LBT before actual vcpu creation, we can borrow the approach used by
other architectures
(kvm_arm_create_scratch_host_vcpu() & kvm_riscv_create_scratch_vcpu()).
Kernel will reject setting unknown CPUCFG bits with -EINVAL, so to probe LBT we
just need to perform
KVM_SET_REGS to scratch vcpu with LBT set to see if it's valid for kernel.
There is no need for any other
probing interface.
I do think scratch CPU interface is also necessary if we are going to implement
cpu = host.
Huacai, would you agree with me?
For me the important thing is consistency, all vm-features or all
vcpu-features are both accepted.
To understand features immediately is difficult job for me. There is
supported features/used features usages etc, overall feature detection
should be VM relative by my knowledge.
Maybe after host machine type and migration feature detection and
checking is finished, there will be further upstanding -:(
Regards
Bibo Mao
Huacai
Thanks
- Jiaxun
Thanks
- Jiaxun
--
- Jiaxun
--
- Jiaxun
Re: [SPAM] [RFC PATCH v42 90/98] hw/sd/sdcard: Add experimental 'x-aspeed-emmc-kludge' property
On 2/7/24 22:05, Philippe Mathieu-Daudé wrote: On 2/7/24 18:21, Cédric Le Goater wrote: On 7/2/24 6:15 PM, Philippe Mathieu-Daudé wrote: On 2/7/24 07:06, Andrew Jeffery wrote: On Fri, 2024-06-28 at 11:16 +0200, Cédric Le Goater wrote: On 6/28/24 9:02 AM, Philippe Mathieu-Daudé wrote: When booting U-boot/Linux on Aspeed boards via eMMC, some commands don't behave as expected from the spec. Add the 'x-aspeed-emmc-kludge' property to allow non standard uses until we figure out the reasons. I am not aware of any singularity in the eMMC logic provided by Aspeed. U-Boot and Linux drivers seem very generic. May be others can tell. I'm not aware of any command kludges. The main problem I had when I wrote the Linux driver for the Aspeed controller was the phase tuning, but that doesn't sound related. Yeah I don't think anything Aspeed nor U-boot related, we model CSD/CID registers per the SD spec, not MMC. Various fields are identical, but few differ, this might be the problem. I rather respect the spec by default, so until we figure the issue, are you OK to use a 'x-emmc-kludge' property and set it on the Aspeed boards? If these differences are eMMC related, why not simply test : if (sd_is_emmc(sd)) ... in commands ALL_SEND_CID and APP_CMD ? The extra property looks ambiguous to me. I'd like to keep the sd_is_emmc() check for code respecting the eMMC spec. I believe the commands in sd_proto_emmc[] in this series do respect it, modulo some register field definitions that are SD specific. So 'x-emmc-kludge' would be a property to allow eMMC use -- without delaying it further --, by bypassing a *bug* in our current model. I'm willing to figure out the problem and fix it, but /after/ the 9.1 release. We are too close of the soft freeze and trying to fix that before is too much pressure on my right now. The problem is in the still unreviewed patch #86 of this series "hw/sd/sdcard: Add emmc_cmd_SEND_OP_COND handler (CMD1)". SEND_OP_COND should put the card in READY state. We are not considering the BOOT_PARTITION_ENABLE feature: > When BOOT_PARTITION_ENABLE bits are set and master send > CMD1 (SEND_OP_COND), slave must enter Card Identification > Mode and respond to the command. > If the slave does not support boot operation mode, which > is compliant with v4.2 or before, or BOOT_PARTITION_ENABLE > bit is cleared, slave automatically enter Idle State after > power-on. Then we don't need the change in the next patch (#91) in ALL_SEND_CID. And likely neither we need #92 (APP_CMD ) but I still need to confirm that.
Re: [RFC v3 1/2] target/loongarch: Add loongson binary translation feature
在2024年7月3日七月 下午6:10，maobibo写道： [...] >>> >>> Huacai, would you agree with me? >> For me the important thing is consistency, all vm-features or all >> vcpu-features are both accepted. > To understand features immediately is difficult job for me. There is > supported features/used features usages etc, overall feature detection > should be VM relative by my knowledge. > > Maybe after host machine type and migration feature detection and > checking is finished, there will be further upstanding -:( Do you agree with the scratch_vcpu approach? This seems to be the only straight forward way to make some progress here. Thanks -- - Jiaxun
Re: [PATCH v2 22/22] qga: centralize logic for disabling/enabling commands
Hello Daniel,
This cleanup seems like a good idea,
On Thu, 13 Jun 2024 18:44, "Daniel P. Berrangé" wrote:
It is confusing having many different pieces of code enabling and
disabling commands, and it is not clear that they all have the same
semantics, especially wrt prioritization of the block/allow lists.
The code attempted to prevent the user from setting both the block
and allow lists concurrently, however, the logic was flawed as it
checked settings in the configuration file separately from the
command line arguments. Thus it was possible to set a block list
in the config file and an allow list via a command line argument.
The --dump-conf option also creates a configuration file with both
keys present, even if unset, which means it is creating a config
that cannot actually be loaded again.
Centralizing the code in a single method "ga_apply_command_filters"
will provide a strong guarantee of consistency and clarify the
intended behaviour. With this there is no compelling technical
reason to prevent concurrent setting of both the allow and block
lists, so this flawed restriction is removed.
Signed-off-by: Daniel P. Berrangé
---
docs/interop/qemu-ga.rst | 14 +
qga/commands-posix.c | 6 --
qga/commands-win32.c | 6 --
qga/main.c | 128 +--
4 files changed, 70 insertions(+), 84 deletions(-)
diff --git a/docs/interop/qemu-ga.rst b/docs/interop/qemu-ga.rst
index e42b370319..e35dcaf0e7 100644
--- a/docs/interop/qemu-ga.rst
+++ b/docs/interop/qemu-ga.rst
@@ -28,6 +28,20 @@ configuration options on the command line. For the same key,
the last
option wins, but the lists accumulate (see below for configuration
file format).
+If an allowed RPCs list is defined in the configuration, then all
+RPCs will be blocked by default, except for the allowed list.
+
+If a blocked RPCs list is defined in the configuration, then all
+RPCs will be allowed by default, except for the blocked list.
+
+If both allowed and blocked RPCs lists are defined in the configuration,
+then all RPCs will be blocked by default, and then allowed list will
+be applied, followed by the blocked list.
Nit: Missing an article here
-then all RPCs will be blocked by default, and then allowed list will
+then all RPCs will be blocked by default, then the allowed list will
+
+While filesystems are frozen, all except for a designated safe set
+of RPCs will blocked, regardless of what the general configuration
+declares.
+
Options
---
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index f4104f2760..578d29f228 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1136,12 +1136,6 @@ error:
#endif /* HAVE_GETIFADDRS */
-/* add unsupported commands to the list of blocked RPCs */
-GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
-{
-return blockedrpcs;
-}
-
/* register init/cleanup routines for stateful command groups */
void ga_command_state_init(GAState *s, GACommandState *cs)
{
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 5866cc2e3c..61b36da469 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1958,12 +1958,6 @@ done:
g_free(rawpasswddata);
}
-/* add unsupported commands to the list of blocked RPCs */
-GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
-{
-return blockedrpcs;
-}
-
/* register init/cleanup routines for stateful command groups */
void ga_command_state_init(GAState *s, GACommandState *cs)
{
diff --git a/qga/main.c b/qga/main.c
index f68a32bf7b..72c16fead8 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -419,60 +419,79 @@ static gint ga_strcmp(gconstpointer str1, gconstpointer
str2)
return strcmp(str1, str2);
}
-/* disable commands that aren't safe for fsfreeze */
-static void ga_disable_not_allowed_freeze(const QmpCommand *cmd, void *opaque)
+static bool ga_command_is_allowed(const QmpCommand *cmd, GAState *state)
{
-bool allowed = false;
int i = 0;
+GAConfig *config = state->config;
const char *name = qmp_command_name(cmd);
+/* Fallback policy is allow everything */
+bool allowed = true;
-while (ga_freeze_allowlist[i] != NULL) {
-if (strcmp(name, ga_freeze_allowlist[i]) == 0) {
+if (config->allowedrpcs) {
+/*
+ * If an allow-list is given, this changes the fallback
+ * policy to deny everything
+ */
+allowed = false;
+
+if (g_list_find_custom(config->allowedrpcs, name, ga_strcmp) != NULL) {
allowed = true;
}
-i++;
}
-if (!allowed) {
-g_debug("disabling command: %s", name);
-qmp_disable_command(&ga_commands, name, "the agent is in frozen
state");
-}
-}
-/* [re-]enable all commands, except those explicitly blocked by user */
-static void ga_enable_non_blocked(const QmpCommand *cmd, void *opaque)
-{
-GAState *s = opaque;
-GList *blockedrpcs = s->blockedrpcs;
-GList *allowedrpcs = s->allowedrpcs;
-const char *name = qmp_comma
Re: [PATCH v4 16/16] tests/qtest/bios-tables-test: Add expected ACPI data files for RISC-V
On Tue, Jul 02, 2024 at 03:02:36PM +0100, Jonathan Cameron wrote: > On Mon, 1 Jul 2024 17:03:43 -0400 > "Michael S. Tsirkin" wrote: > > > On Thu, Jun 27, 2024 at 02:18:03PM +0200, Igor Mammedov wrote: > > > On Tue, 25 Jun 2024 20:38:39 +0530 > > > Sunil V L wrote: > > > > > > > As per the step 5 in the process documented in bios-tables-test.c, > > > > generate the expected ACPI AML data files for RISC-V using the > > > > rebuild-expected-aml.sh script and update the > > > > bios-tables-test-allowed-diff.h. > > > > > > > > These are all new files being added for the first time. Hence, iASL diff > > > > output is not added. > > > > > > > > Signed-off-by: Sunil V L > > > > Acked-by: Alistair Francis > > > > Acked-by: Igor Mammedov > > > > > > Michael, > > > can it go via risc-v tree or > > > do you plan to merge it via your tree? > > > > given patch 1 is merged, I took the rest. > > Looks like your CI runs are catching this as well but > RHCT here is failing. I rebased the GI/GP set on top of this > and ignored that failure by skipping riscv64 tests. > > Jonathan > Hi Jonathan, Michael, Looks like a recent RISC-V PR updated the rva22s64 ISA string affecting the RHCT I had in my series. I see that Michael dropped those 3 RISC-V patches from the PR. So, let me update the expected RHCT AML file in a new series. I will also include Igor's feedback to remove fallback path in that series. Thanks, Sunil Thanks, Sunil
Re: [PATCH v2 18/22] qga: don't disable fsfreeze commands if vss_init fails
On Thu, 13 Jun 2024 18:44, "Daniel P. Berrangé" wrote:
The fsfreeze commands are already written to report an error if
vss_init() fails. Reporting a more specific error message is more
helpful than a generic "command is disabled" message, which cannot
beteween an admin config decision and lack of platform support.
s/beteween/between
Signed-off-by: Daniel P. Berrangé
---
qga/commands-win32.c | 18 +++---
qga/main.c | 4
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 2533e4c748..5866cc2e3c 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1203,7 +1203,7 @@ GuestFilesystemInfoList *qmp_guest_get_fsinfo(Error
**errp)
GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
{
if (!vss_initialized()) {
-error_setg(errp, QERR_UNSUPPORTED);
+error_setg(errp, "fsfreeze not possible as VSS failed to initialize");
return 0;
Should this be return -1 by the way?
}
@@ -1231,7 +1231,7 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool
has_mountpoints,
Error *local_err = NULL;
if (!vss_initialized()) {
-error_setg(errp, QERR_UNSUPPORTED);
+error_setg(errp, "fsfreeze not possible as VSS failed to initialize");
return 0;
}
@@ -1266,7 +1266,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
int i;
if (!vss_initialized()) {
-error_setg(errp, QERR_UNSUPPORTED);
+error_setg(errp, "fsfreeze not possible as VSS failed to initialize");
return 0;
}
@@ -1961,18 +1961,6 @@ done:
/* add unsupported commands to the list of blocked RPCs */
GList *ga_command_init_blockedrpcs(GList *blockedrpcs)
{
-if (!vss_init(true)) {
-g_debug("vss_init failed, vss commands are going to be disabled");
-const char *list[] = {
-"guest-get-fsinfo", "guest-fsfreeze-status",
-"guest-fsfreeze-freeze", "guest-fsfreeze-thaw", NULL};
-char **p = (char **)list;
-
-while (*p) {
-blockedrpcs = g_list_append(blockedrpcs, g_strdup(*p++));
-}
-}
-
return blockedrpcs;
}
diff --git a/qga/main.c b/qga/main.c
index f4d5f15bb3..17b6ce18ac 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -1395,6 +1395,10 @@ static GAState *initialize_agent(GAConfig *config, int
socket_activation)
" '%s': %s", config->state_dir, strerror(errno));
return NULL;
}
+
+if (!vss_init(true)) {
+g_debug("vss_init failed, vss commands will not function");
+}
#endif
if (ga_is_frozen(s)) {
--
2.45.1
Otherwise LGTM:
Reviewed-by: Manos Pitsidianakis
Re: [PATCH 4/4] python: enable testing for 3.13
John Snow writes: > Python 3.13 is in beta and Fedora 41 is preparing to make it the default > system interpreter; enable testing for it. > > (In the event problems develop prior to release, it should only impact > the check-python-tox job, which is not run by default and is allowed to > fail.) > > Signed-off-by: John Snow Reviewed-by: Alex Bennée Tested-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
[PATCH] target/i386: SEV: fix formatting of CPUID mismatch message
Fixes: 70943ad8e4d ("i386/sev: Add support for SNP CPUID validation",
2024-06-05)
Signed-off-by: Paolo Bonzini
---
target/i386/sev.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 3ab8b3c28b7..2a0f94d390d 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -841,7 +841,7 @@ sev_snp_cpuid_report_mismatches(SnpCpuidInfo *old,
size_t i;
if (old->count != new->count) {
-error_report("SEV-SNP: CPUID validation failed due to count mismatch,"
+error_report("SEV-SNP: CPUID validation failed due to count mismatch, "
"provided: %d, expected: %d", old->count, new->count);
return;
}
@@ -853,8 +853,8 @@ sev_snp_cpuid_report_mismatches(SnpCpuidInfo *old,
new_func = &new->entries[i];
if (memcmp(old_func, new_func, sizeof(SnpCpuidFunc))) {
-error_report("SEV-SNP: CPUID validation failed for function 0x%x,
index: 0x%x"
- "provided: eax:0x%08x, ebx: 0x%08x, ecx: 0x%08x, edx:
0x%08x"
+error_report("SEV-SNP: CPUID validation failed for function 0x%x,
index: 0x%x, "
+ "provided: eax:0x%08x, ebx: 0x%08x, ecx: 0x%08x, edx:
0x%08x, "
"expected: eax:0x%08x, ebx: 0x%08x, ecx: 0x%08x, edx:
0x%08x",
old_func->eax_in, old_func->ecx_in,
old_func->eax, old_func->ebx, old_func->ecx,
old_func->edx,
--
2.45.2
[PATCH] target/i386: do not include undefined bits in the AMD topoext leaf
Commit d7c72735f61 ("target/i386: Add new EPYC CPU versions with updated
cache_info", 2023-05-08) ensured that AMD-defined CPU models did not
have the 'complex_indexing' bit set, but left it set in "-cpu host"
which uses the default ("legacy") cache information.
Reimplement that commit using a CPU feature, so that it can be applied
to all guests using a new machine type, independent of the CPU model.
Signed-off-by: Paolo Bonzini
---
target/i386/cpu.h | 3 +++
hw/i386/pc.c | 1 +
target/i386/cpu.c | 4
3 files changed, 8 insertions(+)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 31c9b43849e..c43ac01c794 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2108,6 +2108,9 @@ struct ArchCPU {
/* Only advertise CPUID leaves defined by the vendor */
bool vendor_cpuid_only;
+/* Only advertise TOPOEXT features that AMD defines */
+bool amd_topoext_features_only;
+
/* Enable auto level-increase for Intel Processor Trace leave */
bool intel_pt_auto_level;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 77415064c62..5dff91422ff 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -80,6 +80,7 @@
{ "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
GlobalProperty pc_compat_9_0[] = {
+{ TYPE_X86_CPU, "x-amd-topoext-features-only", "false" },
{ TYPE_X86_CPU, "x-l1-cache-per-thread", "false" },
{ TYPE_X86_CPU, "guest-phys-bits", "0" },
{ "sev-guest", "legacy-vm-type", "true" },
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a042e6b4edb..c9d8f6339a4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6991,6 +6991,9 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
*eax = *ebx = *ecx = *edx = 0;
break;
}
+if (cpu->amd_topoext_features_only) {
+*edx &= CACHE_NO_INVD_SHARING | CACHE_INCLUSIVE;
+}
break;
case 0x801E:
if (cpu->core_id <= 255) {
@@ -8302,6 +8305,7 @@ static Property x86_cpu_properties[] = {
DEFINE_PROP_STRING("hv-vendor-id", X86CPU, hyperv_vendor),
DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
DEFINE_PROP_BOOL("x-vendor-cpuid-only", X86CPU, vendor_cpuid_only, true),
+DEFINE_PROP_BOOL("x-amd-topoext-features-only", X86CPU,
amd_topoext_features_only, true),
DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false),
DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true),
DEFINE_PROP_BOOL("kvm-pv-enforce-cpuid", X86CPU, kvm_pv_enforce_cpuid,
--
2.45.2
Re: [PATCH v10 11/12] hw/pci: Convert rom_bar into OnOffAuto
On Wed, 3 Jul 2024, Michael S. Tsirkin wrote:
On Wed, Jul 03, 2024 at 04:15:23AM +0200, BALATON Zoltan wrote:
On Tue, 2 Jul 2024, Michael S. Tsirkin wrote:
On Thu, Jun 27, 2024 at 03:08:00PM +0900, Akihiko Odaki wrote:
rom_bar is tristate but was defined as uint32_t so convert it into
OnOffAuto.
Signed-off-by: Akihiko Odaki
Commit log should explain why this is an improvement,
not just what's done.
diff --git a/docs/igd-assign.txt b/docs/igd-assign.txt
index e17bb50789ad..35c6c8e28493 100644
--- a/docs/igd-assign.txt
+++ b/docs/igd-assign.txt
@@ -35,7 +35,7 @@ IGD has two different modes for assignment using vfio-pci:
ISA/LPC bridge device (vfio-pci-igd-lpc-bridge) on the root bus at
PCI address 1f.0.
* The IGD device must have a VGA ROM, either provided via the romfile
- option or loaded automatically through vfio (standard). rombar=0
+ option or loaded automatically through vfio (standard). rombar=off
will disable legacy mode support.
* Hotplug of the IGD device is not supported.
* The IGD device must be a SandyBridge or newer model device.
...
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index 39dae72497e0..0e920ed0691a 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -33,7 +33,7 @@
* execution as noticed with the BCM 57810 card for lack of a
* more better way to handle such issues.
* The user can still override by specifying a romfile or
- * rombar=1.
+ * rombar=on.
* Please see https://bugs.launchpad.net/qemu/+bug/1284874
* for an analysis of the 57810 card hang. When adding
* a new vendor id/device id combination below, please also add
So we are apparently breaking a bunch of users who followed
documentation to the dot. Why is this a good idea?
On/off is clearer than 1/0. But isn't 1/0 a synonym for on/off so previous
command lines would still work?
Regards,
BALATON Zoltan
I see nothing in code that would make it so:
const QEnumLookup OnOffAuto_lookup = {
.array = (const char *const[]) {
[ON_OFF_AUTO_AUTO] = "auto",
[ON_OFF_AUTO_ON] = "on",
[ON_OFF_AUTO_OFF] = "off",
},
.size = ON_OFF_AUTO__MAX
};
I also tried with an existing property:
$ ./qemu-system-x86_64 -device intel-hda,msi=0
qemu-system-x86_64: -device intel-hda,msi=0: Parameter 'msi' does not accept
value '0'
Then it was probably bit properties that also accept 0/1, on/off,
true/false. Maybe similar aliases could be added to on/off/auto?
In any case when I first saw rombar I thought it would set the BAR of the
ROM so wondered why it's 1 and not 5 or 6 or an offset. So on/off is
clearer in this case.
Regards,
BALATON Zoltan
[RFC PATCH 1/2] target/i386: add support for masking CPUID features in confidential guests
Some CPUID features may be provided by KVM for some guests, independent of
processor support, for example TSC deadline or TSC adjust. If these are
not supported by the confidential computing firmware, however, the guest
will fail to start. Add support for removing unsupported features from
"-cpu host".
Signed-off-by: Paolo Bonzini
---
target/i386/confidential-guest.h | 24
target/i386/cpu.c| 9 +
target/i386/kvm/kvm.c| 5 +
3 files changed, 38 insertions(+)
diff --git a/target/i386/confidential-guest.h b/target/i386/confidential-guest.h
index 532e172a60b..7342d2843aa 100644
--- a/target/i386/confidential-guest.h
+++ b/target/i386/confidential-guest.h
@@ -39,6 +39,8 @@ struct X86ConfidentialGuestClass {
/* */
int (*kvm_type)(X86ConfidentialGuest *cg);
+uint32_t (*mask_cpuid_features)(X86ConfidentialGuest *cg, uint32_t
feature, uint32_t index,
+int reg, uint32_t value);
};
/**
@@ -56,4 +58,26 @@ static inline int
x86_confidential_guest_kvm_type(X86ConfidentialGuest *cg)
return 0;
}
}
+
+/**
+ * x86_confidential_guest_mask_cpuid_features:
+ *
+ * Removes unsupported features from a confidential guest's CPUID values,
returns
+ * the value with the bits removed. The bits removed should be those that KVM
+ * provides independent of host-supported CPUID features, but are not
supported by
+ * the confidential computing firmware.
+ */
+static inline int
x86_confidential_guest_mask_cpuid_features(X86ConfidentialGuest *cg,
+ uint32_t feature,
uint32_t index,
+ int reg, uint32_t
value)
+{
+X86ConfidentialGuestClass *klass = X86_CONFIDENTIAL_GUEST_GET_CLASS(cg);
+
+if (klass->mask_cpuid_features) {
+return klass->mask_cpuid_features(cg, feature, index, reg, value);
+} else {
+return value;
+}
+}
+
#endif
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index dd8b0f33136..056d117cd11 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -548,6 +548,11 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s,
uint32_t function,
ret |= 1U << KVM_HINTS_REALTIME;
}
+if (current_machine->cgs) {
+ret = x86_confidential_guest_mask_cpuid_features(
+X86_CONFIDENTIAL_GUEST(current_machine->cgs),
+function, index, reg, ret);
+}
return ret;
}
--
2.45.2
[RFC PATCH 0/2] target/i386: SEV: allow running SNP guests with "-cpu host"
Some CPUID features may be provided by KVM for some guests, independent of processor support, for example TSC deadline or TSC adjust. They are not going to be present in named models unless the vendor implements them in hardware, but they will be present in "-cpu host". If these bits are not supported by the confidential computing firmware, however, the guest will fail to start, and indeed this is a problem when you run SNP guests with "-cpu host". This series fixes the issue. However, I am marking this as RFC because it's not future proof. If in the future AMD processors do provide any of these bits, this is going to break (tsc_deadline and tsc_adjust are the most likely one). Including the bits if they are present in host CPUID is not super safe either, since the firmware might not be updated to follow suit. Michael, any ideas? Is there a way for the host to retrieve the supported CPUID bits for SEV-SNP guests? One possibility is to set up a fake guest---either in QEMU or when KVM starts---to do a LAUNCH_UPDATE for the CPUID page, but even that is not perfect. For example, I got > function 0x7, index: 0x0 provided: edx: 0xbc10, expected: edx: 0x even though the FSRM bit (0x10) is supported. That might be just a firmware bug however. Paolo Based-on: <20240627140628.1025317-1-pbonz...@redhat.com> Paolo Bonzini (4): target/i386: add support for masking CPUID features in confidential guests target/i386/SEV: implement mask_cpuid_features target/i386/confidential-guest.h | 24 target/i386/cpu.c| 9 + target/i386/cpu.h| 4 target/i386/kvm/kvm.c| 5 + target/i386/sev.c| 33 + 5 files changed, 75 insertions(+) -- 2.45.2
[RFC PATCH 2/2] target/i386/SEV: implement mask_cpuid_features
SNP firmware rejects several features that KVM implements without needing
hardware support. If these are specified, for example with "-cpu host",
the guest will fail to start.
I am marking this as RFC because it's not future proof. If in the future
AMD processors do provide any of these bits, this is going to break
(tsc_deadline and tsc_adjust are the most likely one). Including the
bits if they are present in host CPUID is not super safe either, since
the firmware might not be updated to follow suit.
Reported-by: Zixi Chen
Not-quite-signed-off-by: Paolo Bonzini
---
target/i386/cpu.h | 4
target/i386/sev.c | 33 +
2 files changed, 37 insertions(+)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 9bea7142bf4..31c9b43849e 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -812,6 +812,8 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu,
FeatureWord w);
/* Support RDFSBASE/RDGSBASE/WRFSBASE/WRGSBASE */
#define CPUID_7_0_EBX_FSGSBASE (1U << 0)
+/* Support TSC adjust MSR */
+#define CPUID_7_0_EBX_TSC_ADJUST(1U << 1)
/* Support SGX */
#define CPUID_7_0_EBX_SGX (1U << 2)
/* 1st Group of Advanced Bit Manipulation Extensions */
@@ -1002,6 +1004,8 @@ uint64_t x86_cpu_get_supported_feature_word(X86CPU *cpu,
FeatureWord w);
#define CPUID_8000_0008_EBX_STIBP_ALWAYS_ON(1U << 17)
/* Speculative Store Bypass Disable */
#define CPUID_8000_0008_EBX_AMD_SSBD(1U << 24)
+/* Paravirtualized Speculative Store Bypass Disable MSR */
+#define CPUID_8000_0008_EBX_VIRT_SSBD (1U << 25)
/* Predictive Store Forwarding Disable */
#define CPUID_8000_0008_EBX_AMD_PSFD(1U << 28)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2a0f94d390d..280eaef8723 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -945,6 +945,38 @@ out:
return ret;
}
+static uint32_t
+sev_snp_mask_cpuid_features(X86ConfidentialGuest *cg, uint32_t feature,
uint32_t index,
+int reg, uint32_t value)
+{
+switch (feature) {
+case 1:
+if (reg == R_ECX) {
+return value & ~CPUID_EXT_TSC_DEADLINE_TIMER;
+}
+break;
+case 7:
+if (index == 0 && reg == R_EBX) {
+return value & ~CPUID_7_0_EBX_TSC_ADJUST;
+}
+if (index == 0 && reg == R_EDX) {
+return value & ~(CPUID_7_0_EDX_SPEC_CTRL |
+ CPUID_7_0_EDX_STIBP |
+ CPUID_7_0_EDX_FLUSH_L1D |
+ CPUID_7_0_EDX_ARCH_CAPABILITIES |
+ CPUID_7_0_EDX_CORE_CAPABILITY |
+ CPUID_7_0_EDX_SPEC_CTRL_SSBD);
+}
+break;
+case 0x8008:
+if (reg == R_EBX) {
+return value & ~CPUID_8000_0008_EBX_VIRT_SSBD;
+}
+break;
+}
+return value;
+}
+
static int
sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
uint8_t *addr, size_t len)
@@ -2315,6 +2347,7 @@ sev_snp_guest_class_init(ObjectClass *oc, void *data)
klass->launch_finish = sev_snp_launch_finish;
klass->launch_update_data = sev_snp_launch_update_data;
klass->kvm_init = sev_snp_kvm_init;
+x86_klass->mask_cpuid_features = sev_snp_mask_cpuid_features;
x86_klass->kvm_type = sev_snp_kvm_type;
object_class_property_add(oc, "policy", "uint64",
--
2.45.2
Re: [PATCH 1/2] qdev: change qdev_prop_set_globals() to use Object*
On Wed, Jul 03, 2024 at 06:46:25AM -0300, Daniel Henrique Barboza wrote:
> Next patch will add Accel globals support and will use
> qdev_prop_set_globals().
>
> At this moment this function is hardwired to be used with DeviceState.
> dev->hotplugged is checked to determine if object_apply_global_props()
> will receive a NULL or an &error_fatal errp.
>
> Change qdev_prop_set_globals() to receive an Object and an errp. The
> logic using dev->hotplugged is moved to the caller, device_post_init().
>
> Signed-off-by: Daniel Henrique Barboza
> ---
> hw/core/qdev-properties.c| 5 ++---
> hw/core/qdev.c | 2 +-
> include/hw/qdev-properties.h | 2 +-
> 3 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 86a583574d..4867d7dd9e 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -920,10 +920,9 @@ int qdev_prop_check_globals(void)
> return ret;
> }
>
> -void qdev_prop_set_globals(DeviceState *dev)
> +void qdev_prop_set_globals(Object *obj, Error **errp)
> {
> -object_apply_global_props(OBJECT(dev), global_props(),
> - dev->hotplugged ? NULL : &error_fatal);
> +object_apply_global_props(obj, global_props(), errp);
> }
If you're going to make this work on Object, instead fo merely
DeviceState, then all this globals functionality should be
moved into qom/object.c, and the methods renamed, as it is no
longer qdev logic.
>
> /* --- 64bit unsigned int 'size' type --- */
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index f3a996f57d..923f9ca74c 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -673,7 +673,7 @@ static void device_post_init(Object *obj)
> * precedence.
> */
> object_apply_compat_props(obj);
> -qdev_prop_set_globals(DEVICE(obj));
> +qdev_prop_set_globals(obj, DEVICE(obj)->hotplugged ? NULL :
> &error_fatal);
> }
>
> /* Unlink device from bus and free the structure. */
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 09aa04ca1e..a1737bf4d8 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -210,7 +210,7 @@ void qdev_prop_register_global(GlobalProperty *prop);
> const GlobalProperty *qdev_find_global_prop(Object *obj,
> const char *name);
> int qdev_prop_check_globals(void);
> -void qdev_prop_set_globals(DeviceState *dev);
> +void qdev_prop_set_globals(Object *obj, Error **errp);
> void error_set_from_qdev_prop_error(Error **errp, int ret, Object *obj,
> const char *name, const char *value);
>
> --
> 2.45.2
>
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[PATCH v4 07/17] sev: Update launch_update_data functions to use Error handling
The class function and implementations for updating launch data return
a code in case of error. In some cases an error message is generated and
in other cases, just the error return value is used.
This small refactor adds an 'Error **errp' parameter to all functions
which consistently set an error condition if a non-zero value is
returned.
Signed-off-by: Roy Hopkins
---
target/i386/sev.c | 59 +--
1 file changed, 31 insertions(+), 28 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 491ca5369e..5eabeadda6 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -121,7 +121,8 @@ struct SevCommonStateClass {
Error **errp);
int (*launch_start)(SevCommonState *sev_common);
void (*launch_finish)(SevCommonState *sev_common);
-int (*launch_update_data)(SevCommonState *sev_common, hwaddr gpa, uint8_t
*ptr, size_t len);
+int (*launch_update_data)(SevCommonState *sev_common, hwaddr gpa,
+ uint8_t *ptr, size_t len, Error **errp);
int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp);
};
@@ -945,14 +946,16 @@ out:
return ret;
}
-static int
-sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
- uint8_t *addr, size_t len)
+static int sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
+ uint8_t *addr, size_t len, Error **errp)
{
int ret, fw_error;
struct kvm_sev_launch_update_data update;
if (!addr || !len) {
+error_setg(errp,
+ "%s: Invalid parameters provided for updating launch data.",
+ __func__);
return 1;
}
@@ -962,8 +965,8 @@ sev_launch_update_data(SevCommonState *sev_common, hwaddr
gpa,
ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_LAUNCH_UPDATE_DATA,
&update, &fw_error);
if (ret) {
-error_report("%s: LAUNCH_UPDATE ret=%d fw_error=%d '%s'",
-__func__, ret, fw_error, fw_error_to_str(fw_error));
+error_setg(errp, "%s: LAUNCH_UPDATE ret=%d fw_error=%d '%s'", __func__,
+ ret, fw_error, fw_error_to_str(fw_error));
}
return ret;
@@ -1091,8 +1094,8 @@ sev_launch_finish(SevCommonState *sev_common)
migrate_add_blocker(&sev_mig_blocker, &error_fatal);
}
-static int
-snp_launch_update_data(uint64_t gpa, void *hva, size_t len, int type)
+static int snp_launch_update_data(uint64_t gpa, void *hva, size_t len,
+ int type, Error **errp)
{
SevLaunchUpdateData *data;
@@ -1107,13 +1110,11 @@ snp_launch_update_data(uint64_t gpa, void *hva, size_t
len, int type)
return 0;
}
-static int
-sev_snp_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
- uint8_t *ptr, size_t len)
+static int sev_snp_launch_update_data(SevCommonState *sev_common, hwaddr gpa,
+ uint8_t *ptr, size_t len, Error **errp)
{
- int ret = snp_launch_update_data(gpa, ptr, len,
- KVM_SEV_SNP_PAGE_TYPE_NORMAL);
- return ret;
+return snp_launch_update_data(gpa, ptr, len,
+ KVM_SEV_SNP_PAGE_TYPE_NORMAL, errp);
}
static int
@@ -1165,8 +1166,8 @@ sev_snp_cpuid_info_fill(SnpCpuidInfo *snp_cpuid_info,
return 0;
}
-static int
-snp_launch_update_cpuid(uint32_t cpuid_addr, void *hva, size_t cpuid_len)
+static int snp_launch_update_cpuid(uint32_t cpuid_addr, void *hva,
+ size_t cpuid_len, Error **errp)
{
KvmCpuidInfo kvm_cpuid_info = {0};
SnpCpuidInfo snp_cpuid_info;
@@ -1183,26 +1184,26 @@ snp_launch_update_cpuid(uint32_t cpuid_addr, void *hva,
size_t cpuid_len)
} while (ret == -E2BIG);
if (ret) {
-error_report("SEV-SNP: unable to query CPUID values for CPU: '%s'",
- strerror(-ret));
+error_setg(errp, "SEV-SNP: unable to query CPUID values for CPU: '%s'",
+ strerror(-ret));
return 1;
}
ret = sev_snp_cpuid_info_fill(&snp_cpuid_info, &kvm_cpuid_info);
if (ret) {
-error_report("SEV-SNP: failed to generate CPUID table information");
+error_setg(errp, "SEV-SNP: failed to generate CPUID table
information");
return 1;
}
memcpy(hva, &snp_cpuid_info, sizeof(snp_cpuid_info));
return snp_launch_update_data(cpuid_addr, hva, cpuid_len,
- KVM_SEV_SNP_PAGE_TYPE_CPUID);
+ KVM_SEV_SNP_PAGE_TYPE_CPUID, errp);
}
-static int
-snp_launch_update_kernel_hashes(SevSnpGuestState *sev_snp, uint32_t addr,
-void *hva, uint32_t len)
+static int snp_launch_update_kernel_hashes(SevSnpGuestState *sev_snp,
+ uint32_t addr, void *hva,
+
[PATCH v4 00/17] Introduce support for IGVM files
Here is v4 of the set of patches to add support for IGVM files to QEMU. This is based on commit 1a2d52c7fc of qemu. This version addresses all of the review comments from v3 along with a couple of small bug fixes. This is a much smaller increment than in the previous version of the series [1]. Thanks once again to the reviewers that have been looking at this series. This v4 patch series is also available on github: [2] The previous version had a build issue when building without debug enabled. Patch 8/17 has been added to fix this and I've updated my own process to test both debug and release builds of QEMU. For testing IGVM support in QEMU you need to generate an IGVM file that is configured for the platform you want to launch. You can use the `buildigvm` test tool [3] to allow generation of IGVM files for all currently supported platforms. Patch 11/17 contains information on how to generate an IGVM file using this tool. Changes in v4: * Remove unused '#ifdef CONFIG_IGVM' sections * Add "'if': 'CONFIG_IGVM'" for IgvmCfgProperties in qom.json * Use error_fatal instead of error_abort in suggested locations * Prevent addition of bios code when an IGVM file is provided and pci_enabled is false * Add patch 6/17 to fix error handling from sev_encrypt_flash() * Revert unrequired changes to return values in sev/*_launch_update() functions * Add documentation to igvm.rst to describe how to use 'buildigvm' * Various convention and code style changes as suggested in reviews * Fix handling of sev_features for kernels that do not support KVM_SEV_INIT2 * Move igvm-cfg from MachineState to X86MachineState Patch summary: 1-12: Add support and documentation for processing IGVM files for SEV, SEV-ES, SEV-SNP and native platforms. 13-16: Processing of policy and SEV-SNP ID_BLOCK from IGVM file. 17: Add pre-processing of IGVM file to support synchronization of 'SEV_FEATURES' from IGVM VMSA to KVM. [1] Link to v3: https://lore.kernel.org/qemu-devel/cover.1718979106.git.roy.hopk...@suse.com/ [2] v4 patches also available here: https://github.com/roy-hopkins/qemu/tree/igvm_master_v4 [3] `buildigvm` tool v0.2.0 https://github.com/roy-hopkins/buildigvm/releases/tag/v0.2.0 Roy Hopkins (17): meson: Add optional dependency on IGVM library backends/confidential-guest-support: Add functions to support IGVM backends/igvm: Add IGVM loader and configuration hw/i386: Add igvm-cfg object and processing for IGVM files i386/pc_sysfw: Ensure sysfw flash configuration does not conflict with IGVM sev: Fix error handling in sev_encrypt_flash() sev: Update launch_update_data functions to use Error handling target/i386: Allow setting of R_LDTR and R_TR with cpu_x86_load_seg_cache() i386/sev: Refactor setting of reset vector and initial CPU state i386/sev: Implement ConfidentialGuestSupport functions for SEV docs/system: Add documentation on support for IGVM docs/interop/firmware.json: Add igvm to FirmwareDevice backends/confidential-guest-support: Add set_guest_policy() function backends/igvm: Process initialization sections in IGVM file backends/igvm: Handle policy for SEV guests i386/sev: Add implementation of CGS set_guest_policy() sev: Provide sev_features flags from IGVM VMSA to KVM_SEV_INIT2 docs/interop/firmware.json | 9 +- docs/system/i386/amd-memory-encryption.rst | 2 + docs/system/igvm.rst | 173 docs/system/index.rst | 1 + meson.build| 8 + qapi/qom.json | 17 + backends/igvm.h| 23 + include/exec/confidential-guest-support.h | 96 +++ include/hw/i386/x86.h | 3 + include/sysemu/igvm-cfg.h | 54 ++ target/i386/cpu.h | 9 +- target/i386/sev.h | 124 +++ backends/confidential-guest-support.c | 43 + backends/igvm-cfg.c| 66 ++ backends/igvm.c| 958 + hw/i386/pc.c | 12 + hw/i386/pc_piix.c | 10 + hw/i386/pc_q35.c | 10 + hw/i386/pc_sysfw.c | 31 +- target/i386/sev.c | 844 -- backends/meson.build | 5 + meson_options.txt | 2 + qemu-options.hx| 25 + scripts/meson-buildoptions.sh | 3 + 24 files changed, 2447 insertions(+), 81 deletions(-) create mode 100644 docs/system/igvm.rst create mode 100644 backends/igvm.h create mode 100644 include/sysemu/igvm-cfg.h create mode 100644 backends/igvm-cfg.c create mode 100644 backends/igvm.c -- 2.43.0
[PATCH v4 09/17] i386/sev: Refactor setting of reset vector and initial CPU state
When an SEV guest is started, the reset vector and state are
extracted from metadata that is contained in the firmware volume.
In preparation for using IGVM to setup the initial CPU state,
the code has been refactored to populate vmcb_save_area for each
CPU which is then applied during guest startup and CPU reset.
Signed-off-by: Roy Hopkins
---
target/i386/sev.h | 110
target/i386/sev.c | 323 +-
2 files changed, 400 insertions(+), 33 deletions(-)
diff --git a/target/i386/sev.h b/target/i386/sev.h
index 858005a119..167dd154d6 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -45,6 +45,116 @@ typedef struct SevKernelLoaderContext {
size_t cmdline_size;
} SevKernelLoaderContext;
+/* Save area definition for SEV-ES and SEV-SNP guests */
+struct QEMU_PACKED sev_es_save_area {
+struct vmcb_seg es;
+struct vmcb_seg cs;
+struct vmcb_seg ss;
+struct vmcb_seg ds;
+struct vmcb_seg fs;
+struct vmcb_seg gs;
+struct vmcb_seg gdtr;
+struct vmcb_seg ldtr;
+struct vmcb_seg idtr;
+struct vmcb_seg tr;
+uint64_t vmpl0_ssp;
+uint64_t vmpl1_ssp;
+uint64_t vmpl2_ssp;
+uint64_t vmpl3_ssp;
+uint64_t u_cet;
+uint8_t reserved_0xc8[2];
+uint8_t vmpl;
+uint8_t cpl;
+uint8_t reserved_0xcc[4];
+uint64_t efer;
+uint8_t reserved_0xd8[104];
+uint64_t xss;
+uint64_t cr4;
+uint64_t cr3;
+uint64_t cr0;
+uint64_t dr7;
+uint64_t dr6;
+uint64_t rflags;
+uint64_t rip;
+uint64_t dr0;
+uint64_t dr1;
+uint64_t dr2;
+uint64_t dr3;
+uint64_t dr0_addr_mask;
+uint64_t dr1_addr_mask;
+uint64_t dr2_addr_mask;
+uint64_t dr3_addr_mask;
+uint8_t reserved_0x1c0[24];
+uint64_t rsp;
+uint64_t s_cet;
+uint64_t ssp;
+uint64_t isst_addr;
+uint64_t rax;
+uint64_t star;
+uint64_t lstar;
+uint64_t cstar;
+uint64_t sfmask;
+uint64_t kernel_gs_base;
+uint64_t sysenter_cs;
+uint64_t sysenter_esp;
+uint64_t sysenter_eip;
+uint64_t cr2;
+uint8_t reserved_0x248[32];
+uint64_t g_pat;
+uint64_t dbgctl;
+uint64_t br_from;
+uint64_t br_to;
+uint64_t last_excp_from;
+uint64_t last_excp_to;
+uint8_t reserved_0x298[80];
+uint32_t pkru;
+uint32_t tsc_aux;
+uint8_t reserved_0x2f0[24];
+uint64_t rcx;
+uint64_t rdx;
+uint64_t rbx;
+uint64_t reserved_0x320; /* rsp already available at 0x01d8 */
+uint64_t rbp;
+uint64_t rsi;
+uint64_t rdi;
+uint64_t r8;
+uint64_t r9;
+uint64_t r10;
+uint64_t r11;
+uint64_t r12;
+uint64_t r13;
+uint64_t r14;
+uint64_t r15;
+uint8_t reserved_0x380[16];
+uint64_t guest_exit_info_1;
+uint64_t guest_exit_info_2;
+uint64_t guest_exit_int_info;
+uint64_t guest_nrip;
+uint64_t sev_features;
+uint64_t vintr_ctrl;
+uint64_t guest_exit_code;
+uint64_t virtual_tom;
+uint64_t tlb_id;
+uint64_t pcpu_id;
+uint64_t event_inj;
+uint64_t xcr0;
+uint8_t reserved_0x3f0[16];
+
+/* Floating point area */
+uint64_t x87_dp;
+uint32_t mxcsr;
+uint16_t x87_ftw;
+uint16_t x87_fsw;
+uint16_t x87_fcw;
+uint16_t x87_fop;
+uint16_t x87_ds;
+uint16_t x87_cs;
+uint64_t x87_rip;
+uint8_t fpreg_x87[80];
+uint8_t fpreg_xmm[256];
+uint8_t fpreg_ymm[256];
+};
+
#ifdef CONFIG_SEV
bool sev_enabled(void);
bool sev_es_enabled(void);
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 5eabeadda6..7487971344 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -49,6 +49,12 @@ OBJECT_DECLARE_TYPE(SevSnpGuestState, SevCommonStateClass,
SEV_SNP_GUEST)
/* hard code sha256 digest size */
#define HASH_SIZE 32
+/* Convert between SEV-ES VMSA and SegmentCache flags/attributes */
+#define FLAGS_VMSA_TO_SEGCACHE(flags) \
+flags) & 0xff00) << 12) | (((flags) & 0xff) << 8))
+#define FLAGS_SEGCACHE_TO_VMSA(flags) \
+flags) & 0xff00) >> 8) | (((flags) & 0xf0) >> 12))
+
typedef struct QEMU_PACKED SevHashTableEntry {
QemuUUID guid;
uint16_t len;
@@ -88,6 +94,14 @@ typedef struct QEMU_PACKED SevHashTableDescriptor {
uint32_t size;
} SevHashTableDescriptor;
+typedef struct SevLaunchVmsa {
+QTAILQ_ENTRY(SevLaunchVmsa) next;
+
+uint16_t cpu_index;
+uint64_t gpa;
+struct sev_es_save_area vmsa;
+} SevLaunchVmsa;
+
struct SevCommonState {
X86ConfidentialGuest parent_obj;
@@ -106,9 +120,7 @@ struct SevCommonState {
int sev_fd;
SevState state;
-uint32_t reset_cs;
-uint32_t reset_ip;
-bool reset_data_valid;
+QTAILQ_HEAD(, SevLaunchVmsa) launch_vmsa;
};
struct SevCommonStateClass {
@@ -371,6 +383,172 @@ static struct RAMBlockNotifier sev_ram_notifier = {
.ram_block_removed = sev_ram_block_removed,
};
+static void sev_apply_cpu_context(CPUState *cpu)
+{
+SevCommonState *sev_common = SEV_COMMON(MACHINE(qdev_
[PATCH v4 17/17] sev: Provide sev_features flags from IGVM VMSA to KVM_SEV_INIT2
IGVM files can contain an initial VMSA that should be applied to each
vcpu as part of the initial guest state. The sev_features flags are
provided as part of the VMSA structure. However, KVM only allows
sev_features to be set during initialization and not as the guest is
being prepared for launch.
This patch queries KVM for the supported set of sev_features flags and
processes the IGVM file during kvm_init to determine any sev_features
flags set in the IGVM file. These are then provided in the call to
KVM_SEV_INIT2 to ensure the guest state matches that specified in the
IGVM file.
This does cause the IGVM file to be processed twice. Firstly to extract
the sev_features then secondly to actually configure the guest. However,
the first pass is largely ignored meaning the overhead is minimal.
Signed-off-by: Roy Hopkins
---
target/i386/sev.c | 160 --
1 file changed, 141 insertions(+), 19 deletions(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index c4d0682d75..be64b3f902 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -117,6 +117,8 @@ struct SevCommonState {
uint32_t cbitpos;
uint32_t reduced_phys_bits;
bool kernel_hashes;
+uint64_t sev_features;
+uint64_t supported_sev_features;
/* runtime state */
uint8_t api_major;
@@ -492,7 +494,40 @@ static void sev_apply_cpu_context(CPUState *cpu)
}
}
-static int check_vmsa_supported(hwaddr gpa, const struct sev_es_save_area
*vmsa,
+static int check_sev_features(SevCommonState *sev_common, uint64_t
sev_features,
+ Error **errp)
+{
+/*
+ * Ensure SEV_FEATURES is configured for correct SEV hardware and that
+ * the requested features are supported. If SEV-SNP is enabled then
+ * that feature must be enabled, otherwise it must be cleared.
+ */
+if (sev_snp_enabled() && !(sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
+error_setg(
+errp,
+"%s: SEV_SNP is enabled but is not enabled in VMSA sev_features",
+__func__);
+return -1;
+} else if (!sev_snp_enabled() &&
+ (sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
+error_setg(
+errp,
+"%s: SEV_SNP is not enabled but is enabled in VMSA sev_features",
+__func__);
+return -1;
+}
+if (sev_features & ~sev_common->supported_sev_features) {
+error_setg(errp,
+ "%s: VMSA contains unsupported sev_features: %lX, "
+ "supported features: %lX",
+ __func__, sev_features, sev_common->supported_sev_features);
+return -1;
+}
+return 0;
+}
+
+static int check_vmsa_supported(SevCommonState *sev_common, hwaddr gpa,
+const struct sev_es_save_area *vmsa,
Error **errp)
{
struct sev_es_save_area vmsa_check;
@@ -558,24 +593,10 @@ static int check_vmsa_supported(hwaddr gpa, const struct
sev_es_save_area *vmsa,
vmsa_check.x87_fcw = 0;
vmsa_check.mxcsr = 0;
-if (sev_snp_enabled()) {
-if (vmsa_check.sev_features != SVM_SEV_FEAT_SNP_ACTIVE) {
-error_setg(errp,
- "%s: sev_features in the VMSA contains an unsupported "
- "value. For SEV-SNP, sev_features must be set to %x.",
- __func__, SVM_SEV_FEAT_SNP_ACTIVE);
-return -1;
-}
-vmsa_check.sev_features = 0;
-} else {
-if (vmsa_check.sev_features != 0) {
-error_setg(errp,
- "%s: sev_features in the VMSA contains an unsupported "
- "value. For SEV-ES and SEV, sev_features must be "
- "set to 0.", __func__);
-return -1;
-}
+if (check_sev_features(sev_common, vmsa_check.sev_features, errp) < 0) {
+return -1;
}
+vmsa_check.sev_features = 0;
if (!buffer_is_zero(&vmsa_check, sizeof(vmsa_check))) {
error_setg(errp,
@@ -1665,6 +1686,39 @@ static int sev_snp_kvm_type(X86ConfidentialGuest *cg)
return KVM_X86_SNP_VM;
}
+static int sev_init_supported_features(ConfidentialGuestSupport *cgs,
+ SevCommonState *sev_common, Error
**errp)
+{
+X86ConfidentialGuestClass *x86_klass =
+ X86_CONFIDENTIAL_GUEST_GET_CLASS(cgs);
+/*
+ * Older kernels do not support query or setting of sev_features. In this
+ * case the set of supported features must be zero to match the settings
+ * in the kernel.
+ */
+if (x86_klass->kvm_type(X86_CONFIDENTIAL_GUEST(sev_common)) ==
+KVM_X86_DEFAULT_VM) {
+sev_common->supported_sev_features = 0;
+return 0;
+}
+
+/* Query KVM for the supported set of sev_features */
+struct kvm_device_attr attr = {
+.group = KVM_X86_GRP_SEV,
+.attr = KVM_X86_SEV_VMSA_
[PATCH v4 15/17] backends/igvm: Handle policy for SEV guests
Adds a handler for the guest policy initialization IGVM section and
builds an SEV policy based on this information and the ID block
directive if present. The policy is applied using by calling
'set_guest_policy()' on the ConfidentialGuestSupport object.
Signed-off-by: Roy Hopkins
---
backends/igvm.c | 138
1 file changed, 138 insertions(+)
diff --git a/backends/igvm.c b/backends/igvm.c
index fa074b9107..3589f977ca 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -28,6 +28,33 @@ typedef struct IgvmParameterData {
uint32_t index;
} IgvmParameterData;
+/*
+ * Some directives are specific to particular confidential computing platforms.
+ * Define required types for each of those platforms here.
+ */
+
+/* SEV/SEV-ES/SEV-SNP */
+struct QEMU_PACKED sev_id_block {
+uint8_t ld[48];
+uint8_t family_id[16];
+uint8_t image_id[16];
+uint32_t version;
+uint32_t guest_svn;
+uint64_t policy;
+};
+
+struct QEMU_PACKED sev_id_authentication {
+uint32_t id_key_alg;
+uint32_t auth_key_algo;
+uint8_t reserved[56];
+uint8_t id_block_sig[512];
+uint8_t id_key[1028];
+uint8_t reserved2[60];
+uint8_t id_key_sig[512];
+uint8_t author_key[1028];
+uint8_t reserved3[892];
+};
+
/*
* QemuIgvm contains the information required during processing
* of a single IGVM file.
@@ -39,6 +66,17 @@ typedef struct QemuIgvm {
uint32_t compatibility_mask;
unsigned current_header_index;
QTAILQ_HEAD(, IgvmParameterData) parameter_data;
+IgvmPlatformType platform_type;
+
+/*
+ * SEV-SNP platforms can contain an ID block and authentication
+ * that should be verified by the guest.
+ */
+struct sev_id_block *id_block;
+struct sev_id_authentication *id_auth;
+
+/* Define the guest policy for SEV guests */
+uint64_t sev_policy;
/* These variables keep track of contiguous page regions */
IGVM_VHS_PAGE_DATA region_prev_page_data;
@@ -64,6 +102,11 @@ static int directive_environment_info(QemuIgvm *ctx, const
uint8_t *header_data,
Error **errp);
static int directive_required_memory(QemuIgvm *ctx, const uint8_t *header_data,
Error **errp);
+static int directive_snp_id_block(QemuIgvm *ctx, const uint8_t *header_data,
+ Error **errp);
+static int initialization_guest_policy(QemuIgvm *ctx,
+ const uint8_t *header_data,
+ Error **errp);
struct IGVMHandler {
uint32_t type;
@@ -87,6 +130,10 @@ static struct IGVMHandler handlers[] = {
directive_environment_info },
{ IGVM_VHT_REQUIRED_MEMORY, IGVM_HEADER_SECTION_DIRECTIVE,
directive_required_memory },
+{ IGVM_VHT_SNP_ID_BLOCK, IGVM_HEADER_SECTION_DIRECTIVE,
+ directive_snp_id_block },
+{ IGVM_VHT_GUEST_POLICY, IGVM_HEADER_SECTION_INITIALIZATION,
+ initialization_guest_policy },
};
static int handler(QemuIgvm *ctx, uint32_t type, Error **errp)
@@ -627,6 +674,70 @@ static int directive_required_memory(QemuIgvm *ctx, const
uint8_t *header_data,
return 0;
}
+static int directive_snp_id_block(QemuIgvm *ctx, const uint8_t *header_data,
+ Error **errp)
+{
+const IGVM_VHS_SNP_ID_BLOCK *igvm_id =
+(const IGVM_VHS_SNP_ID_BLOCK *)header_data;
+
+if (!(igvm_id->compatibility_mask & ctx->compatibility_mask)) {
+return 0;
+}
+
+if (ctx->id_block) {
+error_setg(errp, "IGVM: Multiple ID blocks encountered "
+"in IGVM file.");
+return -1;
+}
+ctx->id_block = g_new0(struct sev_id_block, 1);
+ctx->id_auth = g_new0(struct sev_id_authentication, 1);
+
+memcpy(ctx->id_block->family_id, igvm_id->family_id,
+sizeof(ctx->id_block->family_id));
+memcpy(ctx->id_block->image_id, igvm_id->image_id,
+sizeof(ctx->id_block->image_id));
+ctx->id_block->guest_svn = igvm_id->guest_svn;
+ctx->id_block->version = 1;
+memcpy(ctx->id_block->ld, igvm_id->ld, sizeof(ctx->id_block->ld));
+
+ctx->id_auth->id_key_alg = igvm_id->id_key_algorithm;
+memcpy(ctx->id_auth->id_block_sig, &igvm_id->id_key_signature,
+sizeof(igvm_id->id_key_signature));
+
+ctx->id_auth->auth_key_algo = igvm_id->author_key_algorithm;
+memcpy(ctx->id_auth->id_key_sig, &igvm_id->author_key_signature,
+sizeof(igvm_id->author_key_signature));
+
+/*
+ * SEV and IGVM public key structure population are slightly different.
+ * See SEV Secure Nested Paging Firmware ABI Specification, Chapter 10.
+ */
+*((uint32_t *)ctx->id_auth->id_key) = igvm_id->id_public_key.curve;
+memcpy(&ctx->id_auth->id_key[4], &igvm_id->id_public_key.qx, 72);
+memcpy(&ctx->id_auth->id_key[76], &igvm_id->id_public_key.qy, 72);
+
+*((uint32_t *)ctx->id_auth->a
[PATCH v4 11/17] docs/system: Add documentation on support for IGVM
IGVM support has been implemented for Confidential Guests that support AMD SEV and AMD SEV-ES. Add some documentation that gives some background on the IGVM format and how to use it to configure a confidential guest. Signed-off-by: Roy Hopkins --- docs/system/i386/amd-memory-encryption.rst | 2 + docs/system/igvm.rst | 173 + docs/system/index.rst | 1 + 3 files changed, 176 insertions(+) create mode 100644 docs/system/igvm.rst diff --git a/docs/system/i386/amd-memory-encryption.rst b/docs/system/i386/amd-memory-encryption.rst index 748f5094ba..6c23f3535f 100644 --- a/docs/system/i386/amd-memory-encryption.rst +++ b/docs/system/i386/amd-memory-encryption.rst @@ -1,3 +1,5 @@ +.. _amd-sev: + AMD Secure Encrypted Virtualization (SEV) = diff --git a/docs/system/igvm.rst b/docs/system/igvm.rst new file mode 100644 index 00..36146a81df --- /dev/null +++ b/docs/system/igvm.rst @@ -0,0 +1,173 @@ +Independent Guest Virtual Machine (IGVM) support + + +IGVM files are designed to encapsulate all the information required to launch a +virtual machine on any given virtualization stack in a deterministic way. This +allows the cryptographic measurement of initial guest state for Confidential +Guests to be calculated when the IGVM file is built, allowing a relying party to +verify the initial state of a guest via a remote attestation. + +Although IGVM files are designed with Confidential Computing in mind, they can +also be used to configure non-confidential guests. Multiple platforms can be +defined by a single IGVM file, allowing a single IGVM file to configure a +virtual machine that can run on, for example, TDX, SEV and non-confidential +hosts. + +QEMU supports IGVM files through the user-creatable ``igvm-cfg`` object. This +object is used to define the filename of the IGVM file to process. A reference +to the object is added to the ``-machine`` to configure the virtual machine +to use the IGVM file for configuration. + +Confidential platform support is provided through the use of +the ``ConfidentialGuestSupport`` object. If the virtual machine provides an +instance of this object then this is used by the IGVM loader to configure the +isolation properties of the directives within the file. + +Further Information on IGVM +--- + +Information about the IGVM format, including links to the format specification +and documentation for the Rust and C libraries can be found at the project +repository: + +https://github.com/microsoft/igvm + + +Supported Platforms +--- + +Currently, IGVM files can be provided for Confidential Guests on host systems +that support AMD SEV, SEV-ES and SEV-SNP with KVM. IGVM files can also be +provided for non-confidential guests. + + +Limitations when using IGVM with AMD SEV, SEV-ES and SEV-SNP + + +IGVM files configure the initial state of the guest using a set of directives. +Not every directive is supported by every Confidential Guest type. For example, +AMD SEV does not support encrypted save state regions, therefore setting the +initial CPU state using IGVM for SEV is not possible. When an IGVM file contains +directives that are not supported for the active platform, an error is generated +and the guest launch is aborted. + +The table below describes the list of directives that are supported for SEV, +SEV-ES, SEV-SNP and non-confidential platforms. + +.. list-table:: SEV, SEV-ES, SEV-SNP & non-confidential Supported Directives + :widths: 35 65 + :header-rows: 1 + + * - IGVM directive + - Notes + * - IGVM_VHT_PAGE_DATA + - ``NORMAL`` zero, measured and unmeasured page types are supported. Other + page types result in an error. + * - IGVM_VHT_PARAMETER_AREA + - + * - IGVM_VHT_PARAMETER_INSERT + - + * - IGVM_VHT_VP_COUNT_PARAMETER + - The guest parameter page is populated with the CPU count. + * - IGVM_VHT_ENVIRONMENT_INFO_PARAMETER + - The ``memory_is_shared`` parameter is set to 1 in the guest parameter + page. + +.. list-table:: Additional SEV, SEV-ES & SEV_SNP Supported Directives + :widths: 25 75 + :header-rows: 1 + + * - IGVM directive + - Notes + * - IGVM_VHT_MEMORY_MAP + - The memory map page is populated using entries from the E820 table. + * - IGVM_VHT_REQUIRED_MEMORY + - + +.. list-table:: Additional SEV-ES & SEV-SNP Supported Directives + :widths: 25 75 + :header-rows: 1 + + * - IGVM directive + - Notes + * - IGVM_VHT_VP_CONTEXT + - Setting of the initial CPU state for the boot CPU and additional CPUs is + supported with limitations on the fields that can be provided in the + VMSA. See below for details on which fields are supported. + +Initial CPU state with VMSA +--- + +The initial state of
[PATCH v4 12/17] docs/interop/firmware.json: Add igvm to FirmwareDevice
Create an enum entry within FirmwareDevice for 'igvm' to describe that
an IGVM file can be used to map firmware into memory as an alternative
to pre-existing firmware devices.
Signed-off-by: Roy Hopkins
---
docs/interop/firmware.json | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index 54a1fc6c10..4a696adc22 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -55,10 +55,17 @@
#
# @memory: The firmware is to be mapped into memory.
#
+# @igvm: The firmware is defined by a file conforming to the IGVM
+#specification and mapped into memory according to directives
+#defined in the file. This is similar to @memory but may
+#include additional processing defined by the IGVM file
+#including initial CPU state or population of metadata into
+#the guest address space. Since: 9.1
+#
# Since: 3.0
##
{ 'enum' : 'FirmwareDevice',
- 'data' : [ 'flash', 'kernel', 'memory' ] }
+ 'data' : [ 'flash', 'kernel', 'memory', 'igvm' ] }
##
# @FirmwareTarget:
--
2.43.0
[PATCH v4 02/17] backends/confidential-guest-support: Add functions to support IGVM
In preparation for supporting the processing of IGVM files to configure
guests, this adds a set of functions to ConfidentialGuestSupport
allowing configuration of secure virtual machines that can be
implemented for each supported isolation platform type such as Intel TDX
or AMD SEV-SNP. These functions will be called by IGVM processing code
in subsequent patches.
This commit provides a default implementation of the functions that
either perform no action or generate an error when they are called.
Targets that support ConfidentalGuestSupport should override these
implementations.
Signed-off-by: Roy Hopkins
---
include/exec/confidential-guest-support.h | 75 +++
backends/confidential-guest-support.c | 31 ++
2 files changed, 106 insertions(+)
diff --git a/include/exec/confidential-guest-support.h
b/include/exec/confidential-guest-support.h
index 02dc4e518f..4834efbe38 100644
--- a/include/exec/confidential-guest-support.h
+++ b/include/exec/confidential-guest-support.h
@@ -21,6 +21,7 @@
#ifndef CONFIG_USER_ONLY
#include "qom/object.h"
+#include "exec/hwaddr.h"
#define TYPE_CONFIDENTIAL_GUEST_SUPPORT "confidential-guest-support"
OBJECT_DECLARE_TYPE(ConfidentialGuestSupport,
@@ -28,6 +29,36 @@ OBJECT_DECLARE_TYPE(ConfidentialGuestSupport,
CONFIDENTIAL_GUEST_SUPPORT)
+typedef enum ConfidentialGuestPlatformType {
+CGS_PLATFORM_SEV,
+CGS_PLATFORM_SEV_ES,
+CGS_PLATFORM_SEV_SNP,
+} ConfidentialGuestPlatformType;
+
+typedef enum ConfidentialGuestMemoryType {
+CGS_MEM_RAM,
+CGS_MEM_RESERVED,
+CGS_MEM_ACPI,
+CGS_MEM_NVS,
+CGS_MEM_UNUSABLE,
+} ConfidentialGuestMemoryType;
+
+typedef struct ConfidentialGuestMemoryMapEntry {
+uint64_t gpa;
+uint64_t size;
+ConfidentialGuestMemoryType type;
+} ConfidentialGuestMemoryMapEntry;
+
+typedef enum ConfidentialGuestPageType {
+CGS_PAGE_TYPE_NORMAL,
+CGS_PAGE_TYPE_VMSA,
+CGS_PAGE_TYPE_ZERO,
+CGS_PAGE_TYPE_UNMEASURED,
+CGS_PAGE_TYPE_SECRETS,
+CGS_PAGE_TYPE_CPUID,
+CGS_PAGE_TYPE_REQUIRED_MEMORY,
+} ConfidentialGuestPageType;
+
struct ConfidentialGuestSupport {
Object parent;
@@ -66,6 +97,40 @@ typedef struct ConfidentialGuestSupportClass {
int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp);
int (*kvm_reset)(ConfidentialGuestSupport *cgs, Error **errp);
+
+/*
+ * Check for to see if this confidential guest supports a particular
+ * platform or configuration
+ */
+int (*check_support)(ConfidentialGuestPlatformType platform,
+ uint16_t platform_version, uint8_t highest_vtl,
+ uint64_t shared_gpa_boundary);
+
+/*
+ * Configure part of the state of a guest for a particular set of data,
page
+ * type and gpa. This can be used for example to pre-populate and measure
+ * guest memory contents, define private ranges or set the initial CPU
state
+ * for one or more CPUs.
+ *
+ * If memory_type is CGS_PAGE_TYPE_VMSA then ptr points to the initial CPU
+ * context for a virtual CPU. The format of the data depends on the type of
+ * confidential virtual machine. For example, for SEV-ES ptr will point to
a
+ * vmcb_save_area structure that should be copied into guest memory at the
+ * address specified in gpa. The cpu_index parameter contains the index of
+ * the CPU the VMSA applies to.
+ */
+int (*set_guest_state)(hwaddr gpa, uint8_t *ptr, uint64_t len,
+ ConfidentialGuestPageType memory_type,
+ uint16_t cpu_index, Error **errp);
+
+/*
+ * Iterate the system memory map, getting the entry with the given index
+ * that can be populated into guest memory.
+ *
+ * Returns 0 for ok, 1 if the index is out of range and -1 on error.
+ */
+int (*get_mem_map_entry)(int index, ConfidentialGuestMemoryMapEntry *entry,
+ Error **errp);
} ConfidentialGuestSupportClass;
static inline int confidential_guest_kvm_init(ConfidentialGuestSupport *cgs,
@@ -94,6 +159,16 @@ static inline int
confidential_guest_kvm_reset(ConfidentialGuestSupport *cgs,
return 0;
}
+#define CONFIDENTIAL_GUEST_SUPPORT_CLASS(klass)\
+OBJECT_CLASS_CHECK(ConfidentialGuestSupportClass, (klass), \
+ TYPE_CONFIDENTIAL_GUEST_SUPPORT)
+#define CONFIDENTIAL_GUEST_SUPPORT(obj) \
+OBJECT_CHECK(ConfidentialGuestSupport, (obj), \
+ TYPE_CONFIDENTIAL_GUEST_SUPPORT)
+#define CONFIDENTIAL_GUEST_SUPPORT_GET_CLASS(obj) \
+OBJECT_GET_CLASS(ConfidentialGuestSupportClass, (obj), \
+ TYPE_CONFIDENTIAL_GUEST_SUPPORT)
+
#endif /* !CONFIG_USER_ONLY */
#endif /* QEMU_CONFIDENTIAL_GUEST_SUPPORT_H */
diff --git a/backends/confidential-guest-support.c
b/backends/confidential-guest-support.c
index 052fde8db0..68e6fd9d18 100644
--- a/backends/confid
[PATCH v4 13/17] backends/confidential-guest-support: Add set_guest_policy() function
For confidential guests a policy can be provided that defines the
security level, debug status, expected launch measurement and other
parameters that define the configuration of the confidential platform.
This commit adds a new function named set_guest_policy() that can be
implemented by each confidential platform, such as AMD SEV to set the
policy. This will allow configuration of the policy from a
multi-platform resource such as an IGVM file without the IGVM processor
requiring specific implementation details for each platform.
Signed-off-by: Roy Hopkins
---
include/exec/confidential-guest-support.h | 21 +
backends/confidential-guest-support.c | 12
2 files changed, 33 insertions(+)
diff --git a/include/exec/confidential-guest-support.h
b/include/exec/confidential-guest-support.h
index 4834efbe38..218bab9714 100644
--- a/include/exec/confidential-guest-support.h
+++ b/include/exec/confidential-guest-support.h
@@ -59,6 +59,10 @@ typedef enum ConfidentialGuestPageType {
CGS_PAGE_TYPE_REQUIRED_MEMORY,
} ConfidentialGuestPageType;
+typedef enum ConfidentialGuestPolicyType {
+GUEST_POLICY_SEV,
+} ConfidentialGuestPolicyType;
+
struct ConfidentialGuestSupport {
Object parent;
@@ -123,6 +127,23 @@ typedef struct ConfidentialGuestSupportClass {
ConfidentialGuestPageType memory_type,
uint16_t cpu_index, Error **errp);
+/*
+ * Set the guest policy. The policy can be used to configure the
+ * confidential platform, such as if debug is enabled or not and can
contain
+ * information about expected launch measurements, signed verification of
+ * guest configuration and other platform data.
+ *
+ * The format of the policy data is specific to each platform. For example,
+ * SEV-SNP uses a policy bitfield in the 'policy' argument and provides an
+ * ID block and ID authentication in the 'policy_data' parameters. The type
+ * of policy data is identified by the 'policy_type' argument.
+ */
+int (*set_guest_policy)(ConfidentialGuestPolicyType policy_type,
+uint64_t policy,
+void *policy_data1, uint32_t policy_data1_size,
+void *policy_data2, uint32_t policy_data2_size,
+Error **errp);
+
/*
* Iterate the system memory map, getting the entry with the given index
* that can be populated into guest memory.
diff --git a/backends/confidential-guest-support.c
b/backends/confidential-guest-support.c
index 68e6fd9d18..3c46b2cd6b 100644
--- a/backends/confidential-guest-support.c
+++ b/backends/confidential-guest-support.c
@@ -38,6 +38,17 @@ static int set_guest_state(hwaddr gpa, uint8_t *ptr,
uint64_t len,
return -1;
}
+static int set_guest_policy(ConfidentialGuestPolicyType policy_type,
+uint64_t policy,
+void *policy_data1, uint32_t policy_data1_size,
+void *policy_data2, uint32_t policy_data2_size,
+Error **errp)
+{
+error_setg(errp,
+ "Setting confidential guest policy is not supported for this
platform");
+return -1;
+}
+
static int get_mem_map_entry(int index, ConfidentialGuestMemoryMapEntry *entry,
Error **errp)
{
@@ -52,6 +63,7 @@ static void confidential_guest_support_class_init(ObjectClass
*oc, void *data)
ConfidentialGuestSupportClass *cgsc = CONFIDENTIAL_GUEST_SUPPORT_CLASS(oc);
cgsc->check_support = check_support;
cgsc->set_guest_state = set_guest_state;
+cgsc->set_guest_policy = set_guest_policy;
cgsc->get_mem_map_entry = get_mem_map_entry;
}
--
2.43.0
[PATCH v4 16/17] i386/sev: Add implementation of CGS set_guest_policy()
The new cgs_set_guest_policy() function is provided to receive the guest
policy flags, SNP ID block and SNP ID authentication from guest
configuration such as an IGVM file and apply it to the platform prior to
launching the guest.
The policy is used to populate values for the existing 'policy',
'id_block' and 'id_auth' parameters. When provided, the guest policy is
applied and the ID block configuration is used to verify the launch
measurement and signatures. The guest is only successfully started if
the expected launch measurements match the actual measurements and the
signatures are valid.
Signed-off-by: Roy Hopkins
---
target/i386/sev.h | 12 +++
target/i386/sev.c | 83 +++
2 files changed, 95 insertions(+)
diff --git a/target/i386/sev.h b/target/i386/sev.h
index 2ccd6fe1e8..7b92102bd0 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -157,6 +157,18 @@ struct QEMU_PACKED sev_es_save_area {
uint8_t fpreg_ymm[256];
};
+struct QEMU_PACKED sev_snp_id_authentication {
+uint32_t id_key_alg;
+uint32_t auth_key_algo;
+uint8_t reserved[56];
+uint8_t id_block_sig[512];
+uint8_t id_key[1028];
+uint8_t reserved2[60];
+uint8_t id_key_sig[512];
+uint8_t author_key[1028];
+uint8_t reserved3[892];
+};
+
#ifdef CONFIG_SEV
bool sev_enabled(void);
bool sev_es_enabled(void);
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 9a05b971c8..c4d0682d75 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -2446,6 +2446,88 @@ static int cgs_get_mem_map_entry(int index,
return 0;
}
+static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
+uint64_t policy, void *policy_data1,
+uint32_t policy_data1_size, void *policy_data2,
+uint32_t policy_data2_size, Error **errp)
+{
+if (policy_type != GUEST_POLICY_SEV) {
+error_setg(errp, "%s: Invalid guest policy type provided for SEV: %d",
+__func__, policy_type);
+return -1;
+}
+/*
+ * SEV-SNP handles policy differently. The policy flags are defined in
+ * kvm_start_conf.policy and an ID block and ID auth can be provided.
+ */
+if (sev_snp_enabled()) {
+SevSnpGuestState *sev_snp_guest =
+SEV_SNP_GUEST(MACHINE(qdev_get_machine())->cgs);
+struct kvm_sev_snp_launch_finish *finish =
+&sev_snp_guest->kvm_finish_conf;
+
+/*
+ * The policy consists of flags in 'policy' and optionally an ID block
+ * and ID auth in policy_data1 and policy_data2 respectively. The ID
+ * block and auth are optional so clear any previous ID block and auth
+ * and set them if provided, but always set the policy flags.
+ */
+g_free(sev_snp_guest->id_block);
+g_free((guchar *)finish->id_block_uaddr);
+g_free(sev_snp_guest->id_auth);
+g_free((guchar *)finish->id_auth_uaddr);
+sev_snp_guest->id_block = NULL;
+finish->id_block_uaddr = 0;
+sev_snp_guest->id_auth = NULL;
+finish->id_auth_uaddr = 0;
+
+if (policy_data1_size > 0) {
+struct sev_snp_id_authentication *id_auth =
+(struct sev_snp_id_authentication *)policy_data2;
+
+if (policy_data1_size != KVM_SEV_SNP_ID_BLOCK_SIZE) {
+error_setg(errp, "%s: Invalid SEV-SNP ID block: incorrect
size",
+ __func__);
+return -1;
+}
+if (policy_data2_size != KVM_SEV_SNP_ID_AUTH_SIZE) {
+error_setg(errp,
+ "%s: Invalid SEV-SNP ID auth block: incorrect size",
+ __func__);
+return -1;
+}
+assert(policy_data1 != NULL);
+assert(policy_data2 != NULL);
+
+finish->id_block_uaddr =
+(__u64)g_memdup2(policy_data1, KVM_SEV_SNP_ID_BLOCK_SIZE);
+finish->id_auth_uaddr =
+(__u64)g_memdup2(policy_data2, KVM_SEV_SNP_ID_AUTH_SIZE);
+
+/*
+ * Check if an author key has been provided and use that to flag
+ * whether the author key is enabled. The first of the author key
+ * must be non-zero to indicate the key type, which will currently
+ * always be 2.
+ */
+sev_snp_guest->kvm_finish_conf.auth_key_en =
+id_auth->author_key[0] ? 1 : 0;
+finish->id_block_en = 1;
+}
+sev_snp_guest->kvm_start_conf.policy = policy;
+} else {
+SevGuestState *sev_guest = SEV_GUEST(MACHINE(qdev_get_machine())->cgs);
+/* Only the policy flags are supported for SEV and SEV-ES */
+if ((policy_data1_size > 0) || (policy_data2_size > 0) || !sev_guest) {
+error_setg(errp, "%s: An ID block/ID auth block has been provided "
+
[PATCH v4 14/17] backends/igvm: Process initialization sections in IGVM file
The initialization sections in IGVM files contain configuration that
should be applied to the guest platform before it is started. This
includes guest policy and other information that can affect the security
level and the startup measurement of a guest.
This commit introduces handling of the initialization sections during
processing of the IGVM file.
Signed-off-by: Roy Hopkins
---
backends/igvm.c | 21 +
1 file changed, 21 insertions(+)
diff --git a/backends/igvm.c b/backends/igvm.c
index 97af1a6cb3..fa074b9107 100644
--- a/backends/igvm.c
+++ b/backends/igvm.c
@@ -781,6 +781,27 @@ int igvm_process_file(IgvmCfgState *cfg,
ConfidentialGuestSupport *cgs,
}
}
+header_count =
+igvm_header_count(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION);
+if (header_count < 0) {
+error_setg(
+errp,
+"Invalid initialization header count in IGVM file. Error code: %X",
+header_count);
+return -1;
+}
+
+for (ctx.current_header_index = 0;
+ ctx.current_header_index < (unsigned)header_count;
+ ctx.current_header_index++) {
+IgvmVariableHeaderType type =
+igvm_get_header_type(ctx.file, IGVM_HEADER_SECTION_INITIALIZATION,
+ ctx.current_header_index);
+if (handler(&ctx, type, errp) < 0) {
+goto cleanup;
+}
+}
+
/*
* Contiguous pages of data with compatible flags are grouped together in
* order to reduce the number of memory regions we create. Make sure the
--
2.43.0
[PATCH v4 08/17] target/i386: Allow setting of R_LDTR and R_TR with cpu_x86_load_seg_cache()
The x86 segment registers are identified by the X86Seg enumeration which
includes LDTR and TR as well as the normal segment registers. The
function 'cpu_x86_load_seg_cache()' uses the enum to determine which
segment to set. However, specifying R_LDTR or R_TR results in an
out-of-bounds access of the segment array.
Possibly by coincidence, the function does correctly set LDTR or TR in
this case as the structures for these registers immediately follow the
array which is accessed out of bounds.
This patch adds correct handling for R_LDTR and R_TR in the function.
Signed-off-by: Roy Hopkins
---
target/i386/cpu.h | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 29daf37048..f4daec71cb 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -2242,7 +2242,14 @@ static inline void cpu_x86_load_seg_cache(CPUX86State
*env,
SegmentCache *sc;
unsigned int new_hflags;
-sc = &env->segs[seg_reg];
+if (seg_reg == R_LDTR) {
+sc = &env->ldt;
+} else if (seg_reg == R_TR) {
+sc = &env->tr;
+} else {
+sc = &env->segs[seg_reg];
+}
+
sc->selector = selector;
sc->base = base;
sc->limit = limit;
--
2.43.0
[PATCH v4 10/17] i386/sev: Implement ConfidentialGuestSupport functions for SEV
The ConfidentialGuestSupport object defines a number of virtual
functions that are called during processing of IGVM directives to query
or configure initial guest state. In order to support processing of IGVM
files, these functions need to be implemented by relevant isolation
hardware support code such as SEV.
This commit implements the required functions for SEV-ES and adds
support for processing IGVM files for configuring the guest.
Signed-off-by: Roy Hopkins
---
target/i386/sev.h | 2 +
target/i386/sev.c | 250 --
2 files changed, 242 insertions(+), 10 deletions(-)
diff --git a/target/i386/sev.h b/target/i386/sev.h
index 167dd154d6..2ccd6fe1e8 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -34,6 +34,8 @@
#define SEV_SNP_POLICY_SMT 0x1
#define SEV_SNP_POLICY_DBG 0x8
+#define SVM_SEV_FEAT_SNP_ACTIVE 1
+
typedef struct SevKernelLoaderContext {
char *setup_data;
size_t setup_size;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 7487971344..9a05b971c8 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -39,8 +39,10 @@
#include "qapi/qapi-commands-misc-target.h"
#include "confidential-guest.h"
#include "hw/i386/pc.h"
+#include "hw/i386/e820_memory_layout.h"
#include "exec/address-spaces.h"
#include "qemu/queue.h"
+#include "qemu/cutils.h"
OBJECT_DECLARE_TYPE(SevCommonState, SevCommonStateClass, SEV_COMMON)
OBJECT_DECLARE_TYPE(SevGuestState, SevCommonStateClass, SEV_GUEST)
@@ -49,6 +51,9 @@ OBJECT_DECLARE_TYPE(SevSnpGuestState, SevCommonStateClass,
SEV_SNP_GUEST)
/* hard code sha256 digest size */
#define HASH_SIZE 32
+/* Hard coded GPA that KVM uses for the VMSA */
+#define KVM_VMSA_GPA 0xF000
+
/* Convert between SEV-ES VMSA and SegmentCache flags/attributes */
#define FLAGS_VMSA_TO_SEGCACHE(flags) \
flags) & 0xff00) << 12) | (((flags) & 0xff) << 8))
@@ -487,6 +492,103 @@ static void sev_apply_cpu_context(CPUState *cpu)
}
}
+static int check_vmsa_supported(hwaddr gpa, const struct sev_es_save_area
*vmsa,
+Error **errp)
+{
+struct sev_es_save_area vmsa_check;
+
+/*
+ * KVM always populates the VMSA at a fixed GPA which cannot be modified
+ * from userspace. Specifying a different GPA will not prevent the guest
+ * from starting but will cause the launch measurement to be different
+ * from expected. Therefore check that the provided GPA matches the KVM
+ * hardcoded value.
+ */
+if (gpa != KVM_VMSA_GPA) {
+error_setg(errp,
+"%s: The VMSA GPA must be %lX but is specified as %lX",
+__func__, KVM_VMSA_GPA, gpa);
+return -1;
+}
+
+/*
+ * Clear all supported fields so we can then check the entire structure
+ * is zero.
+ */
+memcpy(&vmsa_check, vmsa, sizeof(struct sev_es_save_area));
+memset(&vmsa_check.es, 0, sizeof(vmsa_check.es));
+memset(&vmsa_check.cs, 0, sizeof(vmsa_check.cs));
+memset(&vmsa_check.ss, 0, sizeof(vmsa_check.ss));
+memset(&vmsa_check.ds, 0, sizeof(vmsa_check.ds));
+memset(&vmsa_check.fs, 0, sizeof(vmsa_check.fs));
+memset(&vmsa_check.gs, 0, sizeof(vmsa_check.gs));
+memset(&vmsa_check.gdtr, 0, sizeof(vmsa_check.gdtr));
+memset(&vmsa_check.idtr, 0, sizeof(vmsa_check.idtr));
+memset(&vmsa_check.ldtr, 0, sizeof(vmsa_check.ldtr));
+memset(&vmsa_check.tr, 0, sizeof(vmsa_check.tr));
+vmsa_check.efer = 0;
+vmsa_check.cr0 = 0;
+vmsa_check.cr3 = 0;
+vmsa_check.cr4 = 0;
+vmsa_check.xcr0 = 0;
+vmsa_check.dr6 = 0;
+vmsa_check.dr7 = 0;
+vmsa_check.rax = 0;
+vmsa_check.rcx = 0;
+vmsa_check.rdx = 0;
+vmsa_check.rbx = 0;
+vmsa_check.rsp = 0;
+vmsa_check.rbp = 0;
+vmsa_check.rsi = 0;
+vmsa_check.rdi = 0;
+vmsa_check.r8 = 0;
+vmsa_check.r9 = 0;
+vmsa_check.r10 = 0;
+vmsa_check.r11 = 0;
+vmsa_check.r12 = 0;
+vmsa_check.r13 = 0;
+vmsa_check.r14 = 0;
+vmsa_check.r15 = 0;
+vmsa_check.rip = 0;
+vmsa_check.rflags = 0;
+
+vmsa_check.g_pat = 0;
+vmsa_check.xcr0 = 0;
+
+vmsa_check.x87_fcw = 0;
+vmsa_check.mxcsr = 0;
+
+if (sev_snp_enabled()) {
+if (vmsa_check.sev_features != SVM_SEV_FEAT_SNP_ACTIVE) {
+error_setg(errp,
+ "%s: sev_features in the VMSA contains an unsupported "
+ "value. For SEV-SNP, sev_features must be set to %x.",
+ __func__, SVM_SEV_FEAT_SNP_ACTIVE);
+return -1;
+}
+vmsa_check.sev_features = 0;
+} else {
+if (vmsa_check.sev_features != 0) {
+error_setg(errp,
+ "%s: sev_features in the VMSA contains an unsupported "
+ "value. For SEV-ES and SEV, sev_features must be "
+ "set to 0.", __func__);
+return -1;
+}
+}
+
+if (!buffer_is_zero
