Re: [PATCH 2/2] hw/arm/virt: Warn when high memory region is disabled
Hi Eric,
On 8/2/22 7:49 PM, Eric Auger wrote:
On 8/2/22 08:45, Gavin Shan wrote:
When one specific high memory region is disabled due to the PA
limit, it'd better to warn user about that. The warning messages
help to identify the cause in some cases. For example, PCIe device
that has large MMIO bar, to be covered by PCIE_MMIO high memory
region, won't work properly if PCIE_MMIO high memory region is
disabled due to the PA limit.
Signed-off-by: Gavin Shan
---
hw/arm/virt.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index bc0cd218f9..c91756e33d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1691,6 +1691,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState
*vms, int idx)
static void virt_memmap_fits(VirtMachineState *vms, int index,
bool *enabled, hwaddr *base, int pa_bits)
{
+const char *region_name;
hwaddr size = extended_memmap[index].size;
/* The region will be disabled if its size isn't given */
@@ -1713,6 +1714,23 @@ static void virt_memmap_fits(VirtMachineState *vms, int
index,
vms->highest_gpa = *base + size - 1;
*base = *base + size;
+} else {
+switch (index) {
+case VIRT_HIGH_GIC_REDIST2:
+region_name = "GIC_REDIST2";
+break;
+case VIRT_HIGH_PCIE_ECAM:
+region_name = "PCIE_ECAM";
+break;
+case VIRT_HIGH_PCIE_MMIO:
+region_name = "PCIE_MMIO";
+break;
+default:
+region_name = "unknown";
+}
when highmem is turned off I don't think we want those warnings because
it is obvious that highmem regions are not meant to be used.
On the other hand I am afraid some users may complain about warnings
that do not affect them. If you miss high MMIO don't you get a warning
on guest side?
Yes, there is error message from guest to complain 64-bits PCI BAR can't
be settled. In this regard, I do think the error messages are duplicate
to some extent.
Lets drop this patch in next revision.
+
+warn_report("Disabled %s high memory region due to PA limit",
+region_name);
}
}
Thanks,
Gavin
Re: [PULL 0/1] semihosting patch queue
On 8/2/22 17:59, Richard Henderson wrote: The following changes since commit 430a388ef4a6e02e762a9c5f86c539f886a6a61a: Merge tag 'pull-migration-20220802c' of https://gitlab.com/dagrh/qemu into staging (2022-08-02 10:03:18 -0700) are available in the Git repository at: https://gitlab.com/rth7680/qemu.git tags/pull-semi-20220802 for you to fetch changes up to d44971e725c02e0656d2f53d4fb564f92e06aef7: target/mips: Advance pc after semihosting exception (2022-08-02 12:34:00 -0700) Fix mips semihosting regression. Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate. r~ Richard Henderson (1): target/mips: Advance pc after semihosting exception target/mips/tcg/translate.h | 4 target/mips/tcg/sysemu/tlb_helper.c | 1 + target/mips/tcg/translate.c | 10 +- target/mips/tcg/micromips_translate.c.inc | 6 +++--- target/mips/tcg/mips16e_translate.c.inc | 2 +- target/mips/tcg/nanomips_translate.c.inc | 4 ++-- 6 files changed, 16 insertions(+), 11 deletions(-)
Re: [PATCH] hw/riscv: remove 'fdt' param from riscv_setup_rom_reset_vec()
On Fri, Jul 29, 2022 at 4:19 AM Daniel Henrique Barboza
wrote:
>
> The 'fdt' param is not being used in riscv_setup_rom_reset_vec().
> Simplify the API by removing it. While we're at it, remove the redundant
> 'return' statement at the end of function.
>
> Cc: Palmer Dabbelt
> Cc: Alistair Francis
> Cc: Bin Meng
> Cc: Vijai Kumar K
> Signed-off-by: Daniel Henrique Barboza
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/riscv/boot.c| 4 +---
> hw/riscv/microchip_pfsoc.c | 2 +-
> hw/riscv/shakti_c.c| 3 +--
> hw/riscv/spike.c | 2 +-
> hw/riscv/virt.c| 2 +-
> include/hw/riscv/boot.h| 2 +-
> 6 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index 06b4fc5ac3..1ae7596873 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -286,7 +286,7 @@ void riscv_setup_rom_reset_vec(MachineState *machine,
> RISCVHartArrayState *harts
> hwaddr start_addr,
> hwaddr rom_base, hwaddr rom_size,
> uint64_t kernel_entry,
> - uint64_t fdt_load_addr, void *fdt)
> + uint64_t fdt_load_addr)
> {
> int i;
> uint32_t start_addr_hi32 = 0x;
> @@ -326,8 +326,6 @@ void riscv_setup_rom_reset_vec(MachineState *machine,
> RISCVHartArrayState *harts
>rom_base, _space_memory);
> riscv_rom_copy_firmware_info(machine, rom_base, rom_size,
> sizeof(reset_vec),
> kernel_entry);
> -
> -return;
> }
>
> void riscv_setup_direct_kernel(hwaddr kernel_addr, hwaddr fdt_addr)
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 10a5d0e501..7313153606 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -583,7 +583,7 @@ static void
> microchip_icicle_kit_machine_init(MachineState *machine)
> riscv_setup_rom_reset_vec(machine, >soc.u_cpus,
> firmware_load_addr,
>memmap[MICROCHIP_PFSOC_ENVM_DATA].base,
>memmap[MICROCHIP_PFSOC_ENVM_DATA].size,
> - kernel_entry, fdt_load_addr, machine->fdt);
> + kernel_entry, fdt_load_addr);
> }
> }
>
> diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
> index 90e2cf609f..e43cc9445c 100644
> --- a/hw/riscv/shakti_c.c
> +++ b/hw/riscv/shakti_c.c
> @@ -66,8 +66,7 @@ static void shakti_c_machine_state_init(MachineState
> *mstate)
> riscv_setup_rom_reset_vec(mstate, >soc.cpus,
>shakti_c_memmap[SHAKTI_C_RAM].base,
>shakti_c_memmap[SHAKTI_C_ROM].base,
> - shakti_c_memmap[SHAKTI_C_ROM].size, 0, 0,
> - NULL);
> + shakti_c_memmap[SHAKTI_C_ROM].size, 0, 0);
> if (mstate->firmware) {
> riscv_load_firmware(mstate->firmware,
> shakti_c_memmap[SHAKTI_C_RAM].base,
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index e41b6aa9f0..5ba34543c8 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -308,7 +308,7 @@ static void spike_board_init(MachineState *machine)
> riscv_setup_rom_reset_vec(machine, >soc[0], memmap[SPIKE_DRAM].base,
>memmap[SPIKE_MROM].base,
>memmap[SPIKE_MROM].size, kernel_entry,
> - fdt_load_addr, s->fdt);
> + fdt_load_addr);
>
> /* initialize HTIF using symbols found in load_kernel */
> htif_mm_init(system_memory, mask_rom,
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index bc424dd2f5..2e9ed2628c 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -1299,7 +1299,7 @@ static void virt_machine_done(Notifier *notifier, void
> *data)
> riscv_setup_rom_reset_vec(machine, >soc[0], start_addr,
>virt_memmap[VIRT_MROM].base,
>virt_memmap[VIRT_MROM].size, kernel_entry,
> - fdt_load_addr, machine->fdt);
> + fdt_load_addr);
>
> /*
> * Only direct boot kernel is currently supported for KVM VM,
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index d2db29721a..a36f7618f5 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -51,7 +51,7 @@ void riscv_setup_rom_reset_vec(MachineState *machine,
> RISCVHartArrayState *harts
> hwaddr saddr,
> hwaddr rom_base, hwaddr rom_size,
> uint64_t kernel_entry,
> - uint64_t fdt_load_addr, void *fdt);
> + uint64_t fdt_load_addr);
> void
Re: [PATCH v5 0/1] target/riscv: Add Zihintpause support
On Wed, Aug 3, 2022 at 9:42 AM Atish Patra wrote: > > On Sun, Jul 24, 2022 at 9:39 PM Alistair Francis wrote: > > > > On Mon, Jul 25, 2022 at 1:48 PM Dao Lu wrote: > > > > > > This patch adds RISC-V Zihintpause support. The extension is set to be > > > enabled > > > by default and opcode has been added to insn32.decode. > > > > > > Added trans_pause to exit the TB and return to main loop. > > > > > > The change can also be found in: > > > https://github.com/dlu42/qemu/tree/zihintpause_support_v1 > > > > > > Tested along with pause support added to cpu_relax function for linux, the > > > changes I made to linux to test can be found here: > > > https://github.com/dlu42/linux/tree/pause_support_v1 > > > > > > > > > Changelog: > > > > > > v1 -> v2 > > > 1. Pause now also exit the TB and return to main loop > > > 2. Move the REQUIRE_ZIHINTPAUSE macro inside the trans_pause function > > > > > > v2 -> v3 > > > No changes, v2 was lost from the list > > > > > > v3 -> v4 > > > No longer break the reservation in trans_pause > > > > > > v4 -> v5 > > > Rabase on top of > > > https://github.com/alistair23/qemu/tree/riscv-to-apply.next > > > > > > Dao Lu (1): > > > Add Zihintpause support > > > > Thanks! > > > > Applied to riscv-to-apply.next > > > > Did you overwrite your tree by mistake ? I pulled riscv-to-apply.next > a few days back where this patch along with Anup's priv version > fixes are there. But I can't find it anymore. I am looking at this. Hey Atish, I created a last minute pull request to get some fixes into 7.1. When I did that I overwrote the current riscv-to-apply.next with a version that only has a few bug fixes. I have pushed the riscv-to-apply.next for 7.2 to my public repo again. Alistair > > https://github.com/alistair23/qemu/commits/riscv-to-apply.next > > I wanted to rebase my sstc series on top of the riscv-to-apply.next. > Let me know if I am missing something. > > > Alistair > > > > > > > > target/riscv/cpu.c | 2 ++ > > > target/riscv/cpu.h | 1 + > > > target/riscv/insn32.decode | 7 ++- > > > target/riscv/insn_trans/trans_rvi.c.inc | 16 > > > 4 files changed, 25 insertions(+), 1 deletion(-) > > > > > > -- > > > 2.25.1 > > > > > > > > > > > -- > Regards, > Atish
Re: [PATCH] hw/nvme: Add helper functions for qid-db conversion
On Wed, Aug 03, 2022 at 09:46:05AM +0800, Jinhao Fan wrote: > at 4:54 PM, Klaus Jensen wrote: > > > I am unsure if the compiler will transform that division into the shift > > if it can infer that the divisor is a power of two (it most likely > > will be able to). > > > > But I see no reason to have a potential division here when we can do > > without and to me it is just as readable when you know the definition of > > DSTRD is `2 ^ (2 + DSTRD)`. > > OK. I will send a new patch with shifts instead of divisions. BTW, why do we > want to avoid divisions? Integer division is at least an order of magnitude more CPU cycles than a shift. Some archs are worse than others, but historically we go out of the way to avoid them in a hot path, so shifting is a more familiar coding pattern. Compilers typically implement division as a shift if you're dividing by a a power of two integer constant expression (ICE). This example here isn't an ICE, but it is a shifted constant power-of-two. I wrote up a simple test to see what my compiler does with that, and it looks like gcc will properly optimize it, but only if compiled with '-O3'.
Re: [PATCH v12 1/6] target/riscv: Add sscofpmf extension support
在 2022/8/3 上午7:33, Atish Patra 写道:
The Sscofpmf ('Ss' for Privileged arch and Supervisor-level extensions,
and 'cofpmf' for Count OverFlow and Privilege Mode Filtering)
extension allows the perf to handle overflow interrupts and filtering
support. This patch provides a framework for programmable
counters to leverage the extension. As the extension doesn't have any
provision for the overflow bit for fixed counters, the fixed events
can also be monitoring using programmable counters. The underlying
counters for cycle and instruction counters are always running. Thus,
a separate timer device is programmed to handle the overflow.
Tested-by: Heiko Stuebner
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu.c | 12 ++
target/riscv/cpu.h | 25 +++
target/riscv/cpu_bits.h | 55 +++
target/riscv/csr.c | 166 ++-
target/riscv/machine.c | 1 +
target/riscv/pmu.c | 357 +++-
target/riscv/pmu.h | 7 +
7 files changed, 612 insertions(+), 11 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d4635c7df46b..dc33b0dc9034 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -22,6 +22,7 @@
#include "qemu/ctype.h"
#include "qemu/log.h"
#include "cpu.h"
+#include "pmu.h"
#include "internals.h"
#include "exec/exec-all.h"
#include "qapi/error.h"
@@ -99,6 +100,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zve64f, true, PRIV_VERSION_1_12_0, ext_zve64f),
ISA_EXT_DATA_ENTRY(zhinx, true, PRIV_VERSION_1_12_0, ext_zhinx),
ISA_EXT_DATA_ENTRY(zhinxmin, true, PRIV_VERSION_1_12_0, ext_zhinxmin),
+ISA_EXT_DATA_ENTRY(sscofpmf, true, PRIV_VERSION_1_12_0, ext_sscofpmf),
ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0, ext_svinval),
ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
@@ -882,6 +884,15 @@ static void riscv_cpu_realize(DeviceState *dev, Error
**errp)
set_misa(env, env->misa_mxl, ext);
}
+#ifndef CONFIG_USER_ONLY
+if (cpu->cfg.pmu_num) {
+if (!riscv_pmu_init(cpu, cpu->cfg.pmu_num) && cpu->cfg.ext_sscofpmf) {
+cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+ riscv_pmu_timer_cb, cpu);
+}
+ }
+#endif
+
riscv_cpu_register_gdb_regs_for_features(cs);
qemu_init_vcpu(cs);
@@ -986,6 +997,7 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
+DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false),
DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 4be4b82a837d..64d90586256d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -137,6 +137,8 @@ typedef struct PMUCTRState {
/* Snapshort value of a counter in RV32 */
target_ulong mhpmcounterh_prev;
bool started;
+/* Value beyond UINT32_MAX/UINT64_MAX before overflow interrupt trigger */
+target_ulong irq_overflow_left;
} PMUCTRState;
struct CPUArchState {
@@ -302,6 +304,9 @@ struct CPUArchState {
/* PMU event selector configured values. First three are unused*/
target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
+/* PMU event selector configured values for RV32*/
+target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS];
+
I think mhpmeventh_val can be uint32_t directly. Or use the usual way
to implement this type of CSRs:
change the type of mhpmevent_val to uint64_t and mhpmeventh csr reuse
the high part of
mhpmeventh_val for RV32
Regards,
Weiwei Li
target_ulong sscratch;
target_ulong mscratch;
@@ -439,6 +444,7 @@ struct RISCVCPUConfig {
bool ext_zve32f;
bool ext_zve64f;
bool ext_zmmul;
+bool ext_sscofpmf;
bool rvv_ta_all_1s;
bool rvv_ma_all_1s;
@@ -486,6 +492,12 @@ struct ArchCPU {
mhpmeventh_val
/* Configuration Settings */
RISCVCPUConfig cfg;
+
+QEMUTimer *pmu_timer;
+/* A bitmask of Available programmable counters */
+uint32_t pmu_avail_ctrs;
+/* Mapping of events to counters */
+GHashTable *pmu_event_ctr_map;
};
static inline int riscv_has_ext(CPURISCVState *env, target_ulong ext)
@@ -746,6 +758,19 @@ enum {
CSR_TABLE_SIZE = 0x1000
};
+/**
+ * The event id are encoded based on the encoding specified in the
+ * SBI specification v0.3
+ */
+
+enum riscv_pmu_event_idx {
+RISCV_PMU_EVENT_HW_CPU_CYCLES = 0x01,
+RISCV_PMU_EVENT_HW_INSTRUCTIONS = 0x02,
+
[PATCH v2] hw/nvme: Add helper functions for qid-db conversion
With the introduction of shadow doorbell and ioeventfd, we need to do
frequent conversion between qid and its doorbell offset. The original
hard-coded calculation is confusing and error-prone. Add several helper
functions to do this task.
Signed-off-by: Jinhao Fan
---
Changes since v1:
- Use left shifts instead of divisions
hw/nvme/ctrl.c | 61 --
1 file changed, 39 insertions(+), 22 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 533ad14e7a..4312e880e1 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -487,6 +487,29 @@ static int nvme_check_cqid(NvmeCtrl *n, uint16_t cqid)
{
return cqid < n->conf_ioqpairs + 1 && n->cq[cqid] != NULL ? 0 : -1;
}
+static inline bool nvme_db_offset_is_cq(NvmeCtrl *n, hwaddr offset)
+{
+hwaddr dstride = NVME_CAP_DSTRD(ldq_le_p(>bar.cap));
+return (offset >> (dstride + 2)) & 1;
+}
+
+static inline uint16_t nvme_db_offset_to_qid(NvmeCtrl *n, hwaddr offset)
+{
+hwaddr dstride = NVME_CAP_DSTRD(ldq_le_p(>bar.cap));
+return offset >> (dstride + 3);
+}
+
+static inline hwaddr nvme_cqid_to_db_offset(NvmeCtrl *n, uint16_t cqid)
+{
+hwaddr stride = 4 << NVME_CAP_DSTRD(ldq_le_p(>bar.cap));
+return stride * (cqid * 2 + 1);
+}
+
+static inline hwaddr nvme_sqid_to_db_offset(NvmeCtrl *n, uint16_t sqid)
+{
+hwaddr stride = 4 << NVME_CAP_DSTRD(ldq_le_p(>bar.cap));
+return stride * sqid * 2;
+}
static void nvme_inc_cq_tail(NvmeCQueue *cq)
{
@@ -4256,7 +4279,7 @@ static void nvme_cq_notifier(EventNotifier *e)
static int nvme_init_cq_ioeventfd(NvmeCQueue *cq)
{
NvmeCtrl *n = cq->ctrl;
-uint16_t offset = (cq->cqid << 3) + (1 << 2);
+uint16_t offset = nvme_cqid_to_db_offset(n, cq->cqid);
int ret;
ret = event_notifier_init(>notifier, 0);
@@ -4283,7 +4306,7 @@ static void nvme_sq_notifier(EventNotifier *e)
static int nvme_init_sq_ioeventfd(NvmeSQueue *sq)
{
NvmeCtrl *n = sq->ctrl;
-uint16_t offset = sq->sqid << 3;
+uint16_t offset = nvme_sqid_to_db_offset(n, sq->sqid);
int ret;
ret = event_notifier_init(>notifier, 0);
@@ -4300,7 +4323,7 @@ static int nvme_init_sq_ioeventfd(NvmeSQueue *sq)
static void nvme_free_sq(NvmeSQueue *sq, NvmeCtrl *n)
{
-uint16_t offset = sq->sqid << 3;
+uint16_t offset = nvme_sqid_to_db_offset(n, sq->sqid);
n->sq[sq->sqid] = NULL;
timer_free(sq->timer);
@@ -4379,8 +4402,8 @@ static void nvme_init_sq(NvmeSQueue *sq, NvmeCtrl *n,
uint64_t dma_addr,
sq->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, nvme_process_sq, sq);
if (n->dbbuf_enabled) {
-sq->db_addr = n->dbbuf_dbs + (sqid << 3);
-sq->ei_addr = n->dbbuf_eis + (sqid << 3);
+sq->db_addr = n->dbbuf_dbs + nvme_sqid_to_db_offset(n, sqid);
+sq->ei_addr = n->dbbuf_eis + nvme_sqid_to_db_offset(n, sqid);
if (n->params.ioeventfd && sq->sqid != 0) {
if (!nvme_init_sq_ioeventfd(sq)) {
@@ -4690,8 +4713,8 @@ static uint16_t nvme_get_log(NvmeCtrl *n, NvmeRequest
*req)
static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n)
{
-uint16_t offset = (cq->cqid << 3) + (1 << 2);
-
+uint16_t offset = nvme_cqid_to_db_offset(n, cq->cqid);
+
n->cq[cq->cqid] = NULL;
timer_free(cq->timer);
if (cq->ioeventfd_enabled) {
@@ -4755,8 +4778,8 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n,
uint64_t dma_addr,
QTAILQ_INIT(>req_list);
QTAILQ_INIT(>sq_list);
if (n->dbbuf_enabled) {
-cq->db_addr = n->dbbuf_dbs + (cqid << 3) + (1 << 2);
-cq->ei_addr = n->dbbuf_eis + (cqid << 3) + (1 << 2);
+cq->db_addr = n->dbbuf_dbs + nvme_cqid_to_db_offset(n, cqid);
+cq->ei_addr = n->dbbuf_eis + nvme_cqid_to_db_offset(n, cqid);
if (n->params.ioeventfd && cqid != 0) {
if (!nvme_init_cq_ioeventfd(cq)) {
@@ -6128,13 +6151,8 @@ static uint16_t nvme_dbbuf_config(NvmeCtrl *n, const
NvmeRequest *req)
NvmeCQueue *cq = n->cq[i];
if (sq) {
-/*
- * CAP.DSTRD is 0, so offset of ith sq db_addr is (i<<3)
- * nvme_process_db() uses this hard-coded way to calculate
- * doorbell offsets. Be consistent with that here.
- */
-sq->db_addr = dbs_addr + (i << 3);
-sq->ei_addr = eis_addr + (i << 3);
+sq->db_addr = dbs_addr + nvme_sqid_to_db_offset(n, i);
+sq->ei_addr = eis_addr + nvme_sqid_to_db_offset(n, i);
pci_dma_write(>parent_obj, sq->db_addr, >tail,
sizeof(sq->tail));
@@ -6146,9 +6164,8 @@ static uint16_t nvme_dbbuf_config(NvmeCtrl *n, const
NvmeRequest *req)
}
if (cq) {
-/* CAP.DSTRD is 0, so offset of ith cq db_addr is (i<<3)+(1<<2) */
-cq->db_addr = dbs_addr + (i << 3) + (1 << 2);
-cq->ei_addr = eis_addr + (i << 3) + (1 << 2);
+cq->db_addr = dbs_addr + nvme_cqid_to_db_offset(n, i);
+
Re: [PATCH] hw/nvme: Add helper functions for qid-db conversion
at 4:54 PM, Klaus Jensen wrote: > I am unsure if the compiler will transform that division into the shift > if it can infer that the divisor is a power of two (it most likely > will be able to). > > But I see no reason to have a potential division here when we can do > without and to me it is just as readable when you know the definition of > DSTRD is `2 ^ (2 + DSTRD)`. OK. I will send a new patch with shifts instead of divisions. BTW, why do we want to avoid divisions?
Re: [PULL 9/9] hw/i386: pass RNG seed via setup_data entry
On 8/2/2022 11:13 PM, Jason A. Donenfeld wrote: Hi Xiaoyao, On Tue, Aug 2, 2022 at 5:06 PM Jason A. Donenfeld wrote: Hi Xiaoyao, On Tue, Aug 02, 2022 at 10:53:07PM +0800, Xiaoyao Li wrote: yes, with >= 7.1, pcmc->legacy_no_rng_seed = false by default, and RNG seed is used. This is intended behavior. Being on by default is basically the whole point of it. Otherwise it's useless. Either way, this shouldn't cause boot failures. It does fail booting OVMF with #PF. Below diff can fix the #PF for me. Huh, interesting. Sounds like maybe there's a bug I need to fix. Can you send me some repro instructions, and I'll look into it right away. I just tried booting Fedora using OVMF and didn't have any problems. I used this command line: qemu-system-x86_64 -machine q35 -enable-kvm -cpu host,-rdrand,-rdseed -smp cores=8 -drive file=disk.qcow2,if=virtio -net nic,model=virtio -net user,hostfwd=tcp::19230-:22 -m 8G -vga qxl -device virtio-serial-pci -device virtserialport,chardev=spicechannel0,name=com.redhat.spice. 0 -chardev spicevmc,id=spicechannel0,name=vdagent -spice unix,addr=/tmp/vm_spice_fedora.socket,disable-ticketing,playback-compression=off,agen t-mouse=on,seamless-migration,gl=on -device virtserialport,chardev=spicechannel1,name=org.spice-space.webdav.0 -chardev spiceport,id=spicechan nel1,name=org.spice-space.webdav.0 -global driver=cfi.pflash01,property=secure,value=on -drive if=pflash,format=raw,unit=0,file=OVMF_CODE.secb oot.fd,readonly=on -drive if=pflash,format=raw,file=OVMF_VARS.secboot.fd Can you tell me what you're using and give me some links with various images and such? Doing the straight forward thing doesn't reproduce it for me. I guess the key to reproduce the issue is using "-kernel" option to load guest kernel with QEMU Thanks, Jason
Re: [PATCH] hw/riscv: remove 'fdt' param from riscv_setup_rom_reset_vec()
On Fri, Jul 29, 2022 at 2:19 AM Daniel Henrique Barboza wrote: > > The 'fdt' param is not being used in riscv_setup_rom_reset_vec(). > Simplify the API by removing it. While we're at it, remove the redundant > 'return' statement at the end of function. > > Cc: Palmer Dabbelt > Cc: Alistair Francis > Cc: Bin Meng > Cc: Vijai Kumar K > Signed-off-by: Daniel Henrique Barboza > --- > hw/riscv/boot.c| 4 +--- > hw/riscv/microchip_pfsoc.c | 2 +- > hw/riscv/shakti_c.c| 3 +-- > hw/riscv/spike.c | 2 +- > hw/riscv/virt.c| 2 +- > include/hw/riscv/boot.h| 2 +- > 6 files changed, 6 insertions(+), 9 deletions(-) > Reviewed-by: Bin Meng
[PULL 1/1] target/mips: Advance pc after semihosting exception
Delay generating the exception until after we know the
insn length, and record that length in env->error_code.
Fixes: 8ec7e3c53d4 ("target/mips: Use an exception for semihosting")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1126
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Richard Henderson
---
target/mips/tcg/translate.h | 4
target/mips/tcg/sysemu/tlb_helper.c | 1 +
target/mips/tcg/translate.c | 10 +-
target/mips/tcg/micromips_translate.c.inc | 6 +++---
target/mips/tcg/mips16e_translate.c.inc | 2 +-
target/mips/tcg/nanomips_translate.c.inc | 4 ++--
6 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index 55053226ae..69f85841d2 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -51,6 +51,10 @@ typedef struct DisasContext {
int gi;
} DisasContext;
+#define DISAS_STOP DISAS_TARGET_0
+#define DISAS_EXIT DISAS_TARGET_1
+#define DISAS_SEMIHOST DISAS_TARGET_2
+
/* MIPS major opcodes */
#define MASK_OP_MAJOR(op) (op & (0x3F << 26))
diff --git a/target/mips/tcg/sysemu/tlb_helper.c
b/target/mips/tcg/sysemu/tlb_helper.c
index 57ffad2902..9d16859c0a 100644
--- a/target/mips/tcg/sysemu/tlb_helper.c
+++ b/target/mips/tcg/sysemu/tlb_helper.c
@@ -1056,6 +1056,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
case EXCP_SEMIHOST:
cs->exception_index = EXCP_NONE;
mips_semihosting(env);
+env->active_tc.PC += env->error_code;
return;
case EXCP_DSS:
env->CP0_Debug |= 1 << CP0DB_DSS;
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 1f6a779808..de1511baaf 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -1213,9 +1213,6 @@ TCGv_i64 fpu_f64[32];
#include "exec/gen-icount.h"
-#define DISAS_STOP DISAS_TARGET_0
-#define DISAS_EXIT DISAS_TARGET_1
-
static const char regnames_HI[][4] = {
"HI0", "HI1", "HI2", "HI3",
};
@@ -13902,7 +13899,7 @@ static void decode_opc_special_r6(CPUMIPSState *env,
DisasContext *ctx)
break;
case R6_OPC_SDBBP:
if (is_uhi(extract32(ctx->opcode, 6, 20))) {
-generate_exception_end(ctx, EXCP_SEMIHOST);
+ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
if (ctx->hflags & MIPS_HFLAG_SBRI) {
gen_reserved_instruction(ctx);
@@ -14314,7 +14311,7 @@ static void decode_opc_special2_legacy(CPUMIPSState
*env, DisasContext *ctx)
break;
case OPC_SDBBP:
if (is_uhi(extract32(ctx->opcode, 6, 20))) {
-generate_exception_end(ctx, EXCP_SEMIHOST);
+ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
/*
* XXX: not clear which exception should be raised
@@ -16098,6 +16095,9 @@ static void mips_tr_translate_insn(DisasContextBase
*dcbase, CPUState *cs)
if (is_slot) {
gen_branch(ctx, insn_bytes);
}
+if (ctx->base.is_jmp == DISAS_SEMIHOST) {
+generate_exception_err(ctx, EXCP_SEMIHOST, insn_bytes);
+}
ctx->base.pc_next += insn_bytes;
if (ctx->base.is_jmp != DISAS_NEXT) {
diff --git a/target/mips/tcg/micromips_translate.c.inc
b/target/mips/tcg/micromips_translate.c.inc
index 274caf2c3c..b2c696f891 100644
--- a/target/mips/tcg/micromips_translate.c.inc
+++ b/target/mips/tcg/micromips_translate.c.inc
@@ -826,7 +826,7 @@ static void gen_pool16c_insn(DisasContext *ctx)
break;
case SDBBP16:
if (is_uhi(extract32(ctx->opcode, 0, 4))) {
-generate_exception_end(ctx, EXCP_SEMIHOST);
+ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
/*
* XXX: not clear which exception should be raised
@@ -942,7 +942,7 @@ static void gen_pool16c_r6_insn(DisasContext *ctx)
case R6_SDBBP16:
/* SDBBP16 */
if (is_uhi(extract32(ctx->opcode, 6, 4))) {
-generate_exception_end(ctx, EXCP_SEMIHOST);
+ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
if (ctx->hflags & MIPS_HFLAG_SBRI) {
generate_exception(ctx, EXCP_RI);
@@ -1311,7 +1311,7 @@ static void gen_pool32axf(CPUMIPSState *env, DisasContext
*ctx, int rt, int rs)
break;
case SDBBP:
if (is_uhi(extract32(ctx->opcode, 16, 10))) {
-generate_exception_end(ctx, EXCP_SEMIHOST);
+ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
check_insn(ctx, ISA_MIPS_R1);
if (ctx->hflags & MIPS_HFLAG_SBRI) {
diff --git a/target/mips/tcg/mips16e_translate.c.inc
b/target/mips/tcg/mips16e_translate.c.inc
index 0a3ba252e4..7568933e23 100644
--- a/target/mips/tcg/mips16e_translate.c.inc
+++ b/target/mips/tcg/mips16e_translate.c.inc
@@ -952,7 +952,7 @@ static int decode_ase_mips16e(CPUMIPSState *env,
DisasContext
Re: [PATCH 1/2] hw/arm/virt: Improve address assignment for highmem IO regions
Hi Eric,
On 8/2/22 7:41 PM, Eric Auger wrote:
On 8/2/22 08:45, Gavin Shan wrote:
There are 3 highmem IO regions as below. They can be disabled in
two situations: (a) The specific region is disabled by user. (b)
The specific region doesn't fit in the PA space. However, the base
address and highest_gpa are still updated no matter if the region
is enabled or disabled. It's incorrectly incurring waste in the PA
space.
If I am not wrong highmem_redists and highmem_mmio are not user selectable
Only highmem ecam depends on machine type & ACPI setup. But I would say
that in server use case it is always set. So is that optimization really
needed?
There are two other cases you missed.
- highmem_ecam is enabled after virt-2.12, meaning it stays disabled
before that.
- The high memory region can be disabled if user is asking large
(normal) memory space through 'maxmem=' option. When the requested
memory by 'maxmem=' is large enough, the high memory regions are
disabled. It means the normal memory has higher priority than those
high memory regions. This is the case I provided in (b) of the
commit log.
In the commit log, I was supposed to say something like below for
(a):
- The specific high memory region can be disabled through changing
the code by user or developer. For example, 'vms->highmem_mmio'
is changed from true to false in virt_instance_init().
Improve address assignment for highmem IO regions to avoid the waste
in the PA space by putting the logic into virt_memmap_fits().
Signed-off-by: Gavin Shan
---
hw/arm/virt.c | 54 +--
1 file changed, 31 insertions(+), 23 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9633f822f3..bc0cd218f9 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1688,6 +1688,34 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState
*vms, int idx)
return arm_cpu_mp_affinity(idx, clustersz);
}
+static void virt_memmap_fits(VirtMachineState *vms, int index,
+ bool *enabled, hwaddr *base, int pa_bits)
+{
+hwaddr size = extended_memmap[index].size;
+
+/* The region will be disabled if its size isn't given */
+if (!*enabled || !size) {
In which case do you have !size?
Yeah, we don't have !size and the condition should be removed.
+*enabled = false;
+vms->memmap[index].base = 0;
+vms->memmap[index].size = 0;
It looks dangerous to me to reset the region's base and size like that.
for instance fdt_add_gic_node() will add dummy data in the dt.
I would guess you missed that the high memory regions won't be exported
through device-tree if they have been disabled. We have a check there,
which is "if (nb_redist_regions == 1)"
+return;
+}
+
+/*
+ * Check if the memory region fits in the PA space. The memory map
+ * and highest_gpa are updated if it fits. Otherwise, it's disabled.
+ */
+*enabled = (ROUND_UP(*base, size) + size <= BIT_ULL(pa_bits));
using a 'fits' local variable would make the code more obvious I think
Lets confirm if you're suggesting something like below?
bool fits;
fits = (ROUND_UP(*base, size) + size <= BIT_ULL(pa_bits));
if (fits) {
/* update *base, memory mapping, highest_gpa */
} else {
*enabled = false;
}
I guess we can simply do
if (ROUND_UP(*base, size) + size <= BIT_ULL(pa_bits)) {
/* update *base, memory mapping, highest_gpa */
} else {
*enabled = false;
}
Please let me know which one looks best to you.
+if (*enabled) {
+*base = ROUND_UP(*base, size);
+vms->memmap[index].base = *base;
+vms->memmap[index].size = size;
+vms->highest_gpa = *base + size - 1;
+
+ *base = *base + size;
+}
+}
+
static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
{
MachineState *ms = MACHINE(vms);
@@ -1744,37 +1772,17 @@ static void virt_set_memmap(VirtMachineState *vms, int
pa_bits)
vms->highest_gpa = memtop - 1;
for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) {
-hwaddr size = extended_memmap[i].size;
-bool fits;
-
-base = ROUND_UP(base, size);
-vms->memmap[i].base = base;
-vms->memmap[i].size = size;
-
-/*
- * Check each device to see if they fit in the PA space,
- * moving highest_gpa as we go.
- *
- * For each device that doesn't fit, disable it.
- */
-fits = (base + size) <= BIT_ULL(pa_bits);
-if (fits) {
-vms->highest_gpa = base + size - 1;
-}
-
we could avoid running the code below in case highmem is not set. We would need
to reset that flags though.
Yeah, I think it's not a big deal. My though is to update the flag in
virt_memmap_fits().
switch (i) {
case VIRT_HIGH_GIC_REDIST2:
-
[PULL 0/1] semihosting patch queue
The following changes since commit 430a388ef4a6e02e762a9c5f86c539f886a6a61a: Merge tag 'pull-migration-20220802c' of https://gitlab.com/dagrh/qemu into staging (2022-08-02 10:03:18 -0700) are available in the Git repository at: https://gitlab.com/rth7680/qemu.git tags/pull-semi-20220802 for you to fetch changes up to d44971e725c02e0656d2f53d4fb564f92e06aef7: target/mips: Advance pc after semihosting exception (2022-08-02 12:34:00 -0700) Fix mips semihosting regression. Richard Henderson (1): target/mips: Advance pc after semihosting exception target/mips/tcg/translate.h | 4 target/mips/tcg/sysemu/tlb_helper.c | 1 + target/mips/tcg/translate.c | 10 +- target/mips/tcg/micromips_translate.c.inc | 6 +++--- target/mips/tcg/mips16e_translate.c.inc | 2 +- target/mips/tcg/nanomips_translate.c.inc | 4 ++-- 6 files changed, 16 insertions(+), 11 deletions(-)
Re: [PATCH v5 0/1] target/riscv: Add Zihintpause support
On Sun, Jul 24, 2022 at 9:39 PM Alistair Francis wrote: > > On Mon, Jul 25, 2022 at 1:48 PM Dao Lu wrote: > > > > This patch adds RISC-V Zihintpause support. The extension is set to be > > enabled > > by default and opcode has been added to insn32.decode. > > > > Added trans_pause to exit the TB and return to main loop. > > > > The change can also be found in: > > https://github.com/dlu42/qemu/tree/zihintpause_support_v1 > > > > Tested along with pause support added to cpu_relax function for linux, the > > changes I made to linux to test can be found here: > > https://github.com/dlu42/linux/tree/pause_support_v1 > > > > > > Changelog: > > > > v1 -> v2 > > 1. Pause now also exit the TB and return to main loop > > 2. Move the REQUIRE_ZIHINTPAUSE macro inside the trans_pause function > > > > v2 -> v3 > > No changes, v2 was lost from the list > > > > v3 -> v4 > > No longer break the reservation in trans_pause > > > > v4 -> v5 > > Rabase on top of https://github.com/alistair23/qemu/tree/riscv-to-apply.next > > > > Dao Lu (1): > > Add Zihintpause support > > Thanks! > > Applied to riscv-to-apply.next > Did you overwrite your tree by mistake ? I pulled riscv-to-apply.next a few days back where this patch along with Anup's priv version fixes are there. But I can't find it anymore. I am looking at this. https://github.com/alistair23/qemu/commits/riscv-to-apply.next I wanted to rebase my sstc series on top of the riscv-to-apply.next. Let me know if I am missing something. > Alistair > > > > > target/riscv/cpu.c | 2 ++ > > target/riscv/cpu.h | 1 + > > target/riscv/insn32.decode | 7 ++- > > target/riscv/insn_trans/trans_rvi.c.inc | 16 > > 4 files changed, 25 insertions(+), 1 deletion(-) > > > > -- > > 2.25.1 > > > > > -- Regards, Atish
[PATCH v12 5/6] target/riscv: Update the privilege field for sscofpmf CSRs
The sscofpmf extension was ratified as a part of priv spec v1.12.
Mark the csr_ops accordingly.
Reviewed-by: Weiwei Li
Reviewed-by: Alistair Francis
Signed-off-by: Atish Patra
---
target/riscv/csr.c | 90 ++
1 file changed, 60 insertions(+), 30 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6690b72ea0e7..8753280e95b2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3889,63 +3889,92 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
write_mhpmevent },
[CSR_MHPMEVENT3H]= { "mhpmevent3h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT4H]= { "mhpmevent4h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT5H]= { "mhpmevent5h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT6H]= { "mhpmevent6h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT7H]= { "mhpmevent7h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT8H]= { "mhpmevent8h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT9H]= { "mhpmevent9h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT10H] = { "mhpmevent10h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT11H] = { "mhpmevent11h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT12H] = { "mhpmevent12h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT13H] = { "mhpmevent13h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT14H] = { "mhpmevent14h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT15H] = { "mhpmevent15h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT16H] = { "mhpmevent16h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT17H] = { "mhpmevent17h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver = PRIV_VERSION_1_12_0},
[CSR_MHPMEVENT18H] = { "mhpmevent18h",sscofpmf, read_mhpmeventh,
- write_mhpmeventh },
+ write_mhpmeventh,
+ .min_priv_ver =
[PATCH v12 3/6] target/riscv: Add few cache related PMU events
From: Atish Patra
Qemu can monitor the following cache related PMU events through
tlb_fill functions.
1. DTLB load/store miss
3. ITLB prefetch miss
Increment the PMU counter in tlb_fill function.
Reviewed-by: Alistair Francis
Tested-by: Heiko Stuebner
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu_helper.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 650574accf0a..c9333cc00389 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -21,11 +21,13 @@
#include "qemu/log.h"
#include "qemu/main-loop.h"
#include "cpu.h"
+#include "pmu.h"
#include "exec/exec-all.h"
#include "instmap.h"
#include "tcg/tcg-op.h"
#include "trace.h"
#include "semihosting/common-semi.h"
+#include "cpu_bits.h"
int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
{
@@ -1183,6 +1185,28 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr
addr,
cpu_loop_exit_restore(cs, retaddr);
}
+
+static void pmu_tlb_fill_incr_ctr(RISCVCPU *cpu, MMUAccessType access_type)
+{
+enum riscv_pmu_event_idx pmu_event_type;
+
+switch (access_type) {
+case MMU_INST_FETCH:
+pmu_event_type = RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS;
+break;
+case MMU_DATA_LOAD:
+pmu_event_type = RISCV_PMU_EVENT_CACHE_DTLB_READ_MISS;
+break;
+case MMU_DATA_STORE:
+pmu_event_type = RISCV_PMU_EVENT_CACHE_DTLB_WRITE_MISS;
+break;
+default:
+return;
+}
+
+riscv_pmu_incr_ctr(cpu, pmu_event_type);
+}
+
bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
MMUAccessType access_type, int mmu_idx,
bool probe, uintptr_t retaddr)
@@ -1281,6 +1305,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int
size,
}
}
} else {
+pmu_tlb_fill_incr_ctr(cpu, access_type);
/* Single stage lookup */
ret = get_physical_address(env, , , address, NULL,
access_type, mmu_idx, true, false, false);
--
2.25.1
[PATCH v12 4/6] hw/riscv: virt: Add PMU DT node to the device tree
Qemu virt machine can support few cache events and cycle/instret counters.
It also supports counter overflow for these events.
Add a DT node so that OpenSBI/Linux kernel is aware of the virt machine
capabilities. There are some dummy nodes added for testing as well.
Acked-by: Alistair Francis
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
hw/riscv/virt.c| 16 +
target/riscv/pmu.c | 57 ++
target/riscv/pmu.h | 1 +
3 files changed, 74 insertions(+)
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index f2ce5663a4c7..b58e6ed8b823 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -30,6 +30,7 @@
#include "hw/char/serial.h"
#include "target/riscv/cpu.h"
#include "hw/core/sysbus-fdt.h"
+#include "target/riscv/pmu.h"
#include "hw/riscv/riscv_hart.h"
#include "hw/riscv/virt.h"
#include "hw/riscv/boot.h"
@@ -715,6 +716,20 @@ static void create_fdt_socket_aplic(RISCVVirtState *s,
aplic_phandles[socket] = aplic_s_phandle;
}
+static void create_fdt_pmu(RISCVVirtState *s)
+{
+char *pmu_name;
+MachineState *mc = MACHINE(s);
+RISCVCPU hart = s->soc[0].harts[0];
+
+pmu_name = g_strdup_printf("/soc/pmu");
+qemu_fdt_add_subnode(mc->fdt, pmu_name);
+qemu_fdt_setprop_string(mc->fdt, pmu_name, "compatible", "riscv,pmu");
+riscv_pmu_generate_fdt_node(mc->fdt, hart.cfg.pmu_num, pmu_name);
+
+g_free(pmu_name);
+}
+
static void create_fdt_sockets(RISCVVirtState *s, const MemMapEntry *memmap,
bool is_32_bit, uint32_t *phandle,
uint32_t *irq_mmio_phandle,
@@ -1043,6 +1058,7 @@ static void create_fdt(RISCVVirtState *s, const
MemMapEntry *memmap,
create_fdt_flash(s, memmap);
create_fdt_fw_cfg(s, memmap);
+create_fdt_pmu(s);
update_bootargs:
if (cmdline && *cmdline) {
diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
index 34096941c0ce..59feb3c243dd 100644
--- a/target/riscv/pmu.c
+++ b/target/riscv/pmu.c
@@ -20,11 +20,68 @@
#include "cpu.h"
#include "pmu.h"
#include "sysemu/cpu-timers.h"
+#include "sysemu/device_tree.h"
#define RISCV_TIMEBASE_FREQ 10 /* 1Ghz */
#define MAKE_32BIT_MASK(shift, length) \
(((uint32_t)(~0UL) >> (32 - (length))) << (shift))
+/**
+ * To keep it simple, any event can be mapped to any programmable counters in
+ * QEMU. The generic cycle & instruction count events can also be monitored
+ * using programmable counters. In that case, mcycle & minstret must continue
+ * to provide the correct value as well. Heterogeneous PMU per hart is not
+ * supported yet. Thus, number of counters are same across all harts.
+ */
+void riscv_pmu_generate_fdt_node(void *fdt, int num_ctrs, char *pmu_name)
+{
+uint32_t fdt_event_ctr_map[20] = {};
+uint32_t cmask;
+
+/* All the programmable counters can map to any event */
+cmask = MAKE_32BIT_MASK(3, num_ctrs);
+
+ /**
+* The event encoding is specified in the SBI specification
+* Event idx is a 20bits wide number encoded as follows:
+* event_idx[19:16] = type
+* event_idx[15:0] = code
+* The code field in cache events are encoded as follows:
+* event_idx.code[15:3] = cache_id
+* event_idx.code[2:1] = op_id
+* event_idx.code[0:0] = result_id
+*/
+
+ /* SBI_PMU_HW_CPU_CYCLES: 0x01 : type(0x00) */
+ fdt_event_ctr_map[0] = cpu_to_be32(0x0001);
+ fdt_event_ctr_map[1] = cpu_to_be32(0x0001);
+ fdt_event_ctr_map[2] = cpu_to_be32(cmask | 1 << 0);
+
+ /* SBI_PMU_HW_INSTRUCTIONS: 0x02 : type(0x00) */
+ fdt_event_ctr_map[3] = cpu_to_be32(0x0002);
+ fdt_event_ctr_map[4] = cpu_to_be32(0x0002);
+ fdt_event_ctr_map[5] = cpu_to_be32(cmask | 1 << 2);
+
+ /* SBI_PMU_HW_CACHE_DTLB : 0x03 READ : 0x00 MISS : 0x00 type(0x01) */
+ fdt_event_ctr_map[6] = cpu_to_be32(0x00010019);
+ fdt_event_ctr_map[7] = cpu_to_be32(0x00010019);
+ fdt_event_ctr_map[8] = cpu_to_be32(cmask);
+
+ /* SBI_PMU_HW_CACHE_DTLB : 0x03 WRITE : 0x01 MISS : 0x00 type(0x01) */
+ fdt_event_ctr_map[9] = cpu_to_be32(0x0001001B);
+ fdt_event_ctr_map[10] = cpu_to_be32(0x0001001B);
+ fdt_event_ctr_map[11] = cpu_to_be32(cmask);
+
+ /* SBI_PMU_HW_CACHE_ITLB : 0x04 READ : 0x00 MISS : 0x00 type(0x01) */
+ fdt_event_ctr_map[12] = cpu_to_be32(0x00010021);
+ fdt_event_ctr_map[13] = cpu_to_be32(0x00010021);
+ fdt_event_ctr_map[14] = cpu_to_be32(cmask);
+
+ /* This a OpenSBI specific DT property documented in OpenSBI docs */
+ qemu_fdt_setprop(fdt, pmu_name, "riscv,event-to-mhpmcounters",
+fdt_event_ctr_map, sizeof(fdt_event_ctr_map));
+}
+
static bool riscv_pmu_counter_valid(RISCVCPU *cpu, uint32_t ctr_idx)
{
if (ctr_idx < 3 || ctr_idx >= RV_MAX_MHPMCOUNTERS ||
diff --git a/target/riscv/pmu.h b/target/riscv/pmu.h
index 036653627f78..3004ce37b636 100644
--- a/target/riscv/pmu.h
+++ b/target/riscv/pmu.h
@@ -31,5 +31,6 @@ int riscv_pmu_init(RISCVCPU *cpu, int num_counters);
[PATCH v12 6/6] target/riscv: Remove additional priv version check for mcountinhibit
With .min_priv_version, additiona priv version check is uncessary
for mcountinhibit read/write functions.
Reviewed-by: Heiko Stuebner
Tested-by: Heiko Stuebner
Signed-off-by: Atish Patra
---
target/riscv/csr.c | 8
1 file changed, 8 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 8753280e95b2..67367e678f38 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1489,10 +1489,6 @@ static RISCVException write_mtvec(CPURISCVState *env,
int csrno,
static RISCVException read_mcountinhibit(CPURISCVState *env, int csrno,
target_ulong *val)
{
-if (env->priv_ver < PRIV_VERSION_1_11_0) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-
*val = env->mcountinhibit;
return RISCV_EXCP_NONE;
}
@@ -1503,10 +1499,6 @@ static RISCVException write_mcountinhibit(CPURISCVState
*env, int csrno,
int cidx;
PMUCTRState *counter;
-if (env->priv_ver < PRIV_VERSION_1_11_0) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-
env->mcountinhibit = val;
/* Check if any other counter is also monitoring cycles/instructions */
--
2.25.1
[PATCH v12 1/6] target/riscv: Add sscofpmf extension support
The Sscofpmf ('Ss' for Privileged arch and Supervisor-level extensions,
and 'cofpmf' for Count OverFlow and Privilege Mode Filtering)
extension allows the perf to handle overflow interrupts and filtering
support. This patch provides a framework for programmable
counters to leverage the extension. As the extension doesn't have any
provision for the overflow bit for fixed counters, the fixed events
can also be monitoring using programmable counters. The underlying
counters for cycle and instruction counters are always running. Thus,
a separate timer device is programmed to handle the overflow.
Tested-by: Heiko Stuebner
Signed-off-by: Atish Patra
Signed-off-by: Atish Patra
---
target/riscv/cpu.c | 12 ++
target/riscv/cpu.h | 25 +++
target/riscv/cpu_bits.h | 55 +++
target/riscv/csr.c | 166 ++-
target/riscv/machine.c | 1 +
target/riscv/pmu.c | 357 +++-
target/riscv/pmu.h | 7 +
7 files changed, 612 insertions(+), 11 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d4635c7df46b..dc33b0dc9034 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -22,6 +22,7 @@
#include "qemu/ctype.h"
#include "qemu/log.h"
#include "cpu.h"
+#include "pmu.h"
#include "internals.h"
#include "exec/exec-all.h"
#include "qapi/error.h"
@@ -99,6 +100,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(zve64f, true, PRIV_VERSION_1_12_0, ext_zve64f),
ISA_EXT_DATA_ENTRY(zhinx, true, PRIV_VERSION_1_12_0, ext_zhinx),
ISA_EXT_DATA_ENTRY(zhinxmin, true, PRIV_VERSION_1_12_0, ext_zhinxmin),
+ISA_EXT_DATA_ENTRY(sscofpmf, true, PRIV_VERSION_1_12_0, ext_sscofpmf),
ISA_EXT_DATA_ENTRY(svinval, true, PRIV_VERSION_1_12_0, ext_svinval),
ISA_EXT_DATA_ENTRY(svnapot, true, PRIV_VERSION_1_12_0, ext_svnapot),
ISA_EXT_DATA_ENTRY(svpbmt, true, PRIV_VERSION_1_12_0, ext_svpbmt),
@@ -882,6 +884,15 @@ static void riscv_cpu_realize(DeviceState *dev, Error
**errp)
set_misa(env, env->misa_mxl, ext);
}
+#ifndef CONFIG_USER_ONLY
+if (cpu->cfg.pmu_num) {
+if (!riscv_pmu_init(cpu, cpu->cfg.pmu_num) && cpu->cfg.ext_sscofpmf) {
+cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+ riscv_pmu_timer_cb, cpu);
+}
+ }
+#endif
+
riscv_cpu_register_gdb_regs_for_features(cs);
qemu_init_vcpu(cs);
@@ -986,6 +997,7 @@ static Property riscv_cpu_extensions[] = {
DEFINE_PROP_BOOL("v", RISCVCPU, cfg.ext_v, false),
DEFINE_PROP_BOOL("h", RISCVCPU, cfg.ext_h, true),
DEFINE_PROP_UINT8("pmu-num", RISCVCPU, cfg.pmu_num, 16),
+DEFINE_PROP_BOOL("sscofpmf", RISCVCPU, cfg.ext_sscofpmf, false),
DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
DEFINE_PROP_BOOL("Zihintpause", RISCVCPU, cfg.ext_zihintpause, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 4be4b82a837d..64d90586256d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -137,6 +137,8 @@ typedef struct PMUCTRState {
/* Snapshort value of a counter in RV32 */
target_ulong mhpmcounterh_prev;
bool started;
+/* Value beyond UINT32_MAX/UINT64_MAX before overflow interrupt trigger */
+target_ulong irq_overflow_left;
} PMUCTRState;
struct CPUArchState {
@@ -302,6 +304,9 @@ struct CPUArchState {
/* PMU event selector configured values. First three are unused*/
target_ulong mhpmevent_val[RV_MAX_MHPMEVENTS];
+/* PMU event selector configured values for RV32*/
+target_ulong mhpmeventh_val[RV_MAX_MHPMEVENTS];
+
target_ulong sscratch;
target_ulong mscratch;
@@ -439,6 +444,7 @@ struct RISCVCPUConfig {
bool ext_zve32f;
bool ext_zve64f;
bool ext_zmmul;
+bool ext_sscofpmf;
bool rvv_ta_all_1s;
bool rvv_ma_all_1s;
@@ -486,6 +492,12 @@ struct ArchCPU {
/* Configuration Settings */
RISCVCPUConfig cfg;
+
+QEMUTimer *pmu_timer;
+/* A bitmask of Available programmable counters */
+uint32_t pmu_avail_ctrs;
+/* Mapping of events to counters */
+GHashTable *pmu_event_ctr_map;
};
static inline int riscv_has_ext(CPURISCVState *env, target_ulong ext)
@@ -746,6 +758,19 @@ enum {
CSR_TABLE_SIZE = 0x1000
};
+/**
+ * The event id are encoded based on the encoding specified in the
+ * SBI specification v0.3
+ */
+
+enum riscv_pmu_event_idx {
+RISCV_PMU_EVENT_HW_CPU_CYCLES = 0x01,
+RISCV_PMU_EVENT_HW_INSTRUCTIONS = 0x02,
+RISCV_PMU_EVENT_CACHE_DTLB_READ_MISS = 0x10019,
+RISCV_PMU_EVENT_CACHE_DTLB_WRITE_MISS = 0x1001B,
+RISCV_PMU_EVENT_CACHE_ITLB_PREFETCH_MISS = 0x10021,
+};
+
/* CSR function table */
extern riscv_csr_operations csr_ops[CSR_TABLE_SIZE];
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 6be5a9e9f046..b63c586be563 100644
--- a/target/riscv/cpu_bits.h
+++
[PATCH v12 2/6] target/riscv: Simplify counter predicate function
All the hpmcounters and the fixed counters (CY, IR, TM) can be represented
as a unified counter. Thus, the predicate function doesn't need handle each
case separately.
Simplify the predicate function so that we just handle things differently
between RV32/RV64 and S/HS mode.
Reviewed-by: Bin Meng
Acked-by: Alistair Francis
Signed-off-by: Atish Patra
---
target/riscv/csr.c | 110 -
1 file changed, 9 insertions(+), 101 deletions(-)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 9b45c49ab45f..6690b72ea0e7 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -74,6 +74,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
CPUState *cs = env_cpu(env);
RISCVCPU *cpu = RISCV_CPU(cs);
int ctr_index;
+target_ulong ctr_mask;
int base_csrno = CSR_CYCLE;
bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false;
@@ -82,122 +83,29 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
base_csrno += 0x80;
}
ctr_index = csrno - base_csrno;
+ctr_mask = BIT(ctr_index);
if ((csrno >= CSR_CYCLE && csrno <= CSR_INSTRET) ||
(csrno >= CSR_CYCLEH && csrno <= CSR_INSTRETH)) {
goto skip_ext_pmu_check;
}
-if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & BIT(ctr_index {
+if (!(cpu->pmu_avail_ctrs & ctr_mask)) {
/* No counter is enabled in PMU or the counter is out of range */
return RISCV_EXCP_ILLEGAL_INST;
}
skip_ext_pmu_check:
-if (env->priv == PRV_S) {
-switch (csrno) {
-case CSR_CYCLE:
-if (!get_field(env->mcounteren, COUNTEREN_CY)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_TIME:
-if (!get_field(env->mcounteren, COUNTEREN_TM)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_INSTRET:
-if (!get_field(env->mcounteren, COUNTEREN_IR)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
-if (!get_field(env->mcounteren, 1 << ctr_index)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-}
-if (rv32) {
-switch (csrno) {
-case CSR_CYCLEH:
-if (!get_field(env->mcounteren, COUNTEREN_CY)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_TIMEH:
-if (!get_field(env->mcounteren, COUNTEREN_TM)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_INSTRETH:
-if (!get_field(env->mcounteren, COUNTEREN_IR)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
-if (!get_field(env->mcounteren, 1 << ctr_index)) {
-return RISCV_EXCP_ILLEGAL_INST;
-}
-break;
-}
-}
+if (((env->priv == PRV_S) && (!get_field(env->mcounteren, ctr_mask))) ||
+ ((env->priv == PRV_U) && (!get_field(env->scounteren, ctr_mask {
+return RISCV_EXCP_ILLEGAL_INST;
}
if (riscv_cpu_virt_enabled(env)) {
-switch (csrno) {
-case CSR_CYCLE:
-if (!get_field(env->hcounteren, COUNTEREN_CY) &&
-get_field(env->mcounteren, COUNTEREN_CY)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-case CSR_TIME:
-if (!get_field(env->hcounteren, COUNTEREN_TM) &&
-get_field(env->mcounteren, COUNTEREN_TM)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-case CSR_INSTRET:
-if (!get_field(env->hcounteren, COUNTEREN_IR) &&
-get_field(env->mcounteren, COUNTEREN_IR)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
-if (!get_field(env->hcounteren, 1 << ctr_index) &&
- get_field(env->mcounteren, 1 << ctr_index)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-}
-if (rv32) {
-switch (csrno) {
-case CSR_CYCLEH:
-if (!get_field(env->hcounteren, COUNTEREN_CY) &&
-get_field(env->mcounteren, COUNTEREN_CY)) {
-return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-}
-break;
-case CSR_TIMEH:
-if (!get_field(env->hcounteren, COUNTEREN_TM) &&
-get_field(env->mcounteren, COUNTEREN_TM)) {
-return
[PATCH v12 0/6] Improve PMU support
The latest version of the SBI specification includes a Performance Monitoring
Unit(PMU) extension[1] which allows the supervisor to start/stop/configure
various PMU events. The Sscofpmf ('Ss' for Privileged arch and Supervisor-level
extensions, and 'cofpmf' for Count OverFlow and Privilege Mode Filtering)
extension[2] allows the perf like tool to handle overflow interrupts and
filtering support.
This series implements remaining PMU infrastructure to support
PMU in virt machine. The first seven patches from the original series
have been merged already.
This will allow us to add any PMU events in future.
Currently, this series enables the following omu events.
1. cycle count
2. instruction count
3. DTLB load/store miss
4. ITLB prefetch miss
The first two are computed using host ticks while last three are counted during
cpu_tlb_fill. We can do both sampling and count from guest userspace.
This series has been tested on both RV64 and RV32. Both Linux[3] and Opensbi[4]
patches are required to get the perf working.
Here is an output of perf stat/report while running hackbench with latest
OpenSBI & Linux kernel.
Perf stat:
==
[root@fedora-riscv ~]# perf stat -e cycles -e instructions -e dTLB-load-misses
-e dTLB-store-misses -e iTLB-load-misses \
> perf bench sched messaging -g 1 -l 10
# Running 'sched/messaging' benchmark:
# 20 sender and receiver processes per group
# 1 groups == 40 processes run
Total time: 0.265 [sec]
Performance counter stats for 'perf bench sched messaging -g 1 -l 10':
4,167,825,362 cycles
4,166,609,256 instructions #1.00 insn per cycle
3,092,026 dTLB-load-misses
258,280 dTLB-store-misses
2,068,966 iTLB-load-misses
0.585791767 seconds time elapsed
0.373802000 seconds user
1.042359000 seconds sys
Perf record:
[root@fedora-riscv ~]# perf record -e cycles -e instructions \
> -e dTLB-load-misses -e dTLB-store-misses -e iTLB-load-misses -c 1 \
> perf bench sched messaging -g 1 -l 10
# Running 'sched/messaging' benchmark:
# 20 sender and receiver processes per group
# 1 groups == 40 processes run
Total time: 1.397 [sec]
[ perf record: Woken up 10 times to write data ]
Check IO/CPU overload!
[ perf record: Captured and wrote 8.211 MB perf.data (214486 samples) ]
[root@fedora-riscv riscv]# perf report
Available samples
107K cycles◆
107K instructions ▒
250 dTLB-load-misses ▒
13 dTLB-store-misses ▒
172 iTLB-load-misses
..
Changes from v11->v12:
1. Rebased on top of the apply-next.
2. Aligned the write function & .min_priv to the previous line.
3. Fixed the FDT generations for multi-socket scenario.
4. Dropped interrupt property from the DT.
5. Generate illegal instruction fault instead of virtual instruction fault
for VS/VU access while mcounteren is not set.
Changes from v10->v11:
1. Rebased on top of the master where first 7 patches were already merged.
2. Removed unnecessary additional check in ctr predicate function.
3. Removed unnecessary priv version checks in mcountinhibit read/write.
4. Added Heiko's reviewed-by/tested-by tags.
Changes from v8->v9:
1. Added the write_done flags to the vmstate.
2. Fixed the hpmcounter read access from M-mode.
Changes from v7->v8:
1. Removeding ordering constraints for mhpmcounter & mhpmevent.
Changes from v6->v7:
1. Fixed all the compilation errors for the usermode.
Changes from v5->v6:
1. Fixed compilation issue with PATCH 1.
2. Addressed other comments.
Changes from v4->v5:
1. Rebased on top of the -next with following patches.
- isa extension
- priv 1.12 spec
2. Addressed all the comments on v4
3. Removed additional isa-ext DT node in favor of riscv,isa string update
Changes from v3->v4:
1. Removed the dummy events from pmu DT node.
2. Fixed pmu_avail_counters mask generation.
3. Added a patch to simplify the predicate function for counters.
Changes from v2->v3:
1. Addressed all the comments on PATCH1-4.
2. Split patch1 into two separate patches.
3. Added explicit comments to explain the event types in DT node.
4. Rebased on latest Qemu.
Changes from v1->v2:
1. Dropped the ACks from v1 as signficant changes happened after v1.
2. sscofpmf support.
3. A generic counter management framework.
[1] https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/riscv-sbi.adoc
[2] https://drive.google.com/file/d/171j4jFjIkKdj5LWcExphq4xG_2sihbfd/edit
[3]
Re: [RFC 0/3] Add Generic SPI GPIO model
Thanks everyone for the insightful feedback! This is really helpful for me. I am taking a look at all the comments now and will investigate into it. Best, Iris
installing qemu on linux
Good Morning, My name is Victor Herrera and I am attempting to install qemu on linux, I am trying to run CAN Bus well in the process of reseaching all these things. I am research assistant with the University of Texas at El Paso. I tried the link provided in the git repository, but there is an error on the page. Is there any other information I can use. Thank You Victor Herrera
Re: [PATCH 05/19] ppc/ppc405: Start QOMification of the SoC
On Tue, 2 Aug 2022, Daniel Henrique Barboza wrote:
On 8/1/22 10:10, Cédric Le Goater wrote:
This moves all the code previously done in the ppc405ep_init() routine
under ppc405_soc_realize().
Signed-off-by: Cédric Le Goater
---
hw/ppc/ppc405.h| 12 ++--
hw/ppc/ppc405_boards.c | 12 ++--
hw/ppc/ppc405_uc.c | 151 -
3 files changed, 84 insertions(+), 91 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index c8cddb71733a..5e4e96d86ceb 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -74,9 +74,14 @@ struct Ppc405SoCState {
MemoryRegion sram;
MemoryRegion ram_memories[2];
hwaddr ram_bases[2], ram_sizes[2];
+bool do_dram_init;
MemoryRegion *dram_mr;
hwaddr ram_size;
+
+uint32_t sysclk;
+PowerPCCPU *cpu;
+DeviceState *uic;
};
/* PowerPC 405 core */
@@ -85,11 +90,4 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env,
ram_addr_t ram_size);
void ppc4xx_plb_init(CPUPPCState *env);
void ppc405_ebc_init(CPUPPCState *env);
-PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem,
-MemoryRegion ram_memories[2],
-hwaddr ram_bases[2],
-hwaddr ram_sizes[2],
-uint32_t sysclk, DeviceState **uicdev,
-int do_init);
-
#endif /* PPC405_H */
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 96db52c5a309..363cb0770506 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -237,9 +237,7 @@ static void ppc405_init(MachineState *machine)
Ppc405MachineState *ppc405 = PPC405_MACHINE(machine);
MachineClass *mc = MACHINE_GET_CLASS(machine);
const char *kernel_filename = machine->kernel_filename;
-PowerPCCPU *cpu;
MemoryRegion *sysmem = get_system_memory();
-DeviceState *uicdev;
if (machine->ram_size != mc->default_ram_size) {
char *sz = size_to_str(mc->default_ram_size);
@@ -254,12 +252,12 @@ static void ppc405_init(MachineState *machine)
machine->ram_size, _fatal);
object_property_set_link(OBJECT(>soc), "dram",
OBJECT(machine->ram), _abort);
+object_property_set_bool(OBJECT(>soc), "dram-init",
+ !(kernel_filename == NULL), _abort);
+object_property_set_uint(OBJECT(>soc), "sys-clk", ,
+ _abort);
qdev_realize(DEVICE(>soc), NULL, _abort);
-cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories,
ppc405->soc.ram_bases,
-ppc405->soc.ram_sizes,
-, , kernel_filename == NULL ? 0 :
1);
-
/* allocate and load BIOS */
if (machine->firmware) {
MemoryRegion *bios = g_new(MemoryRegion, 1);
@@ -315,7 +313,7 @@ static void ppc405_init(MachineState *machine)
/* Load ELF kernel and rootfs.cpio */
} else if (kernel_filename && !machine->firmware) {
-boot_from_kernel(machine, cpu);
+boot_from_kernel(machine, ppc405->soc.cpu);
}
}
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index 156e839b8283..59612504bf3f 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1432,134 +1432,131 @@ static void ppc405ep_cpc_init (CPUPPCState *env,
clk_setup_t clk_setup[8],
#endif
}
-PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem,
-MemoryRegion ram_memories[2],
-hwaddr ram_bases[2],
-hwaddr ram_sizes[2],
-uint32_t sysclk, DeviceState **uicdevp,
-int do_init)
+static void ppc405_soc_realize(DeviceState *dev, Error **errp)
{
+Ppc405SoCState *s = PPC405_SOC(dev);
clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup;
qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4];
-PowerPCCPU *cpu;
CPUPPCState *env;
-DeviceState *uicdev;
-SysBusDevice *uicsbd;
+Error *err = NULL;
+
+/* XXX: fix this ? */
So, this comment, originally from ppc405_boards.c, was added by commit
1a6c088620368 and it seemed to make reference to something with the refering
to the ram_* values:
/* XXX: fix this */
ram_bases[0] = 0x;
ram_sizes[0] = 0x0800;
ram_bases[1] = 0x;
ram_sizes[1] = 0x;
(...)
No more context is provided aside from a git-svn-id from savannah.nongnu.org.
If no one can provide more context about what is to be fixed here, I'll
remove the comment.
I'm not sure about it because I don't know 405 and only vaguely remember
how this was on 440/460EX but I think it might be that the memory
controller can be programmed to map RAM to different places but we don't
fully emulate that nor the different chunks/DIMM sockets that could be
possible and just map all system RAM to address 0 which is what most guest
firmware or OS does anyway. Maybe I'm wrong and
Re: [PATCH v11 2/6] target/riscv: Simplify counter predicate function
On Wed, Jul 27, 2022 at 5:56 PM Weiwei Li wrote:
>
> 在 2022/7/28 上午5:40, Atish Kumar Patra 写道:
>
>
>
> On Wed, Jul 27, 2022 at 1:35 AM Weiwei Li wrote:
>
>>
>> 在 2022/7/27 下午2:49, Atish Patra 写道:
>> > All the hpmcounters and the fixed counters (CY, IR, TM) can be
>> represented
>> > as a unified counter. Thus, the predicate function doesn't need handle
>> each
>> > case separately.
>> >
>> > Simplify the predicate function so that we just handle things
>> differently
>> > between RV32/RV64 and S/HS mode.
>> >
>> > Reviewed-by: Bin Meng
>> > Acked-by: Alistair Francis
>> > Signed-off-by: Atish Patra
>> > ---
>> > target/riscv/csr.c | 112 +
>> > 1 file changed, 11 insertions(+), 101 deletions(-)
>> >
>> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> > index 1233bfa0a726..57dbbf9b09a0 100644
>> > --- a/target/riscv/csr.c
>> > +++ b/target/riscv/csr.c
>> > @@ -74,6 +74,7 @@ static RISCVException ctr(CPURISCVState *env, int
>> csrno)
>> > CPUState *cs = env_cpu(env);
>> > RISCVCPU *cpu = RISCV_CPU(cs);
>> > int ctr_index;
>> > +target_ulong ctr_mask;
>> > int base_csrno = CSR_CYCLE;
>> > bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false;
>> >
>> > @@ -82,122 +83,31 @@ static RISCVException ctr(CPURISCVState *env, int
>> csrno)
>> > base_csrno += 0x80;
>> > }
>> > ctr_index = csrno - base_csrno;
>> > +ctr_mask = BIT(ctr_index);
>> >
>> > if ((csrno >= CSR_CYCLE && csrno <= CSR_INSTRET) ||
>> > (csrno >= CSR_CYCLEH && csrno <= CSR_INSTRETH)) {
>> > goto skip_ext_pmu_check;
>> > }
>> >
>> > -if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs &
>> BIT(ctr_index {
>> > +if (!(cpu->pmu_avail_ctrs & ctr_mask)) {
>> > /* No counter is enabled in PMU or the counter is out of
>> range */
>> > return RISCV_EXCP_ILLEGAL_INST;
>> > }
>> >
>> > skip_ext_pmu_check:
>> >
>> > -if (env->priv == PRV_S) {
>> > -switch (csrno) {
>> > -case CSR_CYCLE:
>> > -if (!get_field(env->mcounteren, COUNTEREN_CY)) {
>> > -return RISCV_EXCP_ILLEGAL_INST;
>> > -}
>> > -break;
>> > -case CSR_TIME:
>> > -if (!get_field(env->mcounteren, COUNTEREN_TM)) {
>> > -return RISCV_EXCP_ILLEGAL_INST;
>> > -}
>> > -break;
>> > -case CSR_INSTRET:
>> > -if (!get_field(env->mcounteren, COUNTEREN_IR)) {
>> > -return RISCV_EXCP_ILLEGAL_INST;
>> > -}
>> > -break;
>> > -case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
>> > -if (!get_field(env->mcounteren, 1 << ctr_index)) {
>> > -return RISCV_EXCP_ILLEGAL_INST;
>> > -}
>> > -break;
>> > -}
>> > -if (rv32) {
>> > -switch (csrno) {
>> > -case CSR_CYCLEH:
>> > -if (!get_field(env->mcounteren, COUNTEREN_CY)) {
>> > -return RISCV_EXCP_ILLEGAL_INST;
>> > -}
>> > -break;
>> > -case CSR_TIMEH:
>> > -if (!get_field(env->mcounteren, COUNTEREN_TM)) {
>> > -return RISCV_EXCP_ILLEGAL_INST;
>> > -}
>> > -break;
>> > -case CSR_INSTRETH:
>> > -if (!get_field(env->mcounteren, COUNTEREN_IR)) {
>> > -return RISCV_EXCP_ILLEGAL_INST;
>> > -}
>> > -break;
>> > -case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
>> > -if (!get_field(env->mcounteren, 1 << ctr_index)) {
>> > -return RISCV_EXCP_ILLEGAL_INST;
>> > -}
>> > -break;
>> > -}
>> > -}
>> > +if (((env->priv == PRV_S) && (!get_field(env->mcounteren,
>> ctr_mask))) ||
>> > + ((env->priv == PRV_U) && (!get_field(env->scounteren,
>> ctr_mask {
>> > +return RISCV_EXCP_ILLEGAL_INST;
>> > }
>> >
>> > if (riscv_cpu_virt_enabled(env)) {
>> > -switch (csrno) {
>> > -case CSR_CYCLE:
>> > -if (!get_field(env->hcounteren, COUNTEREN_CY) &&
>> > -get_field(env->mcounteren, COUNTEREN_CY)) {
>> > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>> > -}
>> > -break;
>> > -case CSR_TIME:
>> > -if (!get_field(env->hcounteren, COUNTEREN_TM) &&
>> > -get_field(env->mcounteren, COUNTEREN_TM)) {
>> > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>> > -}
>> > -break;
>> > -case CSR_INSTRET:
>> > -if (!get_field(env->hcounteren, COUNTEREN_IR) &&
>> > -get_field(env->mcounteren, COUNTEREN_IR)) {
>> > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>> > -}
>> > -
Re: [PATCH 1/1] vfio-user: update submodule to latest
On Tue, 2 Aug 2022 at 05:44, Daniel P. Berrangé wrote: > On Mon, Aug 01, 2022 at 09:24:04PM -0400, Jagannathan Raman wrote: > > Update libvfio-user submodule to the latest > > > > Signed-off-by: Jagannathan Raman > > --- > > subprojects/libvfio-user | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/subprojects/libvfio-user b/subprojects/libvfio-user > > index 0b28d20557..1305f161b7 16 > > --- a/subprojects/libvfio-user > > +++ b/subprojects/libvfio-user > > @@ -1 +1 @@ > > -Subproject commit 0b28d205572c80b568a1003db2c8f37ca333e4d7 > > +Subproject commit 1305f161b7e0dd2c2a420c17efcb0bd49b94dad4 > > Only 3 changes in the submodule with this: > > 1305f161b7e0dd2c2a420c17efcb0bd49b94dad4 disable client-server test by > default (#700) > 36beb63be45ad1412562a98d9373a4c0bd91ab3d support for shadow ioeventfd (#698) > 1c274027bb4f9d68eee846036e8d50dcde2fd7e9 improve README.md (#696) > > That fixes the testing bug we see, the other change affects an API that > QEMU doesn't use. Overall looks safe for applying during soft freeze. > > Reviewed-by: Daniel P. Berrangé CCing Richard so this can be merged. Stefan
Re: [PATCH 04/19] ppc/ppc405: Introduce a PPC405 SoC
On 8/1/22 10:10, Cédric Le Goater wrote:
It is an initial model to start QOMification of the PPC405 board.
Signed-off-by: Cédric Le Goater
---
Reviewed-by: Daniel Henrique Barboza
hw/ppc/ppc405.h| 17 ++
hw/ppc/ppc405_boards.c | 29 ++-
hw/ppc/ppc405_uc.c | 53 ++
3 files changed, 82 insertions(+), 17 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index 83f156f585c8..c8cddb71733a 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -25,6 +25,7 @@
#ifndef PPC405_H
#define PPC405_H
+#include "qom/object.h"
#include "hw/ppc/ppc4xx.h"
#define PPC405EP_SDRAM_BASE 0x
@@ -62,6 +63,22 @@ struct ppc4xx_bd_info_t {
uint32_t bi_iic_fast[2];
};
+#define TYPE_PPC405_SOC "ppc405-soc"
+OBJECT_DECLARE_SIMPLE_TYPE(Ppc405SoCState, PPC405_SOC);
+
+struct Ppc405SoCState {
+/* Private */
+DeviceState parent_obj;
+
+/* Public */
+MemoryRegion sram;
+MemoryRegion ram_memories[2];
+hwaddr ram_bases[2], ram_sizes[2];
+
+MemoryRegion *dram_mr;
+hwaddr ram_size;
+};
+
/* PowerPC 405 core */
ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, ram_addr_t ram_size);
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 24ec948d22a4..96db52c5a309 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -54,6 +54,8 @@ struct Ppc405MachineState {
/* Private */
MachineState parent_obj;
/* Public */
+
+Ppc405SoCState soc;
};
#define TYPE_PPC405_MACHINE MACHINE_TYPE_NAME("ppc405")
@@ -232,12 +234,10 @@ static void boot_from_kernel(MachineState *machine,
PowerPCCPU *cpu)
static void ppc405_init(MachineState *machine)
{
+Ppc405MachineState *ppc405 = PPC405_MACHINE(machine);
MachineClass *mc = MACHINE_GET_CLASS(machine);
const char *kernel_filename = machine->kernel_filename;
PowerPCCPU *cpu;
-MemoryRegion *sram = g_new(MemoryRegion, 1);
-MemoryRegion *ram_memories = g_new(MemoryRegion, 2);
-hwaddr ram_bases[2], ram_sizes[2];
MemoryRegion *sysmem = get_system_memory();
DeviceState *uicdev;
@@ -248,23 +248,18 @@ static void ppc405_init(MachineState *machine)
exit(EXIT_FAILURE);
}
-/* XXX: fix this */
-memory_region_init_alias(_memories[0], NULL, "ef405ep.ram.alias",
- machine->ram, 0, machine->ram_size);
-ram_bases[0] = 0;
-ram_sizes[0] = machine->ram_size;
-memory_region_init(_memories[1], NULL, "ef405ep.ram1", 0);
-ram_bases[1] = 0x;
-ram_sizes[1] = 0x;
+object_initialize_child(OBJECT(machine), "soc", >soc,
+TYPE_PPC405_SOC);
+object_property_set_uint(OBJECT(>soc), "ram-size",
+ machine->ram_size, _fatal);
+object_property_set_link(OBJECT(>soc), "dram",
+ OBJECT(machine->ram), _abort);
+qdev_realize(DEVICE(>soc), NULL, _abort);
-cpu = ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
+cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories,
ppc405->soc.ram_bases,
+ppc405->soc.ram_sizes,
, , kernel_filename == NULL ? 0 : 1);
-/* allocate SRAM */
-memory_region_init_ram(sram, NULL, "ef405ep.sram", PPC405EP_SRAM_SIZE,
- _fatal);
-memory_region_add_subregion(sysmem, PPC405EP_SRAM_BASE, sram);
-
/* allocate and load BIOS */
if (machine->firmware) {
MemoryRegion *bios = g_new(MemoryRegion, 1);
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index d6420c88d3a6..156e839b8283 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -30,6 +30,7 @@
#include "hw/ppc/ppc.h"
#include "hw/i2c/ppc4xx_i2c.h"
#include "hw/irq.h"
+#include "hw/qdev-properties.h"
#include "ppc405.h"
#include "hw/char/serial.h"
#include "qemu/timer.h"
@@ -1530,3 +1531,55 @@ PowerPCCPU *ppc405ep_init(MemoryRegion
*address_space_mem,
return cpu;
}
+
+static void ppc405_soc_realize(DeviceState *dev, Error **errp)
+{
+Ppc405SoCState *s = PPC405_SOC(dev);
+Error *err = NULL;
+
+/* XXX: fix this ? */
+memory_region_init_alias(>ram_memories[0], OBJECT(s),
+ "ef405ep.ram.alias", s->dram_mr, 0, s->ram_size);
+s->ram_bases[0] = 0;
+s->ram_sizes[0] = s->ram_size;
+memory_region_init(>ram_memories[1], OBJECT(s), "ef405ep.ram1", 0);
+s->ram_bases[1] = 0x;
+s->ram_sizes[1] = 0x;
+
+/* allocate SRAM */
+memory_region_init_ram(>sram, OBJECT(s), "ef405ep.sram",
+ PPC405EP_SRAM_SIZE, );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+memory_region_add_subregion(get_system_memory(), PPC405EP_SRAM_BASE,
+>sram);
+}
+
+static Property ppc405_soc_properties[] = {
+
Re: [PATCH 05/19] ppc/ppc405: Start QOMification of the SoC
On 8/1/22 10:10, Cédric Le Goater wrote:
This moves all the code previously done in the ppc405ep_init() routine
under ppc405_soc_realize().
Signed-off-by: Cédric Le Goater
---
hw/ppc/ppc405.h| 12 ++--
hw/ppc/ppc405_boards.c | 12 ++--
hw/ppc/ppc405_uc.c | 151 -
3 files changed, 84 insertions(+), 91 deletions(-)
diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
index c8cddb71733a..5e4e96d86ceb 100644
--- a/hw/ppc/ppc405.h
+++ b/hw/ppc/ppc405.h
@@ -74,9 +74,14 @@ struct Ppc405SoCState {
MemoryRegion sram;
MemoryRegion ram_memories[2];
hwaddr ram_bases[2], ram_sizes[2];
+bool do_dram_init;
MemoryRegion *dram_mr;
hwaddr ram_size;
+
+uint32_t sysclk;
+PowerPCCPU *cpu;
+DeviceState *uic;
};
/* PowerPC 405 core */
@@ -85,11 +90,4 @@ ram_addr_t ppc405_set_bootinfo(CPUPPCState *env, ram_addr_t
ram_size);
void ppc4xx_plb_init(CPUPPCState *env);
void ppc405_ebc_init(CPUPPCState *env);
-PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem,
-MemoryRegion ram_memories[2],
-hwaddr ram_bases[2],
-hwaddr ram_sizes[2],
-uint32_t sysclk, DeviceState **uicdev,
-int do_init);
-
#endif /* PPC405_H */
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 96db52c5a309..363cb0770506 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -237,9 +237,7 @@ static void ppc405_init(MachineState *machine)
Ppc405MachineState *ppc405 = PPC405_MACHINE(machine);
MachineClass *mc = MACHINE_GET_CLASS(machine);
const char *kernel_filename = machine->kernel_filename;
-PowerPCCPU *cpu;
MemoryRegion *sysmem = get_system_memory();
-DeviceState *uicdev;
if (machine->ram_size != mc->default_ram_size) {
char *sz = size_to_str(mc->default_ram_size);
@@ -254,12 +252,12 @@ static void ppc405_init(MachineState *machine)
machine->ram_size, _fatal);
object_property_set_link(OBJECT(>soc), "dram",
OBJECT(machine->ram), _abort);
+object_property_set_bool(OBJECT(>soc), "dram-init",
+ !(kernel_filename == NULL), _abort);
+object_property_set_uint(OBJECT(>soc), "sys-clk", ,
+ _abort);
qdev_realize(DEVICE(>soc), NULL, _abort);
-cpu = ppc405ep_init(sysmem, ppc405->soc.ram_memories, ppc405->soc.ram_bases,
-ppc405->soc.ram_sizes,
-, , kernel_filename == NULL ? 0 : 1);
-
/* allocate and load BIOS */
if (machine->firmware) {
MemoryRegion *bios = g_new(MemoryRegion, 1);
@@ -315,7 +313,7 @@ static void ppc405_init(MachineState *machine)
/* Load ELF kernel and rootfs.cpio */
} else if (kernel_filename && !machine->firmware) {
-boot_from_kernel(machine, cpu);
+boot_from_kernel(machine, ppc405->soc.cpu);
}
}
diff --git a/hw/ppc/ppc405_uc.c b/hw/ppc/ppc405_uc.c
index 156e839b8283..59612504bf3f 100644
--- a/hw/ppc/ppc405_uc.c
+++ b/hw/ppc/ppc405_uc.c
@@ -1432,134 +1432,131 @@ static void ppc405ep_cpc_init (CPUPPCState *env,
clk_setup_t clk_setup[8],
#endif
}
-PowerPCCPU *ppc405ep_init(MemoryRegion *address_space_mem,
-MemoryRegion ram_memories[2],
-hwaddr ram_bases[2],
-hwaddr ram_sizes[2],
-uint32_t sysclk, DeviceState **uicdevp,
-int do_init)
+static void ppc405_soc_realize(DeviceState *dev, Error **errp)
{
+Ppc405SoCState *s = PPC405_SOC(dev);
clk_setup_t clk_setup[PPC405EP_CLK_NB], tlb_clk_setup;
qemu_irq dma_irqs[4], gpt_irqs[5], mal_irqs[4];
-PowerPCCPU *cpu;
CPUPPCState *env;
-DeviceState *uicdev;
-SysBusDevice *uicsbd;
+Error *err = NULL;
+
+/* XXX: fix this ? */
So, this comment, originally from ppc405_boards.c, was added by commit
1a6c088620368 and it seemed to make reference to something with the refering
to the ram_* values:
/* XXX: fix this */
ram_bases[0] = 0x;
ram_sizes[0] = 0x0800;
ram_bases[1] = 0x;
ram_sizes[1] = 0x;
(...)
No more context is provided aside from a git-svn-id from savannah.nongnu.org.
If no one can provide more context about what is to be fixed here, I'll
remove the comment.
+memory_region_init_alias(>ram_memories[0], OBJECT(s),
+ "ef405ep.ram.alias", s->dram_mr, 0, s->ram_size);
As I mentioned in patch 2, ef405ep.ram.alias can be renamed to ppc405.ram.alias
...
+s->ram_bases[0] = 0;
+s->ram_sizes[0] = s->ram_size;
+memory_region_init(>ram_memories[1], OBJECT(s), "ef405ep.ram1", 0);
And this can be renamed to ef405ep.ram1. If you agree with the rename I
can
Re: [PULL 0/5] migration queue
On 8/2/22 08:54, Dr. David Alan Gilbert (git) wrote: From: "Dr. David Alan Gilbert" The following changes since commit 0399521e53336bd2cdc15482bca0ffd3493fdff6: Merge tag 'for-upstream' of git://repo.or.cz/qemu/kevin into staging (2022-08-02 06:52:05 -0700) are available in the Git repository at: https://gitlab.com/dagrh/qemu.git tags/pull-migration-20220802c for you to fetch changes up to a21ba54dd5ca38cd05da9035fc65374d7af54f13: virtiofsd: Disable killpriv_v2 by default (2022-08-02 16:46:52 +0100) Migration fixes pull 2022-08-02 Small migration (and virtiofsd) fixes. Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate. r~ Signed-off-by: Dr. David Alan Gilbert Leonardo Bras (1): migration: add remaining params->has_* = true in migration_instance_init() Peter Maydell (2): migration: Assert that migrate_multifd_compression() returns an in-range value migration: Define BLK_MIG_BLOCK_SIZE as unsigned long long Thomas Huth (1): Revert "migration: Simplify unqueue_page()" Vivek Goyal (1): virtiofsd: Disable killpriv_v2 by default migration/block.c| 2 +- migration/migration.c| 5 + migration/ram.c | 37 ++--- migration/trace-events | 3 ++- tools/virtiofsd/passthrough_ll.c | 13 ++--- 5 files changed, 36 insertions(+), 24 deletions(-)
Re: [PATCH 03/19] ppc/ppc405: Move devices under the ref405ep machine
On 8/1/22 10:10, Cédric Le Goater wrote:
Signed-off-by: Cédric Le Goater
---
Reviewed-by: Daniel Henrique Barboza
hw/ppc/ppc405_boards.c | 31 +++
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 4c269b6526a5..24ec948d22a4 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -230,13 +230,11 @@ static void boot_from_kernel(MachineState *machine,
PowerPCCPU *cpu)
env->load_info = _info;
}
-static void ref405ep_init(MachineState *machine)
+static void ppc405_init(MachineState *machine)
{
MachineClass *mc = MACHINE_GET_CLASS(machine);
const char *kernel_filename = machine->kernel_filename;
PowerPCCPU *cpu;
-DeviceState *dev;
-SysBusDevice *s;
MemoryRegion *sram = g_new(MemoryRegion, 1);
MemoryRegion *ram_memories = g_new(MemoryRegion, 2);
hwaddr ram_bases[2], ram_sizes[2];
@@ -294,15 +292,6 @@ static void ref405ep_init(MachineState *machine)
memory_region_add_subregion(sysmem, (uint32_t)(-bios_size), bios);
}
-/* Register FPGA */
-ref405ep_fpga_init(sysmem, PPC405EP_FPGA_BASE);
-/* Register NVRAM */
-dev = qdev_new("sysbus-m48t08");
-qdev_prop_set_int32(dev, "base-year", 1968);
-s = SYS_BUS_DEVICE(dev);
-sysbus_realize_and_unref(s, _fatal);
-sysbus_mmio_map(s, 0, PPC405EP_NVRAM_BASE);
-
/* Load kernel and initrd using U-Boot images */
if (kernel_filename && machine->firmware) {
target_ulong kernel_base, initrd_base;
@@ -335,6 +324,23 @@ static void ref405ep_init(MachineState *machine)
}
}
+static void ref405ep_init(MachineState *machine)
+{
+DeviceState *dev;
+SysBusDevice *s;
+
+ppc405_init(machine);
+
+/* Register FPGA */
+ref405ep_fpga_init(get_system_memory(), PPC405EP_FPGA_BASE);
+/* Register NVRAM */
+dev = qdev_new("sysbus-m48t08");
+qdev_prop_set_int32(dev, "base-year", 1968);
+s = SYS_BUS_DEVICE(dev);
+sysbus_realize_and_unref(s, _fatal);
+sysbus_mmio_map(s, 0, PPC405EP_NVRAM_BASE);
+}
+
static void ref405ep_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -354,6 +360,7 @@ static void ppc405_machine_class_init(ObjectClass *oc, void
*data)
MachineClass *mc = MACHINE_CLASS(oc);
mc->desc = "PPC405 generic machine";
+mc->init = ppc405_init;
mc->default_ram_size = 0x0800;
mc->default_ram_id = "ppc405.ram";
}
Re: [PATCH 02/19] ppc/ppc405: Introduce a PPC405 generic machine
On 8/1/22 10:10, Cédric Le Goater wrote:
We will use this machine as a base to define the ref405ep and possibly
the PPC405 hotfoot board as found in the Linux kernel.
Signed-off-by: Cédric Le Goater
---
hw/ppc/ppc405_boards.c | 31 ---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 1a4e7588c584..4c269b6526a5 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -50,6 +50,15 @@
#define USE_FLASH_BIOS
+struct Ppc405MachineState {
+/* Private */
+MachineState parent_obj;
+/* Public */
+};
+
+#define TYPE_PPC405_MACHINE MACHINE_TYPE_NAME("ppc405")
+OBJECT_DECLARE_SIMPLE_TYPE(Ppc405MachineState, PPC405_MACHINE);
+
/*/
/* PPC405EP reference board (IBM) */
/* Standalone board with:
@@ -332,18 +341,34 @@ static void ref405ep_class_init(ObjectClass *oc, void
*data)
mc->desc = "ref405ep";
mc->init = ref405ep_init;
-mc->default_ram_size = 0x0800;
-mc->default_ram_id = "ef405ep.ram";
}
static const TypeInfo ref405ep_type = {
.name = MACHINE_TYPE_NAME("ref405ep"),
-.parent = TYPE_MACHINE,
+.parent = TYPE_PPC405_MACHINE,
.class_init = ref405ep_class_init,
};
+static void ppc405_machine_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->desc = "PPC405 generic machine";
+mc->default_ram_size = 0x0800;
+mc->default_ram_id = "ppc405.ram";
I don't mind changing the default_ram_id from "ef405ep.ram" to "ppc405.ram",
but since we're renaming it, might as well rename the remaining instances
of ef405ep.ram:
$ git grep 'ef405ep.ram'
hw/ppc/ppc405_uc.c: "ef405ep.ram.alias", s->dram_mr,
0, s->ram_size);
hw/ppc/ppc405_uc.c:memory_region_init(>ram_memories[1], OBJECT(s),
"ef405ep.ram1", 0);
I believe these can be renamed to "ppc405.ram.alias" and "ppc405.ram1" in
patch 05.
Thanks,
Daniel
+}
+
+static const TypeInfo ppc405_machine_type = {
+.name = TYPE_PPC405_MACHINE,
+.parent = TYPE_MACHINE,
+.instance_size = sizeof(Ppc405MachineState),
+.class_init = ppc405_machine_class_init,
+.abstract = true,
+};
+
static void ppc405_machine_init(void)
{
+type_register_static(_machine_type);
type_register_static(_type);
}
[PATCH v4 2/2] target/s390x: support SHA-512 extensions
In order to fully support MSA_EXT_5, we have to also support the SHA-512
special instructions. So implement those.
The implementation began as something TweetNacl-like, and then was
adjusted to be useful here. It's not very beautiful, but it is quite
short and compact, which is what we're going for.
Cc: Thomas Huth
Cc: David Hildenbrand
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: Cornelia Huck
Cc: Harald Freudenberger
Cc: Holger Dengler
Signed-off-by: Jason A. Donenfeld
---
target/s390x/gen-features.c | 2 +
target/s390x/tcg/crypto_helper.c | 116 +++
2 files changed, 118 insertions(+)
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 3d333e2789..b6d804fa6d 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -751,6 +751,8 @@ static uint16_t qemu_MAX[] = {
S390_FEAT_VECTOR_ENH2,
S390_FEAT_MSA_EXT_5,
S390_FEAT_PRNO_TRNG,
+S390_FEAT_KIMD_SHA_512,
+S390_FEAT_KLMD_SHA_512,
};
/** END FEATURE DEFS **/
diff --git a/target/s390x/tcg/crypto_helper.c b/target/s390x/tcg/crypto_helper.c
index 8ad4ef1ace..475627aa83 100644
--- a/target/s390x/tcg/crypto_helper.c
+++ b/target/s390x/tcg/crypto_helper.c
@@ -19,6 +19,112 @@
#include "exec/exec-all.h"
#include "exec/cpu_ldst.h"
+static uint64_t R(uint64_t x, int c) { return (x >> c) | (x << (64 - c)); }
+static uint64_t Ch(uint64_t x, uint64_t y, uint64_t z) { return (x & y) ^ (~x
& z); }
+static uint64_t Maj(uint64_t x, uint64_t y, uint64_t z) { return (x & y) ^ (x
& z) ^ (y & z); }
+static uint64_t Sigma0(uint64_t x) { return R(x, 28) ^ R(x, 34) ^ R(x, 39); }
+static uint64_t Sigma1(uint64_t x) { return R(x, 14) ^ R(x, 18) ^ R(x, 41); }
+static uint64_t sigma0(uint64_t x) { return R(x, 1) ^ R(x, 8) ^ (x >> 7); }
+static uint64_t sigma1(uint64_t x) { return R(x, 19) ^ R(x, 61) ^ (x >> 6); }
+
+static const uint64_t K[80] = {
+0x428a2f98d728ae22ULL, 0x7137449123ef65cdULL, 0xb5c0fbcfec4d3b2fULL,
+0xe9b5dba58189dbbcULL, 0x3956c25bf348b538ULL, 0x59f111f1b605d019ULL,
+0x923f82a4af194f9bULL, 0xab1c5ed5da6d8118ULL, 0xd807aa98a3030242ULL,
+0x12835b0145706fbeULL, 0x243185be4ee4b28cULL, 0x550c7dc3d5ffb4e2ULL,
+0x72be5d74f27b896fULL, 0x80deb1fe3b1696b1ULL, 0x9bdc06a725c71235ULL,
+0xc19bf174cf692694ULL, 0xe49b69c19ef14ad2ULL, 0xefbe4786384f25e3ULL,
+0x0fc19dc68b8cd5b5ULL, 0x240ca1cc77ac9c65ULL, 0x2de92c6f592b0275ULL,
+0x4a7484aa6ea6e483ULL, 0x5cb0a9dcbd41fbd4ULL, 0x76f988da831153b5ULL,
+0x983e5152ee66dfabULL, 0xa831c66d2db43210ULL, 0xb00327c898fb213fULL,
+0xbf597fc7beef0ee4ULL, 0xc6e00bf33da88fc2ULL, 0xd5a79147930aa725ULL,
+0x06ca6351e003826fULL, 0x142929670a0e6e70ULL, 0x27b70a8546d22ffcULL,
+0x2e1b21385c26c926ULL, 0x4d2c6dfc5ac42aedULL, 0x53380d139d95b3dfULL,
+0x650a73548baf63deULL, 0x766a0abb3c77b2a8ULL, 0x81c2c92e47edaee6ULL,
+0x92722c851482353bULL, 0xa2bfe8a14cf10364ULL, 0xa81a664bbc423001ULL,
+0xc24b8b70d0f89791ULL, 0xc76c51a30654be30ULL, 0xd192e819d6ef5218ULL,
+0xd69906245565a910ULL, 0xf40e35855771202aULL, 0x106aa07032bbd1b8ULL,
+0x19a4c116b8d2d0c8ULL, 0x1e376c085141ab53ULL, 0x2748774cdf8eeb99ULL,
+0x34b0bcb5e19b48a8ULL, 0x391c0cb3c5c95a63ULL, 0x4ed8aa4ae3418acbULL,
+0x5b9cca4f7763e373ULL, 0x682e6ff3d6b2b8a3ULL, 0x748f82ee5defb2fcULL,
+0x78a5636f43172f60ULL, 0x84c87814a1f0ab72ULL, 0x8cc702081a6439ecULL,
+0x90befffa23631e28ULL, 0xa4506cebde82bde9ULL, 0xbef9a3f7b2c67915ULL,
+0xc67178f2e372532bULL, 0xca273eceea26619cULL, 0xd186b8c721c0c207ULL,
+0xeada7dd6cde0eb1eULL, 0xf57d4f7fee6ed178ULL, 0x06f067aa72176fbaULL,
+0x0a637dc5a2c898a6ULL, 0x113f9804bef90daeULL, 0x1b710b35131c471bULL,
+0x28db77f523047d84ULL, 0x32caab7b40c72493ULL, 0x3c9ebe0a15c9bebcULL,
+0x431d67c49c100d4cULL, 0x4cc5d4becb3e42b6ULL, 0x597f299cfc657e2aULL,
+0x5fcb6fab3ad6faecULL, 0x6c44198c4a475817ULL
+};
+
+static void kimd_sha512(CPUS390XState *env, uintptr_t ra, uint64_t
parameter_block,
+uint64_t *message_reg, uint64_t *len_reg, uint8_t
*stack_buffer)
+{
+uint64_t z[8], b[8], a[8], w[16], t;
+int i, j;
+
+for (i = 0; i < 8; ++i)
+z[i] = a[i] = cpu_ldq_be_data_ra(env, wrap_address(env,
parameter_block + 8 * i), ra);
+
+while (*len_reg >= 128) {
+for (i = 0; i < 16; ++i) {
+if (message_reg)
+w[i] = cpu_ldq_be_data_ra(env, wrap_address(env, *message_reg
+ 8 * i), ra);
+else
+w[i] = be64_to_cpu(((uint64_t *)stack_buffer)[i]);
+}
+
+for (i = 0; i < 80; ++i) {
+for (j = 0; j < 8; ++j)
+b[j] = a[j];
+t = a[7] + Sigma1(a[4]) + Ch(a[4], a[5], a[6]) + K[i] + w[i % 16];
+b[7] = t + Sigma0(a[0]) + Maj(a[0], a[1], a[2]);
+b[3] += t;
+for (j = 0; j < 8; ++j)
+a[(j + 1) % 8] = b[j];
+if (i % 16 == 15) {
+for (j = 0; j < 16; ++j)
+
[PATCH v4 1/2] target/s390x: support PRNO_TRNG instruction
In order for hosts running inside of TCG to initialize the kernel's
random number generator, we should support the PRNO_TRNG instruction,
backed in the usual way with the qemu_guest_getrandom helper. This is
confirmed working on Linux 5.19.
Cc: Thomas Huth
Cc: David Hildenbrand
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: Cornelia Huck
Cc: Harald Freudenberger
Cc: Holger Dengler
Signed-off-by: Jason A. Donenfeld
---
target/s390x/gen-features.c | 2 ++
target/s390x/tcg/crypto_helper.c | 30 ++
2 files changed, 32 insertions(+)
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index ad140184b9..3d333e2789 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -749,6 +749,8 @@ static uint16_t qemu_V7_0[] = {
*/
static uint16_t qemu_MAX[] = {
S390_FEAT_VECTOR_ENH2,
+S390_FEAT_MSA_EXT_5,
+S390_FEAT_PRNO_TRNG,
};
/** END FEATURE DEFS **/
diff --git a/target/s390x/tcg/crypto_helper.c b/target/s390x/tcg/crypto_helper.c
index 138d9e7ad9..8ad4ef1ace 100644
--- a/target/s390x/tcg/crypto_helper.c
+++ b/target/s390x/tcg/crypto_helper.c
@@ -12,12 +12,38 @@
#include "qemu/osdep.h"
#include "qemu/main-loop.h"
+#include "qemu/guest-random.h"
#include "s390x-internal.h"
#include "tcg_s390x.h"
#include "exec/helper-proto.h"
#include "exec/exec-all.h"
#include "exec/cpu_ldst.h"
+static void fill_buf_random(CPUS390XState *env, uintptr_t ra,
+uint64_t *buf_reg, uint64_t *len_reg)
+{
+uint8_t tmp[256];
+uint64_t len = *len_reg;
+int reg_len = 64;
+
+if (!(env->psw.mask & PSW_MASK_64)) {
+len = (uint32_t)len;
+reg_len = (env->psw.mask & PSW_MASK_32) ? 32 : 24;
+}
+
+while (len) {
+size_t block = MIN(len, sizeof(tmp));
+
+qemu_guest_getrandom_nofail(tmp, block);
+for (size_t i = 0; i < block; ++i) {
+cpu_stb_data_ra(env, wrap_address(env, *buf_reg), tmp[i], ra);
+*buf_reg = deposit64(*buf_reg, 0, reg_len, *buf_reg + 1);
+--*len_reg;
+}
+len -= block;
+}
+}
+
uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3,
uint32_t type)
{
@@ -52,6 +78,10 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1,
uint32_t r2, uint32_t r3,
cpu_stb_data_ra(env, param_addr, subfunc[i], ra);
}
break;
+case 114: /* CPACF_PRNO_TRNG */
+fill_buf_random(env, ra, >regs[r1], >regs[r1 + 1]);
+fill_buf_random(env, ra, >regs[r2], >regs[r2 + 1]);
+break;
default:
/* we don't implement any other subfunction yet */
g_assert_not_reached();
--
2.35.1
[PATCH v4 0/2] MSA EXT 5 for s390x
In addition to the prior TRNG patch from v3, this v4 adds SHA-512 support. I know, I know, I know -- I fussed around asking if somebody would help me implement this because it was "oh so hard", and offered to do the crypto part if someone would do the rest. But then once I had the crypto part, I wanted some way to test it and then... and then the implementation worked and passed the test vectors. So now these two patches together implement MSA EXT 5, and appear to be working with Linux's drivers for it. Cc: Thomas Huth Cc: David Hildenbrand Cc: Christian Borntraeger Cc: Richard Henderson Cc: Cornelia Huck Cc: Harald Freudenberger Cc: Holger Dengler Jason A. Donenfeld (2): target/s390x: support PRNO_TRNG instruction target/s390x: support SHA-512 extensions target/s390x/gen-features.c | 4 + target/s390x/tcg/crypto_helper.c | 146 +++ 2 files changed, 150 insertions(+) -- 2.35.1
Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
On Tue, 2 Aug 2022 at 17:43, Daniel P. Berrangé wrote:
>
> The latest glibc 2.36 has extended sys/mount.h so that it
> defines the FSCONFIG_* enum constants. These are historically
> defined in linux/mount.h, and thus if you include both headers
> the compiler complains:
>
> In file included from /usr/include/linux/fs.h:19,
> from ../linux-user/syscall.c:98:
> /usr/include/linux/mount.h:95:6: error: redeclaration of 'enum
> fsconfig_command'
>95 | enum fsconfig_command {
> | ^~~~
> In file included from ../linux-user/syscall.c:31:
> /usr/include/sys/mount.h:189:6: note: originally defined here
> 189 | enum fsconfig_command
> | ^~~~
> /usr/include/linux/mount.h:96:9: error: redeclaration of enumerator
> 'FSCONFIG_SET_FLAG'
>96 | FSCONFIG_SET_FLAG = 0,/* Set parameter, supplying
> no value */
> | ^
> /usr/include/sys/mount.h:191:3: note: previous definition of
> 'FSCONFIG_SET_FLAG' with type 'enum fsconfig_command'
> 191 | FSCONFIG_SET_FLAG = 0,/* Set parameter, supplying no
> value */
> | ^
> ...snip...
>
> QEMU doesn't include linux/mount.h, but it does use
> linux/fs.h and thus gets linux/mount.h indirectly.
>
> glibc acknowledges this problem but does not appear to
> be intending to fix it in the forseeable future, simply
> documenting it as a known incompatibility with no
> workaround:
>
>
> https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
> https://sourceware.org/glibc/wiki/Synchronizing_Headers
>
> To address this requires either removing use of sys/mount.h
> or linux/fs.h, despite QEMU needing declarations from
> both.
>
> This patch removes linux/fs.h, meaning we have to define
> various FS_IOC constants that are now unavailable.
I don't suppose anybody has useful contacts with the glibc
developers so we can ask them if they can try to maybe
not break applications like this, or at least if they do
to provide workarounds ? Having to hand-define
a bunch of ioctl constants is particularly messy.
thanks
-- PMM
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
On Tue, Aug 02, 2022 at 05:32:26PM +0200, David Hildenbrand wrote: > On 02.08.22 17:28, Jason A. Donenfeld wrote: > > Hi David, Christian, > > > > While this thread has your attention, I thought I'd reiterate my offer in: > > https://lore.kernel.org/qemu-devel/yueouwzdzbqff...@zx2c4.com/ > > > > Do either of you want to "take ownership" of this patch to bring it > > past the finish line, and I can provide whatever additional crypto > > code you need for that? > > For me the patch is good enough. But sure, having a SHA512 > implementation would be nice ... > > Long story short, I'll wire up whatever crypto stuff you can come up with ;) Long story short, I started to take you up on that offer, but because I am an insane person, before I knew it, the whole thing was done... Patch series incoming. Jason
Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
On Tue, Aug 02, 2022 at 07:29:29PM +0100, Richard W.M. Jones wrote: > Dan, which Fedora glibc package shows this problem? I have > glibc-2.35.9000-31.fc37.x86_64 and qemu compiled fine. (Also nbdkit > which includes linux/fs.h) It would help if I enabled a *-linux-user target ... Yes, I can reproduce this now and the patch fixes it, so: Tested-by: Richard W.M. Jones Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Re: [PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
On Tue, Aug 02, 2022 at 12:41:34PM -0400, Daniel P. Berrangé wrote:
> The latest glibc 2.36 has extended sys/mount.h so that it
> defines the FSCONFIG_* enum constants. These are historically
> defined in linux/mount.h, and thus if you include both headers
> the compiler complains:
>
> In file included from /usr/include/linux/fs.h:19,
> from ../linux-user/syscall.c:98:
> /usr/include/linux/mount.h:95:6: error: redeclaration of 'enum
> fsconfig_command'
>95 | enum fsconfig_command {
> | ^~~~
> In file included from ../linux-user/syscall.c:31:
> /usr/include/sys/mount.h:189:6: note: originally defined here
> 189 | enum fsconfig_command
> | ^~~~
> /usr/include/linux/mount.h:96:9: error: redeclaration of enumerator
> 'FSCONFIG_SET_FLAG'
>96 | FSCONFIG_SET_FLAG = 0,/* Set parameter, supplying
> no value */
> | ^
> /usr/include/sys/mount.h:191:3: note: previous definition of
> 'FSCONFIG_SET_FLAG' with type 'enum fsconfig_command'
> 191 | FSCONFIG_SET_FLAG = 0,/* Set parameter, supplying no
> value */
> | ^
> ...snip...
>
> QEMU doesn't include linux/mount.h, but it does use
> linux/fs.h and thus gets linux/mount.h indirectly.
>
> glibc acknowledges this problem but does not appear to
> be intending to fix it in the forseeable future, simply
> documenting it as a known incompatibility with no
> workaround:
>
>
> https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
> https://sourceware.org/glibc/wiki/Synchronizing_Headers
>
> To address this requires either removing use of sys/mount.h
> or linux/fs.h, despite QEMU needing declarations from
> both.
>
> This patch removes linux/fs.h, meaning we have to define
> various FS_IOC constants that are now unavailable.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> linux-user/syscall.c | 18 ++
> meson.build | 2 ++
> 2 files changed, 20 insertions(+)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index b27a6552aa..52d178afe7 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -95,7 +95,25 @@
> #include
> #include
> #include
> +
> +#ifdef HAVE_SYS_MOUNT_FSCONFIG
> +/*
> + * glibc >= 2.36 linux/mount.h conflicts with sys/mount.h,
> + * which in turn prevents use of linux/fs.h. So we have to
> + * define the constants ourselves for now.
> + */
> +#define FS_IOC_GETFLAGS_IOR('f', 1, long)
> +#define FS_IOC_SETFLAGS_IOW('f', 2, long)
> +#define FS_IOC_GETVERSION _IOR('v', 1, long)
> +#define FS_IOC_SETVERSION _IOW('v', 2, long)
> +#define FS_IOC_FIEMAP _IOWR('f', 11, struct fiemap)
> +#define FS_IOC32_GETFLAGS _IOR('f', 1, int)
> +#define FS_IOC32_SETFLAGS _IOW('f', 2, int)
> +#define FS_IOC32_GETVERSION_IOR('v', 1, int)
> +#define FS_IOC32_SETVERSION_IOW('v', 2, int)
> +#else
> #include
> +#endif
> #include
> #if defined(CONFIG_FIEMAP)
> #include
> diff --git a/meson.build b/meson.build
> index 294e9a8f32..30a380752c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1963,6 +1963,8 @@ config_host_data.set('HAVE_OPTRESET',
> cc.has_header_symbol('getopt.h', 'optreset'))
> config_host_data.set('HAVE_IPPROTO_MPTCP',
> cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
> +config_host_data.set('HAVE_SYS_MOUNT_FSCONFIG',
> + cc.has_header_symbol('sys/mount.h',
> 'FSCONFIG_SET_FLAG'))
>
> # has_member
> config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',
Dan, which Fedora glibc package shows this problem? I have
glibc-2.35.9000-31.fc37.x86_64 and qemu compiled fine. (Also nbdkit
which includes linux/fs.h)
I see various other build failures in Koji, but not this one as far as
I can tell:
https://koji.fedoraproject.org/koji/packageinfo?packageID=3685
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
[PATCH v5 06/10] vdpa: Make vhost_vdpa_net_cvq_map_elem accept any out sg
So its generic enough to accept any out sg buffer and we can inject
NIC state messages.
Signed-off-by: Eugenio Pérez
---
v5: Accept out sg instead of dev_buffers[]
---
net/vhost-vdpa.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 33bf3d6409..2421bca347 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -302,16 +302,16 @@ dma_map_err:
}
/**
- * Copy the guest element into a dedicated buffer suitable to be sent to NIC
+ * Maps out sg and in buffer into dedicated buffers suitable to be sent to NIC
*/
-static bool vhost_vdpa_net_cvq_map_elem(VhostVDPAState *s,
-VirtQueueElement *elem,
-size_t *out_len)
+static bool vhost_vdpa_net_cvq_map_sg(VhostVDPAState *s,
+ const struct iovec *out, size_t out_num,
+ size_t *out_len)
{
size_t in_copied;
bool ok;
-ok = vhost_vdpa_cvq_map_buf(>vhost_vdpa, elem->out_sg, elem->out_num,
+ok = vhost_vdpa_cvq_map_buf(>vhost_vdpa, out, out_num,
vhost_vdpa_net_cvq_cmd_len(),
s->cvq_cmd_out_buffer, out_len, false);
if (unlikely(!ok)) {
@@ -435,7 +435,8 @@ static int
vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
};
bool ok;
-ok = vhost_vdpa_net_cvq_map_elem(s, elem, _buffers[0].iov_len);
+ok = vhost_vdpa_net_cvq_map_sg(s, elem->out_sg, elem->out_num,
+ _buffers[0].iov_len);
if (unlikely(!ok)) {
goto out;
}
--
2.31.1
[PATCH v5 05/10] vdpa: Extract vhost_vdpa_net_cvq_add from vhost_vdpa_net_handle_ctrl_avail
So we can reuse it to inject state messages.
Signed-off-by: Eugenio Pérez
--
v5:
* Do not use an artificial !NULL VirtQueueElement
* Use only out size instead of iovec dev_buffers for these functions.
---
net/vhost-vdpa.c | 73
1 file changed, 49 insertions(+), 24 deletions(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index c6699edfbc..33bf3d6409 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -329,6 +329,52 @@ static bool vhost_vdpa_net_cvq_map_elem(VhostVDPAState *s,
return true;
}
+static virtio_net_ctrl_ack vhost_vdpa_net_cvq_add(VhostVDPAState *s,
+ size_t out_len)
+{
+/* Buffers for the device */
+const struct iovec out = {
+.iov_base = s->cvq_cmd_out_buffer,
+.iov_len = out_len,
+};
+const struct iovec in = {
+.iov_base = s->cvq_cmd_in_buffer,
+.iov_len = sizeof(virtio_net_ctrl_ack),
+};
+VhostShadowVirtqueue *svq = g_ptr_array_index(s->vhost_vdpa.shadow_vqs, 0);
+/* in buffer used for device model */
+virtio_net_ctrl_ack status;
+size_t dev_written;
+int r;
+
+r = vhost_svq_add(svq, , 1, , 1, NULL);
+if (unlikely(r != 0)) {
+if (unlikely(r == -ENOSPC)) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: No space on device queue\n",
+ __func__);
+}
+return VIRTIO_NET_ERR;
+}
+
+/*
+ * We can poll here since we've had BQL from the time we sent the
+ * descriptor. Also, we need to take the answer before SVQ pulls by itself,
+ * when BQL is released
+ */
+dev_written = vhost_svq_poll(svq);
+if (unlikely(dev_written < sizeof(status))) {
+error_report("Insufficient written data (%zu)", dev_written);
+return VIRTIO_NET_ERR;
+}
+
+memcpy(, s->cvq_cmd_in_buffer, sizeof(status));
+if (status != VIRTIO_NET_OK) {
+return VIRTIO_NET_ERR;
+}
+
+return VIRTIO_NET_OK;
+}
+
/**
* Do not forward commands not supported by SVQ. Otherwise, the device could
* accept it and qemu would not know how to update the device model.
@@ -375,7 +421,7 @@ static int
vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
void *opaque)
{
VhostVDPAState *s = opaque;
-size_t in_len, dev_written;
+size_t in_len;
virtio_net_ctrl_ack status = VIRTIO_NET_ERR;
/* out and in buffers sent to the device */
struct iovec dev_buffers[2] = {
@@ -387,7 +433,6 @@ static int
vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
.iov_base = ,
.iov_len = sizeof(status),
};
-int r = -EINVAL;
bool ok;
ok = vhost_vdpa_net_cvq_map_elem(s, elem, _buffers[0].iov_len);
@@ -400,27 +445,7 @@ static int
vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
goto out;
}
-r = vhost_svq_add(svq, _buffers[0], 1, _buffers[1], 1, elem);
-if (unlikely(r != 0)) {
-if (unlikely(r == -ENOSPC)) {
-qemu_log_mask(LOG_GUEST_ERROR, "%s: No space on device queue\n",
- __func__);
-}
-goto out;
-}
-
-/*
- * We can poll here since we've had BQL from the time we sent the
- * descriptor. Also, we need to take the answer before SVQ pulls by itself,
- * when BQL is released
- */
-dev_written = vhost_svq_poll(svq);
-if (unlikely(dev_written < sizeof(status))) {
-error_report("Insufficient written data (%zu)", dev_written);
-goto out;
-}
-
-memcpy(, dev_buffers[1].iov_base, sizeof(status));
+status = vhost_vdpa_net_cvq_add(s, dev_buffers[0].iov_len);
if (status != VIRTIO_NET_OK) {
goto out;
}
@@ -445,7 +470,7 @@ out:
if (dev_buffers[1].iov_base) {
vhost_vdpa_cvq_unmap_buf(>vhost_vdpa, dev_buffers[1].iov_base);
}
-return r;
+return 0;
}
static const VhostShadowVirtqueueOps vhost_vdpa_net_svq_ops = {
--
2.31.1
[PATCH v5 08/10] vdpa: add net_vhost_vdpa_cvq_info NetClientInfo
Next patches will add a new info callback to restore NIC status through
CVQ. Since only the CVQ vhost device is needed, create it with a new
NetClientInfo.
Signed-off-by: Eugenio Pérez
---
v5: Create a new NetClientInfo instead of reusing the dataplane one.
---
net/vhost-vdpa.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 2421bca347..8d400f2dff 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -375,6 +375,16 @@ static virtio_net_ctrl_ack
vhost_vdpa_net_cvq_add(VhostVDPAState *s,
return VIRTIO_NET_OK;
}
+static NetClientInfo net_vhost_vdpa_cvq_info = {
+.type = NET_CLIENT_DRIVER_VHOST_VDPA,
+.size = sizeof(VhostVDPAState),
+.receive = vhost_vdpa_receive,
+.cleanup = vhost_vdpa_cleanup,
+.has_vnet_hdr = vhost_vdpa_has_vnet_hdr,
+.has_ufo = vhost_vdpa_has_ufo,
+.check_peer_type = vhost_vdpa_check_peer_type,
+};
+
/**
* Do not forward commands not supported by SVQ. Otherwise, the device could
* accept it and qemu would not know how to update the device model.
@@ -496,7 +506,7 @@ static NetClientState *net_vhost_vdpa_init(NetClientState
*peer,
nc = qemu_new_net_client(_vhost_vdpa_info, peer, device,
name);
} else {
-nc = qemu_new_net_control_client(_vhost_vdpa_info, peer,
+nc = qemu_new_net_control_client(_vhost_vdpa_cvq_info, peer,
device, name);
}
snprintf(nc->info_str, sizeof(nc->info_str), TYPE_VHOST_VDPA);
--
2.31.1
[PATCH v5 03/10] vhost: Do not depend on !NULL VirtQueueElement on vhost_svq_flush
Since QEMU will be able to inject new elements on CVQ to restore the
state, we need not to depend on a VirtQueueElement to know if a new
element has been used by the device or not. Instead of check that, check
if there are new elements only using used idx on vhost_svq_flush.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index e6eebd0e8d..fdb550c31b 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -491,7 +491,7 @@ static void vhost_svq_flush(VhostShadowVirtqueue *svq,
/**
* Poll the SVQ for one device used buffer.
*
- * This function race with main event loop SVQ polling, so extra
+ * This function races with main event loop SVQ polling, so extra
* synchronization is needed.
*
* Return the length written by the device.
@@ -499,20 +499,20 @@ static void vhost_svq_flush(VhostShadowVirtqueue *svq,
size_t vhost_svq_poll(VhostShadowVirtqueue *svq)
{
int64_t start_us = g_get_monotonic_time();
-do {
+while (true) {
uint32_t len;
-VirtQueueElement *elem = vhost_svq_get_buf(svq, );
-if (elem) {
-return len;
-}
if (unlikely(g_get_monotonic_time() - start_us > 10e6)) {
return 0;
}
-/* Make sure we read new used_idx */
-smp_rmb();
-} while (true);
+if (!vhost_svq_more_used(svq)) {
+continue;
+}
+
+vhost_svq_get_buf(svq, );
+return len;
+}
}
/**
--
2.31.1
Re: [PATCH 01/19] ppc/ppc405: Remove taihu machine
On 8/1/22 10:10, Cédric Le Goater wrote:
It has been deprecated since 7.0.
Signed-off-by: Cédric Le Goater
---
docs/about/deprecated.rst| 9 --
docs/system/ppc/embedded.rst | 1 -
hw/ppc/ppc405_boards.c | 232 ---
3 files changed, 242 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7ee26626d5cf..2f9b41aaea48 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -233,15 +233,6 @@ deprecated; use the new name ``dtb-randomness`` instead.
The new name
better reflects the way this property affects all random data within
the device tree blob, not just the ``kaslr-seed`` node.
-PPC 405 ``taihu`` machine (since 7.0)
-'
-
-The PPC 405 CPU is a system-on-a-chip, so all 405 machines are very similar,
-except for some external periphery. However, the periphery of the ``taihu``
-machine is hardly emulated at all (e.g. neither the LCD nor the USB part had
-been implemented), so there is not much value added by this board. Use the
-``ref405ep`` machine instead.
-
I believe we need to add a note in docs/about/removed-feature.rst as well.
E.g. 50f97a0ec6e81b8 where the swift-bmc Aspeed machine was removed. The
deprecated.rst entry was deleted and an entry in removed-features.rst was
added.
Don't worry re-sending the series just for that. I can fixup in the tree if
this is the only observation for the series.
Thanks,
Daniel
``pc-i440fx-1.4`` up to ``pc-i440fx-1.7`` (since 7.0)
'
diff --git a/docs/system/ppc/embedded.rst b/docs/system/ppc/embedded.rst
index cfffbda24da9..af3b3d9fa460 100644
--- a/docs/system/ppc/embedded.rst
+++ b/docs/system/ppc/embedded.rst
@@ -6,5 +6,4 @@ Embedded family boards
- ``ppce500`` generic paravirt e500 platform
- ``ref405ep`` ref405ep
- ``sam460ex`` aCube Sam460ex
-- ``taihu``taihu
- ``virtex-ml507`` Xilinx Virtex ML507 reference design
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index a66ad05e3ac3..1a4e7588c584 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -342,241 +342,9 @@ static const TypeInfo ref405ep_type = {
.class_init = ref405ep_class_init,
};
-/*/
-/* AMCC Taihu evaluation board */
-/* - PowerPC 405EP processor
- * - SDRAM 128 MB at 0x
- * - Boot flash 2 MB at 0xFFE0
- * - Application flash 32 MB at 0xFC00
- * - 2 serial ports
- * - 2 ethernet PHY
- * - 1 USB 1.1 device0x5000
- * - 1 LCD display 0x5010
- * - 1 CPLD 0x5010
- * - 1 I2C EEPROM
- * - 1 I2C thermal sensor
- * - a set of LEDs
- * - bit-bang SPI port using GPIOs
- * - 1 EBC interface connector 0 0x5020
- * - 1 cardbus controller + expansion slot.
- * - 1 PCI expansion slot.
- */
-typedef struct taihu_cpld_t taihu_cpld_t;
-struct taihu_cpld_t {
-uint8_t reg0;
-uint8_t reg1;
-};
-
-static uint64_t taihu_cpld_read(void *opaque, hwaddr addr, unsigned size)
-{
-taihu_cpld_t *cpld;
-uint32_t ret;
-
-cpld = opaque;
-switch (addr) {
-case 0x0:
-ret = cpld->reg0;
-break;
-case 0x1:
-ret = cpld->reg1;
-break;
-default:
-ret = 0;
-break;
-}
-
-return ret;
-}
-
-static void taihu_cpld_write(void *opaque, hwaddr addr,
- uint64_t value, unsigned size)
-{
-taihu_cpld_t *cpld;
-
-cpld = opaque;
-switch (addr) {
-case 0x0:
-/* Read only */
-break;
-case 0x1:
-cpld->reg1 = value;
-break;
-default:
-break;
-}
-}
-
-static const MemoryRegionOps taihu_cpld_ops = {
-.read = taihu_cpld_read,
-.write = taihu_cpld_write,
-.impl = {
-.min_access_size = 1,
-.max_access_size = 1,
-},
-.endianness = DEVICE_NATIVE_ENDIAN,
-};
-
-static void taihu_cpld_reset (void *opaque)
-{
-taihu_cpld_t *cpld;
-
-cpld = opaque;
-cpld->reg0 = 0x01;
-cpld->reg1 = 0x80;
-}
-
-static void taihu_cpld_init(MemoryRegion *sysmem, uint32_t base)
-{
-taihu_cpld_t *cpld;
-MemoryRegion *cpld_memory = g_new(MemoryRegion, 1);
-
-cpld = g_new0(taihu_cpld_t, 1);
-memory_region_init_io(cpld_memory, NULL, _cpld_ops, cpld, "cpld",
0x100);
-memory_region_add_subregion(sysmem, base, cpld_memory);
-qemu_register_reset(_cpld_reset, cpld);
-}
-
-static void taihu_405ep_init(MachineState *machine)
-{
-MachineClass *mc = MACHINE_GET_CLASS(machine);
-const char *bios_name = machine->firmware ?: BIOS_FILENAME;
-const char *kernel_filename = machine->kernel_filename;
-const char *initrd_filename = machine->initrd_filename;
-char *filename;
-MemoryRegion *sysmem = get_system_memory();
-MemoryRegion *bios;
-
[PATCH v5 10/10] vdpa: Delete CVQ migration blocker
We can restore the device state in the destination via CVQ now. Remove
the migration blocker.
Signed-off-by: Eugenio Pérez
---
include/hw/virtio/vhost-vdpa.h | 1 -
hw/virtio/vhost-vdpa.c | 14 --
net/vhost-vdpa.c | 2 --
3 files changed, 17 deletions(-)
diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index d10a89303e..d85643 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -35,7 +35,6 @@ typedef struct vhost_vdpa {
bool shadow_vqs_enabled;
/* IOVA mapping used by the Shadow Virtqueue */
VhostIOVATree *iova_tree;
-Error *migration_blocker;
GPtrArray *shadow_vqs;
const VhostShadowVirtqueueOps *shadow_vq_ops;
void *shadow_vq_ops_opaque;
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index e44c23dce5..8882077955 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1029,13 +1029,6 @@ static bool vhost_vdpa_svqs_start(struct vhost_dev *dev)
return true;
}
-if (v->migration_blocker) {
-int r = migrate_add_blocker(v->migration_blocker, );
-if (unlikely(r < 0)) {
-return false;
-}
-}
-
for (i = 0; i < v->shadow_vqs->len; ++i) {
VirtQueue *vq = virtio_get_queue(dev->vdev, dev->vq_index + i);
VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, i);
@@ -1078,10 +1071,6 @@ err:
vhost_svq_stop(svq);
}
-if (v->migration_blocker) {
-migrate_del_blocker(v->migration_blocker);
-}
-
return false;
}
@@ -1101,9 +1090,6 @@ static bool vhost_vdpa_svqs_stop(struct vhost_dev *dev)
}
}
-if (v->migration_blocker) {
-migrate_del_blocker(v->migration_blocker);
-}
return true;
}
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index d489fcd91e..f933ba53a3 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -576,8 +576,6 @@ static NetClientState *net_vhost_vdpa_init(NetClientState
*peer,
s->vhost_vdpa.shadow_vq_ops = _vdpa_net_svq_ops;
s->vhost_vdpa.shadow_vq_ops_opaque = s;
-error_setg(>vhost_vdpa.migration_blocker,
- "Migration disabled: vhost-vdpa uses CVQ.");
}
ret = vhost_vdpa_add(nc, (void *)>vhost_vdpa, queue_pair_index, nvqs);
if (ret) {
--
2.31.1
[PATCH v5 07/10] vdpa: add NetClientState->load() callback
It allows per-net client operations right after device's successful
start. In particular, to load the device status.
Vhost-vdpa net will use it to add the CVQ buffers to restore the device
status.
Signed-off-by: Eugenio Pérez
---
v5: Rename start / load, naming it more specifically.
---
include/net/net.h | 2 ++
hw/net/vhost_net.c | 7 +++
2 files changed, 9 insertions(+)
diff --git a/include/net/net.h b/include/net/net.h
index 523136c7ac..a8d47309cd 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -44,6 +44,7 @@ typedef struct NICConf {
typedef void (NetPoll)(NetClientState *, bool enable);
typedef bool (NetCanReceive)(NetClientState *);
+typedef int (NetLoad)(NetClientState *);
typedef ssize_t (NetReceive)(NetClientState *, const uint8_t *, size_t);
typedef ssize_t (NetReceiveIOV)(NetClientState *, const struct iovec *, int);
typedef void (NetCleanup) (NetClientState *);
@@ -71,6 +72,7 @@ typedef struct NetClientInfo {
NetReceive *receive_raw;
NetReceiveIOV *receive_iov;
NetCanReceive *can_receive;
+NetLoad *load;
NetCleanup *cleanup;
LinkStatusChanged *link_status_changed;
QueryRxFilter *query_rx_filter;
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index ccac5b7a64..a9bf72dcda 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -274,6 +274,13 @@ static int vhost_net_start_one(struct vhost_net *net,
}
}
}
+
+if (net->nc->info->load) {
+r = net->nc->info->load(net->nc);
+if (r < 0) {
+goto fail;
+}
+}
return 0;
fail:
file.fd = -1;
--
2.31.1
[PATCH v5 02/10] vhost: use SVQ element ndescs instead of opaque data for desc validation
Since we're going to allow SVQ to add elements without the guest's
knowledge and without its own VirtQueueElement, it's easier to check if
an element is a valid head checking a different thing than the
VirtQueueElement.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index ffd2b2c972..e6eebd0e8d 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -414,7 +414,7 @@ static VirtQueueElement
*vhost_svq_get_buf(VhostShadowVirtqueue *svq,
return NULL;
}
-if (unlikely(!svq->desc_state[used_elem.id].elem)) {
+if (unlikely(!svq->desc_state[used_elem.id].ndescs)) {
qemu_log_mask(LOG_GUEST_ERROR,
"Device %s says index %u is used, but it was not available",
svq->vdev->name, used_elem.id);
@@ -422,6 +422,7 @@ static VirtQueueElement
*vhost_svq_get_buf(VhostShadowVirtqueue *svq,
}
num = svq->desc_state[used_elem.id].ndescs;
+svq->desc_state[used_elem.id].ndescs = 0;
last_used_chain = vhost_svq_last_desc_of_chain(svq, num, used_elem.id);
svq->desc_next[last_used_chain] = svq->free_head;
svq->free_head = used_elem.id;
--
2.31.1
[PATCH v5 09/10] vdpa: Add virtio-net mac address via CVQ at start
This is needed so the destination vdpa device see the same state a the
guest set in the source.
Signed-off-by: Eugenio Pérez
---
v5:
* Rename s/start/load/
* Use independent NetClientInfo to only add load callback on cvq.
---
net/vhost-vdpa.c | 50
1 file changed, 50 insertions(+)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 8d400f2dff..d489fcd91e 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -375,10 +375,60 @@ static virtio_net_ctrl_ack
vhost_vdpa_net_cvq_add(VhostVDPAState *s,
return VIRTIO_NET_OK;
}
+static int vhost_vdpa_net_load(NetClientState *nc)
+{
+VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
+struct vhost_vdpa *v = >vhost_vdpa;
+VirtIONet *n;
+uint64_t features;
+
+assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
+
+if (!v->shadow_vqs_enabled) {
+return 0;
+}
+
+n = VIRTIO_NET(v->dev->vdev);
+features = v->dev->vdev->host_features;
+if (features & BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR)) {
+const struct virtio_net_ctrl_hdr ctrl = {
+.class = VIRTIO_NET_CTRL_MAC,
+.cmd = VIRTIO_NET_CTRL_MAC_ADDR_SET,
+};
+uint8_t mac[6];
+const struct iovec out[] = {
+{
+.iov_base = (void *),
+.iov_len = sizeof(ctrl),
+},{
+.iov_base = mac,
+.iov_len = sizeof(mac),
+},
+};
+size_t out_len;
+bool ok;
+virtio_net_ctrl_ack state;
+
+ok = vhost_vdpa_net_cvq_map_sg(s, out, ARRAY_SIZE(out), _len);
+if (unlikely(!ok)) {
+return -1;
+}
+
+memcpy(mac, n->mac, sizeof(mac));
+state = vhost_vdpa_net_cvq_add(s, out_len);
+vhost_vdpa_cvq_unmap_buf(v, s->cvq_cmd_out_buffer);
+vhost_vdpa_cvq_unmap_buf(v, s->cvq_cmd_in_buffer);
+return state == VIRTIO_NET_OK ? 0 : 1;
+}
+
+return 0;
+}
+
static NetClientInfo net_vhost_vdpa_cvq_info = {
.type = NET_CLIENT_DRIVER_VHOST_VDPA,
.size = sizeof(VhostVDPAState),
.receive = vhost_vdpa_receive,
+.load = vhost_vdpa_net_load,
.cleanup = vhost_vdpa_cleanup,
.has_vnet_hdr = vhost_vdpa_has_vnet_hdr,
.has_ufo = vhost_vdpa_has_ufo,
--
2.31.1
[PATCH v5 04/10] vdpa: Get buffers from VhostVDPAState on vhost_vdpa_net_cvq_map_elem
There is no need to get them by parameter, since they're contained in
VhostVDPAState. The only useful information was the written length in
out.
Simplify the function removing those.
Signed-off-by: Eugenio Pérez
---
net/vhost-vdpa.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index ac1810723c..c6699edfbc 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -303,34 +303,29 @@ dma_map_err:
/**
* Copy the guest element into a dedicated buffer suitable to be sent to NIC
- *
- * @iov: [0] is the out buffer, [1] is the in one
*/
static bool vhost_vdpa_net_cvq_map_elem(VhostVDPAState *s,
VirtQueueElement *elem,
-struct iovec *iov)
+size_t *out_len)
{
size_t in_copied;
bool ok;
-iov[0].iov_base = s->cvq_cmd_out_buffer;
ok = vhost_vdpa_cvq_map_buf(>vhost_vdpa, elem->out_sg, elem->out_num,
-vhost_vdpa_net_cvq_cmd_len(), iov[0].iov_base,
-[0].iov_len, false);
+vhost_vdpa_net_cvq_cmd_len(),
+s->cvq_cmd_out_buffer, out_len, false);
if (unlikely(!ok)) {
return false;
}
-iov[1].iov_base = s->cvq_cmd_in_buffer;
ok = vhost_vdpa_cvq_map_buf(>vhost_vdpa, NULL, 0,
-sizeof(virtio_net_ctrl_ack), iov[1].iov_base,
-_copied, true);
+sizeof(virtio_net_ctrl_ack),
+s->cvq_cmd_in_buffer, _copied, true);
if (unlikely(!ok)) {
vhost_vdpa_cvq_unmap_buf(>vhost_vdpa, s->cvq_cmd_out_buffer);
return false;
}
-iov[1].iov_len = sizeof(virtio_net_ctrl_ack);
return true;
}
@@ -395,7 +390,7 @@ static int
vhost_vdpa_net_handle_ctrl_avail(VhostShadowVirtqueue *svq,
int r = -EINVAL;
bool ok;
-ok = vhost_vdpa_net_cvq_map_elem(s, elem, dev_buffers);
+ok = vhost_vdpa_net_cvq_map_elem(s, elem, _buffers[0].iov_len);
if (unlikely(!ok)) {
goto out;
}
--
2.31.1
[PATCH v5 01/10] vhost: stop transfer elem ownership in vhost_handle_guest_kick
It was easier to allow vhost_svq_add to handle the memory. Now that we
will allow qemu to add elements to a SVQ without the guest's knowledge,
it's better to handle it in the caller.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index e4956728dd..ffd2b2c972 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -233,9 +233,6 @@ static void vhost_svq_kick(VhostShadowVirtqueue *svq)
/**
* Add an element to a SVQ.
*
- * The caller must check that there is enough slots for the new element. It
- * takes ownership of the element: In case of failure not ENOSPC, it is free.
- *
* Return -EINVAL if element is invalid, -ENOSPC if dev queue is full
*/
int vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec *out_sg,
@@ -252,7 +249,6 @@ int vhost_svq_add(VhostShadowVirtqueue *svq, const struct
iovec *out_sg,
ok = vhost_svq_add_split(svq, out_sg, out_num, in_sg, in_num, _head);
if (unlikely(!ok)) {
-g_free(elem);
return -EINVAL;
}
@@ -293,7 +289,7 @@ static void vhost_handle_guest_kick(VhostShadowVirtqueue
*svq)
virtio_queue_set_notification(svq->vq, false);
while (true) {
-VirtQueueElement *elem;
+g_autofree VirtQueueElement *elem;
int r;
if (svq->next_guest_avail_elem) {
@@ -324,12 +320,14 @@ static void vhost_handle_guest_kick(VhostShadowVirtqueue
*svq)
* queue the current guest descriptor and ignore kicks
* until some elements are used.
*/
-svq->next_guest_avail_elem = elem;
+svq->next_guest_avail_elem = g_steal_pointer();
}
/* VQ is full or broken, just return and ignore kicks */
return;
}
+/* elem belongs to SVQ or external caller now */
+elem = NULL;
}
virtio_queue_set_notification(svq->vq, true);
--
2.31.1
[PATCH v5 00/10] NIC vhost-vdpa state restore via Shadow CVQ
CVQ of net vhost-vdpa devices can be intercepted since the work of [1]. The virtio-net device model is updated. The migration was blocked because although the state can be megrated between VMM it was not possible to restore on the destination NIC. This series add support for SVQ to inject external messages without the guest's knowledge, so before the guest is resumed all the guest visible state is restored. It is done using standard CVQ messages, so the vhost-vdpa device does not need to learn how to restore it: As long as they have the feature, they know how to handle it. This series needs fixes [1], [2] and [3] to be applied to achieve full live migration. Thanks! [1] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg02984.html [2] https://lists.nongnu.org/archive/html/qemu-devel/2022-07/msg03993.html [3] https://lists.nongnu.org/archive/html/qemu-devel/2022-08/msg00325.html v5: - Rename s/start/load/ - Use independent NetClientInfo to only add load callback on cvq. - Accept out sg instead of dev_buffers[] at vhost_vdpa_net_cvq_map_elem - Use only out size instead of iovec dev_buffers to know if the descriptor is effectively available, allowing to delete artificial !NULL VirtQueueElement on vhost_svq_add call. v4: - Actually use NetClientInfo callback. v3: - Route vhost-vdpa start code through NetClientInfo callback. - Delete extra vhost_net_stop_one() call. v2: - Fix SIGSEGV dereferencing SVQ when not in svq mode v1 from RFC: - Do not reorder DRIVER_OK & enable patches. - Delete leftovers Eugenio Pérez (10): vhost: stop transfer elem ownership in vhost_handle_guest_kick vhost: use SVQ element ndescs instead of opaque data for desc validation vhost: Do not depend on !NULL VirtQueueElement on vhost_svq_flush vdpa: Get buffers from VhostVDPAState on vhost_vdpa_net_cvq_map_elem vdpa: Extract vhost_vdpa_net_cvq_add from vhost_vdpa_net_handle_ctrl_avail vdpa: Make vhost_vdpa_net_cvq_map_elem accept any out sg vdpa: add NetClientState->load() callback vdpa: add net_vhost_vdpa_cvq_info NetClientInfo vdpa: Add virtio-net mac address via CVQ at start vdpa: Delete CVQ migration blocker include/hw/virtio/vhost-vdpa.h | 1 - include/net/net.h | 2 + hw/net/vhost_net.c | 7 ++ hw/virtio/vhost-shadow-virtqueue.c | 31 +++--- hw/virtio/vhost-vdpa.c | 14 --- net/vhost-vdpa.c | 163 + 6 files changed, 145 insertions(+), 73 deletions(-) -- 2.31.1
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
On Wed, Jul 20, 2022 at 02:08:59PM +0200, Jason A. Donenfeld wrote: > +case 114: > +if (r1 & 1 || !r1 || r2 & 1 || !r2) > +tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra); This is already handled in op_msa. I'm going to remove it for v4.
Re: [PATCH v4 6/7] vdpa: Add virtio-net mac address via CVQ at start
On Mon, Aug 1, 2022 at 9:09 AM Eugenio Perez Martin wrote:
>
> On Mon, Jul 25, 2022 at 11:32 AM Jason Wang wrote:
> >
> >
> > 在 2022/7/22 19:12, Eugenio Pérez 写道:
> > > This is needed so the destination vdpa device see the same state a the
> > > guest set in the source.
> > >
> > > Signed-off-by: Eugenio Pérez
> > > ---
> > > net/vhost-vdpa.c | 61
> > > 1 file changed, 61 insertions(+)
> > >
> > > diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> > > index 61516b1432..3e15a42c35 100644
> > > --- a/net/vhost-vdpa.c
> > > +++ b/net/vhost-vdpa.c
> > > @@ -365,10 +365,71 @@ static virtio_net_ctrl_ack
> > > vhost_vdpa_net_cvq_add(VhostShadowVirtqueue *svq,
> > > return VIRTIO_NET_OK;
> > > }
> > >
> > > +static int vhost_vdpa_net_start(NetClientState *nc)
> > > +{
> > > +VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
> > > +struct vhost_vdpa *v = >vhost_vdpa;
> > > +VirtIONet *n;
> > > +uint64_t features;
> > > +VhostShadowVirtqueue *svq;
> > > +
> > > +assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
> > > +
> > > +if (!v->shadow_vqs_enabled) {
> > > +return 0;
> > > +}
> > > +
> > > +if (v->dev->nvqs != 1 &&
> > > +v->dev->vq_index + v->dev->nvqs != v->dev->vq_index_end) {
> > > +/* Only interested in CVQ */
> > > +return 0;
> > > +}
> >
> >
> > I'd have a dedicated NetClientInfo for cvq.
> >
>
> I'll try and come back to you.
>
> >
> > > +
> > > +n = VIRTIO_NET(v->dev->vdev);
> > > +features = v->dev->vdev->host_features;
> > > +svq = g_ptr_array_index(v->shadow_vqs, 0);
> > > +if (features & BIT_ULL(VIRTIO_NET_F_CTRL_MAC_ADDR)) {
> > > +const struct virtio_net_ctrl_hdr ctrl = {
> > > +.class = VIRTIO_NET_CTRL_MAC,
> > > +.cmd = VIRTIO_NET_CTRL_MAC_ADDR_SET,
> > > +};
> > > +uint8_t mac[6];
> > > +const struct iovec out[] = {
> > > +{
> > > +.iov_base = (void *),
> > > +.iov_len = sizeof(ctrl),
> > > +},{
> > > +.iov_base = mac,
> > > +.iov_len = sizeof(mac),
> > > +},
> > > +};
> > > +struct iovec dev_buffers[2] = {
> > > +{ .iov_base = s->cvq_cmd_out_buffer },
> > > +{ .iov_base = s->cvq_cmd_in_buffer },
> > > +};
> > > +bool ok;
> > > +virtio_net_ctrl_ack state;
> > > +
> > > +ok = vhost_vdpa_net_cvq_map_sg(s, out, ARRAY_SIZE(out),
> > > dev_buffers);
> >
> >
> > To speed up the state recovery, can we map those buffers during svq start?
> >
>
> Not sure if I follow you here. This is the callback that is called
> during the device startup.
>
> If you mean to make these buffers permanently mapped I think that can
> be done for this series, but extra care will be needed when we
> introduce ASID support to not make them visible from the guest. I'm ok
> if you prefer to make it that way for this series.
>
Sending v4 without this part, please let me know if it needs further changes.
Thanks!
[PATCH for 7.1] linux-user: fix compat with glibc >= 2.36 sys/mount.h
The latest glibc 2.36 has extended sys/mount.h so that it
defines the FSCONFIG_* enum constants. These are historically
defined in linux/mount.h, and thus if you include both headers
the compiler complains:
In file included from /usr/include/linux/fs.h:19,
from ../linux-user/syscall.c:98:
/usr/include/linux/mount.h:95:6: error: redeclaration of 'enum fsconfig_command'
95 | enum fsconfig_command {
| ^~~~
In file included from ../linux-user/syscall.c:31:
/usr/include/sys/mount.h:189:6: note: originally defined here
189 | enum fsconfig_command
| ^~~~
/usr/include/linux/mount.h:96:9: error: redeclaration of enumerator
'FSCONFIG_SET_FLAG'
96 | FSCONFIG_SET_FLAG = 0,/* Set parameter, supplying no
value */
| ^
/usr/include/sys/mount.h:191:3: note: previous definition of
'FSCONFIG_SET_FLAG' with type 'enum fsconfig_command'
191 | FSCONFIG_SET_FLAG = 0,/* Set parameter, supplying no value
*/
| ^
...snip...
QEMU doesn't include linux/mount.h, but it does use
linux/fs.h and thus gets linux/mount.h indirectly.
glibc acknowledges this problem but does not appear to
be intending to fix it in the forseeable future, simply
documenting it as a known incompatibility with no
workaround:
https://sourceware.org/glibc/wiki/Release/2.36#Usage_of_.3Clinux.2Fmount.h.3E_and_.3Csys.2Fmount.h.3E
https://sourceware.org/glibc/wiki/Synchronizing_Headers
To address this requires either removing use of sys/mount.h
or linux/fs.h, despite QEMU needing declarations from
both.
This patch removes linux/fs.h, meaning we have to define
various FS_IOC constants that are now unavailable.
Signed-off-by: Daniel P. Berrangé
---
linux-user/syscall.c | 18 ++
meson.build | 2 ++
2 files changed, 20 insertions(+)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b27a6552aa..52d178afe7 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -95,7 +95,25 @@
#include
#include
#include
+
+#ifdef HAVE_SYS_MOUNT_FSCONFIG
+/*
+ * glibc >= 2.36 linux/mount.h conflicts with sys/mount.h,
+ * which in turn prevents use of linux/fs.h. So we have to
+ * define the constants ourselves for now.
+ */
+#define FS_IOC_GETFLAGS_IOR('f', 1, long)
+#define FS_IOC_SETFLAGS_IOW('f', 2, long)
+#define FS_IOC_GETVERSION _IOR('v', 1, long)
+#define FS_IOC_SETVERSION _IOW('v', 2, long)
+#define FS_IOC_FIEMAP _IOWR('f', 11, struct fiemap)
+#define FS_IOC32_GETFLAGS _IOR('f', 1, int)
+#define FS_IOC32_SETFLAGS _IOW('f', 2, int)
+#define FS_IOC32_GETVERSION_IOR('v', 1, int)
+#define FS_IOC32_SETVERSION_IOW('v', 2, int)
+#else
#include
+#endif
#include
#if defined(CONFIG_FIEMAP)
#include
diff --git a/meson.build b/meson.build
index 294e9a8f32..30a380752c 100644
--- a/meson.build
+++ b/meson.build
@@ -1963,6 +1963,8 @@ config_host_data.set('HAVE_OPTRESET',
cc.has_header_symbol('getopt.h', 'optreset'))
config_host_data.set('HAVE_IPPROTO_MPTCP',
cc.has_header_symbol('netinet/in.h', 'IPPROTO_MPTCP'))
+config_host_data.set('HAVE_SYS_MOUNT_FSCONFIG',
+ cc.has_header_symbol('sys/mount.h', 'FSCONFIG_SET_FLAG'))
# has_member
config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',
--
2.37.1
Re: [PATCH v7 11/14] KVM: Register/unregister the guest private memory regions
On Tue, Aug 02, 2022, Sean Christopherson wrote: > I think we should avoid UNMAPPABLE even on the KVM side of things for the core > memslots functionality and instead be very literal, e.g. > > KVM_HAS_FD_BASED_MEMSLOTS > KVM_MEM_FD_VALID > > We'll still need KVM_HAS_USER_UNMAPPABLE_MEMORY, but it won't be tied > directly to > the memslot. Decoupling the two thingis will require a bit of extra work, > but the > code impact should be quite small, e.g. explicitly query and propagate > MEMFILE_F_USER_INACCESSIBLE to kvm_memory_slot to track if a memslot can be > private. > And unless I'm missing something, it won't require an additional memslot flag. > The biggest oddity (if we don't also add KVM_MEM_PRIVATE) is that KVM would > effectively ignore the hva for fd-based memslots for VM types that don't > support > private memory, i.e. userspace can't opt out of using the fd-based backing, > but that > doesn't seem like a deal breaker. Hrm, but basing private memory on top of a generic FD_VALID would effectively require shared memory to use hva-based memslots for confidential VMs. That'd yield a very weird API, e.g. non-confidential VMs could be backed entirely by fd-based memslots, but confidential VMs would be forced to use hva-based memslots. Ignore this idea for now. If there's an actual use case for generic fd-based memory then we'll want a separate flag, fd, and offset, i.e. that support could be added independent of KVM_MEM_PRIVATE.
Re: [Qemu-devel] [PULL 15/36] memory: fix race between TCG and accesses to dirty bitmap
On Tue, 20 Aug 2019 at 08:12, Paolo Bonzini wrote:
>
> There is a race between TCG and accesses to the dirty log:
>
> vCPU thread reader thread
> --- ---
> TLB check -> slow path
> notdirty_mem_write
> write to RAM
> set dirty flag
>clear dirty flag
> TLB check -> fast path
>read memory
> write to RAM
>
> Fortunately, in order to fix it, no change is required to the
> vCPU thread. However, the reader thread must delay the read after
> the vCPU thread has finished the write. This can be approximated
> conservatively by run_on_cpu, which waits for the end of the current
> translation block.
>
> A similar technique is used by KVM, which has to do a synchronous TLB
> flush after doing a test-and-clear of the dirty-page flags.
>
> Reported-by: Dr. David Alan Gilbert
> Signed-off-by: Paolo Bonzini
> +static void tcg_log_global_after_sync(MemoryListener *listener)
> +{
> +CPUAddressSpace *cpuas;
> +
> +/* Wait for the CPU to end the current TB. This avoids the following
> + * incorrect race:
> + *
> + * vCPU migration
> + * -- -
> + * TLB check -> slow path
> + *notdirty_mem_write
> + * write to RAM
> + * mark dirty
> + * clear dirty flag
> + * TLB check -> fast path
> + * read memory
> + *write to RAM
> + *
> + * by pushing the migration thread's memory read after the vCPU thread
> has
> + * written the memory.
> + */
> +cpuas = container_of(listener, CPUAddressSpace, tcg_as_listener);
> +run_on_cpu(cpuas->cpu, do_nothing, RUN_ON_CPU_NULL);
> +}
So I think this implementation has two problems that we've only just
noticed:
(1) if memory_global_after_dirty_log_sync() is called in a context
where the caller holds the BQL, then the implementation here will
temporarily drop the BQL inside run_on_cpu(). That breaks
assumptions in pretty much all the graphics device models which
call memory_region_snapshot_and_clear_dirty() and assume that
things they'd determined before the call stay valid (pointers
into DisplaySurfaces, information about size and location of
the framebuffer in guest memory, etc).
(2) if memory_global_after_dirty_log_sync() is called in a context
where the caller does *not* hold the BQL, this is undefined
behaviour, because run_on_cpu() calls qemu_cond_wait() passing
it the BKL, and at least for the posix implementation that
ends up being posix_cond_wait() and calling that without the
mutex held is undefined behaviour. This happens for the migration
code for everything except the final iteration.
Does anybody have any bright ideas for fixing these? (2) I guess
we could handle just by having the migration code (or this
code right here) grab the BQL if it doesn't already have it.
(1) is much harder: you kind of conceptually want to do a "go back
and restart the update-display operation", but there's no clear
way to tell if you need to do that. You can postpone some things
(like "get the current UI display surface") until after the
call to memory_region_snapshot_and_clear_dirty(), and you could
have a "check a flag to see if the guest re-entered the display
device while we were inside memory_region_snapshot_and_clear_dirty()"
test, but some things the guest might do could potentially invalidate
things in a non-obvious action-at-a-distance way that doesn't
require an access to the display device. (e.g. a display device that
lets the guest program the FB to be anywhere in the address map,
plus the ability for the guest to cause remapping of RAM banks
or similar -- the display device can't tell if the RAM banks
got remapped or not, that's handled by a different device.)
thanks
-- PMM
[PULL 5/5] virtiofsd: Disable killpriv_v2 by default
From: Vivek Goyal
We are having bunch of issues with killpriv_v2 enabled by default. First
of all it relies on clearing suid/sgid bits as needed by dropping
capability CAP_FSETID. This does not work for remote filesystems like
NFS (and possibly others).
Secondly, we are noticing other issues related to clearing of SGID
which leads to failures for xfstests generic/355 and generic/193.
Thirdly, there are other issues w.r.t caching of metadata (suid/sgid)
bits in fuse client with killpriv_v2 enabled. Guest can cache that
data for sometime even if cleared on server.
Second and Third issue are fixable. Just that it might take a little
while to get it fixed in kernel. First one will probably not see
any movement for a long time.
Given these issues, killpriv_v2 does not seem to be a good candidate
for enabling by default. We have already disabled it by default in
rust version of virtiofsd.
Hence this patch disabled killpriv_v2 by default. User can choose to
enable it by passing option "-o killpriv_v2".
Signed-off-by: Vivek Goyal
Message-Id:
Reviewed-by: Dr. David Alan Gilbert
Signed-off-by: Dr. David Alan Gilbert
---
tools/virtiofsd/passthrough_ll.c | 13 ++---
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 7a73dfcce9..371a7bead6 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -767,19 +767,10 @@ static void lo_init(void *userdata, struct fuse_conn_info
*conn)
fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling killpriv_v2\n");
conn->want |= FUSE_CAP_HANDLE_KILLPRIV_V2;
lo->killpriv_v2 = 1;
-} else if (lo->user_killpriv_v2 == -1 &&
- conn->capable & FUSE_CAP_HANDLE_KILLPRIV_V2) {
-/*
- * User did not specify a value for killpriv_v2. By default enable it
- * if connection offers this capability
- */
-fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling killpriv_v2\n");
-conn->want |= FUSE_CAP_HANDLE_KILLPRIV_V2;
-lo->killpriv_v2 = 1;
} else {
/*
- * Either user specified to disable killpriv_v2, or connection does
- * not offer this capability. Disable killpriv_v2 in both the cases
+ * Either user specified to disable killpriv_v2, or did not
+ * specify anything. Disable killpriv_v2 in both the cases.
*/
fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling killpriv_v2\n");
conn->want &= ~FUSE_CAP_HANDLE_KILLPRIV_V2;
--
2.37.1
[PULL 4/5] migration: Define BLK_MIG_BLOCK_SIZE as unsigned long long
From: Peter Maydell When we use BLK_MIG_BLOCK_SIZE in expressions like block_mig_state.submitted * BLK_MIG_BLOCK_SIZE, this multiplication is done as 32 bits, because both operands are 32 bits. Coverity complains about possible overflows because we then accumulate that into a 64 bit variable. Define BLK_MIG_BLOCK_SIZE as unsigned long long using the ULL suffix. The only two current uses of it with this problem are both in block_save_pending(), so we could just cast to uint64_t there, but using the ULL suffix is simpler and ensures that we don't accidentally introduce new variants of the same issue in future. Resolves: Coverity CID 1487136, 1487175 Signed-off-by: Peter Maydell Message-Id: <20220721115207.729615-3-peter.mayd...@linaro.org> Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/migration/block.c b/migration/block.c index 9e5aae5898..3577c815a9 100644 --- a/migration/block.c +++ b/migration/block.c @@ -28,7 +28,7 @@ #include "sysemu/block-backend.h" #include "trace.h" -#define BLK_MIG_BLOCK_SIZE (1 << 20) +#define BLK_MIG_BLOCK_SIZE (1ULL << 20) #define BDRV_SECTORS_PER_DIRTY_CHUNK (BLK_MIG_BLOCK_SIZE >> BDRV_SECTOR_BITS) #define BLK_MIG_FLAG_DEVICE_BLOCK 0x01 -- 2.37.1
[PULL 1/5] migration: add remaining params->has_* = true in migration_instance_init()
From: Leonardo Bras
Some of params->has_* = true are missing in migration_instance_init, this
causes migrate_params_check() to skip some tests, allowing some
unsupported scenarios.
Fix this by adding all missing params->has_* = true in
migration_instance_init().
Fixes: 69ef1f36b0 ("migration: define 'tls-creds' and 'tls-hostname' migration
parameters")
Fixes: 1d58872a91 ("migration: do not wait for free thread")
Fixes: d2f1d29b95 ("migration: add support for a "tls-authz" migration
parameter")
Signed-off-by: Leonardo Bras
Message-Id: <20220726010235.342927-1-leob...@redhat.com>
Reviewed-by: Peter Xu
Signed-off-by: Dr. David Alan Gilbert
---
migration/migration.c | 4
1 file changed, 4 insertions(+)
diff --git a/migration/migration.c b/migration/migration.c
index e03f698a3c..82fbe0cf55 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -4451,6 +4451,7 @@ static void migration_instance_init(Object *obj)
/* Set has_* up only for parameter checks */
params->has_compress_level = true;
params->has_compress_threads = true;
+params->has_compress_wait_thread = true;
params->has_decompress_threads = true;
params->has_throttle_trigger_threshold = true;
params->has_cpu_throttle_initial = true;
@@ -4471,6 +4472,9 @@ static void migration_instance_init(Object *obj)
params->has_announce_max = true;
params->has_announce_rounds = true;
params->has_announce_step = true;
+params->has_tls_creds = true;
+params->has_tls_hostname = true;
+params->has_tls_authz = true;
qemu_sem_init(>postcopy_pause_sem, 0);
qemu_sem_init(>postcopy_pause_rp_sem, 0);
--
2.37.1
[PULL 2/5] Revert "migration: Simplify unqueue_page()"
From: Thomas Huth
This reverts commit cfd66f30fb0f735df06ff4220e5000290a43dad3.
The simplification of unqueue_page() introduced a bug that sometimes
breaks migration on s390x hosts.
The problem is not fully understood yet, but since we are already in
the freeze for QEMU 7.1 and we need something working there, let's
revert this patch for the upcoming release. The optimization can be
redone later again in a proper way if necessary.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2099934
Signed-off-by: Thomas Huth
Message-Id: <20220802061949.331576-1-th...@redhat.com>
Reviewed-by: Dr. David Alan Gilbert
Signed-off-by: Dr. David Alan Gilbert
---
migration/ram.c| 37 ++---
migration/trace-events | 3 ++-
2 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index b94669ba5d..dc1de9ddbc 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1612,7 +1612,6 @@ static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t
*offset)
{
struct RAMSrcPageRequest *entry;
RAMBlock *block = NULL;
-size_t page_size;
if (!postcopy_has_request(rs)) {
return NULL;
@@ -1629,13 +1628,10 @@ static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t
*offset)
entry = QSIMPLEQ_FIRST(>src_page_requests);
block = entry->rb;
*offset = entry->offset;
-page_size = qemu_ram_pagesize(block);
-/* Each page request should only be multiple page size of the ramblock */
-assert((entry->len % page_size) == 0);
-if (entry->len > page_size) {
-entry->len -= page_size;
-entry->offset += page_size;
+if (entry->len > TARGET_PAGE_SIZE) {
+entry->len -= TARGET_PAGE_SIZE;
+entry->offset += TARGET_PAGE_SIZE;
} else {
memory_region_unref(block->mr);
QSIMPLEQ_REMOVE_HEAD(>src_page_requests, next_req);
@@ -1643,9 +1639,6 @@ static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t
*offset)
migration_consume_urgent_request();
}
-trace_unqueue_page(block->idstr, *offset,
- test_bit((*offset >> TARGET_PAGE_BITS), block->bmap));
-
return block;
}
@@ -2069,8 +2062,30 @@ static bool get_queued_page(RAMState *rs,
PageSearchStatus *pss)
{
RAMBlock *block;
ram_addr_t offset;
+bool dirty;
+
+do {
+block = unqueue_page(rs, );
+/*
+ * We're sending this page, and since it's postcopy nothing else
+ * will dirty it, and we must make sure it doesn't get sent again
+ * even if this queue request was received after the background
+ * search already sent it.
+ */
+if (block) {
+unsigned long page;
+
+page = offset >> TARGET_PAGE_BITS;
+dirty = test_bit(page, block->bmap);
+if (!dirty) {
+trace_get_queued_page_not_dirty(block->idstr, (uint64_t)offset,
+page);
+} else {
+trace_get_queued_page(block->idstr, (uint64_t)offset, page);
+}
+}
-block = unqueue_page(rs, );
+} while (block && !dirty);
if (block) {
/* See comment above postcopy_preempted_contains() */
diff --git a/migration/trace-events b/migration/trace-events
index a34afe7b85..57003edcbd 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -85,6 +85,8 @@ put_qlist_end(const char *field_name, const char *vmsd_name)
"%s(%s)"
qemu_file_fclose(void) ""
# ram.c
+get_queued_page(const char *block_name, uint64_t tmp_offset, unsigned long
page_abs) "%s/0x%" PRIx64 " page_abs=0x%lx"
+get_queued_page_not_dirty(const char *block_name, uint64_t tmp_offset,
unsigned long page_abs) "%s/0x%" PRIx64 " page_abs=0x%lx"
migration_bitmap_sync_start(void) ""
migration_bitmap_sync_end(uint64_t dirty_pages) "dirty_pages %" PRIu64
migration_bitmap_clear_dirty(char *str, uint64_t start, uint64_t size,
unsigned long page) "rb %s start 0x%"PRIx64" size 0x%"PRIx64" page 0x%lx"
@@ -110,7 +112,6 @@ ram_save_iterate_big_wait(uint64_t milliconds, int
iterations) "big wait: %" PRI
ram_load_complete(int ret, uint64_t seq_iter) "exit_code %d seq iteration %"
PRIu64
ram_write_tracking_ramblock_start(const char *block_id, size_t page_size, void
*addr, size_t length) "%s: page_size: %zu addr: %p length: %zu"
ram_write_tracking_ramblock_stop(const char *block_id, size_t page_size, void
*addr, size_t length) "%s: page_size: %zu addr: %p length: %zu"
-unqueue_page(char *block, uint64_t offset, bool dirty) "ramblock '%s' offset
0x%"PRIx64" dirty %d"
postcopy_preempt_triggered(char *str, unsigned long page) "during sending
ramblock %s offset 0x%lx"
postcopy_preempt_restored(char *str, unsigned long page) "ramblock %s offset
0x%lx"
postcopy_preempt_hit(char *str, uint64_t offset) "ramblock %s offset 0x%"PRIx64
--
2.37.1
[PULL 3/5] migration: Assert that migrate_multifd_compression() returns an in-range value
From: Peter Maydell Coverity complains that when we use the return value from migrate_multifd_compression() as an array index: multifd_recv_state->ops = multifd_ops[migrate_multifd_compression()]; that this might overrun the array (which is declared to have size MULTIFD_COMPRESSION__MAX). This is because the function return type is MultiFDCompression, which is an autogenerated enum. The code generator includes the "one greater than the maximum possible value" MULTIFD_COMPRESSION__MAX in the enum, even though this is not actually a valid value for the enum, and this makes Coverity think that migrate_multifd_compression() could return that __MAX value and index off the end of the array. Suppress the Coverity error by asserting that the value we're going to return is within range. Resolves: Coverity CID 1487239, 1487254 Signed-off-by: Peter Maydell Message-Id: <20220721115207.729615-2-peter.mayd...@linaro.org> Reviewed-by: Dr. David Alan Gilbert Reviewed-by: Juan Quintela Signed-off-by: Dr. David Alan Gilbert --- migration/migration.c | 1 + 1 file changed, 1 insertion(+) diff --git a/migration/migration.c b/migration/migration.c index 82fbe0cf55..bb8bbddfe4 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2617,6 +2617,7 @@ MultiFDCompression migrate_multifd_compression(void) s = migrate_get_current(); +assert(s->parameters.multifd_compression < MULTIFD_COMPRESSION__MAX); return s->parameters.multifd_compression; } -- 2.37.1
[PULL 0/5] migration queue
From: "Dr. David Alan Gilbert" The following changes since commit 0399521e53336bd2cdc15482bca0ffd3493fdff6: Merge tag 'for-upstream' of git://repo.or.cz/qemu/kevin into staging (2022-08-02 06:52:05 -0700) are available in the Git repository at: https://gitlab.com/dagrh/qemu.git tags/pull-migration-20220802c for you to fetch changes up to a21ba54dd5ca38cd05da9035fc65374d7af54f13: virtiofsd: Disable killpriv_v2 by default (2022-08-02 16:46:52 +0100) Migration fixes pull 2022-08-02 Small migration (and virtiofsd) fixes. Signed-off-by: Dr. David Alan Gilbert Leonardo Bras (1): migration: add remaining params->has_* = true in migration_instance_init() Peter Maydell (2): migration: Assert that migrate_multifd_compression() returns an in-range value migration: Define BLK_MIG_BLOCK_SIZE as unsigned long long Thomas Huth (1): Revert "migration: Simplify unqueue_page()" Vivek Goyal (1): virtiofsd: Disable killpriv_v2 by default migration/block.c| 2 +- migration/migration.c| 5 + migration/ram.c | 37 ++--- migration/trace-events | 3 ++- tools/virtiofsd/passthrough_ll.c | 13 ++--- 5 files changed, 36 insertions(+), 24 deletions(-)
Re: [PULL 0/7] Block layer patches
On 8/2/22 06:37, Kevin Wolf wrote: The following changes since commit 60205b71421cbc529ca60b12c79e0eeace007319: Merge tag 'pull-aspeed-20220801' of https://github.com/legoater/qemu into staging (2022-08-01 13:55:11 -0700) are available in the Git repository at: git://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to 21b1d974595b3986c68fe80a1f7e9b87886d4bae: main loop: add missing documentation links to GS/IO macros (2022-08-02 12:02:17 +0200) Block layer patches - libvduse: Coverity fixes - hd-geometry: Fix ignored bios-chs-trans setting - io_uring: Fix compiler warning (missing #include) - main loop: add missing documentation links to GS/IO macros - qemu-iotests: Discard stderr when probing devices Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate. r~ Cole Robinson (1): qemu-iotests: Discard stderr when probing devices Emanuele Giuseppe Esposito (1): main loop: add missing documentation links to GS/IO macros Jinhao Fan (1): block/io_uring: add missing include file Lev Kujawski (1): hw/block/hd-geometry: Do not override specified bios-chs-trans Xie Yongji (3): libvduse: Fix the incorrect function name libvduse: Replace strcpy() with strncpy() libvduse: Pass positive value to strerror() include/qemu/main-loop.h| 18 +++--- block/io_uring.c| 1 + hw/block/hd-geometry.c | 7 ++- subprojects/libvduse/libvduse.c | 13 +++-- tests/qemu-iotests/common.rc| 4 ++-- 5 files changed, 31 insertions(+), 12 deletions(-)
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
On 02.08.22 17:28, Jason A. Donenfeld wrote: > Hi David, Christian, > > While this thread has your attention, I thought I'd reiterate my offer in: > https://lore.kernel.org/qemu-devel/yueouwzdzbqff...@zx2c4.com/ > > Do either of you want to "take ownership" of this patch to bring it > past the finish line, and I can provide whatever additional crypto > code you need for that? For me the patch is good enough. But sure, having a SHA512 implementation would be nice ... Long story short, I'll wire up whatever crypto stuff you can come up with ;) -- Thanks, David / dhildenb
Re: [PATCH] virtio: remove unnecessary host_features in ->get_features()
On Tue, Aug 02 2022, Stefan Hajnoczi wrote:
> Since at least commit 6b8f1020540c27246277377aa2c3331ad2bfb160 ("virtio:
> move host_features") the ->get_features() function has been called with
> host_features as an argument.
>
> Some devices manually add host_features in ->get_features() although the
> features argument already contains host_features. Make all devices
> consistent by dropping the unnecessary code.
I wonder whether we should add that explicitly to the contract for
get_features()?
>
> Cc: Cornelia Huck
> Signed-off-by: Stefan Hajnoczi
> ---
> hw/block/virtio-blk.c | 3 ---
> hw/char/virtio-serial-bus.c | 1 -
> hw/net/virtio-net.c | 3 ---
> hw/scsi/vhost-scsi-common.c | 3 ---
> hw/scsi/virtio-scsi.c | 4
> hw/virtio/virtio-balloon.c | 2 --
> 6 files changed, 16 deletions(-)
The change seems sane to me.
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
Hi David, Christian, While this thread has your attention, I thought I'd reiterate my offer in: https://lore.kernel.org/qemu-devel/yueouwzdzbqff...@zx2c4.com/ Do either of you want to "take ownership" of this patch to bring it past the finish line, and I can provide whatever additional crypto code you need for that? Jason
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
On 02.08.22 17:15, Christian Borntraeger wrote:
>
>
> Am 02.08.22 um 16:53 schrieb David Hildenbrand:
>> On 02.08.22 16:01, Christian Borntraeger wrote:
>>>
>>>
>>> Am 02.08.22 um 15:54 schrieb David Hildenbrand:
On 02.08.22 15:26, Christian Borntraeger wrote:
>
>
> Am 20.07.22 um 14:08 schrieb Jason A. Donenfeld:
>> In order for hosts running inside of TCG to initialize the kernel's
>> random number generator, we should support the PRNO_TRNG instruction,
>> backed in the usual way with the qemu_guest_getrandom helper. This is
>> confirmed working on Linux 5.19-rc6.
>>
>> Cc: Thomas Huth
>> Cc: David Hildenbrand
>> Cc: Richard Henderson
>> Cc: Cornelia Huck
>> Cc: Harald Freudenberger
>> Cc: Holger Dengler
>> Signed-off-by: Jason A. Donenfeld
> [...]
>> +case 114:
>> +if (r1 & 1 || !r1 || r2 & 1 || !r2)
>> +tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> +fill_buf_random(env, ra, >regs[r1], >regs[r1 + 1]);
>> +fill_buf_random(env, ra, >regs[r2], >regs[r2 + 1]);
>> +break;
>
> I think I agree with Harald that some aspects are missing.
> Linux does not seem to check, but we should also modify the query
> function to
> indicate the availability of 114.
>
> As the msa helper deals with many instructions
> ...
> target/s390x/tcg/insn-data.def:D(0xb91e, KMAC,RRE, MSA, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_KMAC)
> target/s390x/tcg/insn-data.def:D(0xb928, PCKMO, RRE, MSA3, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_PCKMO)
> target/s390x/tcg/insn-data.def:D(0xb92a, KMF, RRE, MSA4, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_KMF)
> target/s390x/tcg/insn-data.def:D(0xb92b, KMO, RRE, MSA4, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_KMO)
> target/s390x/tcg/insn-data.def:D(0xb92c, PCC, RRE, MSA4, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_PCC)
> target/s390x/tcg/insn-data.def:D(0xb92d, KMCTR, RRF_b, MSA4, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_KMCTR)
> target/s390x/tcg/insn-data.def:D(0xb92e, KM, RRE, MSA, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_KM)
> target/s390x/tcg/insn-data.def:D(0xb92f, KMC, RRE, MSA, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_KMC)
> target/s390x/tcg/insn-data.def:D(0xb929, KMA, RRF_b, MSA8, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_KMA)
> target/s390x/tcg/insn-data.def:D(0xb93c, PPNO,RRE, MSA5, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_PPNO)
> target/s390x/tcg/insn-data.def:D(0xb93e, KIMD,RRE, MSA, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_KIMD)
> target/s390x/tcg/insn-data.def:D(0xb93f, KLMD,RRE, MSA, 0, 0,
> 0, 0, msa, 0, S390_FEAT_TYPE_KLMD)
> ...
> and in theory other instructions might also have 114 we should at least
> check that this is ppno/prno.
> Or we split out a prno helper from the msa helper.
>
Doesn't
s390_get_feat_block(type, subfunc);
if (!test_be_bit(fc, subfunc)) {
tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
}
check that? As long as we don't implement 114 for any other instruction.
that should properly fence off the other instructions.
>>>
>>> Right that would help. We should still take care of the query function.
>>>
>> s390_get_feat_block() should already take care of that as well, no?
>
> Ah right, yes it fills subfunc. So yes, that should do the trick. Sorry for
> the noise.
>
I had to look at that 2 times as well ...
--
Thanks,
David / dhildenb
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
Am 02.08.22 um 16:53 schrieb David Hildenbrand:
On 02.08.22 16:01, Christian Borntraeger wrote:
Am 02.08.22 um 15:54 schrieb David Hildenbrand:
On 02.08.22 15:26, Christian Borntraeger wrote:
Am 20.07.22 um 14:08 schrieb Jason A. Donenfeld:
In order for hosts running inside of TCG to initialize the kernel's
random number generator, we should support the PRNO_TRNG instruction,
backed in the usual way with the qemu_guest_getrandom helper. This is
confirmed working on Linux 5.19-rc6.
Cc: Thomas Huth
Cc: David Hildenbrand
Cc: Richard Henderson
Cc: Cornelia Huck
Cc: Harald Freudenberger
Cc: Holger Dengler
Signed-off-by: Jason A. Donenfeld
[...]
+case 114:
+if (r1 & 1 || !r1 || r2 & 1 || !r2)
+tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+fill_buf_random(env, ra, >regs[r1], >regs[r1 + 1]);
+fill_buf_random(env, ra, >regs[r2], >regs[r2 + 1]);
+break;
I think I agree with Harald that some aspects are missing.
Linux does not seem to check, but we should also modify the query function to
indicate the availability of 114.
As the msa helper deals with many instructions
...
target/s390x/tcg/insn-data.def:D(0xb91e, KMAC,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMAC)
target/s390x/tcg/insn-data.def:D(0xb928, PCKMO, RRE, MSA3, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PCKMO)
target/s390x/tcg/insn-data.def:D(0xb92a, KMF, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMF)
target/s390x/tcg/insn-data.def:D(0xb92b, KMO, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMO)
target/s390x/tcg/insn-data.def:D(0xb92c, PCC, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PCC)
target/s390x/tcg/insn-data.def:D(0xb92d, KMCTR, RRF_b, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMCTR)
target/s390x/tcg/insn-data.def:D(0xb92e, KM, RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KM)
target/s390x/tcg/insn-data.def:D(0xb92f, KMC, RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMC)
target/s390x/tcg/insn-data.def:D(0xb929, KMA, RRF_b, MSA8, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMA)
target/s390x/tcg/insn-data.def:D(0xb93c, PPNO,RRE, MSA5, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PPNO)
target/s390x/tcg/insn-data.def:D(0xb93e, KIMD,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KIMD)
target/s390x/tcg/insn-data.def:D(0xb93f, KLMD,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KLMD)
...
and in theory other instructions might also have 114 we should at least check
that this is ppno/prno.
Or we split out a prno helper from the msa helper.
Doesn't
s390_get_feat_block(type, subfunc);
if (!test_be_bit(fc, subfunc)) {
tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
}
check that? As long as we don't implement 114 for any other instruction.
that should properly fence off the other instructions.
Right that would help. We should still take care of the query function.
s390_get_feat_block() should already take care of that as well, no?
Ah right, yes it fills subfunc. So yes, that should do the trick. Sorry for the
noise.
Re: [PULL 9/9] hw/i386: pass RNG seed via setup_data entry
Hi Xiaoyao, On Tue, Aug 2, 2022 at 5:06 PM Jason A. Donenfeld wrote: > > Hi Xiaoyao, > > On Tue, Aug 02, 2022 at 10:53:07PM +0800, Xiaoyao Li wrote: > > yes, with >= 7.1, pcmc->legacy_no_rng_seed = false by default, and RNG > > seed is used. > > This is intended behavior. Being on by default is basically the whole > point of it. Otherwise it's useless. > > > > > > Either way, this shouldn't cause boot failures. > > > > It does fail booting OVMF with #PF. Below diff can fix the #PF for me. > > Huh, interesting. Sounds like maybe there's a bug I need to fix. Can you > send me some repro instructions, and I'll look into it right away. I just tried booting Fedora using OVMF and didn't have any problems. I used this command line: qemu-system-x86_64 -machine q35 -enable-kvm -cpu host,-rdrand,-rdseed -smp cores=8 -drive file=disk.qcow2,if=virtio -net nic,model=virtio -net user,hostfwd=tcp::19230-:22 -m 8G -vga qxl -device virtio-serial-pci -device virtserialport,chardev=spicechannel0,name=com.redhat.spice. 0 -chardev spicevmc,id=spicechannel0,name=vdagent -spice unix,addr=/tmp/vm_spice_fedora.socket,disable-ticketing,playback-compression=off,agen t-mouse=on,seamless-migration,gl=on -device virtserialport,chardev=spicechannel1,name=org.spice-space.webdav.0 -chardev spiceport,id=spicechan nel1,name=org.spice-space.webdav.0 -global driver=cfi.pflash01,property=secure,value=on -drive if=pflash,format=raw,unit=0,file=OVMF_CODE.secb oot.fd,readonly=on -drive if=pflash,format=raw,file=OVMF_VARS.secboot.fd Can you tell me what you're using and give me some links with various images and such? Doing the straight forward thing doesn't reproduce it for me. Thanks, Jason
RE: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16 bit accesses
Hi Peter,
CC'ing Philippe.
> -Original Message-
> From: Qemu-devel bounces+fkonrad=amd@nongnu.org> On Behalf Of Peter Maydell
> Sent: 02 August 2022 14:19
> To: qemu-devel@nongnu.org
> Cc: Fabien Chouteau ; Frederic Konrad
>
> Subject: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16 bit
> accesses
>
> In real hardware, the APB and AHB PNP data tables can be accessed
> with byte and halfword reads as well as word reads. Our
> implementation currently only handles word reads. Add support for
> the 8 and 16 bit accesses. Note that we only need to handle aligned
> accesses -- unaligned accesses should continue to trap, as happens on
> hardware.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1132
> Signed-off-by: Peter Maydell
> ---
> It would be nice if we could just set the .valid.min_access_size in
> the MemoryRegionOps to 1 and have the memory system core synthesize
> the 1 and 2 byte accesses from a 4 byte read, but currently that
> doesn't work (see various past mailing list threads).
That looks good to me but I thought this was fixed by 1a5a5570 and 0fbe394a
because RTEMS do bytes accesses?
Did that break at some point?
Regards,
Fred
> ---
> hw/misc/grlib_ahb_apb_pnp.c | 10 ++
> hw/misc/trace-events| 4 ++--
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/hw/misc/grlib_ahb_apb_pnp.c b/hw/misc/grlib_ahb_apb_pnp.c
> index 43e001c3c7b..5b05f158592 100644
> --- a/hw/misc/grlib_ahb_apb_pnp.c
> +++ b/hw/misc/grlib_ahb_apb_pnp.c
> @@ -136,7 +136,8 @@ static uint64_t grlib_ahb_pnp_read(void *opaque,
> hwaddr offset, unsigned size)
> uint32_t val;
>
> val = ahb_pnp->regs[offset >> 2];
> -trace_grlib_ahb_pnp_read(offset, val);
> +val = extract32(val, (4 - (offset & 3) - size) * 8, size * 8);
> +trace_grlib_ahb_pnp_read(offset, size, val);
>
> return val;
> }
> @@ -152,7 +153,7 @@ static const MemoryRegionOps grlib_ahb_pnp_ops =
> {
> .write = grlib_ahb_pnp_write,
> .endianness = DEVICE_BIG_ENDIAN,
> .impl = {
> -.min_access_size = 4,
> +.min_access_size = 1,
> .max_access_size = 4,
> },
> };
> @@ -247,7 +248,8 @@ static uint64_t grlib_apb_pnp_read(void *opaque,
> hwaddr offset, unsigned size)
> uint32_t val;
>
> val = apb_pnp->regs[offset >> 2];
> -trace_grlib_apb_pnp_read(offset, val);
> +val = extract32(val, (4 - (offset & 3) - size) * 8, size * 8);
> +trace_grlib_apb_pnp_read(offset, size, val);
>
> return val;
> }
> @@ -263,7 +265,7 @@ static const MemoryRegionOps grlib_apb_pnp_ops =
> {
> .write = grlib_apb_pnp_write,
> .endianness = DEVICE_BIG_ENDIAN,
> .impl = {
> -.min_access_size = 4,
> +.min_access_size = 1,
> .max_access_size = 4,
> },
> };
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 4d51a80de1d..c18bc0605e8 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -247,8 +247,8 @@ via1_adb_poll(uint8_t data, const char *vadbint, int
> status, int index, int size
> via1_auxmode(int mode) "setting auxmode to %d"
>
> # grlib_ahb_apb_pnp.c
> -grlib_ahb_pnp_read(uint64_t addr, uint32_t value) "AHB PnP read
> addr:0x%03"PRIx64" data:0x%08x"
> -grlib_apb_pnp_read(uint64_t addr, uint32_t value) "APB PnP read
> addr:0x%03"PRIx64" data:0x%08x"
> +grlib_ahb_pnp_read(uint64_t addr, unsigned size, uint32_t value) "AHB PnP
> read addr:0x%03"PRIx64" size:%u data:0x%08x"
> +grlib_apb_pnp_read(uint64_t addr, unsigned size, uint32_t value) "APB PnP
> read addr:0x%03"PRIx64" size:%u data:0x%08x"
>
> # led.c
> led_set_intensity(const char *color, const char *desc, uint8_t
> intensity_percent) "LED desc:'%s' color:%s intensity: %u%%"
> --
> 2.25.1
>
Re: [PULL 9/9] hw/i386: pass RNG seed via setup_data entry
Hi Xiaoyao, On Tue, Aug 02, 2022 at 10:53:07PM +0800, Xiaoyao Li wrote: > yes, with >= 7.1, pcmc->legacy_no_rng_seed = false by default, and RNG > seed is used. This is intended behavior. Being on by default is basically the whole point of it. Otherwise it's useless. > > > Either way, this shouldn't cause boot failures. > > It does fail booting OVMF with #PF. Below diff can fix the #PF for me. Huh, interesting. Sounds like maybe there's a bug I need to fix. Can you send me some repro instructions, and I'll look into it right away. Jason
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
On 02.08.22 16:01, Christian Borntraeger wrote:
>
>
> Am 02.08.22 um 15:54 schrieb David Hildenbrand:
>> On 02.08.22 15:26, Christian Borntraeger wrote:
>>>
>>>
>>> Am 20.07.22 um 14:08 schrieb Jason A. Donenfeld:
In order for hosts running inside of TCG to initialize the kernel's
random number generator, we should support the PRNO_TRNG instruction,
backed in the usual way with the qemu_guest_getrandom helper. This is
confirmed working on Linux 5.19-rc6.
Cc: Thomas Huth
Cc: David Hildenbrand
Cc: Richard Henderson
Cc: Cornelia Huck
Cc: Harald Freudenberger
Cc: Holger Dengler
Signed-off-by: Jason A. Donenfeld
>>> [...]
+case 114:
+if (r1 & 1 || !r1 || r2 & 1 || !r2)
+tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+fill_buf_random(env, ra, >regs[r1], >regs[r1 + 1]);
+fill_buf_random(env, ra, >regs[r2], >regs[r2 + 1]);
+break;
>>>
>>> I think I agree with Harald that some aspects are missing.
>>> Linux does not seem to check, but we should also modify the query function
>>> to
>>> indicate the availability of 114.
>>>
>>> As the msa helper deals with many instructions
>>> ...
>>> target/s390x/tcg/insn-data.def:D(0xb91e, KMAC,RRE, MSA, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_KMAC)
>>> target/s390x/tcg/insn-data.def:D(0xb928, PCKMO, RRE, MSA3, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_PCKMO)
>>> target/s390x/tcg/insn-data.def:D(0xb92a, KMF, RRE, MSA4, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_KMF)
>>> target/s390x/tcg/insn-data.def:D(0xb92b, KMO, RRE, MSA4, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_KMO)
>>> target/s390x/tcg/insn-data.def:D(0xb92c, PCC, RRE, MSA4, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_PCC)
>>> target/s390x/tcg/insn-data.def:D(0xb92d, KMCTR, RRF_b, MSA4, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_KMCTR)
>>> target/s390x/tcg/insn-data.def:D(0xb92e, KM, RRE, MSA, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_KM)
>>> target/s390x/tcg/insn-data.def:D(0xb92f, KMC, RRE, MSA, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_KMC)
>>> target/s390x/tcg/insn-data.def:D(0xb929, KMA, RRF_b, MSA8, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_KMA)
>>> target/s390x/tcg/insn-data.def:D(0xb93c, PPNO,RRE, MSA5, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_PPNO)
>>> target/s390x/tcg/insn-data.def:D(0xb93e, KIMD,RRE, MSA, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_KIMD)
>>> target/s390x/tcg/insn-data.def:D(0xb93f, KLMD,RRE, MSA, 0, 0, 0,
>>> 0, msa, 0, S390_FEAT_TYPE_KLMD)
>>> ...
>>> and in theory other instructions might also have 114 we should at least
>>> check that this is ppno/prno.
>>> Or we split out a prno helper from the msa helper.
>>>
>>
>> Doesn't
>>
>> s390_get_feat_block(type, subfunc);
>> if (!test_be_bit(fc, subfunc)) {
>> tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> }
>>
>> check that? As long as we don't implement 114 for any other instruction.
>> that should properly fence off the other instructions.
>
> Right that would help. We should still take care of the query function.
>
s390_get_feat_block() should already take care of that as well, no?
--
Thanks,
David / dhildenb
Re: [PULL 9/9] hw/i386: pass RNG seed via setup_data entry
On 8/2/2022 9:21 PM, Jason A. Donenfeld wrote:
Hi,
On Tue, Aug 02, 2022 at 11:28:15AM +0800, Xiaoyao Li wrote:
static void pc_q35_7_0_machine_options(MachineClass *m)
{
+PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
pc_q35_7_1_machine_options(m);
m->alias = NULL;
+pcmc->legacy_no_rng_seed = true;
Is making .legacy_no_rng_seed default false and opt-in it for old
machines correct?
Not sure I follow what you're saying. ≤7.0 won't pass the RNG seed. It's
only on ≥7.1 that the RNG seed is used.
yes, with >= 7.1, pcmc->legacy_no_rng_seed = false by default, and RNG
seed is used.
Either way, this shouldn't cause boot failures.
It does fail booting OVMF with #PF. Below diff can fix the #PF for me.
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7280c02ce3d5..1f62971759bf 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1903,6 +1903,7 @@ static void pc_machine_class_init(ObjectClass *oc,
void *data)
pcmc->has_reserved_memory = true;
pcmc->kvmclock_enabled = true;
pcmc->enforce_aligned_dimm = true;
+pcmc->legacy_no_rng_seed = true;
pcmc->enforce_amd_1tb_hole = true;
/* BIOS ACPI tables: 128K. Other BIOS datastructures: less than 4K
reported
* to be used at the moment, 32K should be enough for a while. */
Jason
Re: [PATCH] vdpa: do not save failed dma maps in SVQ iova tree
On Tue, Aug 2, 2022 at 4:41 PM Eugenio Pérez wrote:
>
> If a map fails for whatever reason, it must not be saved in the tree.
> Otherwise, qemu will try to unmap it in cleanup, leaving to more errors.
>
I forgot to add:
Reported-by: Lei Yang
> Fixes: 34e3c94eda ("vdpa: Add custom IOTLB translations to SVQ")
> Signed-off-by: Eugenio Pérez
> ---
> hw/virtio/vhost-vdpa.c | 20 +---
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 3ff9ce3501..e44c23dce5 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -176,6 +176,7 @@ static void vhost_vdpa_listener_commit(MemoryListener
> *listener)
> static void vhost_vdpa_listener_region_add(MemoryListener *listener,
> MemoryRegionSection *section)
> {
> +DMAMap mem_region = {};
> struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa,
> listener);
> hwaddr iova;
> Int128 llend, llsize;
> @@ -212,13 +213,13 @@ static void
> vhost_vdpa_listener_region_add(MemoryListener *listener,
>
> llsize = int128_sub(llend, int128_make64(iova));
> if (v->shadow_vqs_enabled) {
> -DMAMap mem_region = {
> -.translated_addr = (hwaddr)(uintptr_t)vaddr,
> -.size = int128_get64(llsize) - 1,
> -.perm = IOMMU_ACCESS_FLAG(true, section->readonly),
> -};
> +int r;
>
> -int r = vhost_iova_tree_map_alloc(v->iova_tree, _region);
> +mem_region.translated_addr = (hwaddr)(uintptr_t)vaddr,
> +mem_region.size = int128_get64(llsize) - 1,
> +mem_region.perm = IOMMU_ACCESS_FLAG(true, section->readonly),
> +
> +r = vhost_iova_tree_map_alloc(v->iova_tree, _region);
> if (unlikely(r != IOVA_OK)) {
> error_report("Can't allocate a mapping (%d)", r);
> goto fail;
> @@ -232,11 +233,16 @@ static void
> vhost_vdpa_listener_region_add(MemoryListener *listener,
> vaddr, section->readonly);
> if (ret) {
> error_report("vhost vdpa map fail!");
> -goto fail;
> +goto fail_map;
> }
>
> return;
>
> +fail_map:
> +if (v->shadow_vqs_enabled) {
> +vhost_iova_tree_remove(v->iova_tree, _region);
> +}
> +
> fail:
> /*
> * On the initfn path, store the first error in the container so we
> --
> 2.31.1
>
>
[PATCH] vdpa: do not save failed dma maps in SVQ iova tree
If a map fails for whatever reason, it must not be saved in the tree.
Otherwise, qemu will try to unmap it in cleanup, leaving to more errors.
Fixes: 34e3c94eda ("vdpa: Add custom IOTLB translations to SVQ")
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-vdpa.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 3ff9ce3501..e44c23dce5 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -176,6 +176,7 @@ static void vhost_vdpa_listener_commit(MemoryListener
*listener)
static void vhost_vdpa_listener_region_add(MemoryListener *listener,
MemoryRegionSection *section)
{
+DMAMap mem_region = {};
struct vhost_vdpa *v = container_of(listener, struct vhost_vdpa, listener);
hwaddr iova;
Int128 llend, llsize;
@@ -212,13 +213,13 @@ static void vhost_vdpa_listener_region_add(MemoryListener
*listener,
llsize = int128_sub(llend, int128_make64(iova));
if (v->shadow_vqs_enabled) {
-DMAMap mem_region = {
-.translated_addr = (hwaddr)(uintptr_t)vaddr,
-.size = int128_get64(llsize) - 1,
-.perm = IOMMU_ACCESS_FLAG(true, section->readonly),
-};
+int r;
-int r = vhost_iova_tree_map_alloc(v->iova_tree, _region);
+mem_region.translated_addr = (hwaddr)(uintptr_t)vaddr,
+mem_region.size = int128_get64(llsize) - 1,
+mem_region.perm = IOMMU_ACCESS_FLAG(true, section->readonly),
+
+r = vhost_iova_tree_map_alloc(v->iova_tree, _region);
if (unlikely(r != IOVA_OK)) {
error_report("Can't allocate a mapping (%d)", r);
goto fail;
@@ -232,11 +233,16 @@ static void vhost_vdpa_listener_region_add(MemoryListener
*listener,
vaddr, section->readonly);
if (ret) {
error_report("vhost vdpa map fail!");
-goto fail;
+goto fail_map;
}
return;
+fail_map:
+if (v->shadow_vqs_enabled) {
+vhost_iova_tree_remove(v->iova_tree, _region);
+}
+
fail:
/*
* On the initfn path, store the first error in the container so we
--
2.31.1
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
Am 02.08.22 um 15:54 schrieb David Hildenbrand:
On 02.08.22 15:26, Christian Borntraeger wrote:
Am 20.07.22 um 14:08 schrieb Jason A. Donenfeld:
In order for hosts running inside of TCG to initialize the kernel's
random number generator, we should support the PRNO_TRNG instruction,
backed in the usual way with the qemu_guest_getrandom helper. This is
confirmed working on Linux 5.19-rc6.
Cc: Thomas Huth
Cc: David Hildenbrand
Cc: Richard Henderson
Cc: Cornelia Huck
Cc: Harald Freudenberger
Cc: Holger Dengler
Signed-off-by: Jason A. Donenfeld
[...]
+case 114:
+if (r1 & 1 || !r1 || r2 & 1 || !r2)
+tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+fill_buf_random(env, ra, >regs[r1], >regs[r1 + 1]);
+fill_buf_random(env, ra, >regs[r2], >regs[r2 + 1]);
+break;
I think I agree with Harald that some aspects are missing.
Linux does not seem to check, but we should also modify the query function to
indicate the availability of 114.
As the msa helper deals with many instructions
...
target/s390x/tcg/insn-data.def:D(0xb91e, KMAC,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMAC)
target/s390x/tcg/insn-data.def:D(0xb928, PCKMO, RRE, MSA3, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PCKMO)
target/s390x/tcg/insn-data.def:D(0xb92a, KMF, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMF)
target/s390x/tcg/insn-data.def:D(0xb92b, KMO, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMO)
target/s390x/tcg/insn-data.def:D(0xb92c, PCC, RRE, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PCC)
target/s390x/tcg/insn-data.def:D(0xb92d, KMCTR, RRF_b, MSA4, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMCTR)
target/s390x/tcg/insn-data.def:D(0xb92e, KM, RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KM)
target/s390x/tcg/insn-data.def:D(0xb92f, KMC, RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMC)
target/s390x/tcg/insn-data.def:D(0xb929, KMA, RRF_b, MSA8, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KMA)
target/s390x/tcg/insn-data.def:D(0xb93c, PPNO,RRE, MSA5, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_PPNO)
target/s390x/tcg/insn-data.def:D(0xb93e, KIMD,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KIMD)
target/s390x/tcg/insn-data.def:D(0xb93f, KLMD,RRE, MSA, 0, 0, 0, 0,
msa, 0, S390_FEAT_TYPE_KLMD)
...
and in theory other instructions might also have 114 we should at least check
that this is ppno/prno.
Or we split out a prno helper from the msa helper.
Doesn't
s390_get_feat_block(type, subfunc);
if (!test_be_bit(fc, subfunc)) {
tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
}
check that? As long as we don't implement 114 for any other instruction.
that should properly fence off the other instructions.
Right that would help. We should still take care of the query function.
Re: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16 bit accesses
On Tue, 2 Aug 2022 at 15:20, Konrad, Frederic wrote: > > Hi Peter, > > CC'ing Philippe. > > > -Original Message- > > From: Qemu-devel > bounces+fkonrad=amd@nongnu.org> On Behalf Of Peter Maydell > > Sent: 02 August 2022 14:19 > > To: qemu-devel@nongnu.org > > Cc: Fabien Chouteau ; Frederic Konrad > > > > Subject: [PATCH for-7.1] hw/misc/grlib_ahb_apb_pnp: Support 8 and 16 bit > > accesses > > > > In real hardware, the APB and AHB PNP data tables can be accessed > > with byte and halfword reads as well as word reads. Our > > implementation currently only handles word reads. Add support for > > the 8 and 16 bit accesses. Note that we only need to handle aligned > > accesses -- unaligned accesses should continue to trap, as happens on > > hardware. > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1132 > > Signed-off-by: Peter Maydell > > --- > > It would be nice if we could just set the .valid.min_access_size in > > the MemoryRegionOps to 1 and have the memory system core synthesize > > the 1 and 2 byte accesses from a 4 byte read, but currently that > > doesn't work (see various past mailing list threads). > > That looks good to me but I thought this was fixed by 1a5a5570 and 0fbe394a > because RTEMS do bytes accesses? > > Did that break at some point? I definitely tried letting the .impl vs .valid settings handle this, but the access_with_adjusted_size() code doesn't do the right thing. (In particular, the test case ELF in the bug report works with this patch, and doesn't work without it...) I'm pretty sure the problem with access_with_adjusted_size() is a long-standing bug -- I found a couple of mailing list threads about it. We really ought to fix that properly, but that's definitely not for-7.1 material. -- PMM
Re: [PATCH] tests/avocado: fix replay-linux test
On Tue, Aug 2, 2022 at 12:46 PM Pavel Dovgalyuk wrote: > > Last line of the test is missing by accident. > This patch fixes the script. > > Signed-off-by: Pavel Dovgalyuk > --- > tests/avocado/replay_linux.py |1 + > 1 file changed, 1 insertion(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] target/mips: Advance pc after semihosting exception
On Tue, Aug 2, 2022 at 4:11 PM Richard Henderson
wrote:
> On 8/1/22 23:48, Philippe Mathieu-Daudé wrote:
> > Hi Richard,
> >
> > On 30/7/22 04:18, Richard Henderson wrote:
> >> Delay generating the exception until after we know the
> >> insn length, and record that length in env->error_code.
> >>
> >> Fixes: 8ec7e3c53d4 ("target/mips: Use an exception for semihosting")
> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1126
> >> Signed-off-by: Richard Henderson
> >> ---
> >> target/mips/tcg/translate.h | 4
> >> target/mips/tcg/sysemu/tlb_helper.c | 1 +
> >> target/mips/tcg/translate.c | 10 +-
> >> target/mips/tcg/micromips_translate.c.inc | 6 +++---
> >> target/mips/tcg/mips16e_translate.c.inc | 2 +-
> >> target/mips/tcg/nanomips_translate.c.inc | 4 ++--
> >> 6 files changed, 16 insertions(+), 11 deletions(-)
> >> @@ -16098,6 +16095,9 @@ static void
> >> mips_tr_translate_insn(DisasContextBase *dcbase,
> >> CPUState *cs)
> >> if (is_slot) {
> >> gen_branch(ctx, insn_bytes);
> >> }
> >> +if (ctx->base.is_jmp == DISAS_SEMIHOST) {
> >> +generate_exception_err(ctx, EXCP_SEMIHOST, insn_bytes);
> >
> > Hmm this API takes an error_code argument:
> >
> >int error_code;
> >#define EXCP_TLB_NOMATCH 0x1
> >#define EXCP_INST_NOTAVAIL 0x2 /* No valid instruction word for BadInstr
> > */
> >
> > Now we pass bytes. What about adding an extra helper?
> >
> >void generate_exception_err_displace(DisasContext *ctx,
> > int excp, int err,
> > target_long displacement);
>
> These error codes are specific to the matching EXCP.
> We don't need to introduce extra storage, I don't think.
OK, fine then.
> > Reviewed-by: Philippe Mathieu-Daudé
Re: [RFC 0/3] Add Generic SPI GPIO model
On 7/31/22 00:06, Peter Delevoryas wrote:
On Sat, Jul 30, 2022 at 11:18:33PM +0200, Cédric Le Goater wrote:
On 7/29/22 19:30, Peter Delevoryas wrote:
On Fri, Jul 29, 2022 at 03:25:55PM +0200, Cédric Le Goater wrote:
Hello Iris,
On 7/29/22 01:23, Iris Chen wrote:
Hey everyone,
I have been working on a project to add support for SPI-based TPMs in QEMU.
Currently, most of our vboot platforms using a SPI-based TPM use the Linux
SPI-GPIO driver to "bit-bang" the SPI protocol. This is because the Aspeed
SPI controller (modeled in QEMU under hw/ssi/aspeed_smc.c) has an implementation
deficiency that prevents bi-directional operations.
aspeed_smc models the Aspeed FMC/SPI controllers which have a well defined
HW interface. Your model proposal adds support for a new SPI controller
using bitbang GPIOs. These are really two differents models. I don't see
how you could reuse aspeed_smc for this purpose.
or you mean that Linux is using the SPI-GPIO driver because the Linux
Aspeed SMC driver doesn't match the need ? It is true that the Linux
driver is not generic, it deals with flash devices only. But that's
another problem.
Thus, in order to connect
a TPM to this bus, my patch implements a QEMU SPI-GPIO driver (as the QEMU
counterpart of the Linux SPI-GPIO driver).
As we use SPI-based TPMs on many of our BMCs for the secure-boot implementation,
I have already tested this implementation locally with our Yosemite-v3.5
platform
and Facebook-OpenBMC. This model was tested by connecting a generic SPI-NOR
(m25p80
for example) to the Yosemite-v3.5 SPI bus containing the TPM.
This patch is an RFC because I have several questions about design. Although the
model is working, I understand there are many areas where the design decision
is not deal (ie. abstracting hard coded GPIO values). Below are some details of
the
patch and specific areas where I would appreciate feedback on how to make this
better:
hw/arm/aspeed.c:
I am not sure where the best place is to instantiate the spi_gpio besides the
aspeed_machine_init.
The SPI GPIO device would be a platform device and not a SoC device.
Hence, it should be instantiated at the machine level, like the I2C
device are, using properties to let the model know about the GPIOs
that should be driven to implement the SPI protocol.
Agreed, should be like an I2C device.
Ideally, the state of the GPIO controller pins and the SPI GPIO state
should be shared. I think that's what you are trying to do that with
attribute 'controller_state' in your patch ? But, the way it is done
today, is too tightly coupled (names) with the Aspeed GPIO model to
be generic.
I think we need an intermediate QOM interface, or a base class, to
implement an abstract SPI GPIO model and an Aspeed SPI GPIO model
on top which would be linked to the Aspeed GPIO model of the SoC
in use.
Disagree, I feel like we can accomplish this without inheritance.
Or we could introduce some kind of generic GPIO controller that
we would link the SPI GPIO model with (using a property). The
Aspeed GPIO model would be of that kind and the SPI GPIO model
would be able to drive the pins using a common interface.
That's another way to do it.
Agree, I would like to go in this direction if at all possible.
Let's give it a try then. I would introduce a new QOM interface,
something like :
#define TYPE_GPIO_INTERFACE "gpio-interface"
#define GPIO_INTERFACE(obj) \
INTERFACE_CHECK(GpioInterface, (obj), TYPE_GPIO_INTERFACE)
typedef struct GpioInterfaceClass GpioInterfaceClass;
DECLARE_CLASS_CHECKERS(GpioInterfaceClass, GPIO_INTERFACE,
TYPE_GPIO_INTERFACE)
struct GpioInterfaceClass {
InterfaceClass parent;
int (*get)(GpioInterface *gi, ...);
int (*set)(GpioInterface *gi, ...);
...
};
and implement the interface handlers under the AspeedGPIO model.
The SPI GPIO model would have a link to such an interface to drive
the GPIO pins.
See IPMI and XIVE for some more complete models.
This sounds good, but I just want to clarify first:
Is it necessary to introduce a GPIO interface?
Well, my feeling is that we need an abstract layer to interface the
SPI GPIO model with any model of GPIO controller.
Or, could we connect the IRQ's just using the existing
QOM/sysbus/object/IRQ infrastructure?
and we would use the QOM canonical path to identify the GPIO pins
and QOM routines to get and set the values ? It looks feasible.
That's how I would do it in a script but not in model.
I'll investigate if we can connect the IRQ's without introducing a new
interface. We can continue with this design for now though.
OK. Let's see.
Could we add the ability to instantiate it on the command line?
It might be complex to identify the QOM object to use as the GPIO
controller from the command line since it is on the SoC and not
a platform device. In that case, an Aspeed SPI GPIO model would
be a
Re: [PATCH] target/mips: Advance pc after semihosting exception
On 8/1/22 23:48, Philippe Mathieu-Daudé wrote:
Hi Richard,
On 30/7/22 04:18, Richard Henderson wrote:
Delay generating the exception until after we know the
insn length, and record that length in env->error_code.
Fixes: 8ec7e3c53d4 ("target/mips: Use an exception for semihosting")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1126
Signed-off-by: Richard Henderson
---
target/mips/tcg/translate.h | 4
target/mips/tcg/sysemu/tlb_helper.c | 1 +
target/mips/tcg/translate.c | 10 +-
target/mips/tcg/micromips_translate.c.inc | 6 +++---
target/mips/tcg/mips16e_translate.c.inc | 2 +-
target/mips/tcg/nanomips_translate.c.inc | 4 ++--
6 files changed, 16 insertions(+), 11 deletions(-)
diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index 55053226ae..69f85841d2 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -51,6 +51,10 @@ typedef struct DisasContext {
int gi;
} DisasContext;
+#define DISAS_STOP DISAS_TARGET_0
+#define DISAS_EXIT DISAS_TARGET_1
+#define DISAS_SEMIHOST DISAS_TARGET_2
+
/* MIPS major opcodes */
#define MASK_OP_MAJOR(op) (op & (0x3F << 26))
diff --git a/target/mips/tcg/sysemu/tlb_helper.c
b/target/mips/tcg/sysemu/tlb_helper.c
index 57ffad2902..9d16859c0a 100644
--- a/target/mips/tcg/sysemu/tlb_helper.c
+++ b/target/mips/tcg/sysemu/tlb_helper.c
@@ -1056,6 +1056,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
case EXCP_SEMIHOST:
cs->exception_index = EXCP_NONE;
mips_semihosting(env);
+ env->active_tc.PC += env->error_code;
return;
case EXCP_DSS:
env->CP0_Debug |= 1 << CP0DB_DSS;
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 1f6a779808..de1511baaf 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -1213,9 +1213,6 @@ TCGv_i64 fpu_f64[32];
#include "exec/gen-icount.h"
-#define DISAS_STOP DISAS_TARGET_0
-#define DISAS_EXIT DISAS_TARGET_1
-
static const char regnames_HI[][4] = {
"HI0", "HI1", "HI2", "HI3",
};
@@ -13902,7 +13899,7 @@ static void decode_opc_special_r6(CPUMIPSState *env,
DisasContext *ctx)
break;
case R6_OPC_SDBBP:
if (is_uhi(extract32(ctx->opcode, 6, 20))) {
- generate_exception_end(ctx, EXCP_SEMIHOST);
+ ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
if (ctx->hflags & MIPS_HFLAG_SBRI) {
gen_reserved_instruction(ctx);
@@ -14314,7 +14311,7 @@ static void decode_opc_special2_legacy(CPUMIPSState *env,
DisasContext *ctx)
break;
case OPC_SDBBP:
if (is_uhi(extract32(ctx->opcode, 6, 20))) {
- generate_exception_end(ctx, EXCP_SEMIHOST);
+ ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
/*
* XXX: not clear which exception should be raised
@@ -16098,6 +16095,9 @@ static void mips_tr_translate_insn(DisasContextBase *dcbase,
CPUState *cs)
if (is_slot) {
gen_branch(ctx, insn_bytes);
}
+ if (ctx->base.is_jmp == DISAS_SEMIHOST) {
+ generate_exception_err(ctx, EXCP_SEMIHOST, insn_bytes);
Hmm this API takes an error_code argument:
int error_code;
#define EXCP_TLB_NOMATCH 0x1
#define EXCP_INST_NOTAVAIL 0x2 /* No valid instruction word for BadInstr */
Now we pass bytes. What about adding an extra helper?
void generate_exception_err_displace(DisasContext *ctx,
int excp, int err,
target_long displacement);
These error codes are specific to the matching EXCP.
We don't need to introduce extra storage, I don't think.
r~
Otherwise LGTM.
Reviewed-by: Philippe Mathieu-Daudé
+ }
ctx->base.pc_next += insn_bytes;
if (ctx->base.is_jmp != DISAS_NEXT) {
diff --git a/target/mips/tcg/micromips_translate.c.inc
b/target/mips/tcg/micromips_translate.c.inc
index 274caf2c3c..b2c696f891 100644
--- a/target/mips/tcg/micromips_translate.c.inc
+++ b/target/mips/tcg/micromips_translate.c.inc
@@ -826,7 +826,7 @@ static void gen_pool16c_insn(DisasContext *ctx)
break;
case SDBBP16:
if (is_uhi(extract32(ctx->opcode, 0, 4))) {
- generate_exception_end(ctx, EXCP_SEMIHOST);
+ ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
/*
* XXX: not clear which exception should be raised
@@ -942,7 +942,7 @@ static void gen_pool16c_r6_insn(DisasContext *ctx)
case R6_SDBBP16:
/* SDBBP16 */
if (is_uhi(extract32(ctx->opcode, 6, 4))) {
- generate_exception_end(ctx, EXCP_SEMIHOST);
+ ctx->base.is_jmp = DISAS_SEMIHOST;
} else {
if (ctx->hflags & MIPS_HFLAG_SBRI) {
generate_exception(ctx, EXCP_RI);
@@ -1311,7 +1311,7 @@ static void
Re: [PATCH] hw/usb/hcd-xhci: Fix endless loop in case the DMA access fails (CVE-2020-14394)
On Tue, 2 Aug 2022 at 14:53, Thomas Huth wrote: > > The XHCI code could enter an endless loop in case the guest points > QEMU to fetch TRBs from invalid memory areas. Fix it by properly > checking the return value of dma_memory_read(). It certainly makes sense to check the return value from dma_memory_read(), but how can we end up in an infinite loop if it fails? Either: (1) there is some combination of data values which allow an infinite loop, so we can hit those if the DMA access fails and we use bogus uninitialized data. But then the guest could maliciously pass us those bad values and create an infinite loop without a failing DMA access, ie commit 05f43d44e4b missed a case. If so we need to fix that! Or (2) there isn't actually an infinite loop possible here, and we're just tidying up a case which is C undefined behaviour but in practice unlikely to have effects any worse than the guest could provoke anyway (ie running up to the TRB_LINK_LIMIT and stopping). In this case the commit message here is a bit misleading. thanks -- PMM
Re: [PATCH 0/2] migration: fix coverity nits
* Peter Maydell (peter.mayd...@linaro.org) wrote: > On Thu, 21 Jul 2022 at 12:52, Peter Maydell wrote: > > > > This patchset fixes four Coverity nits in the migration code. > > The first patch is just adding an assert() to clue coverity in > > that an array index must be in-bounds. The second adds an ULL > > suffix to force a multiplication to be done at 64 bits. > > > > thanks > > -- PMM > > > > Peter Maydell (2): > > migration: Assert that migrate_multifd_compression() returns an > > in-range value > > migration: Define BLK_MIG_BLOCK_SIZE as unsigned long long > > Ping? This series got reviewed but doesn't seem to have > made it into a migration pullreq yet. Queued > thanks > -- PMM > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH] virtiofsd: Disable killpriv_v2 by default
* Vivek Goyal (vgo...@redhat.com) wrote:
> We are having bunch of issues with killpriv_v2 enabled by default. First
> of all it relies on clearing suid/sgid bits as needed by dropping
> capability CAP_FSETID. This does not work for remote filesystems like
> NFS (and possibly others).
>
> Secondly, we are noticing other issues related to clearing of SGID
> which leads to failures for xfstests generic/355 and generic/193.
>
> Thirdly, there are other issues w.r.t caching of metadata (suid/sgid)
> bits in fuse client with killpriv_v2 enabled. Guest can cache that
> data for sometime even if cleared on server.
>
> Second and Third issue are fixable. Just that it might take a little
> while to get it fixed in kernel. First one will probably not see
> any movement for a long time.
>
> Given these issues, killpriv_v2 does not seem to be a good candidate
> for enabling by default. We have already disabled it by default in
> rust version of virtiofsd.
>
> Hence this patch disabled killpriv_v2 by default. User can choose to
> enable it by passing option "-o killpriv_v2".
>
> Signed-off-by: Vivek Goyal
Queued
> ---
> tools/virtiofsd/passthrough_ll.c | 13 ++---
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c
> ===
> --- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c 2022-07-29
> 08:19:05.925119947 -0400
> +++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c2022-07-29
> 08:27:08.048049096 -0400
> @@ -767,19 +767,10 @@ static void lo_init(void *userdata, stru
> fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling killpriv_v2\n");
> conn->want |= FUSE_CAP_HANDLE_KILLPRIV_V2;
> lo->killpriv_v2 = 1;
> -} else if (lo->user_killpriv_v2 == -1 &&
> - conn->capable & FUSE_CAP_HANDLE_KILLPRIV_V2) {
> -/*
> - * User did not specify a value for killpriv_v2. By default enable it
> - * if connection offers this capability
> - */
> -fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling killpriv_v2\n");
> -conn->want |= FUSE_CAP_HANDLE_KILLPRIV_V2;
> -lo->killpriv_v2 = 1;
> } else {
> /*
> - * Either user specified to disable killpriv_v2, or connection does
> - * not offer this capability. Disable killpriv_v2 in both the cases
> + * Either user specified to disable killpriv_v2, or did not
> + * specify anything. Disable killpriv_v2 in both the cases.
> */
> fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling killpriv_v2\n");
> conn->want &= ~FUSE_CAP_HANDLE_KILLPRIV_V2;
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
On 02.08.22 15:26, Christian Borntraeger wrote:
>
>
> Am 20.07.22 um 14:08 schrieb Jason A. Donenfeld:
>> In order for hosts running inside of TCG to initialize the kernel's
>> random number generator, we should support the PRNO_TRNG instruction,
>> backed in the usual way with the qemu_guest_getrandom helper. This is
>> confirmed working on Linux 5.19-rc6.
>>
>> Cc: Thomas Huth
>> Cc: David Hildenbrand
>> Cc: Richard Henderson
>> Cc: Cornelia Huck
>> Cc: Harald Freudenberger
>> Cc: Holger Dengler
>> Signed-off-by: Jason A. Donenfeld
> [...]
>> +case 114:
>> +if (r1 & 1 || !r1 || r2 & 1 || !r2)
>> +tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
>> +fill_buf_random(env, ra, >regs[r1], >regs[r1 + 1]);
>> +fill_buf_random(env, ra, >regs[r2], >regs[r2 + 1]);
>> +break;
>
> I think I agree with Harald that some aspects are missing.
> Linux does not seem to check, but we should also modify the query function to
> indicate the availability of 114.
>
> As the msa helper deals with many instructions
> ...
> target/s390x/tcg/insn-data.def:D(0xb91e, KMAC,RRE, MSA, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_KMAC)
> target/s390x/tcg/insn-data.def:D(0xb928, PCKMO, RRE, MSA3, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_PCKMO)
> target/s390x/tcg/insn-data.def:D(0xb92a, KMF, RRE, MSA4, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_KMF)
> target/s390x/tcg/insn-data.def:D(0xb92b, KMO, RRE, MSA4, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_KMO)
> target/s390x/tcg/insn-data.def:D(0xb92c, PCC, RRE, MSA4, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_PCC)
> target/s390x/tcg/insn-data.def:D(0xb92d, KMCTR, RRF_b, MSA4, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_KMCTR)
> target/s390x/tcg/insn-data.def:D(0xb92e, KM, RRE, MSA, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_KM)
> target/s390x/tcg/insn-data.def:D(0xb92f, KMC, RRE, MSA, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_KMC)
> target/s390x/tcg/insn-data.def:D(0xb929, KMA, RRF_b, MSA8, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_KMA)
> target/s390x/tcg/insn-data.def:D(0xb93c, PPNO,RRE, MSA5, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_PPNO)
> target/s390x/tcg/insn-data.def:D(0xb93e, KIMD,RRE, MSA, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_KIMD)
> target/s390x/tcg/insn-data.def:D(0xb93f, KLMD,RRE, MSA, 0, 0, 0,
> 0, msa, 0, S390_FEAT_TYPE_KLMD)
> ...
> and in theory other instructions might also have 114 we should at least check
> that this is ppno/prno.
> Or we split out a prno helper from the msa helper.
>
Doesn't
s390_get_feat_block(type, subfunc);
if (!test_be_bit(fc, subfunc)) {
tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
}
check that? As long as we don't implement 114 for any other instruction.
that should properly fence off the other instructions.
--
Thanks,
David / dhildenb
Re: [PATCH v7 05/14] qapi: net: add stream and dgram netdevs
Laurent Vivier writes:
> On 02/08/2022 10:37, Markus Armbruster wrote:
>> Laurent Vivier writes:
>>
> ...
>>> diff --git a/qemu-options.hx b/qemu-options.hx
>>> index 79e00916a11f..170117e1adf0 100644
>>> --- a/qemu-options.hx
>>> +++ b/qemu-options.hx
>>> @@ -2726,6 +2726,18 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>>> "-netdev socket,id=str[,fd=h][,udp=host:port][,localaddr=host:port]\n"
>>> "configure a network backend to connect to another
>>> network\n"
>>> "using an UDP tunnel\n"
>>> +"-netdev
>>> stream,id=str[,server=on|off],addr.type=inet,addr.host=host,addr.port=port\n"
>>> +"-netdev stream,id=str[,server=on|off],addr.type=fd,addr.str=h\n"
>>> +"configure a network backend to connect to another
>>> network\n"
>>> +"using a socket connection in stream mode.\n"
>
> From v6:
>> This part needs to match NetdevStreamOptions above.
>>
>> Missing here: the optional members of InetSocketAddress: numeric, to,
>> ipv4, ... Do we care?
>
> At this patch level, no, because we decode them manually and not using
> socket_connect()/socket_listen(). But the doc should be updated for PATCH
> 13/14 as I move
> stream.c to QIO.
>
>>
>> The next part needs to match NetdevDgramOptions above.
>
>
>>> +"-netdev
>>> dgram,id=str,remote.type=inet,remote.host=maddr,remote.port=port[,local.type=inet,local.host=addr]\n"
>>> +"-netdev
>>> dgram,id=str,remote.type=inet,remote.host=maddr,remote.port=port[,local.type=fd,local.str=h]\n"
>>> +"configure a network backend to connect to a multicast
>>> maddr and port\n"
>>> +"use ``local.host=addr`` to specify the host address
>>> to send packets from\n"
>
> From v6:
>> I figure this covers table rows
>>
>>#@remote @local | okay?
>>#+
>>#multicast absent | yes
>>#multicast present | yes
>>
>> for remote.type=inet and any local.type.
>>
>> What about remote.type=fd?
>
> multicast is only supported with remote.type=inet, not fd or unix
>
> In net_dgram_init(), we initiate a multicast connection if remote.type is
> inet and address
> type is multicast, otherwise it's an unicast connection.
Hmm.
With .type=inet, .host=... determines multicast.
With .type=fd, we *assume* unicast? What if the underlying socket is
actually bound to a multicast address?
>>> +"-netdev
>>> dgram,id=str,local.type=inet,local.host=addr,local.port=port[,remote.type=inet,remote.host=addr,remote.port=port]\n"
>
> From v6:
>> I figure this covers table rows
>>
>> #absent present | yes
>> #not multicast present | yes
>>
>> for *.type=inet.
>
>
>>> +"-netdev dgram,id=str,local.type=fd,local.str=h\n"
>>> +"configure a network backend to connect to another
>>> network\n"
>>> +"using an UDP tunnel\n"
>
> From v6:
>> I figure this covers table row
>>
>>#absent present | yes
>>
>> for local.type=fd.
>>
>> Together, they cover table row
>>
>> #absent present | yes
>>
>> for any local.type. Good.
>>
>> Table row
>>
>> #not multicast present | yes
>>
>> is only covered for *.type=inet. Could either of the types be fd?
>
> In v7, I've update the table to include the case of fd:
>
> = =
> remote local okay?
> = =
> absent absentno
> absent not fdno
> absent fdyes
> multicast absentyes
> multicast present yes
> not multicast absentno
> not multicast present yes
> = =
>
> For local, if it's not specified otherwise, fd is supported.
> Remote and local type must be the same (inet or unix), if local is fd, remote
> must not be
> provided.
My brain is melting.
[PATCH] hw/usb/hcd-xhci: Fix endless loop in case the DMA access fails (CVE-2020-14394)
The XHCI code could enter an endless loop in case the guest points
QEMU to fetch TRBs from invalid memory areas. Fix it by properly
checking the return value of dma_memory_read().
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/646
Signed-off-by: Thomas Huth
---
hw/usb/hcd-xhci.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 296cc6c8e6..63d428a444 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -21,6 +21,7 @@
#include "qemu/osdep.h"
#include "qemu/timer.h"
+#include "qemu/log.h"
#include "qemu/module.h"
#include "qemu/queue.h"
#include "migration/vmstate.h"
@@ -679,8 +680,12 @@ static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing
*ring, XHCITRB *trb,
while (1) {
TRBType type;
-dma_memory_read(xhci->as, ring->dequeue, trb, TRB_SIZE,
-MEMTXATTRS_UNSPECIFIED);
+if (dma_memory_read(xhci->as, ring->dequeue, trb, TRB_SIZE,
+MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: DMA memory access failed!\n",
+ __func__);
+return 0;
+}
trb->addr = ring->dequeue;
trb->ccs = ring->ccs;
le64_to_cpus(>parameter);
@@ -727,8 +732,12 @@ static int xhci_ring_chain_length(XHCIState *xhci, const
XHCIRing *ring)
while (1) {
TRBType type;
-dma_memory_read(xhci->as, dequeue, , TRB_SIZE,
-MEMTXATTRS_UNSPECIFIED);
+if (dma_memory_read(xhci->as, dequeue, , TRB_SIZE,
+MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: DMA memory access failed!\n",
+ __func__);
+return -length;
+}
le64_to_cpus();
le32_to_cpus();
le32_to_cpus();
--
2.31.1
[PULL 7/7] main loop: add missing documentation links to GS/IO macros
From: Emanuele Giuseppe Esposito
If we go directly to GLOBAL_STATE_CODE, IO_CODE or IO_OR_GS_CODE
definition, we just find that they "mark and check that the function
is part of the {category} API".
However, ther is no definition on what {category} API is, they are
in include/block/block-*.h
Therefore, add a comment that refers to such documentation.
Signed-off-by: Emanuele Giuseppe Esposito
Message-Id: <20220609122206.1016936-1-eespo...@redhat.com>
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Kevin Wolf
---
include/qemu/main-loop.h | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index 5518845299..c50d1b7e3a 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -279,7 +279,11 @@ bool qemu_mutex_iothread_locked(void);
*/
bool qemu_in_main_thread(void);
-/* Mark and check that the function is part of the global state API. */
+/*
+ * Mark and check that the function is part of the Global State API.
+ * Please refer to include/block/block-global-state.h for more
+ * information about GS API.
+ */
#ifdef CONFIG_COCOA
/*
* When using the Cocoa UI, addRemovableDevicesMenuItems() is called from
@@ -298,13 +302,21 @@ bool qemu_in_main_thread(void);
} while (0)
#endif /* CONFIG_COCOA */
-/* Mark and check that the function is part of the I/O API. */
+/*
+ * Mark and check that the function is part of the I/O API.
+ * Please refer to include/block/block-io.h for more
+ * information about IO API.
+ */
#define IO_CODE() \
do {\
/* nop */ \
} while (0)
-/* Mark and check that the function is part of the "I/O OR GS" API. */
+/*
+ * Mark and check that the function is part of the "I/O OR GS" API.
+ * Please refer to include/block/block-io.h for more
+ * information about "IO or GS" API.
+ */
#define IO_OR_GS_CODE() \
do {\
/* nop */ \
--
2.35.3
[PULL 6/7] qemu-iotests: Discard stderr when probing devices
From: Cole Robinson
qemu-iotests fails in the following setup:
./configure --enable-modules --enable-smartcard \
--target-list=x86_64-softmmu,s390x-softmmu
make
cd build
QEMU_PROG=`pwd`/s390x-softmmu/qemu-system-s390x \
../tests/check-block.sh qcow2
...
--- /home/crobinso/src/qemu/tests/qemu-iotests/127.out
+++ /home/crobinso/src/qemu/build/tests/qemu-iotests/scratch/127.out.bad
@@ -1,4 +1,18 @@
QA output created by 127
+Failed to open module: /home/crobinso/src/qemu/build/hw-usb-smartcard.so:
undefined symbol: ccid_card_ccid_attach
...
--- /home/crobinso/src/qemu/tests/qemu-iotests/267.out
+++ /home/crobinso/src/qemu/build/tests/qemu-iotests/scratch/267.out.bad
@@ -1,4 +1,11 @@
QA output created by 267
+Failed to open module: /home/crobinso/src/qemu/build/hw-usb-smartcard.so:
undefined symbol: ccid_card_ccid_attach
The stderr spew is its own known issue, but seems like iotests should
be discarding stderr in this case.
Signed-off-by: Cole Robinson
Reviewed-by: Thomas Huth
Signed-off-by: Kevin Wolf
---
tests/qemu-iotests/common.rc | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 165b54a61e..db757025cb 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -982,7 +982,7 @@ _require_large_file()
#
_require_devices()
{
-available=$($QEMU -M none -device help | \
+available=$($QEMU -M none -device help 2> /dev/null | \
grep ^name | sed -e 's/^name "//' -e 's/".*$//')
for device
do
@@ -994,7 +994,7 @@ _require_devices()
_require_one_device_of()
{
-available=$($QEMU -M none -device help | \
+available=$($QEMU -M none -device help 2> /dev/null | \
grep ^name | sed -e 's/^name "//' -e 's/".*$//')
for device
do
--
2.35.3
[PULL 3/7] libvduse: Replace strcpy() with strncpy()
From: Xie Yongji Coverity reported a string overflow issue since we copied "name" to "dev_config->name" without checking the length. This should be a false positive since we already checked the length of "name" in vduse_name_is_invalid(). But anyway, let's replace strcpy() with strncpy() (as a general library, we'd like to minimize dependencies on other libraries, so we didn't use g_strlcpy() here) to fix the coverity complaint. Fixes: Coverity CID 1490224 Signed-off-by: Xie Yongji Reviewed-by: Markus Armbruster Message-Id: <20220706095624.328-3-xieyon...@bytedance.com> Signed-off-by: Kevin Wolf --- subprojects/libvduse/libvduse.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c index 6374933881..1e36227388 100644 --- a/subprojects/libvduse/libvduse.c +++ b/subprojects/libvduse/libvduse.c @@ -1309,7 +1309,8 @@ VduseDev *vduse_dev_create(const char *name, uint32_t device_id, goto err_dev; } -strcpy(dev_config->name, name); +strncpy(dev_config->name, name, VDUSE_NAME_MAX); +dev_config->name[VDUSE_NAME_MAX - 1] = '\0'; dev_config->device_id = device_id; dev_config->vendor_id = vendor_id; dev_config->features = features; -- 2.35.3
[PULL 2/7] libvduse: Fix the incorrect function name
From: Xie Yongji
In vduse_name_is_valid(), we actually check whether
the name is invalid or not. So let's change the
function name to vduse_name_is_invalid() to match
the behavior.
Signed-off-by: Xie Yongji
Reviewed-by: Markus Armbruster
Message-Id: <20220706095624.328-2-xieyon...@bytedance.com>
Signed-off-by: Kevin Wolf
---
subprojects/libvduse/libvduse.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index 9a2bcec282..6374933881 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -1193,7 +1193,7 @@ static int vduse_dev_init(VduseDev *dev, const char *name,
return 0;
}
-static inline bool vduse_name_is_valid(const char *name)
+static inline bool vduse_name_is_invalid(const char *name)
{
return strlen(name) >= VDUSE_NAME_MAX || strstr(name, "..");
}
@@ -1242,7 +1242,7 @@ VduseDev *vduse_dev_create_by_name(const char *name,
uint16_t num_queues,
VduseDev *dev;
int ret;
-if (!name || vduse_name_is_valid(name) || !ops ||
+if (!name || vduse_name_is_invalid(name) || !ops ||
!ops->enable_queue || !ops->disable_queue) {
fprintf(stderr, "Invalid parameter for vduse\n");
return NULL;
@@ -1276,7 +1276,7 @@ VduseDev *vduse_dev_create(const char *name, uint32_t
device_id,
struct vduse_dev_config *dev_config;
size_t size = offsetof(struct vduse_dev_config, config);
-if (!name || vduse_name_is_valid(name) ||
+if (!name || vduse_name_is_invalid(name) ||
!has_feature(features, VIRTIO_F_VERSION_1) || !config ||
!config_size || !ops || !ops->enable_queue || !ops->disable_queue) {
fprintf(stderr, "Invalid parameter for vduse\n");
--
2.35.3
[PULL 5/7] hw/block/hd-geometry: Do not override specified bios-chs-trans
From: Lev Kujawski
For small disk images (<4 GiB), QEMU and SeaBIOS default to the
LARGE/ECHS disk translation method, but it is not uncommon for other
BIOS software to use LBA in these cases as well. Some operating
system boot loaders (e.g., NT 4) do not handle LARGE translations
outside of fixed configurations. See, e.g., Q154052:
"When starting an x86 based computer, Ntdetect.com retrieves and
stores Interrupt 13 information. . . If the disk controller is using a
32 sector/64 head translation scheme, this boundary will be 1 GB. If
the controller uses 63 sector/255 head translation [AUTHOR: i.e.,
LBA], the limit will be 4 GB."
To accommodate these situations, hd_geometry_guess() now follows the
disk translation specified by the user even when the ATA disk geometry
is guessed.
hd_geometry_guess():
* Only set the disk translation when translation is AUTO.
* Show the soon-to-be active translation (*ptrans) in the trace rather
than what was guessed.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/56
Buglink: https://bugs.launchpad.net/qemu/+bug/1745312
Signed-off-by: Lev Kujawski
Message-Id: <20220707204045.999544-1-lku...@member.fsf.org>
Signed-off-by: Kevin Wolf
---
hw/block/hd-geometry.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 112094358e..dae13ab14d 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -150,7 +150,12 @@ void hd_geometry_guess(BlockBackend *blk,
translation = BIOS_ATA_TRANSLATION_NONE;
}
if (ptrans) {
-*ptrans = translation;
+if (*ptrans == BIOS_ATA_TRANSLATION_AUTO) {
+*ptrans = translation;
+} else {
+/* Defer to the translation specified by the user. */
+translation = *ptrans;
+}
}
trace_hd_geometry_guess(blk, *pcyls, *pheads, *psecs, translation);
}
--
2.35.3
[PULL 1/7] block/io_uring: add missing include file
From: Jinhao Fan
The commit "Use io_uring_register_ring_fd() to skip fd operations" uses
warn_report but did not include the header file "qemu/error-report.h".
This causes "error: implicit declaration of function ‘warn_report’".
Include this header file.
Fixes: e2848bc574 ("Use io_uring_register_ring_fd() to skip fd operations")
Signed-off-by: Jinhao Fan
Message-Id: <20220721065645.577404-1-fanjinhao...@ict.ac.cn>
Reviewed-by: Stefano Garzarella
Signed-off-by: Kevin Wolf
---
block/io_uring.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/block/io_uring.c b/block/io_uring.c
index f8a19fd97f..a1760152e0 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -11,6 +11,7 @@
#include "qemu/osdep.h"
#include
#include "block/aio.h"
+#include "qemu/error-report.h"
#include "qemu/queue.h"
#include "block/block.h"
#include "block/raw-aio.h"
--
2.35.3
[PULL 0/7] Block layer patches
The following changes since commit 60205b71421cbc529ca60b12c79e0eeace007319: Merge tag 'pull-aspeed-20220801' of https://github.com/legoater/qemu into staging (2022-08-01 13:55:11 -0700) are available in the Git repository at: git://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to 21b1d974595b3986c68fe80a1f7e9b87886d4bae: main loop: add missing documentation links to GS/IO macros (2022-08-02 12:02:17 +0200) Block layer patches - libvduse: Coverity fixes - hd-geometry: Fix ignored bios-chs-trans setting - io_uring: Fix compiler warning (missing #include) - main loop: add missing documentation links to GS/IO macros - qemu-iotests: Discard stderr when probing devices Cole Robinson (1): qemu-iotests: Discard stderr when probing devices Emanuele Giuseppe Esposito (1): main loop: add missing documentation links to GS/IO macros Jinhao Fan (1): block/io_uring: add missing include file Lev Kujawski (1): hw/block/hd-geometry: Do not override specified bios-chs-trans Xie Yongji (3): libvduse: Fix the incorrect function name libvduse: Replace strcpy() with strncpy() libvduse: Pass positive value to strerror() include/qemu/main-loop.h| 18 +++--- block/io_uring.c| 1 + hw/block/hd-geometry.c | 7 ++- subprojects/libvduse/libvduse.c | 13 +++-- tests/qemu-iotests/common.rc| 4 ++-- 5 files changed, 31 insertions(+), 12 deletions(-)
[PULL 4/7] libvduse: Pass positive value to strerror()
From: Xie Yongji
The value passed to strerror() should be positive.
So let's fix it.
Fixes: Coverity CID 1490226, 1490223
Signed-off-by: Xie Yongji
Reviewed-by: Richard Henderson
Reviewed-by: Markus Armbruster
Message-Id: <20220706095624.328-4-xieyon...@bytedance.com>
Signed-off-by: Kevin Wolf
---
subprojects/libvduse/libvduse.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/subprojects/libvduse/libvduse.c b/subprojects/libvduse/libvduse.c
index 1e36227388..1a5981445c 100644
--- a/subprojects/libvduse/libvduse.c
+++ b/subprojects/libvduse/libvduse.c
@@ -1257,7 +1257,7 @@ VduseDev *vduse_dev_create_by_name(const char *name,
uint16_t num_queues,
ret = vduse_dev_init(dev, name, num_queues, ops, priv);
if (ret < 0) {
fprintf(stderr, "Failed to init vduse device %s: %s\n",
-name, strerror(ret));
+name, strerror(-ret));
free(dev);
return NULL;
}
@@ -1331,7 +1331,7 @@ VduseDev *vduse_dev_create(const char *name, uint32_t
device_id,
ret = vduse_dev_init(dev, name, num_queues, ops, priv);
if (ret < 0) {
fprintf(stderr, "Failed to init vduse device %s: %s\n",
-name, strerror(ret));
+name, strerror(-ret));
goto err;
}
--
2.35.3
Re: [PATCH for-7.1] Revert "migration: Simplify unqueue_page()"
* Thomas Huth (th...@redhat.com) wrote: > On 02/08/2022 10.47, Dr. David Alan Gilbert wrote: > > * Thomas Huth (th...@redhat.com) wrote: > > > This reverts commit cfd66f30fb0f735df06ff4220e5000290a43dad3. > > > > > > The simplification of unqueue_page() introduced a bug that sometimes > > > breaks migration on s390x hosts. Seems like there are still pages here > > > that do not have their dirty bit set. > > > > I don't think it's about 'not having their dirty bit set' - it's > > perfectly fine to have the bits clear (which indicates the page has > > already been sent to the destination, sometime inbetween the page request > > being sent from the destination and it being unqueued). > > Ok, could you maybe simply drop that sentence from the commit description > when picking up the patch? Or shall I resend a v2? Queued > Thomas > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PULL 0/1] riscv-to-apply queue
On 8/1/22 16:02, Alistair Francis wrote: From: Alistair Francis The following changes since commit 0e0c2cf6de0bc6538840837c63b25817cd417347: Merge tag 'pull-target-arm-20220801' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2022-08-01 12:00:08 -0700) are available in the Git repository at: g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20220802 for you to fetch changes up to 1eaa63429a9944265c92efdb94c02fabb231f564: linux-user/riscv: Align signal frame to 16 bytes (2022-08-02 08:56:49 +1000) Seventh RISC-V PR for QEMU 7.1 This is a second PR to go in for RC1. It fixes a bug we have had for awhile, but it's a simple fix so let's pull it in for RC1. * linux-user/riscv: Align signal frame to 16 bytes Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate. r~ Richard Henderson (1): linux-user/riscv: Align signal frame to 16 bytes linux-user/riscv/signal.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
[PATCH] virtio: remove unnecessary host_features in ->get_features()
Since at least commit 6b8f1020540c27246277377aa2c3331ad2bfb160 ("virtio:
move host_features") the ->get_features() function has been called with
host_features as an argument.
Some devices manually add host_features in ->get_features() although the
features argument already contains host_features. Make all devices
consistent by dropping the unnecessary code.
Cc: Cornelia Huck
Signed-off-by: Stefan Hajnoczi
---
hw/block/virtio-blk.c | 3 ---
hw/char/virtio-serial-bus.c | 1 -
hw/net/virtio-net.c | 3 ---
hw/scsi/vhost-scsi-common.c | 3 ---
hw/scsi/virtio-scsi.c | 4
hw/virtio/virtio-balloon.c | 2 --
6 files changed, 16 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e9ba752f6b..429aedcf2b 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -996,9 +996,6 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev,
uint64_t features,
{
VirtIOBlock *s = VIRTIO_BLK(vdev);
-/* Firstly sync all virtio-blk possible supported features */
-features |= s->host_features;
-
virtio_add_feature(, VIRTIO_BLK_F_SEG_MAX);
virtio_add_feature(, VIRTIO_BLK_F_GEOMETRY);
virtio_add_feature(, VIRTIO_BLK_F_TOPOLOGY);
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 7d4601cb5d..1414fb85ae 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -557,7 +557,6 @@ static uint64_t get_features(VirtIODevice *vdev, uint64_t
features,
vser = VIRTIO_SERIAL(vdev);
-features |= vser->host_features;
if (vser->bus.max_nr_ports > 1) {
virtio_add_feature(, VIRTIO_CONSOLE_F_MULTIPORT);
}
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index dd0d056fde..8ecdc1cd83 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -715,9 +715,6 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev,
uint64_t features,
VirtIONet *n = VIRTIO_NET(vdev);
NetClientState *nc = qemu_get_queue(n->nic);
-/* Firstly sync all virtio-net possible supported features */
-features |= n->host_features;
-
virtio_add_feature(, VIRTIO_NET_F_MAC);
if (!peer_has_vnet_hdr(n)) {
diff --git a/hw/scsi/vhost-scsi-common.c b/hw/scsi/vhost-scsi-common.c
index 767f827e55..8b26f90aa1 100644
--- a/hw/scsi/vhost-scsi-common.c
+++ b/hw/scsi/vhost-scsi-common.c
@@ -124,9 +124,6 @@ uint64_t vhost_scsi_common_get_features(VirtIODevice *vdev,
uint64_t features,
{
VHostSCSICommon *vsc = VHOST_SCSI_COMMON(vdev);
-/* Turn on predefined features supported by this device */
-features |= vsc->host_features;
-
return vhost_get_features(>dev, vsc->feature_bits, features);
}
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 414151..f754611dfe 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -816,10 +816,6 @@ static uint64_t virtio_scsi_get_features(VirtIODevice
*vdev,
uint64_t requested_features,
Error **errp)
{
-VirtIOSCSI *s = VIRTIO_SCSI(vdev);
-
-/* Firstly sync all virtio-scsi possible supported features */
-requested_features |= s->host_features;
return requested_features;
}
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 73ac5eb675..0e9ca71b15 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -796,8 +796,6 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f,
Error **errp)
{
-VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
-f |= dev->host_features;
virtio_add_feature(, VIRTIO_BALLOON_F_STATS_VQ);
return f;
--
2.37.1
Re: [PATCH 8/9] scripts/qapi-gen: add -i option
Marc-André Lureau writes:
> Hi
>
>
> On Tue, Jun 21, 2022 at 6:14 PM Markus Armbruster wrote:
>>
>> marcandre.lur...@redhat.com writes:
>>
>> > From: Marc-André Lureau
>> >
>> > Replace hard-coded "qemu/osdep.h" include with a qapi-gen option to
>> > specify the headers to include. This will allow to substitute QEMU
>> > osdep.h with glib.h for example, for projects with different
>> > global headers.
>> >
>> > For historical reasons, we can keep the default as "qemu/osdep.h".
>> >
>> > Signed-off-by: Marc-André Lureau
>>
>> I wish we'd all agree on "config.h" (conventional with autoconf). But
>> we don't.
>>
>> Precedence for tweaking generated code with command line options: -p.
>>
>> I suspect that we'll always specify the same -p and -i for a given
>> schema. To me, that suggests they should both go into the schema
>> instead. Observation, not demand.
>>
>> > ---
>> > scripts/qapi/commands.py | 15 ++-
>> > scripts/qapi/events.py | 17 +++--
>> > scripts/qapi/gen.py| 17 +
>> > scripts/qapi/introspect.py | 11 +++
>> > scripts/qapi/main.py | 17 +++--
>> > scripts/qapi/types.py | 17 +++--
>> > scripts/qapi/visit.py | 17 +++--
>> > 7 files changed, 78 insertions(+), 33 deletions(-)
>> >
>> > diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
>> > index 38ca38a7b9dd..781491b6390d 100644
>> > --- a/scripts/qapi/commands.py
>> > +++ b/scripts/qapi/commands.py
>> > @@ -294,9 +294,9 @@ def gen_register_command(name: str,
>> >
>> >
>> > class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
>> > -def __init__(self, prefix: str, gen_tracing: bool):
>> > +def __init__(self, prefix: str, include: List[str], gen_tracing:
>> > bool):
>>
>> Ignorant question: why ist this List[str], not str? Do multiple options
>> accumulate into a list?
>>
>> Alright, I'm back from further down: looks like they do.
>>
>> > super().__init__(
>> > -prefix, 'qapi-commands',
>> > +prefix, include, 'qapi-commands',
>> > ' * Schema-defined QAPI/QMP commands', None, __doc__,
>> > gen_tracing=gen_tracing)
>> > self._visited_ret_types: Dict[QAPIGenC, Set[QAPISchemaType]] = {}
>> > @@ -308,7 +308,8 @@ def _begin_user_module(self, name: str) -> None:
>> > types = self._module_basename('qapi-types', name)
>> > visit = self._module_basename('qapi-visit', name)
>> > self._genc.add(mcgen('''
>> > -#include "qemu/osdep.h"
>> > +%(include)s
>> > +
>> > #include "qapi/compat-policy.h"
>> > #include "qapi/visitor.h"
>> > #include "qapi/qmp/qdict.h"
>> > @@ -318,6 +319,7 @@ def _begin_user_module(self, name: str) -> None:
>> > #include "%(commands)s.h"
>> >
>> > ''',
>> > + include=self.genc_include(),
>> > commands=commands, visit=visit))
>> >
>> > if self._gen_tracing and commands != 'qapi-commands':
>> > @@ -344,7 +346,8 @@ def visit_begin(self, schema: QAPISchema) -> None:
>> > ''',
>> > c_prefix=c_name(self._prefix,
>> > protect=False)))
>> > self._genc.add(mcgen('''
>> > -#include "qemu/osdep.h"
>> > +%(include)s
>> > +
>> > #include "%(prefix)sqapi-commands.h"
>> > #include "%(prefix)sqapi-init-commands.h"
>> >
>> > @@ -353,6 +356,7 @@ def visit_begin(self, schema: QAPISchema) -> None:
>> > QTAILQ_INIT(cmds);
>> >
>> > ''',
>> > + include=self.genc_include(),
>> > prefix=self._prefix,
>> > c_prefix=c_name(self._prefix,
>> > protect=False)))
>> >
>> > @@ -404,7 +408,8 @@ def visit_command(self,
>> > def gen_commands(schema: QAPISchema,
>> > output_dir: str,
>> > prefix: str,
>> > + include: List[str],
>> > gen_tracing: bool) -> None:
>> > -vis = QAPISchemaGenCommandVisitor(prefix, gen_tracing)
>> > +vis = QAPISchemaGenCommandVisitor(prefix, include, gen_tracing)
>> > schema.visit(vis)
>> > vis.write(output_dir)
>> > diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
>> > index 27b44c49f5e9..6e677d11d2e0 100644
>> > --- a/scripts/qapi/events.py
>> > +++ b/scripts/qapi/events.py
>> > @@ -175,9 +175,9 @@ def gen_event_send(name: str,
>> >
>> > class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
>> >
>> > -def __init__(self, prefix: str):
>> > +def __init__(self, prefix: str, include: List[str]):
>> > super().__init__(
>> > -prefix, 'qapi-events',
>> > +prefix, include, 'qapi-events',
>> > ' * Schema-defined QAPI/QMP events', None, __doc__)
>> > self._event_enum_name = c_name(prefix + 'QAPIEvent',
>> > protect=False)
>> > self._event_enum_members: List[QAPISchemaEnumMember] = []
>> > @@ -188,7 +188,8 @@ def
Re: [PATCH v3] target/s390x: support PRNO_TRNG instruction
Am 20.07.22 um 14:08 schrieb Jason A. Donenfeld: In order for hosts running inside of TCG to initialize the kernel's random number generator, we should support the PRNO_TRNG instruction, backed in the usual way with the qemu_guest_getrandom helper. This is confirmed working on Linux 5.19-rc6. Cc: Thomas Huth Cc: David Hildenbrand Cc: Richard Henderson Cc: Cornelia Huck Cc: Harald Freudenberger Cc: Holger Dengler Signed-off-by: Jason A. Donenfeld [...] +case 114: +if (r1 & 1 || !r1 || r2 & 1 || !r2) +tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra); +fill_buf_random(env, ra, >regs[r1], >regs[r1 + 1]); +fill_buf_random(env, ra, >regs[r2], >regs[r2 + 1]); +break; I think I agree with Harald that some aspects are missing. Linux does not seem to check, but we should also modify the query function to indicate the availability of 114. As the msa helper deals with many instructions ... target/s390x/tcg/insn-data.def:D(0xb91e, KMAC,RRE, MSA, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMAC) target/s390x/tcg/insn-data.def:D(0xb928, PCKMO, RRE, MSA3, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_PCKMO) target/s390x/tcg/insn-data.def:D(0xb92a, KMF, RRE, MSA4, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMF) target/s390x/tcg/insn-data.def:D(0xb92b, KMO, RRE, MSA4, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMO) target/s390x/tcg/insn-data.def:D(0xb92c, PCC, RRE, MSA4, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_PCC) target/s390x/tcg/insn-data.def:D(0xb92d, KMCTR, RRF_b, MSA4, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMCTR) target/s390x/tcg/insn-data.def:D(0xb92e, KM, RRE, MSA, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KM) target/s390x/tcg/insn-data.def:D(0xb92f, KMC, RRE, MSA, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMC) target/s390x/tcg/insn-data.def:D(0xb929, KMA, RRF_b, MSA8, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KMA) target/s390x/tcg/insn-data.def:D(0xb93c, PPNO,RRE, MSA5, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_PPNO) target/s390x/tcg/insn-data.def:D(0xb93e, KIMD,RRE, MSA, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KIMD) target/s390x/tcg/insn-data.def:D(0xb93f, KLMD,RRE, MSA, 0, 0, 0, 0, msa, 0, S390_FEAT_TYPE_KLMD) ... and in theory other instructions might also have 114 we should at least check that this is ppno/prno. Or we split out a prno helper from the msa helper.
Re: [PATCH v1 1/1] migration: add remaining params->has_* = true in migration_instance_init()
* Leonardo Bras Soares Passos (leob...@redhat.com) wrote:
> Please include:
>
> Fixes: 69ef1f36b0 ("migration: define 'tls-creds' and 'tls-hostname'
> migration parameters")
> Fixes: 1d58872a91 ("migration: do not wait for free thread")
> Fixes: d2f1d29b95 ("migration: add support for a "tls-authz" migration
> parameter")
Queued
> On Mon, Jul 25, 2022 at 10:02 PM Leonardo Bras wrote:
> >
> > Some of params->has_* = true are missing in migration_instance_init, this
> > causes migrate_params_check() to skip some tests, allowing some
> > unsupported scenarios.
> >
> > Fix this by adding all missing params->has_* = true in
> > migration_instance_init().
> >
> > Signed-off-by: Leonardo Bras
> > ---
> > migration/migration.c | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index e03f698a3c..82fbe0cf55 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -4451,6 +4451,7 @@ static void migration_instance_init(Object *obj)
> > /* Set has_* up only for parameter checks */
> > params->has_compress_level = true;
> > params->has_compress_threads = true;
> > +params->has_compress_wait_thread = true;
> > params->has_decompress_threads = true;
> > params->has_throttle_trigger_threshold = true;
> > params->has_cpu_throttle_initial = true;
> > @@ -4471,6 +4472,9 @@ static void migration_instance_init(Object *obj)
> > params->has_announce_max = true;
> > params->has_announce_rounds = true;
> > params->has_announce_step = true;
> > +params->has_tls_creds = true;
> > +params->has_tls_hostname = true;
> > +params->has_tls_authz = true;
> >
> > qemu_sem_init(>postcopy_pause_sem, 0);
> > qemu_sem_init(>postcopy_pause_rp_sem, 0);
> > --
> > 2.37.1
> >
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
