qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v8 00/20] target/riscv, KVM: fixes and enhancements


From: Daniel Henrique Barboza
Subject: [PATCH v8 00/20] target/riscv, KVM: fixes and enhancements
Date: Wed, 5 Jul 2023 18:39:35 -0300

Hi,

This version has a last minute change in patch 14. It's a bug fix and a
design change.

The bug fix: ioctl() will always error out with -1 and return the error
code in 'errno'. I was checking the ioctl() return value for EINVAL,
which doesn't work.

The design change has to do with discussions between Andrew and Anup and
myself in our internal Slack. RISC-V KVM is overusing the EINVAL error
code in set_one_reg() and get_one_reg() APIs, making it very hard for
userspace (such as QEMU, crosvm, etc) to tell what went wrong. In our
case, patch 14 is making an EINVAL assumption that we're not confident
about because this error code can mean almost anything.

We'll push for a KVM change in the next few days. As far as QEMU goes
we're going to do what we consider the right thing: check for ENOENT
instead of EINVAL in patch 14. The reason why we're doing this change
right now in QEMU, instead of waiting for KVM to change first, can be
better explained by Drew's comment in version 7 [1]:

" But, also as discussed internally, based on our upcoming plans to use
ENOENT for missing registers, we should change this check to be for
ENOENT now. While that may seem premature, I think it's OK, because
until a KVM which returns ENOENT for missing registers exists and is
used, QEMU command lines which disable unknown registers will be
rejected. But, that will also happen even after a KVM that returns
ENOENT exits if an older KVM is used. In both cases that's fine, as
rejecting is the more conservative behavior for an error. Finally, if
the yet-to-be-posted KVM ENOENT patch never gets merged, then we may be
stuck rejecting forever anyway, since EINVAL is quite generic and
probably isn't safe to use for this purpose."

Checking for ENOENT is the right approach and we'll change QEMU to
implement it right off the gate for 8.1. In case KVM refuses to change
we'll error out in all cases in patch 14, which is still a better
solution than making guesses about EINVAL means.


Series based on top of Alistair's riscv-to-apply.next.

Patches missing review: 14

Changes from v7:
- Patch 14:
  - use 'errno' to check the error code from ioctl()
  - test for ENOENT instead of EINVAL
- v7 link: 
https://lore.kernel.org/qemu-devel/20230630100811.287315-1-dbarboza@ventanamicro.com/

[1] https://lore.kernel.org/qemu-devel/20230705-091906904fcc54a4ce96e625@orel/

Daniel Henrique Barboza (20):
  target/riscv: skip features setup for KVM CPUs
  hw/riscv/virt.c: skip 'mmu-type' FDT if satp mode not set
  target/riscv/cpu.c: restrict 'mvendorid' value
  target/riscv/cpu.c: restrict 'mimpid' value
  target/riscv/cpu.c: restrict 'marchid' value
  target/riscv: use KVM scratch CPUs to init KVM properties
  target/riscv: read marchid/mimpid in kvm_riscv_init_machine_ids()
  target/riscv: handle mvendorid/marchid/mimpid for KVM CPUs
  linux-headers: Update to v6.4-rc1
  target/riscv/kvm.c: init 'misa_ext_mask' with scratch CPU
  target/riscv/cpu: add misa_ext_info_arr[]
  target/riscv: add KVM specific MISA properties
  target/riscv/kvm.c: update KVM MISA bits
  target/riscv/kvm.c: add multi-letter extension KVM properties
  target/riscv/cpu.c: add satp_mode properties earlier
  target/riscv/cpu.c: remove priv_ver check from riscv_isa_string_ext()
  target/riscv/cpu.c: create KVM mock properties
  target/riscv: update multi-letter extension KVM properties
  target/riscv/kvm.c: add kvmconfig_get_cfg_addr() helper
  target/riscv/kvm.c: read/write (cbom|cboz)_blocksize in KVM

 hw/riscv/virt.c                               |  14 +-
 include/standard-headers/linux/const.h        |   2 +-
 include/standard-headers/linux/virtio_blk.h   |  18 +-
 .../standard-headers/linux/virtio_config.h    |   6 +
 include/standard-headers/linux/virtio_net.h   |   1 +
 linux-headers/asm-arm64/kvm.h                 |  33 ++
 linux-headers/asm-riscv/kvm.h                 |  53 +-
 linux-headers/asm-riscv/unistd.h              |   9 +
 linux-headers/asm-s390/unistd_32.h            |   1 +
 linux-headers/asm-s390/unistd_64.h            |   1 +
 linux-headers/asm-x86/kvm.h                   |   3 +
 linux-headers/linux/const.h                   |   2 +-
 linux-headers/linux/kvm.h                     |  12 +-
 linux-headers/linux/psp-sev.h                 |   7 +
 linux-headers/linux/userfaultfd.h             |  17 +-
 target/riscv/cpu.c                            | 341 ++++++++++--
 target/riscv/cpu.h                            |   7 +-
 target/riscv/kvm.c                            | 499 +++++++++++++++++-
 target/riscv/kvm_riscv.h                      |   1 +
 19 files changed, 940 insertions(+), 87 deletions(-)

-- 
2.41.0




reply via email to

[Prev in Thread] Current Thread [Next in Thread]