[PATCH v2 0/3] hw/ufs: Add Universal Flash Storage (UFS) support
Since v1: - use macros of "hw/registerfields.h" (Addressed Philippe's review comments) This patch series adds support for a new PCI-based UFS device. The UFS pci device id (PCI_DEVICE_ID_REDHAT_UFS) is not registered in the Linux kernel yet, so it does not work right away, but I confirmed that it works with Linux when the UFS pci device id is registered. I have also verified that it works with Windows 10. Jeuk Kim (3): hw/ufs: Initial commit for emulated Universal-Flash-Storage hw/ufs: Support for Query Transfer Requests hw/ufs: Support for UFS logical unit MAINTAINERS |6 + hw/Kconfig |1 + hw/meson.build |1 + hw/ufs/Kconfig |4 + hw/ufs/lu.c | 1441 hw/ufs/meson.build |1 + hw/ufs/trace-events | 58 ++ hw/ufs/trace.h |1 + hw/ufs/ufs.c | 1511 ++ hw/ufs/ufs.h | 130 include/block/ufs.h | 1048 ++ include/hw/pci/pci.h |1 + include/hw/pci/pci_ids.h |1 + include/scsi/constants.h |1 + meson.build |1 + 15 files changed, 4206 insertions(+) create mode 100644 hw/ufs/Kconfig create mode 100644 hw/ufs/lu.c create mode 100644 hw/ufs/meson.build create mode 100644 hw/ufs/trace-events create mode 100644 hw/ufs/trace.h create mode 100644 hw/ufs/ufs.c create mode 100644 hw/ufs/ufs.h create mode 100644 include/block/ufs.h -- 2.34.1
Re: [PATCH] chardev/char-win-stdio: Support VT sequences on Windows 11 host
From: Huasen Zhang Hello, On Thu, 15 Jun 2023 12:57:55 +0200 Marc-André Lureau wrote: > Hi > > On Thu, Jun 15, 2023 at 12:36 PM Zhang Huasen > wrote: > > > If the monitor or the serial port use STDIO as backend on Windows 11 host, > > e.g. -nographic options is used, the monitor or the guest Linux do not > > response to arrow keys. > > > > When Windows creates a console, ENABLE_VIRTUAL_PROCESS_INPUT is disabled > > by default. Arrow keys cannot be retrieved by ReadFile or ReadConsoleInput > > functions. > > > > Add ENABLE_VIRTUAL_PROCESS_INPUT to the flag which is passed to > > SetConsoleMode, > > when opening stdio console. > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1674 > > > > Signed-off-by: Zhang Huasen > > --- > > chardev/char-win-stdio.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/chardev/char-win-stdio.c b/chardev/char-win-stdio.c > > index eb830eabd9..1a18999e78 100644 > > --- a/chardev/char-win-stdio.c > > +++ b/chardev/char-win-stdio.c > > @@ -190,7 +190,7 @@ static void qemu_chr_open_stdio(Chardev *chr, > > } > > } > > > > -dwMode |= ENABLE_LINE_INPUT; > > +dwMode |= ENABLE_LINE_INPUT | ENABLE_VIRTUAL_TERMINAL_INPUT; > > > > I think we should set it only when is_console (although that may not make a > difference otherwise) It is okay to set ENABLE_VIRTUAL_TERMINAL_INPUT only when is_console is TRUE. I do not understand some points of original code. If the stdin is not a console, i.e. GetConsoleMode fails, we still call SetConsoleMode and set ENABLE_LINE_INPUT. Could you please tell what the purpose is? > thanks > > > > if (is_console) { > > /* set the terminal in raw mode */ > > -- > > 2.41.0.windows.1 > > > >
Re: [PATCH 4/4] ppc/spapr: Move spapr nested HV to a new file
On 6/15/23 17:21, Nicholas Piggin wrote: On Thu Jun 15, 2023 at 4:30 PM AEST, Harsh Prateek Bora wrote: On 6/8/23 14:43, Nicholas Piggin wrote: Create spapr_nested.c for most of the nested HV implementation. Signed-off-by: Nicholas Piggin --- hw/ppc/meson.build | 1 + hw/ppc/spapr_hcall.c | 415 +- hw/ppc/spapr_nested.c | 496 + include/hw/ppc/spapr.h | 61 + 4 files changed, 499 insertions(+), 474 deletions(-) create mode 100644 hw/ppc/spapr_nested.c [snip] diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c new file mode 100644 index 00..c06dd8903c --- /dev/null +++ b/hw/ppc/spapr_nested.c @@ -0,0 +1,496 @@ +#include "qemu/osdep.h" +#include "qemu/cutils.h" +#include "exec/exec-all.h" +#include "helper_regs.h" +#include "hw/ppc/ppc.h" +#include "hw/ppc/spapr.h" +#include "hw/ppc/spapr_cpu_core.h" + +/* + * Register state for entering a nested guest with H_ENTER_NESTED. + * New member must be added at the end. + */ +struct kvmppc_hv_guest_state { +uint64_t version; /* version of this structure layout, must be first */ +uint32_t lpid; +uint32_t vcpu_token; +/* These registers are hypervisor privileged (at least for writing) */ +uint64_t lpcr; +uint64_t pcr; +uint64_t amor; +uint64_t dpdes; +uint64_t hfscr; +int64_t tb_offset; +uint64_t dawr0; +uint64_t dawrx0; +uint64_t ciabr; +uint64_t hdec_expiry; +uint64_t purr; +uint64_t spurr; +uint64_t ic; +uint64_t vtb; +uint64_t hdar; +uint64_t hdsisr; +uint64_t heir; +uint64_t asdr; +/* These are OS privileged but need to be set late in guest entry */ +uint64_t srr0; +uint64_t srr1; +uint64_t sprg[4]; +uint64_t pidr; +uint64_t cfar; +uint64_t ppr; +/* Version 1 ends here */ +uint64_t dawr1; +uint64_t dawrx1; +/* Version 2 ends here */ +}; + +/* Latest version of hv_guest_state structure */ +#define HV_GUEST_STATE_VERSION 2 + +/* Linux 64-bit powerpc pt_regs struct, used by nested HV */ +struct kvmppc_pt_regs { +uint64_t gpr[32]; +uint64_t nip; +uint64_t msr; +uint64_t orig_gpr3;/* Used for restarting system calls */ +uint64_t ctr; +uint64_t link; +uint64_t xer; +uint64_t ccr; +uint64_t softe;/* Soft enabled/disabled */ +uint64_t trap; /* Reason for being here */ +uint64_t dar; /* Fault registers */ +uint64_t dsisr;/* on 4xx/Book-E used for ESR */ +uint64_t result; /* Result of a system call */ +}; Now that we have a separated spapr_nested.c for nested related code, Can above definitions and other nested related defines (like struct nested_ppc_state below) be moved to a new spapr_nested.h as well? Otherwise, looks good to me. They're private to this file, so do we need to? I'm on the fence about it, maybe because they're hcall ABI. I don't object to adding a new spapr_nested.h for nested related things in general though. Yeh, I would recommend moving nested specific declarations to spapr_nested.h as would be more organized. Thanks Harsh Thanks, Nick
Re: [PATCH 0/7] Add new CPU model EmeraldRapids and GraniteRapids
On Fri, Jun 16, 2023 at 12:01:52PM +0800, Wang, Lei wrote: > On 6/16/2023 11:23, Tao Su wrote: > > This patch series mainly updates SapphireRapids CPU model and adds > > new CPU model EmeraldRapids and GraniteRapids. > > > > Bit 13 (ARCH_CAP_FBSDP_NO), bit 14 (ARCH_CAP_FBSDP_NO) and bit 15 > > Bit 13 should be MSR_ARCH_CAP_SBDR_SSDP_NO, right? Yes, copied by mistake, thanks! Tao > > > (ARCH_CAP_PSDP_NO) of MSR_IA32_ARCH_CAPABILITIES are enumerated starting > > from latest SapphireRapids, which are missed in current SapphireRapids > > CPU model, so add a new version for SapphireRapids CPU model to expose > > these bits. > > > > Add EmeraldRapids CPU model to this series, since EmeraldRapids also > > enumerates these bits. The original patch of EmeraldRapids CPU model is > > in [1]. > > > > GraniteRapids is Intel's successor to EmeraldRapids, an Intel 3 process > > microarchitecture for enthusiasts and servers, which adds new features > > based on SapphireRapids and EmeraldRapids. > > > > [1] > > https://lore.kernel.org/qemu-devel/20230515025308.1050277-1-qian@intel.com/ > > > > Lei Wang (1): > > target/i386: Add few security fix bits in ARCH_CAPABILITIES into > > SapphireRapids CPU model > > > > Qian Wen (1): > > target/i386: Add new CPU model EmeraldRapids > > > > Tao Su (5): > > target/i386: Add FEAT_7_1_EDX to adjust feature level > > target/i386: Add support for MCDT_NO in CPUID enumeration > > target/i386: Allow MCDT_NO if host supports > > target/i386: Add new bit definitions of MSR_IA32_ARCH_CAPABILITIES > > target/i386: Add new CPU model GraniteRapids > > > > target/i386/cpu.c | 303 +- > > target/i386/cpu.h | 8 ++ > > target/i386/kvm/kvm.c | 5 + > > 3 files changed, 314 insertions(+), 2 deletions(-) > > > > > > base-commit: 7efd65423ab22e6f5890ca08ae40c84d6660242f
Re: [PATCH 0/7] Add new CPU model EmeraldRapids and GraniteRapids
On 6/16/2023 11:23, Tao Su wrote: > This patch series mainly updates SapphireRapids CPU model and adds > new CPU model EmeraldRapids and GraniteRapids. > > Bit 13 (ARCH_CAP_FBSDP_NO), bit 14 (ARCH_CAP_FBSDP_NO) and bit 15 Bit 13 should be MSR_ARCH_CAP_SBDR_SSDP_NO, right? > (ARCH_CAP_PSDP_NO) of MSR_IA32_ARCH_CAPABILITIES are enumerated starting > from latest SapphireRapids, which are missed in current SapphireRapids > CPU model, so add a new version for SapphireRapids CPU model to expose > these bits. > > Add EmeraldRapids CPU model to this series, since EmeraldRapids also > enumerates these bits. The original patch of EmeraldRapids CPU model is > in [1]. > > GraniteRapids is Intel's successor to EmeraldRapids, an Intel 3 process > microarchitecture for enthusiasts and servers, which adds new features > based on SapphireRapids and EmeraldRapids. > > [1] > https://lore.kernel.org/qemu-devel/20230515025308.1050277-1-qian@intel.com/ > > Lei Wang (1): > target/i386: Add few security fix bits in ARCH_CAPABILITIES into > SapphireRapids CPU model > > Qian Wen (1): > target/i386: Add new CPU model EmeraldRapids > > Tao Su (5): > target/i386: Add FEAT_7_1_EDX to adjust feature level > target/i386: Add support for MCDT_NO in CPUID enumeration > target/i386: Allow MCDT_NO if host supports > target/i386: Add new bit definitions of MSR_IA32_ARCH_CAPABILITIES > target/i386: Add new CPU model GraniteRapids > > target/i386/cpu.c | 303 +- > target/i386/cpu.h | 8 ++ > target/i386/kvm/kvm.c | 5 + > 3 files changed, 314 insertions(+), 2 deletions(-) > > > base-commit: 7efd65423ab22e6f5890ca08ae40c84d6660242f
[PATCH 0/7] Add new CPU model EmeraldRapids and GraniteRapids
This patch series mainly updates SapphireRapids CPU model and adds new CPU model EmeraldRapids and GraniteRapids. Bit 13 (ARCH_CAP_FBSDP_NO), bit 14 (ARCH_CAP_FBSDP_NO) and bit 15 (ARCH_CAP_PSDP_NO) of MSR_IA32_ARCH_CAPABILITIES are enumerated starting from latest SapphireRapids, which are missed in current SapphireRapids CPU model, so add a new version for SapphireRapids CPU model to expose these bits. Add EmeraldRapids CPU model to this series, since EmeraldRapids also enumerates these bits. The original patch of EmeraldRapids CPU model is in [1]. GraniteRapids is Intel's successor to EmeraldRapids, an Intel 3 process microarchitecture for enthusiasts and servers, which adds new features based on SapphireRapids and EmeraldRapids. [1] https://lore.kernel.org/qemu-devel/20230515025308.1050277-1-qian@intel.com/ Lei Wang (1): target/i386: Add few security fix bits in ARCH_CAPABILITIES into SapphireRapids CPU model Qian Wen (1): target/i386: Add new CPU model EmeraldRapids Tao Su (5): target/i386: Add FEAT_7_1_EDX to adjust feature level target/i386: Add support for MCDT_NO in CPUID enumeration target/i386: Allow MCDT_NO if host supports target/i386: Add new bit definitions of MSR_IA32_ARCH_CAPABILITIES target/i386: Add new CPU model GraniteRapids target/i386/cpu.c | 303 +- target/i386/cpu.h | 8 ++ target/i386/kvm/kvm.c | 5 + 3 files changed, 314 insertions(+), 2 deletions(-) base-commit: 7efd65423ab22e6f5890ca08ae40c84d6660242f -- 2.34.1
[PATCH 3/7] target/i386: Allow MCDT_NO if host supports
MCDT_NO bit indicates HW contains the security fix and doesn't need to be mitigated to avoid data-dependent behaviour for certain instructions. It needs no hypervisor support. Treat it as supported regardless of what KVM reports. Signed-off-by: Tao Su Reviewed-by: Xiaoyao Li --- target/i386/kvm/kvm.c | 5 + 1 file changed, 5 insertions(+) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index de531842f6..4defd8b479 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -432,6 +432,11 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, uint32_t eax; host_cpuid(7, 1, &eax, &unused, &unused, &unused); ret |= eax & (CPUID_7_1_EAX_FZRM | CPUID_7_1_EAX_FSRS | CPUID_7_1_EAX_FSRC); +} else if (function == 7 && index == 2 && reg == R_EDX) { +/* Not new instructions, just an optimization. */ +uint32_t edx; +host_cpuid(7, 2, &unused, &unused, &unused, &edx); +ret |= edx & CPUID_7_2_EDX_MCDT_NO; } else if (function == 0xd && index == 0 && (reg == R_EAX || reg == R_EDX)) { /* -- 2.34.1
[PATCH 4/7] target/i386: Add new bit definitions of MSR_IA32_ARCH_CAPABILITIES
Currently, bit 13, 14, 15 and 24 of MSR_IA32_ARCH_CAPABILITIES are disclosed for fixing security issues, so add those bit definitions and feature names. Signed-off-by: Tao Su --- target/i386/cpu.c | 4 ++-- target/i386/cpu.h | 4 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 7898a4c79a..b5321240c6 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -1069,10 +1069,10 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { "rdctl-no", "ibrs-all", "rsba", "skip-l1dfl-vmentry", "ssb-no", "mds-no", "pschange-mc-no", "tsx-ctrl", "taa-no", NULL, NULL, NULL, -NULL, NULL, NULL, NULL, +NULL, "sbdr-ssdp-no", "fbsdp-no", "psdp-no", NULL, "fb-clear", NULL, NULL, NULL, NULL, NULL, NULL, -NULL, NULL, NULL, NULL, +"pbrsb-no", NULL, NULL, NULL, NULL, NULL, NULL, NULL, }, .msr = { diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 64d50acf41..6221b1c0a4 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1022,7 +1022,11 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w, #define MSR_ARCH_CAP_PSCHANGE_MC_NO (1U << 6) #define MSR_ARCH_CAP_TSX_CTRL_MSR (1U << 7) #define MSR_ARCH_CAP_TAA_NO (1U << 8) +#define MSR_ARCH_CAP_SBDR_SSDP_NO (1u << 13) +#define MSR_ARCH_CAP_FBSDP_NO (1u << 14) +#define MSR_ARCH_CAP_PSDP_NO(1u << 15) #define MSR_ARCH_CAP_FB_CLEAR (1U << 17) +#define MSR_ARCH_CAP_PBRSB_NO (1U << 24) #define MSR_CORE_CAP_SPLIT_LOCK_DETECT (1U << 5) -- 2.34.1
[PATCH 7/7] target/i386: Add new CPU model GraniteRapids
The GraniteRapids CPU model mainly adds the following new features based on SapphireRapids: - PREFETCHITI CPUID.(EAX=7,ECX=1):EDX[bit 14] - AMX-FP16 CPUID.(EAX=7,ECX=1):EAX[bit 21] - MCDT_NO CPUID.(EAX=7,ECX=2):EDX[bit 5] - SBDR_SSDP_NO MSR_IA32_ARCH_CAPABILITIES[bit 13] - FBSDP_NO MSR_IA32_ARCH_CAPABILITIES[bit 14] - PSDP_NO MSR_IA32_ARCH_CAPABILITIES[bit 15] - PBRSB_NO MSR_IA32_ARCH_CAPABILITIES[bit 24] Signed-off-by: Tao Su Tested-by: Xuelian Guo Reviewed-by: Xiaoyao Li --- target/i386/cpu.c | 136 ++ 1 file changed, 136 insertions(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 7faf6dfaee..860106fc24 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -3993,6 +3993,142 @@ static const X86CPUDefinition builtin_x86_defs[] = { { /* end of list */ }, }, }, +{ +.name = "GraniteRapids", +.level = 0x20, +.vendor = CPUID_VENDOR_INTEL, +.family = 6, +.model = 173, +.stepping = 0, +/* + * please keep the ascending order so that we can have a clear view of + * bit position of each feature. + */ +.features[FEAT_1_EDX] = +CPUID_FP87 | CPUID_VME | CPUID_DE | CPUID_PSE | CPUID_TSC | +CPUID_MSR | CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | +CPUID_SEP | CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | +CPUID_PAT | CPUID_PSE36 | CPUID_CLFLUSH | CPUID_MMX | CPUID_FXSR | +CPUID_SSE | CPUID_SSE2, +.features[FEAT_1_ECX] = +CPUID_EXT_SSE3 | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSSE3 | +CPUID_EXT_FMA | CPUID_EXT_CX16 | CPUID_EXT_PCID | CPUID_EXT_SSE41 | +CPUID_EXT_SSE42 | CPUID_EXT_X2APIC | CPUID_EXT_MOVBE | +CPUID_EXT_POPCNT | CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_AES | +CPUID_EXT_XSAVE | CPUID_EXT_AVX | CPUID_EXT_F16C | CPUID_EXT_RDRAND, +.features[FEAT_8000_0001_EDX] = +CPUID_EXT2_SYSCALL | CPUID_EXT2_NX | CPUID_EXT2_PDPE1GB | +CPUID_EXT2_RDTSCP | CPUID_EXT2_LM, +.features[FEAT_8000_0001_ECX] = +CPUID_EXT3_LAHF_LM | CPUID_EXT3_ABM | CPUID_EXT3_3DNOWPREFETCH, +.features[FEAT_8000_0008_EBX] = +CPUID_8000_0008_EBX_WBNOINVD, +.features[FEAT_7_0_EBX] = +CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_HLE | +CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | +CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID | CPUID_7_0_EBX_RTM | +CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ | +CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX | CPUID_7_0_EBX_SMAP | +CPUID_7_0_EBX_AVX512IFMA | CPUID_7_0_EBX_CLFLUSHOPT | +CPUID_7_0_EBX_CLWB | CPUID_7_0_EBX_AVX512CD | CPUID_7_0_EBX_SHA_NI | +CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512VL, +.features[FEAT_7_0_ECX] = +CPUID_7_0_ECX_AVX512_VBMI | CPUID_7_0_ECX_UMIP | CPUID_7_0_ECX_PKU | +CPUID_7_0_ECX_AVX512_VBMI2 | CPUID_7_0_ECX_GFNI | +CPUID_7_0_ECX_VAES | CPUID_7_0_ECX_VPCLMULQDQ | +CPUID_7_0_ECX_AVX512VNNI | CPUID_7_0_ECX_AVX512BITALG | +CPUID_7_0_ECX_AVX512_VPOPCNTDQ | CPUID_7_0_ECX_LA57 | +CPUID_7_0_ECX_RDPID | CPUID_7_0_ECX_BUS_LOCK_DETECT, +.features[FEAT_7_0_EDX] = +CPUID_7_0_EDX_FSRM | CPUID_7_0_EDX_SERIALIZE | +CPUID_7_0_EDX_TSX_LDTRK | CPUID_7_0_EDX_AMX_BF16 | +CPUID_7_0_EDX_AVX512_FP16 | CPUID_7_0_EDX_AMX_TILE | +CPUID_7_0_EDX_AMX_INT8 | CPUID_7_0_EDX_SPEC_CTRL | +CPUID_7_0_EDX_ARCH_CAPABILITIES | CPUID_7_0_EDX_SPEC_CTRL_SSBD, +.features[FEAT_ARCH_CAPABILITIES] = +MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_IBRS_ALL | +MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO | +MSR_ARCH_CAP_PSCHANGE_MC_NO | MSR_ARCH_CAP_TAA_NO | +MSR_ARCH_CAP_SBDR_SSDP_NO | MSR_ARCH_CAP_FBSDP_NO | +MSR_ARCH_CAP_PSDP_NO | MSR_ARCH_CAP_PBRSB_NO, +.features[FEAT_XSAVE] = +CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC | +CPUID_XSAVE_XGETBV1 | CPUID_XSAVE_XSAVES | CPUID_D_1_EAX_XFD, +.features[FEAT_6_EAX] = +CPUID_6_EAX_ARAT, +.features[FEAT_7_1_EAX] = +CPUID_7_1_EAX_AVX_VNNI | CPUID_7_1_EAX_AVX512_BF16 | +CPUID_7_1_EAX_FZRM | CPUID_7_1_EAX_FSRS | CPUID_7_1_EAX_FSRC | +CPUID_7_1_EAX_AMX_FP16, +.features[FEAT_7_1_EDX] = +CPUID_7_1_EDX_PREFETCHITI, +.features[FEAT_7_2_EDX] = +CPUID_7_2_EDX_MCDT_NO, +.features[FEAT_VMX_BASIC] = +MSR_VMX_BASIC_INS_OUTS | MSR_VMX_BASIC_TRUE_CTLS, +.features[FEAT_VMX_ENTRY_CTLS] = +VMX_VM_ENTRY_LOAD_DEBUG_CONTROLS | VMX_VM_ENTRY_IA32E_MODE | +VMX_VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL | +VMX
[PATCH 5/7] target/i386: Add few security fix bits in ARCH_CAPABILITIES into SapphireRapids CPU model
From: Lei Wang Latest stepping (8) of SapphireRapids has bit 13, 14 and 15 of MSR_IA32_ARCH_CAPABILITIES enabled, which are related to some security fixes. Add version 2 of SapphireRapids CPU model with those bits enabled also. Signed-off-by: Lei Wang Signed-off-by: Tao Su --- target/i386/cpu.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index b5321240c6..f84fd20bb1 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -3854,8 +3854,17 @@ static const X86CPUDefinition builtin_x86_defs[] = { .model_id = "Intel Xeon Processor (SapphireRapids)", .versions = (X86CPUVersionDefinition[]) { { .version = 1 }, -{ /* end of list */ }, -}, +{ +.version = 2, +.props = (PropValue[]) { +{ "sbdr-ssdp-no", "on" }, +{ "fbsdp-no", "on" }, +{ "psdp-no", "on" }, +{ /* end of list */ } +} +}, +{ /* end of list */ } +} }, { .name = "Denverton", -- 2.34.1
[PATCH 6/7] target/i386: Add new CPU model EmeraldRapids
From: Qian Wen Emerald Rapids (EMR) is the next generation of Xeon server processor after Sapphire Rapids (SPR). Currently, regarding the feature set that can be exposed to guest, there isn't any one new comparing with SPR cpu model, except that EMR has a different model number. Though it's practicable to define EMR as an alias of a new version of SPR by only updating the model number and model name, it loses the flexibility when new version of EMR cpu model are needed for adding new features (that hasn't virtalized/supported by KVM yet). So just add EMR as a standalone cpu model. Signed-off-by: Qian Wen Reviewed-by: Xiaoyao Li Signed-off-by: Tao Su --- Changes to original patch (https://lore.kernel.org/qemu-devel/20230515025308.1050277-1-qian@intel.com/) - Add MSR_ARCH_CAP_SBDR_SSDP_NO, MSR_ARCH_CAP_FBSDP_NO and MSR_ARCH_CAP_PSDP_NO --- target/i386/cpu.c | 127 ++ 1 file changed, 127 insertions(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index f84fd20bb1..7faf6dfaee 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -3866,6 +3866,133 @@ static const X86CPUDefinition builtin_x86_defs[] = { { /* end of list */ } } }, +{ +.name = "EmeraldRapids", +.level = 0x20, +.vendor = CPUID_VENDOR_INTEL, +.family = 6, +.model = 207, +.stepping = 1, +.features[FEAT_1_EDX] = +CPUID_FP87 | CPUID_VME | CPUID_DE | CPUID_PSE | CPUID_TSC | +CPUID_MSR | CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | +CPUID_SEP | CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | +CPUID_PAT | CPUID_PSE36 | CPUID_CLFLUSH | CPUID_MMX | CPUID_FXSR | +CPUID_SSE | CPUID_SSE2, +.features[FEAT_1_ECX] = +CPUID_EXT_SSE3 | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSSE3 | +CPUID_EXT_FMA | CPUID_EXT_CX16 | CPUID_EXT_PCID | CPUID_EXT_SSE41 | +CPUID_EXT_SSE42 | CPUID_EXT_X2APIC | CPUID_EXT_MOVBE | +CPUID_EXT_POPCNT | CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_AES | +CPUID_EXT_XSAVE | CPUID_EXT_AVX | CPUID_EXT_F16C | CPUID_EXT_RDRAND, +.features[FEAT_8000_0001_EDX] = +CPUID_EXT2_SYSCALL | CPUID_EXT2_NX | CPUID_EXT2_PDPE1GB | +CPUID_EXT2_RDTSCP | CPUID_EXT2_LM, +.features[FEAT_8000_0001_ECX] = +CPUID_EXT3_LAHF_LM | CPUID_EXT3_ABM | CPUID_EXT3_3DNOWPREFETCH, +.features[FEAT_8000_0008_EBX] = +CPUID_8000_0008_EBX_WBNOINVD, +.features[FEAT_7_0_EBX] = +CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_HLE | +CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | +CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID | CPUID_7_0_EBX_RTM | +CPUID_7_0_EBX_AVX512F | CPUID_7_0_EBX_AVX512DQ | +CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX | CPUID_7_0_EBX_SMAP | +CPUID_7_0_EBX_AVX512IFMA | CPUID_7_0_EBX_CLFLUSHOPT | +CPUID_7_0_EBX_CLWB | CPUID_7_0_EBX_AVX512CD | CPUID_7_0_EBX_SHA_NI | +CPUID_7_0_EBX_AVX512BW | CPUID_7_0_EBX_AVX512VL, +.features[FEAT_7_0_ECX] = +CPUID_7_0_ECX_AVX512_VBMI | CPUID_7_0_ECX_UMIP | CPUID_7_0_ECX_PKU | +CPUID_7_0_ECX_AVX512_VBMI2 | CPUID_7_0_ECX_GFNI | +CPUID_7_0_ECX_VAES | CPUID_7_0_ECX_VPCLMULQDQ | +CPUID_7_0_ECX_AVX512VNNI | CPUID_7_0_ECX_AVX512BITALG | +CPUID_7_0_ECX_AVX512_VPOPCNTDQ | CPUID_7_0_ECX_LA57 | +CPUID_7_0_ECX_RDPID | CPUID_7_0_ECX_BUS_LOCK_DETECT, +.features[FEAT_7_0_EDX] = +CPUID_7_0_EDX_FSRM | CPUID_7_0_EDX_SERIALIZE | +CPUID_7_0_EDX_TSX_LDTRK | CPUID_7_0_EDX_AMX_BF16 | +CPUID_7_0_EDX_AVX512_FP16 | CPUID_7_0_EDX_AMX_TILE | +CPUID_7_0_EDX_AMX_INT8 | CPUID_7_0_EDX_SPEC_CTRL | +CPUID_7_0_EDX_ARCH_CAPABILITIES | CPUID_7_0_EDX_SPEC_CTRL_SSBD, +.features[FEAT_ARCH_CAPABILITIES] = +MSR_ARCH_CAP_RDCL_NO | MSR_ARCH_CAP_IBRS_ALL | +MSR_ARCH_CAP_SKIP_L1DFL_VMENTRY | MSR_ARCH_CAP_MDS_NO | +MSR_ARCH_CAP_PSCHANGE_MC_NO | MSR_ARCH_CAP_TAA_NO | +MSR_ARCH_CAP_SBDR_SSDP_NO | MSR_ARCH_CAP_FBSDP_NO | +MSR_ARCH_CAP_PSDP_NO, +.features[FEAT_XSAVE] = +CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC | +CPUID_XSAVE_XGETBV1 | CPUID_XSAVE_XSAVES | CPUID_D_1_EAX_XFD, +.features[FEAT_6_EAX] = +CPUID_6_EAX_ARAT, +.features[FEAT_7_1_EAX] = +CPUID_7_1_EAX_AVX_VNNI | CPUID_7_1_EAX_AVX512_BF16 | +CPUID_7_1_EAX_FZRM | CPUID_7_1_EAX_FSRS | CPUID_7_1_EAX_FSRC, +.features[FEAT_VMX_BASIC] = +MSR_VMX_BASIC_INS_OUTS | MSR_VMX_BASIC_TRUE_CTLS, +.features[FEAT_VMX_ENTRY_CTLS] = +VMX_VM_ENTRY_LOAD_DEBUG_CONTROLS | VMX_VM_ENTRY_IA32E_MODE | +VMX_VM_ENTRY_LOAD_IA32_PERF_GLOBAL_C
[PATCH 1/7] target/i386: Add FEAT_7_1_EDX to adjust feature level
Considering the case of FEAT_7_1_EAX being 0 and FEAT_7_1_EDX being non-zero, guest may report wrong maximum number sub-leaves in leaf 07H. So add FEAT_7_1_EDX to adjust feature level. Fixes: eaaa197d5b11 ("target/i386: Add support for AVX-VNNI-INT8 in CPUID enumeration") Signed-off-by: Tao Su Reviewed-by: Xiaoyao Li --- target/i386/cpu.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 1242bd541a..e8a70c35d2 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6778,6 +6778,7 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp) x86_cpu_adjust_feat_level(cpu, FEAT_6_EAX); x86_cpu_adjust_feat_level(cpu, FEAT_7_0_ECX); x86_cpu_adjust_feat_level(cpu, FEAT_7_1_EAX); +x86_cpu_adjust_feat_level(cpu, FEAT_7_1_EDX); x86_cpu_adjust_feat_level(cpu, FEAT_8000_0001_EDX); x86_cpu_adjust_feat_level(cpu, FEAT_8000_0001_ECX); x86_cpu_adjust_feat_level(cpu, FEAT_8000_0007_EDX); -- 2.34.1
[PATCH 2/7] target/i386: Add support for MCDT_NO in CPUID enumeration
CPUID.(EAX=7,ECX=2):EDX[bit 5] enumerates MCDT_NO. Processors enumerate this bit as 1 do not exhibit MXCSR Configuration Dependent Timing (MCDT) behavior and do not need to be mitigated to avoid data-dependent behavior for certain instructions. Since MCDT_NO is in a new sub-leaf, add a new CPUID feature word FEAT_7_2_EDX. Also update cpuid_level_func7 by FEAT_7_2_EDX. Signed-off-by: Tao Su Reviewed-by: Xiaoyao Li --- target/i386/cpu.c | 26 ++ target/i386/cpu.h | 4 2 files changed, 30 insertions(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index e8a70c35d2..7898a4c79a 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -668,6 +668,7 @@ void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, #define TCG_7_1_EAX_FEATURES (CPUID_7_1_EAX_FZRM | CPUID_7_1_EAX_FSRS | \ CPUID_7_1_EAX_FSRC) #define TCG_7_1_EDX_FEATURES 0 +#define TCG_7_2_EDX_FEATURES 0 #define TCG_APM_FEATURES 0 #define TCG_6_EAX_FEATURES CPUID_6_EAX_ARAT #define TCG_XSAVE_FEATURES (CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XGETBV1) @@ -910,6 +911,25 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, .tcg_features = TCG_7_1_EDX_FEATURES, }, +[FEAT_7_2_EDX] = { +.type = CPUID_FEATURE_WORD, +.feat_names = { +NULL, NULL, NULL, NULL, +NULL, "mcdt-no", NULL, NULL, +NULL, NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +}, +.cpuid = { +.eax = 7, +.needs_ecx = true, .ecx = 2, +.reg = R_EDX, +}, +.tcg_features = TCG_7_2_EDX_FEATURES, +}, [FEAT_8000_0007_EDX] = { .type = CPUID_FEATURE_WORD, .feat_names = { @@ -5919,6 +5939,11 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, *edx = env->features[FEAT_7_1_EDX]; *ebx = 0; *ecx = 0; +} else if (count == 2) { +*edx = env->features[FEAT_7_2_EDX]; +*eax = 0; +*ebx = 0; +*ecx = 0; } else { *eax = 0; *ebx = 0; @@ -6779,6 +6804,7 @@ void x86_cpu_expand_features(X86CPU *cpu, Error **errp) x86_cpu_adjust_feat_level(cpu, FEAT_7_0_ECX); x86_cpu_adjust_feat_level(cpu, FEAT_7_1_EAX); x86_cpu_adjust_feat_level(cpu, FEAT_7_1_EDX); +x86_cpu_adjust_feat_level(cpu, FEAT_7_2_EDX); x86_cpu_adjust_feat_level(cpu, FEAT_8000_0001_EDX); x86_cpu_adjust_feat_level(cpu, FEAT_8000_0001_ECX); x86_cpu_adjust_feat_level(cpu, FEAT_8000_0007_EDX); diff --git a/target/i386/cpu.h b/target/i386/cpu.h index cd047e0410..64d50acf41 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -628,6 +628,7 @@ typedef enum FeatureWord { FEAT_XSAVE_XSS_LO, /* CPUID[EAX=0xd,ECX=1].ECX */ FEAT_XSAVE_XSS_HI, /* CPUID[EAX=0xd,ECX=1].EDX */ FEAT_7_1_EDX, /* CPUID[EAX=7,ECX=1].EDX */ +FEAT_7_2_EDX, /* CPUID[EAX=7,ECX=2].EDX */ FEATURE_WORDS, } FeatureWord; @@ -932,6 +933,9 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w, /* PREFETCHIT0/1 Instructions */ #define CPUID_7_1_EDX_PREFETCHITI (1U << 14) +/* Do not exhibit MXCSR Configuration Dependent Timing (MCDT) behavior */ +#define CPUID_7_2_EDX_MCDT_NO (1U << 5) + /* XFD Extend Feature Disabled */ #define CPUID_D_1_EAX_XFD (1U << 4) -- 2.34.1
RE: [PATCH] vfio/migration: Fix return value of vfio_migration_realize()
>-Original Message- >From: Joao Martins >Sent: Thursday, June 15, 2023 6:23 PM >To: Duan, Zhenzhong >Cc: alex.william...@redhat.com; c...@redhat.com; qemu-devel@nongnu.org; >Peng, Chao P >Subject: Re: [PATCH] vfio/migration: Fix return value of >vfio_migration_realize() > >On 15/06/2023 10:19, Duan, Zhenzhong wrote: >>> -Original Message- >>> From: Joao Martins >>> Sent: Thursday, June 15, 2023 4:54 PM >>> To: Duan, Zhenzhong >>> Cc: alex.william...@redhat.com; c...@redhat.com; >>> qemu-devel@nongnu.org; Peng, Chao P >>> Subject: Re: [PATCH] vfio/migration: Fix return value of >>> vfio_migration_realize() >>> >>> >>> >>> On 15/06/2023 09:18, Zhenzhong Duan wrote: We should print "Migration disabled" when migration is blocked in vfio_migration_realize(). Fix it by reverting return value of migrate_add_blocker(), meanwhile error out directly once migrate_add_blocker() failed. >>> >>> It wasn't immediately obvious from commit message that this is mainly >>> about just printing an error message when blockers get added and that >>> well >>> migrate_add_blocker() returns 0 on success and caller of >>> vfio_migration_realize expects the opposite when blockers are added. >>> >>> Perhaps better wording would be: >>> >>> migrate_add_blocker() returns 0 when successfully adding the >>> migration blocker. However, the caller of vfio_migration_realize() >>> considers that migration was blocked when the latter returned an >>> error. Fix it by negating the return value obtained by >>> migrate_add_blocker(). What matters for migration is that the blocker >>> is added in core migration, so this cleans up usability such that >>> user sees "Migrate disabled" when any of the vfio migration blockers are >active. >> >> Great, I'll use your words. >> >>> Signed-off-by: Zhenzhong Duan --- hw/vfio/common.c| 4 ++-- hw/vfio/migration.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index fa8fd949b1cf..8505385798f3 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -399,7 +399,7 @@ int vfio_block_multiple_devices_migration(Error >>> **errp) multiple_devices_migration_blocker = NULL; } -return ret; +return !ret; } void vfio_unblock_multiple_devices_migration(void) @@ -444,7 +444,7 @@ int vfio_block_giommu_migration(Error **errp) giommu_migration_blocker = NULL; } -return ret; +return !ret; } void vfio_migration_finalize(void) diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index 6b58dddb8859..0146521d129a 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -646,12 +646,12 @@ int vfio_migration_realize(VFIODevice *vbasedev, >>> Error **errp) } ret = vfio_block_multiple_devices_migration(errp); -if (ret) { +if (ret || (errp && *errp)) { >>> >>> Why do you need this extra clause? >> >> Now that error happens, no need to add other blockers which will fail >> for same reason. >> > >But you don't need the (errp && *errp) for that as that's the whole point of >negating the result. > >And if there's an error set it means migrate_add_blocker failed to add the >blocker (e.g. snapshotting IIUC), and you would be failing here unnecessarily? If there is an error qdev_device_add() will fail, continue execution is meaningless here? There is ERRP_GUARD in this path, so it looks (*errp) is enough. If I removed (errp && *errp) to continue, need below change to bypass trace_vfio_migration_probe Do you prefer this way? if (!*errp) { trace_vfio_migration_probe(vbasedev->name); } Thanks Zhenzhong > >Still it feels strange to use the error pointer to check for that, it's better >to >return error appropriately. > >>> return ret; } ret = vfio_block_giommu_migration(errp); -if (ret) { +if (ret || (errp && *errp)) { >>> >>> Same here? >> >> To avoid calling into trace_vfio_migration_probe() which hints vfio migration >realize succeed. >> > >That was clear, my question was more related to second clause you are >adding.. > >> Thanks >> Zhenzhong >> >>> return ret; } @@ -667,7 +667,7 @@ add_blocker: error_free(vbasedev->migration_blocker); vbasedev->migration_blocker = NULL; } -return ret; +return !ret; } void vfio_migration_exit(VFIODevice *vbasedev)
Re: [PATCH v15 00/10] TCG code quality tracking
On 6/13/2023 11:29 AM, Wu, Fei wrote: > On 6/7/2023 8:24 PM, Fei Wu wrote: >> v15 >> --- >> This is a large change: >> * remove all time related stuffs, including cmd 'info profile' >> * remove the per-TB flag, use global flag instead >> * remove tb_stats pause/filter, but add status >> * remove qemu_log changes, and use monitor_printf >> * use array instead of list for sorting >> * remove async_safe_run_on_cpu for cmd info tb-list & tb >> * use monitor_disas instead of regenerate TB, but **doesn't work yet** >> * other cleanups >> > Hi Richard, > > Could you please take a look at this series? I hope most of the comments > on v14 have been addressed. > > Next revision I will change: > * add async_safe_run_on_cpu back in case of any concurrency issue > * add tbs->gpa_pc for monitor_disas, which requires > get_page_addr_code_hostp() return both ram_addr_t and gpa > * finalize the commit logs and documents > Should I send the new revision with these changes, or you take a look first then I address all of them together? Thanks, Fei. > Thanks, > Fei. > >> >> Alex Bennée (1): >> tb-stats: reset the tracked TBs on a tb_flush >> >> Fei Wu (5): >> accel/tcg: remove CONFIG_PROFILER >> accel/tcg: add jit stats to TBStatistics >> debug: add -d tb_stats to control TBStatistics >> tb-stats: dump hot TBs at the end of the execution >> docs: add tb-stats how to >> >> Vanderson M. do Rosario (4): >> accel/tcg: introduce TBStatistics structure >> accel: collecting TB execution count >> monitor: adding tb_stats hmp command >> tb-stats: Adding info [tb-list|tb] commands to HMP (WIP) >> >> MAINTAINERS | 1 + >> accel/tcg/cpu-exec.c | 6 + >> accel/tcg/meson.build | 1 + >> accel/tcg/monitor.c | 184 +++-- >> accel/tcg/tb-context.h| 1 + >> accel/tcg/tb-hash.h | 7 + >> accel/tcg/tb-maint.c | 20 ++ >> accel/tcg/tb-stats.c | 365 ++ >> accel/tcg/tcg-accel-ops.c | 10 - >> accel/tcg/tcg-runtime.c | 1 + >> accel/tcg/translate-all.c | 110 ++ >> accel/tcg/translator.c| 30 +++ >> disas/disas.c | 2 + >> docs/devel/tcg-tbstats.rst| 97 + >> hmp-commands-info.hx | 31 +-- >> hmp-commands.hx | 16 ++ >> include/exec/exec-all.h | 3 + >> include/exec/gen-icount.h | 1 + >> include/exec/tb-stats-dump.h | 21 ++ >> include/exec/tb-stats-flags.h | 29 +++ >> include/exec/tb-stats.h | 130 >> include/monitor/hmp.h | 3 + >> include/qemu/log.h| 1 + >> include/qemu/timer.h | 9 - >> include/tcg/tcg.h | 26 +-- >> linux-user/exit.c | 2 + >> meson.build | 2 - >> meson_options.txt | 2 - >> qapi/machine.json | 18 -- >> scripts/meson-buildoptions.sh | 3 - >> softmmu/runstate.c| 11 +- >> stubs/meson.build | 1 + >> stubs/tb-stats.c | 36 >> tcg/tcg.c | 237 +++--- >> tests/qtest/qmp-cmd-test.c| 3 - >> util/log.c| 26 +++ >> 36 files changed, 1093 insertions(+), 353 deletions(-) >> create mode 100644 accel/tcg/tb-stats.c >> create mode 100644 docs/devel/tcg-tbstats.rst >> create mode 100644 include/exec/tb-stats-dump.h >> create mode 100644 include/exec/tb-stats-flags.h >> create mode 100644 include/exec/tb-stats.h >> create mode 100644 stubs/tb-stats.c >> >
[PULL v5 10/11] meson.build: enable xenpv machine build for ARM
From: Vikram Garhwal Add CONFIG_XEN for aarch64 device to support build for ARM targets. Signed-off-by: Vikram Garhwal Signed-off-by: Stefano Stabellini Reviewed-by: Alex Bennée --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 481865bfa9..cfa98e9e25 100644 --- a/meson.build +++ b/meson.build @@ -136,7 +136,7 @@ endif if cpu in ['x86', 'x86_64', 'arm', 'aarch64'] # i386 emulator provides xenpv machine type for multiple architectures accelerator_targets += { -'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu'], +'CONFIG_XEN': ['i386-softmmu', 'x86_64-softmmu', 'aarch64-softmmu'], } endif if cpu in ['x86', 'x86_64'] -- 2.25.1
[PULL v5 09/11] hw/arm: introduce xenpvh machine
From: Vikram Garhwal Add a new machine xenpvh which creates a IOREQ server to register/connect with Xen Hypervisor. Optional: When CONFIG_TPM is enabled, it also creates a tpm-tis-device, adds a TPM emulator and connects to swtpm running on host machine via chardev socket and support TPM functionalities for a guest domain. Extra command line for aarch64 xenpvh QEMU to connect to swtpm: -chardev socket,id=chrtpm,path=/tmp/myvtpm2/swtpm-sock \ -tpmdev emulator,id=tpm0,chardev=chrtpm \ -machine tpm-base-addr=0x0c00 \ swtpm implements a TPM software emulator(TPM 1.2 & TPM 2) built on libtpms and provides access to TPM functionality over socket, chardev and CUSE interface. Github repo: https://github.com/stefanberger/swtpm Example for starting swtpm on host machine: mkdir /tmp/vtpm2 swtpm socket --tpmstate dir=/tmp/vtpm2 \ --ctrl type=unixio,path=/tmp/vtpm2/swtpm-sock & Signed-off-by: Vikram Garhwal Signed-off-by: Stefano Stabellini Reviewed-by: Stefano Stabellini --- docs/system/arm/xenpvh.rst| 34 +++ docs/system/target-arm.rst| 1 + hw/arm/meson.build| 2 + hw/arm/xen_arm.c | 181 ++ include/hw/arm/xen_arch_hvm.h | 9 ++ include/hw/xen/arch_hvm.h | 2 + 6 files changed, 229 insertions(+) create mode 100644 docs/system/arm/xenpvh.rst create mode 100644 hw/arm/xen_arm.c create mode 100644 include/hw/arm/xen_arch_hvm.h diff --git a/docs/system/arm/xenpvh.rst b/docs/system/arm/xenpvh.rst new file mode 100644 index 00..e1655c7ab8 --- /dev/null +++ b/docs/system/arm/xenpvh.rst @@ -0,0 +1,34 @@ +XENPVH (``xenpvh``) += +This machine creates a IOREQ server to register/connect with Xen Hypervisor. + +When TPM is enabled, this machine also creates a tpm-tis-device at a user input +tpm base address, adds a TPM emulator and connects to a swtpm application +running on host machine via chardev socket. This enables xenpvh to support TPM +functionalities for a guest domain. + +More information about TPM use and installing swtpm linux application can be +found at: docs/specs/tpm.rst. + +Example for starting swtpm on host machine: +.. code-block:: console + +mkdir /tmp/vtpm2 +swtpm socket --tpmstate dir=/tmp/vtpm2 \ +--ctrl type=unixio,path=/tmp/vtpm2/swtpm-sock & + +Sample QEMU xenpvh commands for running and connecting with Xen: +.. code-block:: console + +qemu-system-aarch64 -xen-domid 1 \ +-chardev socket,id=libxl-cmd,path=qmp-libxl-1,server=on,wait=off \ +-mon chardev=libxl-cmd,mode=control \ +-chardev socket,id=libxenstat-cmd,path=qmp-libxenstat-1,server=on,wait=off \ +-mon chardev=libxenstat-cmd,mode=control \ +-xen-attach -name guest0 -vnc none -display none -nographic \ +-machine xenpvh -m 1301 \ +-chardev socket,id=chrtpm,path=tmp/vtpm2/swtpm-sock \ +-tpmdev emulator,id=tpm0,chardev=chrtpm -machine tpm-base-addr=0x0C00 + +In above QEMU command, last two lines are for connecting xenpvh QEMU to swtpm +via chardev socket. diff --git a/docs/system/target-arm.rst b/docs/system/target-arm.rst index a12b6bca05..790ac1b8a2 100644 --- a/docs/system/target-arm.rst +++ b/docs/system/target-arm.rst @@ -107,6 +107,7 @@ undocumented; you can get a complete list by running arm/stm32 arm/virt arm/xlnx-versal-virt + arm/xenpvh Emulated CPU architecture support = diff --git a/hw/arm/meson.build b/hw/arm/meson.build index 870ec67376..4f94f821b0 100644 --- a/hw/arm/meson.build +++ b/hw/arm/meson.build @@ -63,6 +63,8 @@ arm_ss.add(when: 'CONFIG_FSL_IMX7', if_true: files('fsl-imx7.c', 'mcimx7d-sabre. arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c')) arm_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-imx6ul.c', 'mcimx6ul-evk.c')) arm_ss.add(when: 'CONFIG_NRF51_SOC', if_true: files('nrf51_soc.c')) +arm_ss.add(when: 'CONFIG_XEN', if_true: files('xen_arm.c')) +arm_ss.add_all(xen_ss) softmmu_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmu-common.c')) softmmu_ss.add(when: 'CONFIG_EXYNOS4', if_true: files('exynos4_boards.c')) diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c new file mode 100644 index 00..19b1cb81ad --- /dev/null +++ b/hw/arm/xen_arm.c @@ -0,0 +1,181 @@ +/* + * QEMU ARM Xen PVH Machine + * + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "A
[PULL v5 03/11] hw/i386/xen/xen-hvm: move x86-specific fields out of XenIOState
From: Stefano Stabellini In preparation to moving most of xen-hvm code to an arch-neutral location, move: - shared_vmport_page - log_for_dirtybit - dirty_bitmap - suspend - wakeup out of XenIOState struct as these are only used on x86, especially the ones related to dirty logging. Updated XenIOState can be used for both aarch64 and x86. Also, remove free_phys_offset as it was unused. Signed-off-by: Stefano Stabellini Signed-off-by: Vikram Garhwal Reviewed-by: Paul Durrant Reviewed-by: Alex Bennée --- hw/i386/xen/xen-hvm.c | 58 --- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index 7a7764240e..01bf947f1c 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -74,6 +74,7 @@ struct shared_vmport_iopage { }; typedef struct shared_vmport_iopage shared_vmport_iopage_t; #endif +static shared_vmport_iopage_t *shared_vmport_page; static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i) { @@ -96,6 +97,11 @@ typedef struct XenPhysmap { } XenPhysmap; static QLIST_HEAD(, XenPhysmap) xen_physmap; +static const XenPhysmap *log_for_dirtybit; +/* Buffer used by xen_sync_dirty_bitmap */ +static unsigned long *dirty_bitmap; +static Notifier suspend; +static Notifier wakeup; typedef struct XenPciDevice { PCIDevice *pci_dev; @@ -106,7 +112,6 @@ typedef struct XenPciDevice { typedef struct XenIOState { ioservid_t ioservid; shared_iopage_t *shared_page; -shared_vmport_iopage_t *shared_vmport_page; buffered_iopage_t *buffered_io_page; xenforeignmemory_resource_handle *fres; QEMUTimer *buffered_io_timer; @@ -126,14 +131,8 @@ typedef struct XenIOState { MemoryListener io_listener; QLIST_HEAD(, XenPciDevice) dev_list; DeviceListener device_listener; -hwaddr free_phys_offset; -const XenPhysmap *log_for_dirtybit; -/* Buffer used by xen_sync_dirty_bitmap */ -unsigned long *dirty_bitmap; Notifier exit; -Notifier suspend; -Notifier wakeup; } XenIOState; /* Xen specific function for piix pci */ @@ -463,10 +462,10 @@ static int xen_remove_from_physmap(XenIOState *state, } QLIST_REMOVE(physmap, list); -if (state->log_for_dirtybit == physmap) { -state->log_for_dirtybit = NULL; -g_free(state->dirty_bitmap); -state->dirty_bitmap = NULL; +if (log_for_dirtybit == physmap) { +log_for_dirtybit = NULL; +g_free(dirty_bitmap); +dirty_bitmap = NULL; } g_free(physmap); @@ -627,16 +626,16 @@ static void xen_sync_dirty_bitmap(XenIOState *state, return; } -if (state->log_for_dirtybit == NULL) { -state->log_for_dirtybit = physmap; -state->dirty_bitmap = g_new(unsigned long, bitmap_size); -} else if (state->log_for_dirtybit != physmap) { +if (log_for_dirtybit == NULL) { +log_for_dirtybit = physmap; +dirty_bitmap = g_new(unsigned long, bitmap_size); +} else if (log_for_dirtybit != physmap) { /* Only one range for dirty bitmap can be tracked. */ return; } rc = xen_track_dirty_vram(xen_domid, start_addr >> TARGET_PAGE_BITS, - npages, state->dirty_bitmap); + npages, dirty_bitmap); if (rc < 0) { #ifndef ENODATA #define ENODATA ENOENT @@ -651,7 +650,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state, } for (i = 0; i < bitmap_size; i++) { -unsigned long map = state->dirty_bitmap[i]; +unsigned long map = dirty_bitmap[i]; while (map != 0) { j = ctzl(map); map &= ~(1ul << j); @@ -677,12 +676,10 @@ static void xen_log_start(MemoryListener *listener, static void xen_log_stop(MemoryListener *listener, MemoryRegionSection *section, int old, int new) { -XenIOState *state = container_of(listener, XenIOState, memory_listener); - if (old & ~new & (1 << DIRTY_MEMORY_VGA)) { -state->log_for_dirtybit = NULL; -g_free(state->dirty_bitmap); -state->dirty_bitmap = NULL; +log_for_dirtybit = NULL; +g_free(dirty_bitmap); +dirty_bitmap = NULL; /* Disable dirty bit tracking */ xen_track_dirty_vram(xen_domid, 0, 0, NULL); } @@ -1022,9 +1019,9 @@ static void handle_vmport_ioreq(XenIOState *state, ioreq_t *req) { vmware_regs_t *vmport_regs; -assert(state->shared_vmport_page); +assert(shared_vmport_page); vmport_regs = -&state->shared_vmport_page->vcpu_vmport_regs[state->send_vcpu]; +&shared_vmport_page->vcpu_vmport_regs[state->send_vcpu]; QEMU_BUILD_BUG_ON(sizeof(*req) < sizeof(*vmport_regs)); current_cpu = state->cpu_by_vcpu_id[state->send_vcpu]; @@ -1472,7 +1469,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory) state->memory_listener = xen_memory_listener;
[PULL v5 02/11] hw/i386/xen: rearrange xen_hvm_init_pc
From: Vikram Garhwal In preparation to moving most of xen-hvm code to an arch-neutral location, move non IOREQ references to: - xen_get_vmport_regs_pfn - xen_suspend_notifier - xen_wakeup_notifier - xen_ram_init towards the end of the xen_hvm_init_pc() function. This is done to keep the common ioreq functions in one place which will be moved to new function in next patch in order to make it common to both x86 and aarch64 machines. Signed-off-by: Vikram Garhwal Signed-off-by: Stefano Stabellini Reviewed-by: Paul Durrant --- hw/i386/xen/xen-hvm.c | 49 ++- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c index ab8f1b61ee..7a7764240e 100644 --- a/hw/i386/xen/xen-hvm.c +++ b/hw/i386/xen/xen-hvm.c @@ -1419,12 +1419,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory) state->exit.notify = xen_exit_notifier; qemu_add_exit_notifier(&state->exit); -state->suspend.notify = xen_suspend_notifier; -qemu_register_suspend_notifier(&state->suspend); - -state->wakeup.notify = xen_wakeup_notifier; -qemu_register_wakeup_notifier(&state->wakeup); - /* * Register wake-up support in QMP query-current-machine API */ @@ -1435,23 +1429,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory) goto err; } -rc = xen_get_vmport_regs_pfn(xen_xc, xen_domid, &ioreq_pfn); -if (!rc) { -DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn); -state->shared_vmport_page = -xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ|PROT_WRITE, - 1, &ioreq_pfn, NULL); -if (state->shared_vmport_page == NULL) { -error_report("map shared vmport IO page returned error %d handle=%p", - errno, xen_xc); -goto err; -} -} else if (rc != -ENOSYS) { -error_report("get vmport regs pfn returned error %d, rc=%d", - errno, rc); -goto err; -} - /* Note: cpus is empty at this point in init */ state->cpu_by_vcpu_id = g_new0(CPUState *, max_cpus); @@ -1490,7 +1467,6 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory) #else xen_map_cache_init(NULL, state); #endif -xen_ram_init(pcms, ms->ram_size, ram_memory); qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state); @@ -1511,6 +1487,31 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory) QLIST_INIT(&xen_physmap); xen_read_physmap(state); +state->suspend.notify = xen_suspend_notifier; +qemu_register_suspend_notifier(&state->suspend); + +state->wakeup.notify = xen_wakeup_notifier; +qemu_register_wakeup_notifier(&state->wakeup); + +rc = xen_get_vmport_regs_pfn(xen_xc, xen_domid, &ioreq_pfn); +if (!rc) { +DPRINTF("shared vmport page at pfn %lx\n", ioreq_pfn); +state->shared_vmport_page = +xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ|PROT_WRITE, + 1, &ioreq_pfn, NULL); +if (state->shared_vmport_page == NULL) { +error_report("map shared vmport IO page returned error %d handle=%p", + errno, xen_xc); +goto err; +} +} else if (rc != -ENOSYS) { +error_report("get vmport regs pfn returned error %d, rc=%d", + errno, rc); +goto err; +} + +xen_ram_init(pcms, ms->ram_size, ram_memory); + /* Disable ACPI build because Xen handles it */ pcms->acpi_build_enabled = false; -- 2.25.1
[PULL v5 05/11] include/hw/xen/xen_common: return error from xen_create_ioreq_server
From: Stefano Stabellini This is done to prepare for enabling xenpv support for ARM architecture. On ARM it is possible to have a functioning xenpv machine with only the PV backends and no IOREQ server. If the IOREQ server creation fails, continue to the PV backends initialization. Signed-off-by: Stefano Stabellini Signed-off-by: Vikram Garhwal Reviewed-by: Stefano Stabellini Reviewed-by: Paul Durrant --- include/hw/xen/xen_native.h | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/include/hw/xen/xen_native.h b/include/hw/xen/xen_native.h index f11eb423e3..4dce905fde 100644 --- a/include/hw/xen/xen_native.h +++ b/include/hw/xen/xen_native.h @@ -463,8 +463,8 @@ static inline void xen_unmap_pcidev(domid_t dom, PCI_FUNC(pci_dev->devfn)); } -static inline void xen_create_ioreq_server(domid_t dom, - ioservid_t *ioservid) +static inline int xen_create_ioreq_server(domid_t dom, + ioservid_t *ioservid) { int rc = xendevicemodel_create_ioreq_server(xen_dmod, dom, HVM_IOREQSRV_BUFIOREQ_ATOMIC, @@ -472,12 +472,14 @@ static inline void xen_create_ioreq_server(domid_t dom, if (rc == 0) { trace_xen_ioreq_server_create(*ioservid); -return; +return rc; } *ioservid = 0; use_default_ioreq_server = true; trace_xen_default_ioreq_server(); + +return rc; } static inline void xen_destroy_ioreq_server(domid_t dom, -- 2.25.1
[PULL v5 06/11] hw/xen/xen-hvm-common: skip ioreq creation on ioreq registration failure
From: Stefano Stabellini On ARM it is possible to have a functioning xenpv machine with only the PV backends and no IOREQ server. If the IOREQ server creation fails continue to the PV backends initialization. Also, moved the IOREQ registration and mapping subroutine to new function xen_do_ioreq_register(). Signed-off-by: Stefano Stabellini Signed-off-by: Vikram Garhwal Reviewed-by: Stefano Stabellini Reviewed-by: Paul Durrant --- hw/xen/xen-hvm-common.c | 57 +++-- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c index a31b067404..cb82f4b83d 100644 --- a/hw/xen/xen-hvm-common.c +++ b/hw/xen/xen-hvm-common.c @@ -764,27 +764,12 @@ void xen_shutdown_fatal_error(const char *fmt, ...) qemu_system_shutdown_request(SHUTDOWN_CAUSE_HOST_ERROR); } -void xen_register_ioreq(XenIOState *state, unsigned int max_cpus, -MemoryListener xen_memory_listener) +static void xen_do_ioreq_register(XenIOState *state, + unsigned int max_cpus, + MemoryListener xen_memory_listener) { int i, rc; -setup_xen_backend_ops(); - -state->xce_handle = qemu_xen_evtchn_open(); -if (state->xce_handle == NULL) { -perror("xen: event channel open"); -goto err; -} - -state->xenstore = xs_daemon_open(); -if (state->xenstore == NULL) { -perror("xen: xenstore open"); -goto err; -} - -xen_create_ioreq_server(xen_domid, &state->ioservid); - state->exit.notify = xen_exit_notifier; qemu_add_exit_notifier(&state->exit); @@ -849,12 +834,46 @@ void xen_register_ioreq(XenIOState *state, unsigned int max_cpus, QLIST_INIT(&state->dev_list); device_listener_register(&state->device_listener); +return; + +err: +error_report("xen hardware virtual machine initialisation failed"); +exit(1); +} + +void xen_register_ioreq(XenIOState *state, unsigned int max_cpus, +MemoryListener xen_memory_listener) +{ +int rc; + +setup_xen_backend_ops(); + +state->xce_handle = qemu_xen_evtchn_open(); +if (state->xce_handle == NULL) { +perror("xen: event channel open"); +goto err; +} + +state->xenstore = xs_daemon_open(); +if (state->xenstore == NULL) { +perror("xen: xenstore open"); +goto err; +} + +rc = xen_create_ioreq_server(xen_domid, &state->ioservid); +if (!rc) { +xen_do_ioreq_register(state, max_cpus, xen_memory_listener); +} else { +warn_report("xen: failed to create ioreq server"); +} + xen_bus_init(); xen_be_init(); return; + err: -error_report("xen hardware virtual machine initialisation failed"); +error_report("xen hardware virtual machine backend registration failed"); exit(1); } -- 2.25.1
[PULL v5 07/11] hw/xen/xen-hvm-common: Use g_new and error_report
From: Vikram Garhwal Replace g_malloc with g_new and perror with error_report. Signed-off-by: Vikram Garhwal Reviewed-by: Stefano Stabellini Reviewed-by: Paul Durrant --- hw/xen/xen-hvm-common.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c index cb82f4b83d..42339c96bd 100644 --- a/hw/xen/xen-hvm-common.c +++ b/hw/xen/xen-hvm-common.c @@ -33,7 +33,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr, trace_xen_ram_alloc(ram_addr, size); nr_pfn = size >> TARGET_PAGE_BITS; -pfn_list = g_malloc(sizeof (*pfn_list) * nr_pfn); +pfn_list = g_new(xen_pfn_t, nr_pfn); for (i = 0; i < nr_pfn; i++) { pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i; @@ -730,7 +730,7 @@ void destroy_hvm_domain(bool reboot) return; } if (errno != ENOTTY /* old Xen */) { -perror("xendevicemodel_shutdown failed"); +error_report("xendevicemodel_shutdown failed with error %d", errno); } /* well, try the old thing then */ } @@ -784,7 +784,7 @@ static void xen_do_ioreq_register(XenIOState *state, } /* Note: cpus is empty at this point in init */ -state->cpu_by_vcpu_id = g_malloc0(max_cpus * sizeof(CPUState *)); +state->cpu_by_vcpu_id = g_new0(CPUState *, max_cpus); rc = xen_set_ioreq_server_state(xen_domid, state->ioservid, true); if (rc < 0) { @@ -793,7 +793,7 @@ static void xen_do_ioreq_register(XenIOState *state, goto err; } -state->ioreq_local_port = g_malloc0(max_cpus * sizeof (evtchn_port_t)); +state->ioreq_local_port = g_new0(evtchn_port_t, max_cpus); /* FIXME: how about if we overflow the page here? */ for (i = 0; i < max_cpus; i++) { @@ -850,13 +850,13 @@ void xen_register_ioreq(XenIOState *state, unsigned int max_cpus, state->xce_handle = qemu_xen_evtchn_open(); if (state->xce_handle == NULL) { -perror("xen: event channel open"); +error_report("xen: event channel open failed with error %d", errno); goto err; } state->xenstore = xs_daemon_open(); if (state->xenstore == NULL) { -perror("xen: xenstore open"); +error_report("xen: xenstore open failed with error %d", errno); goto err; } -- 2.25.1
[PULL v5 11/11] test/qtest: add xepvh to skip list for qtest
From: Vikram Garhwal Like existing xen machines, xenpvh also cannot be used for qtest. Signed-off-by: Vikram Garhwal Reviewed-by: Stefano Stabellini --- tests/qtest/libqtest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index 77de16227f..de03ef5f60 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -1465,7 +1465,8 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine), for (i = 0; machines[i].name != NULL; i++) { /* Ignore machines that cannot be used for qtests */ if (!strncmp("xenfv", machines[i].name, 5) || -g_str_equal("xenpv", machines[i].name)) { +g_str_equal("xenpv", machines[i].name) || +g_str_equal("xenpvh", machines[i].name)) { continue; } if (!skip_old_versioned || -- 2.25.1
[PULL v5 01/11] hw/i386/xen/: move xen-mapcache.c to hw/xen/
From: Vikram Garhwal xen-mapcache.c contains common functions which can be used for enabling Xen on aarch64 with IOREQ handling. Moving it out from hw/i386/xen to hw/xen to make it accessible for both aarch64 and x86. Signed-off-by: Vikram Garhwal Signed-off-by: Stefano Stabellini Reviewed-by: Paul Durrant --- hw/i386/meson.build | 1 + hw/i386/xen/meson.build | 1 - hw/i386/xen/trace-events | 5 - hw/xen/meson.build | 4 hw/xen/trace-events | 5 + hw/{i386 => }/xen/xen-mapcache.c | 0 6 files changed, 10 insertions(+), 6 deletions(-) rename hw/{i386 => }/xen/xen-mapcache.c (100%) diff --git a/hw/i386/meson.build b/hw/i386/meson.build index 213e2e82b3..cfdbfdcbcb 100644 --- a/hw/i386/meson.build +++ b/hw/i386/meson.build @@ -33,5 +33,6 @@ subdir('kvm') subdir('xen') i386_ss.add_all(xenpv_ss) +i386_ss.add_all(xen_ss) hw_arch += {'i386': i386_ss} diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build index 2e64a34e16..3dc4c4f106 100644 --- a/hw/i386/xen/meson.build +++ b/hw/i386/xen/meson.build @@ -1,6 +1,5 @@ i386_ss.add(when: 'CONFIG_XEN', if_true: files( 'xen-hvm.c', - 'xen-mapcache.c', 'xen_apic.c', 'xen_pvdevice.c', )) diff --git a/hw/i386/xen/trace-events b/hw/i386/xen/trace-events index 5d6be61090..a0c89d91c4 100644 --- a/hw/i386/xen/trace-events +++ b/hw/i386/xen/trace-events @@ -21,8 +21,3 @@ xen_map_resource_ioreq(uint32_t id, void *addr) "id: %u addr: %p" cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x" cpu_ioreq_config_write(void *req, uint32_t sbdf, uint32_t reg, uint32_t size, uint32_t data) "I/O=%p sbdf=0x%x reg=%u size=%u data=0x%x" -# xen-mapcache.c -xen_map_cache(uint64_t phys_addr) "want 0x%"PRIx64 -xen_remap_bucket(uint64_t index) "index 0x%"PRIx64 -xen_map_cache_return(void* ptr) "%p" - diff --git a/hw/xen/meson.build b/hw/xen/meson.build index 19c6aabc7c..202752e557 100644 --- a/hw/xen/meson.build +++ b/hw/xen/meson.build @@ -26,3 +26,7 @@ else endif specific_ss.add_all(when: ['CONFIG_XEN', xen], if_true: xen_specific_ss) + +xen_ss = ss.source_set() + +xen_ss.add(when: 'CONFIG_XEN', if_true: files('xen-mapcache.c')) diff --git a/hw/xen/trace-events b/hw/xen/trace-events index 55c9e1df68..f977c7c8c6 100644 --- a/hw/xen/trace-events +++ b/hw/xen/trace-events @@ -41,3 +41,8 @@ xs_node_vprintf(char *path, char *value) "%s %s" xs_node_vscanf(char *path, char *value) "%s %s" xs_node_watch(char *path) "%s" xs_node_unwatch(char *path) "%s" + +# xen-mapcache.c +xen_map_cache(uint64_t phys_addr) "want 0x%"PRIx64 +xen_remap_bucket(uint64_t index) "index 0x%"PRIx64 +xen_map_cache_return(void* ptr) "%p" diff --git a/hw/i386/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c similarity index 100% rename from hw/i386/xen/xen-mapcache.c rename to hw/xen/xen-mapcache.c -- 2.25.1
[PULL v5 08/11] meson.build: do not set have_xen_pci_passthrough for aarch64 targets
From: Stefano Stabellini have_xen_pci_passthrough is only used for Xen x86 VMs. Signed-off-by: Stefano Stabellini Reviewed-by: Alex Bennée --- meson.build | 2 ++ 1 file changed, 2 insertions(+) diff --git a/meson.build b/meson.build index 34306a6205..481865bfa9 100644 --- a/meson.build +++ b/meson.build @@ -1726,6 +1726,8 @@ have_xen_pci_passthrough = get_option('xen_pci_passthrough') \ error_message: 'Xen PCI passthrough requested but Xen not enabled') \ .require(targetos == 'linux', error_message: 'Xen PCI passthrough not available on this platform') \ + .require(cpu == 'x86' or cpu == 'x86_64', + error_message: 'Xen PCI passthrough not available on this platform') \ .allowed() -- 2.25.1
[PULL v5 04/11] xen-hvm: reorganize xen-hvm and move common function to xen-hvm-common
From: Stefano Stabellini This patch does following: 1. creates arch_handle_ioreq() and arch_xen_set_memory(). This is done in preparation for moving most of xen-hvm code to an arch-neutral location, move the x86-specific portion of xen_set_memory to arch_xen_set_memory. Also, move handle_vmport_ioreq to arch_handle_ioreq. 2. Pure code movement: move common functions to hw/xen/xen-hvm-common.c Extract common functionalities from hw/i386/xen/xen-hvm.c and move them to hw/xen/xen-hvm-common.c. These common functions are useful for creating an IOREQ server. xen_hvm_init_pc() contains the architecture independent code for creating and mapping a IOREQ server, connecting memory and IO listeners, initializing a xen bus and registering backends. Moved this common xen code to a new function xen_register_ioreq() which can be used by both x86 and ARM machines. Following functions are moved to hw/xen/xen-hvm-common.c: xen_vcpu_eport(), xen_vcpu_ioreq(), xen_ram_alloc(), xen_set_memory(), xen_region_add(), xen_region_del(), xen_io_add(), xen_io_del(), xen_device_realize(), xen_device_unrealize(), cpu_get_ioreq_from_shared_memory(), cpu_get_ioreq(), do_inp(), do_outp(), rw_phys_req_item(), read_phys_req_item(), write_phys_req_item(), cpu_ioreq_pio(), cpu_ioreq_move(), cpu_ioreq_config(), handle_ioreq(), handle_buffered_iopage(), handle_buffered_io(), cpu_handle_ioreq(), xen_main_loop_prepare(), xen_hvm_change_state_handler(), xen_exit_notifier(), xen_map_ioreq_server(), destroy_hvm_domain() and xen_shutdown_fatal_error() 3. Removed static type from below functions: 1. xen_region_add() 2. xen_region_del() 3. xen_io_add() 4. xen_io_del() 5. xen_device_realize() 6. xen_device_unrealize() 7. xen_hvm_change_state_handler() 8. cpu_ioreq_pio() 9. xen_exit_notifier() 4. Replace TARGET_PAGE_SIZE with XC_PAGE_SIZE to match the page side with Xen. Signed-off-by: Vikram Garhwal Signed-off-by: Stefano Stabellini Acked-by: Stefano Stabellini --- hw/i386/xen/trace-events| 14 - hw/i386/xen/xen-hvm.c | 1016 ++- hw/xen/meson.build |5 +- hw/xen/trace-events | 14 + hw/xen/xen-hvm-common.c | 860 ++ include/hw/i386/xen_arch_hvm.h | 11 + include/hw/xen/arch_hvm.h |3 + include/hw/xen/xen-hvm-common.h | 99 +++ 8 files changed, 1054 insertions(+), 968 deletions(-) create mode 100644 hw/xen/xen-hvm-common.c create mode 100644 include/hw/i386/xen_arch_hvm.h create mode 100644 include/hw/xen/arch_hvm.h create mode 100644 include/hw/xen/xen-hvm-common.h diff --git a/hw/i386/xen/trace-events b/hw/i386/xen/trace-events index a0c89d91c4..5d0a8d6dcf 100644 --- a/hw/i386/xen/trace-events +++ b/hw/i386/xen/trace-events @@ -7,17 +7,3 @@ xen_platform_log(char *s) "xen platform: %s" xen_pv_mmio_read(uint64_t addr) "WARNING: read from Xen PV Device MMIO space (address 0x%"PRIx64")" xen_pv_mmio_write(uint64_t addr) "WARNING: write to Xen PV Device MMIO space (address 0x%"PRIx64")" -# xen-hvm.c -xen_ram_alloc(unsigned long ram_addr, unsigned long size) "requested: 0x%lx, size 0x%lx" -xen_client_set_memory(uint64_t start_addr, unsigned long size, bool log_dirty) "0x%"PRIx64" size 0x%lx, log_dirty %i" -handle_ioreq(void *req, uint32_t type, uint32_t dir, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p type=%d dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -handle_ioreq_read(void *req, uint32_t type, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p read type=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -handle_ioreq_write(void *req, uint32_t type, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p write type=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -cpu_ioreq_pio(void *req, uint32_t dir, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p pio dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -cpu_ioreq_pio_read_reg(void *req, uint64_t data, uint64_t addr, uint32_t size) "I/O=%p pio read reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d" -cpu_ioreq_pio_write_reg(void *req, uint64_t data, uint64_t addr, uint32_t size) "I/O=%p pio write reg data=0x%"PRIx64" port=0x%"PRIx64" size=%d" -cpu_ioreq_move(void *req, uint32_t dir, uint32_t df, uint32_t data_is_ptr, uint64_t addr, uint64_t data, uint32_t count, uint32_t size) "I/O=%p copy dir=%d df=%d ptr=%d port=0x%"PRIx64" data=0x%"PRIx64" count=%d size=%d" -xen_map_resource_ioreq(uint32_t id, void *addr) "id: %u addr: %p" -cpu_ioreq_config_read(void *req, uint32_t sbdf, uint32_t reg,
[PULL v5 00/11] xenpvh5-tag
Hi Peter, Richard, Vikram fixed the gitlab test problem yet again. I appended a tiny qtest fix at the end of the series. Cheers, Stefano The following changes since commit 7efd65423ab22e6f5890ca08ae40c84d6660242f: Merge tag 'pull-riscv-to-apply-20230614' of https://github.com/alistair23/qemu into staging (2023-06-14 05:28:51 +0200) are available in the Git repository at: https://gitlab.com/sstabellini/qemu.git xenpvh5-tag for you to fetch changes up to d8a714eba68cd7221d44a6acb6b8a69cf6f2f86b: test/qtest: add xepvh to skip list for qtest (2023-06-15 16:46:58 -0700) Stefano Stabellini (5): hw/i386/xen/xen-hvm: move x86-specific fields out of XenIOState xen-hvm: reorganize xen-hvm and move common function to xen-hvm-common include/hw/xen/xen_common: return error from xen_create_ioreq_server hw/xen/xen-hvm-common: skip ioreq creation on ioreq registration failure meson.build: do not set have_xen_pci_passthrough for aarch64 targets Vikram Garhwal (6): hw/i386/xen/: move xen-mapcache.c to hw/xen/ hw/i386/xen: rearrange xen_hvm_init_pc hw/xen/xen-hvm-common: Use g_new and error_report hw/arm: introduce xenpvh machine meson.build: enable xenpv machine build for ARM test/qtest: add xepvh to skip list for qtest docs/system/arm/xenpvh.rst | 34 ++ docs/system/target-arm.rst |1 + hw/arm/meson.build |2 + hw/arm/xen_arm.c | 181 +++ hw/i386/meson.build |1 + hw/i386/xen/meson.build |1 - hw/i386/xen/trace-events | 19 - hw/i386/xen/xen-hvm.c| 1075 -- hw/xen/meson.build |7 + hw/xen/trace-events | 19 + hw/xen/xen-hvm-common.c | 879 +++ hw/{i386 => }/xen/xen-mapcache.c |0 include/hw/arm/xen_arch_hvm.h|9 + include/hw/i386/xen_arch_hvm.h | 11 + include/hw/xen/arch_hvm.h|5 + include/hw/xen/xen-hvm-common.h | 99 include/hw/xen/xen_native.h |8 +- meson.build |4 +- tests/qtest/libqtest.c |3 +- 19 files changed, 1349 insertions(+), 1009 deletions(-) create mode 100644 docs/system/arm/xenpvh.rst create mode 100644 hw/arm/xen_arm.c create mode 100644 hw/xen/xen-hvm-common.c rename hw/{i386 => }/xen/xen-mapcache.c (100%) create mode 100644 include/hw/arm/xen_arch_hvm.h create mode 100644 include/hw/i386/xen_arch_hvm.h create mode 100644 include/hw/xen/arch_hvm.h create mode 100644 include/hw/xen/xen-hvm-common.h
Re: [QEMU][PATCH v8 11/11] test/qtest: add xepvh to skip list for qtest
On Wed, 14 Jun 2023, Vikram Garhwal wrote: > Like existing xen machines, xenpvh also cannot be used for qtest. > > Signed-off-by: Vikram Garhwal Reviewed-by: Stefano Stabellini > --- > tests/qtest/libqtest.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c > index 77de16227f..de03ef5f60 100644 > --- a/tests/qtest/libqtest.c > +++ b/tests/qtest/libqtest.c > @@ -1465,7 +1465,8 @@ void qtest_cb_for_every_machine(void (*cb)(const char > *machine), > for (i = 0; machines[i].name != NULL; i++) { > /* Ignore machines that cannot be used for qtests */ > if (!strncmp("xenfv", machines[i].name, 5) || > -g_str_equal("xenpv", machines[i].name)) { > +g_str_equal("xenpv", machines[i].name) || > +g_str_equal("xenpvh", machines[i].name)) { > continue; > } > if (!skip_old_versioned || > -- > 2.17.1 >
[PATCH v3 12/14] target/ppc: Clean up ifdefs in excp_helper.c, part 1
Use #ifdef, #ifndef for brevity and add comments to #endif that are more than a few lines apart for clarity. Signed-off-by: BALATON Zoltan --- target/ppc/excp_helper.c | 49 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index cfb652445a..858f657128 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -34,7 +34,7 @@ /*/ /* Exception processing */ -#if !defined(CONFIG_USER_ONLY) +#ifndef CONFIG_USER_ONLY static const char *powerpc_excp_name(int excp) { @@ -165,7 +165,7 @@ static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp) env->error_code); } -#if defined(TARGET_PPC64) +#ifdef TARGET_PPC64 static int powerpc_reset_wakeup(CPUPPCState *env, int excp, target_ulong *msr) { /* We no longer are in a PM state */ @@ -359,7 +359,7 @@ static void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp, target_ulong msr, } } } -#endif +#endif /* TARGET_PPC64 */ static void powerpc_reset_excp_state(PowerPCCPU *cpu) { @@ -1100,7 +1100,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp) break; } -#if defined(TARGET_PPC64) +#ifdef TARGET_PPC64 if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) { /* Cat.64-bit: EPCR.ICM is copied to MSR.CM */ new_msr |= (target_ulong)1 << MSR_CM; @@ -1419,7 +1419,7 @@ static inline void powerpc_excp_books(PowerPCCPU *cpu, int excp) { g_assert_not_reached(); } -#endif +#endif /* TARGET_PPC64 */ static void powerpc_excp(PowerPCCPU *cpu, int excp) { @@ -1470,7 +1470,7 @@ void ppc_cpu_do_interrupt(CPUState *cs) powerpc_excp(cpu, cs->exception_index); } -#if defined(TARGET_PPC64) +#ifdef TARGET_PPC64 #define P7_UNUSED_INTERRUPTS \ (PPC_INTERRUPT_RESET | PPC_INTERRUPT_HVIRT | PPC_INTERRUPT_CEXT | \ PPC_INTERRUPT_WDT | PPC_INTERRUPT_CDOORBELL | PPC_INTERRUPT_FIT | \ @@ -1801,7 +1801,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env) return 0; } -#endif +#endif /* TARGET_PPC64 */ static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env) { @@ -1918,7 +1918,7 @@ static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env) static int ppc_next_unmasked_interrupt(CPUPPCState *env) { switch (env->excp_model) { -#if defined(TARGET_PPC64) +#ifdef TARGET_PPC64 case POWERPC_EXCP_POWER7: return p7_next_unmasked_interrupt(env); case POWERPC_EXCP_POWER8: @@ -1957,7 +1957,7 @@ void ppc_maybe_interrupt(CPUPPCState *env) } } -#if defined(TARGET_PPC64) +#ifdef TARGET_PPC64 static void p7_deliver_interrupt(CPUPPCState *env, int interrupt) { PowerPCCPU *cpu = env_archcpu(env); @@ -2159,7 +2159,7 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt) interrupt); } } -#endif +#endif /* TARGET_PPC64 */ static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt) { @@ -2268,7 +2268,7 @@ static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt) static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt) { switch (env->excp_model) { -#if defined(TARGET_PPC64) +#ifdef TARGET_PPC64 case POWERPC_EXCP_POWER7: p7_deliver_interrupt(env, interrupt); break; @@ -2378,9 +2378,9 @@ void helper_raise_exception(CPUPPCState *env, uint32_t exception) { raise_exception_err_ra(env, exception, 0, 0); } -#endif +#endif /* CONFIG_TCG */ -#if !defined(CONFIG_USER_ONLY) +#ifndef CONFIG_USER_ONLY #ifdef CONFIG_TCG void helper_store_msr(CPUPPCState *env, target_ulong val) { @@ -2397,7 +2397,7 @@ void helper_ppc_maybe_interrupt(CPUPPCState *env) ppc_maybe_interrupt(env); } -#if defined(TARGET_PPC64) +#ifdef TARGET_PPC64 void helper_scv(CPUPPCState *env, uint32_t lev) { if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) { @@ -2420,7 +2420,7 @@ void helper_pminsn(CPUPPCState *env, uint32_t insn) ppc_maybe_interrupt(env); } -#endif /* defined(TARGET_PPC64) */ +#endif /* TARGET_PPC64 */ static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) { @@ -2431,7 +2431,7 @@ static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) if (env->flags & POWERPC_FLAG_TGPR) msr &= ~(1ULL << MSR_TGPR); -#if defined(TARGET_PPC64) +#ifdef TARGET_PPC64 /* Switching to 32-bit ? Crop the nip */ if (!msr_is_64bit(env, msr)) { nip = (uint32_t)nip; @@ -2460,7 +2460,7 @@ void helper_rfi(CPUPPCState *env) do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xul); } -#if defined(TARGET_PPC64) +#ifdef TARGET_PPC64 void helper_rfid(CPUPPCState *env) { /* @@ -2481,7 +2481,7 @@ void helper_hrfid(CPUPPCState *env) { do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]); } -#endif +#endif /* TARGET_PPC64 */ #if defined(TARGET_PPC6
[PATCH v3 02/14] target/ppc: Remove unneeded parameter from powerpc_reset_wakeup()
CPUState is rarely needed by this function (only for logging a fatal error) and it's easy to get from the env parameter so passing it separately is not necessary. Signed-off-by: BALATON Zoltan Acked-by: Nicholas Piggin --- target/ppc/excp_helper.c | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 8298217e78..3783315fdb 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -166,8 +166,7 @@ static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp) } #if defined(TARGET_PPC64) -static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp, -target_ulong *msr) +static int powerpc_reset_wakeup(CPUPPCState *env, int excp, target_ulong *msr) { /* We no longer are in a PM state */ env->resume_as_sreset = false; @@ -202,8 +201,8 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp, *msr |= SRR1_WAKEHVI; break; default: -cpu_abort(cs, "Unsupported exception %d in Power Save mode\n", - excp); +cpu_abort(env_cpu(env), + "Unsupported exception %d in Power Save mode\n", excp); } return POWERPC_EXCP_RESET; } @@ -1353,7 +1352,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) * P7/P8/P9 */ if (env->resume_as_sreset) { -excp = powerpc_reset_wakeup(cs, env, excp, &msr); +excp = powerpc_reset_wakeup(env, excp, &msr); } /* -- 2.30.9
[PATCH v3 06/14] target/ppc: Readability improvements in exception handlers
Improve readability by shortening some long comments, removing comments that state the obvious and dropping some empty lines so they don't distract when reading the code. Signed-off-by: BALATON Zoltan Acked-by: Nicholas Piggin --- target/ppc/cpu.h | 1 + target/ppc/excp_helper.c | 180 +++ 2 files changed, 33 insertions(+), 148 deletions(-) diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 0ee2adc105..d7acd65176 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -2739,6 +2739,7 @@ static inline bool ppc_has_spr(PowerPCCPU *cpu, int spr) } #if !defined(CONFIG_USER_ONLY) +/* Sort out endianness of interrupt. Depends on the CPU, HV mode, etc. */ static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu, bool hv) { PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index a175865fa9..f19a0f2d1d 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -382,9 +382,8 @@ static void powerpc_set_excp_state(PowerPCCPU *cpu, target_ulong vector, * We don't use hreg_store_msr here as already have treated any * special case that could occur. Just store MSR and update hflags * - * Note: We *MUST* not use hreg_store_msr() as-is anyway because it - * will prevent setting of the HV bit which some exceptions might need - * to do. + * Note: We *MUST* not use hreg_store_msr() as-is anyway because it will + * prevent setting of the HV bit which some exceptions might need to do. */ env->nip = vector; env->msr = msr; @@ -426,25 +425,15 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) { CPUPPCState *env = &cpu->env; target_ulong msr, new_msr, vector; -int srr0, srr1; +int srr0 = SPR_SRR0, srr1 = SPR_SRR1; /* new srr1 value excluding must-be-zero bits */ msr = env->msr & ~0x783fULL; -/* - * new interrupt handler msr preserves existing ME unless - * explicitly overriden. - */ +/* new interrupt handler msr preserves ME unless explicitly overriden */ new_msr = env->msr & (((target_ulong)1 << MSR_ME)); -/* target registers */ -srr0 = SPR_SRR0; -srr1 = SPR_SRR1; - -/* - * Hypervisor emulation assistance interrupt only exists on server - * arch 2.05 server or later. - */ +/* HV emu assistance interrupt only exists on server arch 2.05 or later */ if (excp == POWERPC_EXCP_HV_EMU) { excp = POWERPC_EXCP_PROGRAM; } @@ -454,7 +443,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) cpu_abort(env_cpu(env), "Raised an exception without defined vector %d\n", excp); } - vector |= env->excp_prefix; switch (excp) { @@ -466,7 +454,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) powerpc_mcheck_checkstop(env); /* machine check exceptions don't have ME set */ new_msr &= ~((target_ulong)1 << MSR_ME); - srr0 = SPR_40x_SRR2; srr1 = SPR_40x_SRR3; break; @@ -537,12 +524,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) break; } -/* Save PC */ env->spr[srr0] = env->nip; - -/* Save MSR */ env->spr[srr1] = msr; - powerpc_set_excp_state(cpu, vector, new_msr); } @@ -554,16 +537,10 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) /* new srr1 value excluding must-be-zero bits */ msr = env->msr & ~0x783fULL; -/* - * new interrupt handler msr preserves existing ME unless - * explicitly overriden - */ +/* new interrupt handler msr preserves ME unless explicitly overriden */ new_msr = env->msr & ((target_ulong)1 << MSR_ME); -/* - * Hypervisor emulation assistance interrupt only exists on server - * arch 2.05 server or later. - */ +/* HV emu assistance interrupt only exists on server arch 2.05 or later */ if (excp == POWERPC_EXCP_HV_EMU) { excp = POWERPC_EXCP_PROGRAM; } @@ -573,7 +550,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) cpu_abort(env_cpu(env), "Raised an exception without defined vector %d\n", excp); } - vector |= env->excp_prefix; switch (excp) { @@ -583,7 +559,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) powerpc_mcheck_checkstop(env); /* machine check exceptions don't have ME set */ new_msr &= ~((target_ulong)1 << MSR_ME); - break; case POWERPC_EXCP_DSI: /* Data storage exception */ trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]); @@ -611,11 +586,9 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) powerpc_reset_excp_state(cpu); return; } - /* - * FP exceptions always have NIP pointing to the faulting - * instruction,
[PATCH v3 13/14] target/ppc: Clean up ifdefs in excp_helper.c, part 2
Remove check for !defined(CONFIG_USER_ONLY) as this is already within an #ifndef CONFIG_USER_ONLY block. Signed-off-by: BALATON Zoltan --- target/ppc/excp_helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 858f657128..01e61464ab 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -2483,7 +2483,7 @@ void helper_hrfid(CPUPPCState *env) } #endif /* TARGET_PPC64 */ -#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) +#ifdef TARGET_PPC64 void helper_rfebb(CPUPPCState *env, target_ulong s) { target_ulong msr = env->msr; @@ -2558,7 +2558,7 @@ void raise_ebb_perfm_exception(CPUPPCState *env) do_ebb(env, POWERPC_EXCP_PERFM_EBB); } -#endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */ +#endif /* TARGET_PPC64 */ /*/ /* Embedded PowerPC specific helpers */ -- 2.30.9
[PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv
From: Nicholas Piggin Unlike sc, for scv a facility unavailable interrupt must be generated if FSCR[SCV]=0 so we can't raise the exception with nip set to next instruction but we can move advancing nip if the FSCR check passes to helper_scv so the exception handler does not need to change it. [balaton: added commit message] Signed-off-by: BALATON Zoltan --- This needs SoB from Nick target/ppc/excp_helper.c | 2 +- target/ppc/translate.c | 6 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 903216c2a6..ef363b0285 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -1304,7 +1304,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) case POWERPC_EXCP_SYSCALL_VECTORED: /* scv exception */ lev = env->error_code; dump_syscall(env); -env->nip += 4; new_msr |= env->msr & ((target_ulong)1 << MSR_EE); new_msr |= env->msr & ((target_ulong)1 << MSR_RI); @@ -2410,6 +2409,7 @@ void helper_ppc_maybe_interrupt(CPUPPCState *env) void helper_scv(CPUPPCState *env, uint32_t lev) { if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) { +env->nip += 4; raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev); } else { raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV); diff --git a/target/ppc/translate.c b/target/ppc/translate.c index 4260d3d66f..0360a17fb3 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -4433,7 +4433,11 @@ static void gen_scv(DisasContext *ctx) { uint32_t lev = (ctx->opcode >> 5) & 0x7F; -/* Set the PC back to the faulting instruction. */ +/* + * Set the PC back to the scv instruction (unlike sc), because a facility + * unavailable interrupt must be generated if FSCR[SCV]=0. The helper + * advances nip if the FSCR check passes. + */ gen_update_nip(ctx, ctx->cia); gen_helper_scv(cpu_env, tcg_constant_i32(lev)); -- 2.30.9
[PATCH v3 05/14] target/ppc: Remove some more local CPUState variables only used once
Some helpers only have a CPUState local to call cpu_interrupt_exittb() but we can use env_cpu for that and remove the local. Signed-off-by: BALATON Zoltan --- target/ppc/excp_helper.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 122e2a6e41..a175865fa9 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -2551,8 +2551,7 @@ void helper_store_msr(CPUPPCState *env, target_ulong val) uint32_t excp = hreg_store_msr(env, val, 0); if (excp != 0) { -CPUState *cs = env_cpu(env); -cpu_interrupt_exittb(cs); +cpu_interrupt_exittb(env_cpu(env)); raise_exception(env, excp); } } @@ -2589,8 +2588,6 @@ void helper_pminsn(CPUPPCState *env, uint32_t insn) static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) { -CPUState *cs = env_cpu(env); - /* MSR:POW cannot be set by any form of rfi */ msr &= ~(1ULL << MSR_POW); @@ -2614,7 +2611,7 @@ static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr) * No need to raise an exception here, as rfi is always the last * insn of a TB */ -cpu_interrupt_exittb(cs); +cpu_interrupt_exittb(env_cpu(env)); /* Reset the reservation */ env->reserve_addr = -1; -- 2.30.9
[PATCH v3 00/14] Misc clean ups to target/ppc exception handling
These are some small clean ups for target/ppc/excp_helper.c trying to make this code a bit simpler. No functional change is intended. v2: Patch 3 changes according to review, added tags v3: Address more review comments: don't change cpu_interrupt_exittb() parameter, add back lev, add scv patch from Nick + add some more patches to clean up #ifdefs Regards, BALATON Zoltan BALATON Zoltan (13): target/ppc: Remove some superfluous parentheses target/ppc: Remove unneeded parameter from powerpc_reset_wakeup() target/ppc: Move common check in exception handlers to a function target/ppc: Use env_cpu for cpu_abort in excp_helper target/ppc: Remove some more local CPUState variables only used once target/ppc: Readability improvements in exception handlers target/ppd: Remove unused define target/ppc: Fix gen_sc to use correct nip target/ppc: Simplify syscall exception handlers target/ppc: Get CPUState in one step target/ppc: Clean up ifdefs in excp_helper.c, part 1 target/ppc: Clean up ifdefs in excp_helper.c, part 2 target/ppc: Clean up ifdefs in excp_helper.c, part 3 Nicholas Piggin (1): target/ppc: Move patching nip from exception handler to helper_scv target/ppc/cpu.h | 1 + target/ppc/excp_helper.c | 570 --- target/ppc/translate.c | 15 +- 3 files changed, 178 insertions(+), 408 deletions(-) -- 2.30.9
[PATCH v3 11/14] target/ppc: Get CPUState in one step
We can get CPUState from env with env_cpu without going through PowerPCCPU and casting that. Signed-off-by: BALATON Zoltan Acked-by: Nicholas Piggin --- target/ppc/excp_helper.c | 15 +++ 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index a62103b8ac..cfb652445a 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -1503,8 +1503,8 @@ static int p7_interrupt_powersave(CPUPPCState *env) static int p7_next_unmasked_interrupt(CPUPPCState *env) { -PowerPCCPU *cpu = env_archcpu(env); -CPUState *cs = CPU(cpu); +CPUState *cs = env_cpu(env); + /* Ignore MSR[EE] when coming out of some power management states */ bool msr_ee = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset; @@ -1593,8 +1593,8 @@ static int p8_interrupt_powersave(CPUPPCState *env) static int p8_next_unmasked_interrupt(CPUPPCState *env) { -PowerPCCPU *cpu = env_archcpu(env); -CPUState *cs = CPU(cpu); +CPUState *cs = env_cpu(env); + /* Ignore MSR[EE] when coming out of some power management states */ bool msr_ee = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset; @@ -1714,8 +1714,8 @@ static int p9_interrupt_powersave(CPUPPCState *env) static int p9_next_unmasked_interrupt(CPUPPCState *env) { -PowerPCCPU *cpu = env_archcpu(env); -CPUState *cs = CPU(cpu); +CPUState *cs = env_cpu(env); + /* Ignore MSR[EE] when coming out of some power management states */ bool msr_ee = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset; @@ -2410,9 +2410,8 @@ void helper_scv(CPUPPCState *env, uint32_t lev) void helper_pminsn(CPUPPCState *env, uint32_t insn) { -CPUState *cs; +CPUState *cs = env_cpu(env); -cs = env_cpu(env); cs->halted = 1; /* Condition for waking up at 0x100 */ -- 2.30.9
[PATCH v3 14/14] target/ppc: Clean up ifdefs in excp_helper.c, part 3
Concatenate #if blocks that are ending then beginning on the next line again. Signed-off-by: BALATON Zoltan --- target/ppc/excp_helper.c | 15 ++- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 01e61464ab..d909c61202 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -2378,10 +2378,8 @@ void helper_raise_exception(CPUPPCState *env, uint32_t exception) { raise_exception_err_ra(env, exception, 0, 0); } -#endif /* CONFIG_TCG */ #ifndef CONFIG_USER_ONLY -#ifdef CONFIG_TCG void helper_store_msr(CPUPPCState *env, target_ulong val) { uint32_t excp = hreg_store_msr(env, val, 0); @@ -2481,9 +2479,7 @@ void helper_hrfid(CPUPPCState *env) { do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]); } -#endif /* TARGET_PPC64 */ -#ifdef TARGET_PPC64 void helper_rfebb(CPUPPCState *env, target_ulong s) { target_ulong msr = env->msr; @@ -2583,10 +2579,8 @@ void helper_rfmci(CPUPPCState *env) /* FIXME: choose CSRR1 or MCSRR1 based on cpu type */ do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]); } -#endif /* CONFIG_TCG */ -#endif /* !defined(CONFIG_USER_ONLY) */ +#endif /* !CONFIG_USER_ONLY */ -#ifdef CONFIG_TCG void helper_tw(CPUPPCState *env, target_ulong arg1, target_ulong arg2, uint32_t flags) { @@ -2614,9 +2608,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, target_ulong arg2, } } #endif /* TARGET_PPC64 */ -#endif /* CONFIG_TCG */ -#ifdef CONFIG_TCG static uint32_t helper_SIMON_LIKE_32_64(uint32_t x, uint64_t key, uint32_t lane) { const uint16_t c = 0xfffc; @@ -2727,11 +2719,8 @@ HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true, NPHIE) HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false, NPHIE) HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true, PHIE) HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false, PHIE) -#endif /* CONFIG_TCG */ #ifndef CONFIG_USER_ONLY -#ifdef CONFIG_TCG - /* Embedded.Processor Control */ static int dbell2irq(target_ulong rb) { @@ -2898,5 +2887,5 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, env->error_code = insn & 0x03FF; cpu_loop_exit(cs); } -#endif /* CONFIG_TCG */ #endif /* !CONFIG_USER_ONLY */ +#endif /* CONFIG_TCG */ -- 2.30.9
[PATCH v3 08/14] target/ppc: Fix gen_sc to use correct nip
Most exceptions are raised with nip pointing to the faulting instruction but the sc instruction generating a syscall exception leaves nip pointing to next instruction. Fix gen_sc to not use gen_exception_err() which sets nip back but correctly set nip to pc_next so we don't have to patch this in the exception handlers. This changes the nip logged in dump_syscall and dump_hcall debug functions but now this matches how nip would be on a real CPU. Signed-off-by: BALATON Zoltan --- target/ppc/excp_helper.c | 39 --- target/ppc/translate.c | 8 +--- 2 files changed, 5 insertions(+), 42 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index f19a0f2d1d..903216c2a6 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -495,12 +495,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) break; case POWERPC_EXCP_SYSCALL: /* System call exception*/ dump_syscall(env); - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; break; case POWERPC_EXCP_FIT: /* Fixed-interval timer interrupt */ trace_ppc_excp_print("FIT"); @@ -611,12 +605,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) break; case POWERPC_EXCP_SYSCALL: /* System call exception*/ dump_syscall(env); - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; break; case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ case POWERPC_EXCP_DECR: /* Decrementer exception*/ @@ -759,13 +747,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp) } else { dump_syscall(env); } - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; - /* * The Virtual Open Firmware (VOF) relies on the 'sc 1' * instruction to communicate with QEMU. The pegasos2 machine @@ -910,13 +891,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp) } else { dump_syscall(env); } - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; - /* * The Virtual Open Firmware (VOF) relies on the 'sc 1' * instruction to communicate with QEMU. The pegasos2 machine @@ -1075,12 +1049,6 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp) break; case POWERPC_EXCP_SYSCALL: /* System call exception*/ dump_syscall(env); - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; break; case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ case POWERPC_EXCP_APU: /* Auxiliary processor unavailable */ @@ -1322,13 +1290,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) } else { dump_syscall(env); } - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; - /* "PAPR mode" built-in hypercall emulation */ if (lev == 1 && books_vhyp_handles_hcall(cpu)) { PPCVirtualHypervisorClass *vhc = diff --git a/target/ppc/translate.c b/target/ppc/translate.c index a32a9b8a5f..4260d3d66f 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -4419,10 +4419,12 @@ static void gen_hrfid(DisasContext *ctx) #endif static void gen_sc(DisasContext *ctx) { -uint32_t lev; +uint32_t lev = (ctx->opcode >> 5) & 0x7F; -lev = (ctx->opcode >> 5) & 0x7F; -gen_exception_err(ctx, POWERPC_SYSCALL, lev); +gen_update_nip(ctx, ctx->base.pc_next); +gen_helper_raise_exception_err(cpu_env, tcg_constant_i32(POWERPC_SYSCALL), + tcg_constant_i32(lev)); +ctx->base.is_jmp = DISAS_NORETURN; } #if defined(TARGET_PPC64) -- 2.30.9
[PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper
Use the env_cpu function to get the CPUState for cpu_abort. These are only needed in case of fatal errors so this allows to avoid casting and storing CPUState in a local variable wnen not needed. Signed-off-by: BALATON Zoltan --- target/ppc/excp_helper.c | 118 +-- 1 file changed, 63 insertions(+), 55 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 79f5ca1034..122e2a6e41 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -424,7 +424,6 @@ static void powerpc_mcheck_checkstop(CPUPPCState *env) static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) { -CPUState *cs = CPU(cpu); CPUPPCState *env = &cpu->env; target_ulong msr, new_msr, vector; int srr0, srr1; @@ -452,8 +451,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) vector = env->excp_vectors[excp]; if (vector == (target_ulong)-1ULL) { -cpu_abort(cs, "Raised an exception without defined vector %d\n", - excp); +cpu_abort(env_cpu(env), + "Raised an exception without defined vector %d\n", excp); } vector |= env->excp_prefix; @@ -502,7 +501,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) env->spr[SPR_40x_ESR] = ESR_PTR; break; default: -cpu_abort(cs, "Invalid program exception %d. Aborting\n", +cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n", env->error_code); break; } @@ -529,11 +528,12 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) trace_ppc_excp_print("PIT"); break; case POWERPC_EXCP_DEBUG: /* Debug interrupt */ -cpu_abort(cs, "%s exception not implemented\n", +cpu_abort(env_cpu(env), "%s exception not implemented\n", powerpc_excp_name(excp)); break; default: -cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp); +cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n", + excp); break; } @@ -548,7 +548,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) { -CPUState *cs = CPU(cpu); CPUPPCState *env = &cpu->env; target_ulong msr, new_msr, vector; @@ -571,8 +570,8 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) vector = env->excp_vectors[excp]; if (vector == (target_ulong)-1ULL) { -cpu_abort(cs, "Raised an exception without defined vector %d\n", - excp); +cpu_abort(env_cpu(env), + "Raised an exception without defined vector %d\n", excp); } vector |= env->excp_prefix; @@ -632,7 +631,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) break; default: /* Should never occur */ -cpu_abort(cs, "Invalid program exception %d. Aborting\n", +cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n", env->error_code); break; } @@ -654,8 +653,9 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) break; case POWERPC_EXCP_RESET: /* System reset exception */ if (FIELD_EX64(env->msr, MSR, POW)) { -cpu_abort(cs, "Trying to deliver power-saving system reset " - "exception %d with no HV support\n", excp); +cpu_abort(env_cpu(env), + "Trying to deliver power-saving system reset exception " + "%d with no HV support\n", excp); } break; case POWERPC_EXCP_TRACE: /* Trace exception */ @@ -682,11 +682,12 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) case POWERPC_EXCP_SMI: /* System management interrupt */ case POWERPC_EXCP_MEXTBR:/* Maskable external breakpoint */ case POWERPC_EXCP_NMEXTBR: /* Non maskable external breakpoint */ -cpu_abort(cs, "%s exception not implemented\n", +cpu_abort(env_cpu(env), "%s exception not implemented\n", powerpc_excp_name(excp)); break; default: -cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp); +cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n", + excp); break; } @@ -709,7 +710,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp) { -CPUState *cs = CPU(cpu); CPUPPCState *env = &cpu->env; target_ulong msr, new_msr, vector; @@ -732,8 +732,8 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp) vector = env->excp_vectors[excp]; if (vector == (target_ulong)-1ULL) { -
[PATCH v3 01/14] target/ppc: Remove some superfluous parentheses
Signed-off-by: BALATON Zoltan Acked-by: Nicholas Piggin --- target/ppc/excp_helper.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 12d8a7257b..8298217e78 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -1009,7 +1009,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp) { int lev = env->error_code; -if ((lev == 1) && cpu->vhyp) { +if (lev == 1 && cpu->vhyp) { dump_hcall(env); } else { dump_syscall(env); @@ -1027,7 +1027,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp) * uses VOF and the 74xx CPUs, so although the 74xx don't have * HV mode, we need to keep hypercall support. */ -if ((lev == 1) && cpu->vhyp) { +if (lev == 1 && cpu->vhyp) { PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp); vhc->hypercall(cpu->vhyp, cpu); @@ -1481,7 +1481,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) case POWERPC_EXCP_SYSCALL: /* System call exception*/ lev = env->error_code; -if ((lev == 1) && cpu->vhyp) { +if (lev == 1 && cpu->vhyp) { dump_hcall(env); } else { dump_syscall(env); @@ -1494,7 +1494,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) env->nip += 4; /* "PAPR mode" built-in hypercall emulation */ -if ((lev == 1) && books_vhyp_handles_hcall(cpu)) { +if (lev == 1 && books_vhyp_handles_hcall(cpu)) { PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp); vhc->hypercall(cpu->vhyp, cpu); -- 2.30.9
[PATCH v3 10/14] target/ppc: Simplify syscall exception handlers
After previous changes the hypercall handling in 7xx and 74xx exception handlers can be folded into one if statement to simpilfy this code. Signed-off-by: BALATON Zoltan --- target/ppc/excp_helper.c | 24 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index ef363b0285..a62103b8ac 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -741,25 +741,21 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp) case POWERPC_EXCP_SYSCALL: /* System call exception*/ { int lev = env->error_code; - -if (lev == 1 && cpu->vhyp) { -dump_hcall(env); -} else { -dump_syscall(env); -} /* * The Virtual Open Firmware (VOF) relies on the 'sc 1' * instruction to communicate with QEMU. The pegasos2 machine * uses VOF and the 7xx CPUs, so although the 7xx don't have * HV mode, we need to keep hypercall support. */ -if (lev == 1 && cpu->vhyp) { +if (unlikely(lev == 1 && cpu->vhyp)) { PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp); +dump_hcall(env); vhc->hypercall(cpu->vhyp, cpu); return; +} else { +dump_syscall(env); } - break; } case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ @@ -885,25 +881,21 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp) case POWERPC_EXCP_SYSCALL: /* System call exception*/ { int lev = env->error_code; - -if (lev == 1 && cpu->vhyp) { -dump_hcall(env); -} else { -dump_syscall(env); -} /* * The Virtual Open Firmware (VOF) relies on the 'sc 1' * instruction to communicate with QEMU. The pegasos2 machine * uses VOF and the 74xx CPUs, so although the 74xx don't have * HV mode, we need to keep hypercall support. */ -if (lev == 1 && cpu->vhyp) { +if (unlikely(lev == 1 && cpu->vhyp)) { PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp); +dump_hcall(env); vhc->hypercall(cpu->vhyp, cpu); return; +} else { +dump_syscall(env); } - break; } case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ -- 2.30.9
Re: [PATCH 08/10] target/ppc: Fix gen_sc to use correct nip
On Thu, 15 Jun 2023, Nicholas Piggin wrote: On Thu Jun 15, 2023 at 7:27 AM AEST, BALATON Zoltan wrote: On Wed, 14 Jun 2023, Nicholas Piggin wrote: On Mon Jun 12, 2023 at 8:42 AM AEST, BALATON Zoltan wrote: Most exceptions are raised with nip pointing to the faulting instruction but the sc instruction generating a syscall exception leaves nip pointing to next instruction. Fix gen_sc to not use gen_exception_err() which sets nip back but correctly set nip to pc_next so we don't have to patch this in the exception handlers. This changes the nip logged in dump_syscall and dump_hcall debug functions but now this matches how nip would be on a real CPU. Signed-off-by: BALATON Zoltan --- target/ppc/excp_helper.c | 39 --- target/ppc/translate.c | 8 +--- 2 files changed, 5 insertions(+), 42 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 885e479301..4f6a6dfb19 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -493,12 +493,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) break; case POWERPC_EXCP_SYSCALL: /* System call exception*/ dump_syscall(env); - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; break; case POWERPC_EXCP_FIT: /* Fixed-interval timer interrupt */ trace_ppc_excp_print("FIT"); @@ -609,12 +603,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) break; case POWERPC_EXCP_SYSCALL: /* System call exception*/ dump_syscall(env); - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; break; case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ case POWERPC_EXCP_DECR: /* Decrementer exception*/ @@ -757,13 +745,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp) } else { dump_syscall(env); } - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; - /* * The Virtual Open Firmware (VOF) relies on the 'sc 1' * instruction to communicate with QEMU. The pegasos2 machine @@ -908,13 +889,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp) } else { dump_syscall(env); } - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; - /* * The Virtual Open Firmware (VOF) relies on the 'sc 1' * instruction to communicate with QEMU. The pegasos2 machine @@ -1073,12 +1047,6 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp) break; case POWERPC_EXCP_SYSCALL: /* System call exception*/ dump_syscall(env); - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; break; case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ case POWERPC_EXCP_APU: /* Auxiliary processor unavailable */ @@ -1320,13 +1288,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) } else { dump_syscall(env); } - -/* - * We need to correct the NIP which in this case is supposed - * to point to the next instruction - */ -env->nip += 4; - /* "PAPR mode" built-in hypercall emulation */ if (lev == 1 && books_vhyp_handles_hcall(cpu)) { PPCVirtualHypervisorClass *vhc = diff --git a/target/ppc/translate.c b/target/ppc/translate.c index a32a9b8a5f..4260d3d66f 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -4419,10 +4419,12 @@ static void gen_hrfid(DisasContext *ctx) #endif static void gen_sc(DisasContext *ctx) { -uint32_t lev; +uint32_t lev = (ctx->opcode >> 5) & 0x7F; -lev = (ctx->opcode >> 5) & 0x7F; -gen_exception_err(ctx, POWERPC_SYSCALL, lev); +gen_update_nip(ctx, ctx->base.pc_next); +gen_helper_raise_exception_err(cpu_env, tcg_constant_i32(POWERPC_SYSCALL), + tcg_constant_i32(lev)); +ctx->base.is_jmp = DISAS_NORETURN; Generally for blame and bisect I don't like to mix cleanup with real change, I guess this is pretty minor though. Great cleanup though, sc is certainly defined to set SRR0 to the instruction past the sc unlike other interrupts so it is more natural and less hacky feeling do it like this. Could you do scv while you are here? It has the same s
[PATCH v3 07/14] target/ppd: Remove unused define
Commit 7a3fe174b12d removed usage of POWERPC_SYSCALL_VECTORED, drop the unused define as well. Signed-off-by: BALATON Zoltan Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Nicholas Piggin --- target/ppc/translate.c | 1 - 1 file changed, 1 deletion(-) diff --git a/target/ppc/translate.c b/target/ppc/translate.c index b591f2e496..a32a9b8a5f 100644 --- a/target/ppc/translate.c +++ b/target/ppc/translate.c @@ -4416,7 +4416,6 @@ static void gen_hrfid(DisasContext *ctx) #define POWERPC_SYSCALL POWERPC_EXCP_SYSCALL_USER #else #define POWERPC_SYSCALL POWERPC_EXCP_SYSCALL -#define POWERPC_SYSCALL_VECTORED POWERPC_EXCP_SYSCALL_VECTORED #endif static void gen_sc(DisasContext *ctx) { -- 2.30.9
[PATCH v3 03/14] target/ppc: Move common check in exception handlers to a function
All powerpc exception handlers share some code when handling machine check exceptions. Move this to a common function. Signed-off-by: BALATON Zoltan Reviewed-by: Nicholas Piggin --- target/ppc/excp_helper.c | 114 +-- 1 file changed, 25 insertions(+), 89 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 3783315fdb..79f5ca1034 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -403,6 +403,25 @@ static void powerpc_set_excp_state(PowerPCCPU *cpu, target_ulong vector, env->reserve_addr = -1; } +static void powerpc_mcheck_checkstop(CPUPPCState *env) +{ +CPUState *cs = env_cpu(env); + +if (FIELD_EX64(env->msr, MSR, ME)) { +return; +} + +/* Machine check exception is not enabled. Enter checkstop state. */ +fprintf(stderr, "Machine check while not allowed. " +"Entering checkstop state\n"); +if (qemu_log_separate()) { +qemu_log("Machine check while not allowed. " + "Entering checkstop state\n"); +} +cs->halted = 1; +cpu_interrupt_exittb(cs); +} + static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) { CPUState *cs = CPU(cpu); @@ -445,21 +464,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) srr1 = SPR_40x_SRR3; break; case POWERPC_EXCP_MCHECK:/* Machine check exception */ -if (!FIELD_EX64(env->msr, MSR, ME)) { -/* - * Machine check exception is not enabled. Enter - * checkstop state. - */ -fprintf(stderr, "Machine check while not allowed. " -"Entering checkstop state\n"); -if (qemu_log_separate()) { -qemu_log("Machine check while not allowed. " -"Entering checkstop state\n"); -} -cs->halted = 1; -cpu_interrupt_exittb(cs); -} - +powerpc_mcheck_checkstop(env); /* machine check exceptions don't have ME set */ new_msr &= ~((target_ulong)1 << MSR_ME); @@ -576,21 +581,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp) case POWERPC_EXCP_CRITICAL:/* Critical input */ break; case POWERPC_EXCP_MCHECK:/* Machine check exception */ -if (!FIELD_EX64(env->msr, MSR, ME)) { -/* - * Machine check exception is not enabled. Enter - * checkstop state. - */ -fprintf(stderr, "Machine check while not allowed. " -"Entering checkstop state\n"); -if (qemu_log_separate()) { -qemu_log("Machine check while not allowed. " -"Entering checkstop state\n"); -} -cs->halted = 1; -cpu_interrupt_exittb(cs); -} - +powerpc_mcheck_checkstop(env); /* machine check exceptions don't have ME set */ new_msr &= ~((target_ulong)1 << MSR_ME); @@ -749,21 +740,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp) switch (excp) { case POWERPC_EXCP_MCHECK:/* Machine check exception */ -if (!FIELD_EX64(env->msr, MSR, ME)) { -/* - * Machine check exception is not enabled. Enter - * checkstop state. - */ -fprintf(stderr, "Machine check while not allowed. " -"Entering checkstop state\n"); -if (qemu_log_separate()) { -qemu_log("Machine check while not allowed. " -"Entering checkstop state\n"); -} -cs->halted = 1; -cpu_interrupt_exittb(cs); -} - +powerpc_mcheck_checkstop(env); /* machine check exceptions don't have ME set */ new_msr &= ~((target_ulong)1 << MSR_ME); @@ -934,21 +911,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp) switch (excp) { case POWERPC_EXCP_MCHECK:/* Machine check exception */ -if (!FIELD_EX64(env->msr, MSR, ME)) { -/* - * Machine check exception is not enabled. Enter - * checkstop state. - */ -fprintf(stderr, "Machine check while not allowed. " -"Entering checkstop state\n"); -if (qemu_log_separate()) { -qemu_log("Machine check while not allowed. " -"Entering checkstop state\n"); -} -cs->halted = 1; -cpu_interrupt_exittb(cs); -} - +powerpc_mcheck_checkstop(env); /* machine check exceptions don't have ME set */ new_msr &= ~((target_ulong)1 << MSR_ME); @@ -1129,21 +1092,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp) srr1 = SPR_BOOKE_CSRR1; break; case POWERPC_EXCP_MCHECK:
Re: [PATCH v2 09/10] target/ppc: Simplify syscall exception handlers
On Thu, 15 Jun 2023, Nicholas Piggin wrote: On Thu Jun 15, 2023 at 7:25 PM AEST, BALATON Zoltan wrote: On Thu, 15 Jun 2023, Nicholas Piggin wrote: On Thu Jun 15, 2023 at 7:34 AM AEST, BALATON Zoltan wrote: After previous changes the hypercall handling in 7xx and 74xx exception handlers can be folded into one if statement to simpilfy this code. Signed-off-by: BALATON Zoltan --- target/ppc/excp_helper.c | 26 ++ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 1682b988ba..662457f342 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -740,26 +740,23 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp) break; case POWERPC_EXCP_SYSCALL: /* System call exception*/ { -int lev = env->error_code; I would still keep lev. Self documenting and consistent with books handler. lev is still there in the books version, but probably not really needed in these 7xx versions which does not really have level parameter. This hack should likely go away and replaced with something else on the long run as this won't work with KVM but that needs some support from VOF or compiling a different version for pegasos2 which wasn't considered so far. I can add the local back if you really insist but I don't think it really makes much sense in these cases for 7xx and 74xx. It is using the sc 1 instruction which does have a lev field though? The hardware might not have such a thing but what is being emulatd here does, so I think lev makes sense. Removing this would be fine, but while you have it yes please just leave it as lev. +PowerPCCPU *cpu = env_archcpu(env); Is this necessary? Yes, for cpu->vhyp below. cpu->vhyp was there before your patch... Originally I had another patch that chnaged these functions to take an env pointer and since cpu is only needed here this was declared local to where it's used. Now that we don't have those patches that change more functions parameters to env then it's indeed not needed. I've added back lev in v3 instead. Regards, BALATON Zoltan
Re: [PATCH v2 05/10] target/ppc: Change parameter of cpu_interrupt_exittb() to an env pointer
On Thu, 15 Jun 2023, Nicholas Piggin wrote: On Thu Jun 15, 2023 at 7:19 PM AEST, BALATON Zoltan wrote: On Thu, 15 Jun 2023, Nicholas Piggin wrote: On Thu Jun 15, 2023 at 7:34 AM AEST, BALATON Zoltan wrote: Changing the parameter of cpu_interrupt_exittb() from CPUState to env allows removing some more local CPUState variables in callers. I think it's more consistent to keep cs, which is same as cpu_interrupt(). But with this patch it's more consistent with the other functions devlared in helper_regs.h and gets rid of the #ifdef in hreg_store_msr() so I'd still like to keep this patch. Callers already have env so it should not matter. Being consistent with functions of the same file is not important or really makes sense. It's important to be consistent with functions of similar type. cpu_interrupt_exittb() is a helper to call cpu_interrupt() so makes sense to be similar. At best it seems like pointless churn. OK I've revised it in v3 and dropped most of this patch. Regards, BALATON Zoltan
Re: [PATCH V2 1/4] qapi: strList_from_string
On 6/13/2023 8:33 AM, Markus Armbruster wrote: > Steven Sistare writes: >> On 2/10/2023 4:25 AM, Markus Armbruster wrote: >>> Steven Sistare writes: On 2/9/2023 1:59 PM, Markus Armbruster wrote: > Steven Sistare writes: >> On 2/9/2023 11:46 AM, Markus Armbruster wrote: >>> Steven Sistare writes: >>> >>> [...] >>> For more context, this patch has been part of my larger series for live update, and I am submitting this separately to reduce the size of that series and make forward progress: https://lore.kernel.org/qemu-devel/1658851843-236870-1-git-send-email-steven.sist...@oracle.com/ In that series, strList_from_string is used to parse a space-separated list of args in an HMP command, and pass them to the new qemu binary. https://lore.kernel.org/qemu-devel/1658851843-236870-16-git-send-email-steven.sist...@oracle.com/ I moved and renamed the generalized function because I thought it might be useful to others in the future, along with the other functions in this 'string list functions' patch series. But if you disagree, I can minimally modify hmp_split_at_comma() in its current location. >>> >>> I'm fine with moving it out of monitor/ if there are uses outside the >>> monitor. I just don't think qapi/ is the right home. >> >> I don't know where else it would go, as strList is a QAPI type. >> include/qapi/util.h already defines QAPI_LIST_PREPEND and >> QAPI_LIST_APPEND, so it >> seems like the natural place to add qapi strList functions. I am open to >> suggestions. > > What about util/? Plenty of QAPI use there already. > > Another thought. Current hmp_split_at_comma() does two things: > > strList *hmp_split_at_comma(const char *str) > { > > One, split a comma-separated string into NULL-terminated a dynamically > allocated char *[]: > > char **split = g_strsplit(str ?: "", ",", -1); > > Two, convert a dynamically allocated char *[] into a strList: > > strList *res = NULL; > strList **tail = &res; > int i; > > for (i = 0; split[i]; i++) { > QAPI_LIST_APPEND(tail, split[i]); > } > > g_free(split); > return res; > } > > Part two could live in qapi/. Works for me. >>> >>> Note that I'm not demanding such a split. I'm merely throwing in >>> another idea for you to use or reject. >> >> I decided to not split the function. IMO having part 2 free memory allocated >> by its caller is not clean. >> >> However, I will base it on your original function, slightly modified: >> >> strList *strList_from_string(const char *str, char *delim) >> { >> g_autofree char **split = g_strsplit(str ?: "", delim, -1); >> strList *res = NULL; >> strList **tail = &res; >> >> for (; *split; split++) { >> QAPI_LIST_APPEND(tail, *split); >> } >> >> return res; >> } >> For future reference, what is your organizing principle for putting things in qapi/ vs util/ ? I see plenty of calls to g_str* functions from qapi/*, so I don't know why removing g_strsplit changes the answer. Per your principle, where does strv_from_strList (patch 3) belong? And if I substitute char ** for GStrv, does the answer change? >>> >>> As is, qapi/qapi-util provides: >>> >>> 1. Helpers for qapi/ and QAPI-generated code. Some of them are >>>used elsewhere, too. That's fine. >>> >>> 2. Tools for working with QAPI data types such as GenericList. >>> >>> strv_from_strList() would fall under 2. Same if you use char ** >>> instead. >>> >>> hmp_split_at_comma() admittedly also falls under 2. I just dislike >>> putting things under qapi/ that contradict QAPI design principles. >> >> What design principle does strList_from_string contradict? Are you OK with >> putting the simplified version shown above in qapi-util? > > The design principle is "use JSON to encode structured data as text in > QAPI/QMP". > > Do: "mumble": [1, 2, 3] > > Don't: "mumble": "1,2,3" I don't mumble, but I sometimes mutter and ramble. > We violate the principle in a couple of places. Some are arguably > mistakes, some are pragmatic exceptions. > > The principle implies "the only parser QAPI needs is the JSON parser". > > By adding other parsers to QAPI, we send a misleading signal, namely > that encoding structured data in a way that requires parsing is okay. > It's not, generally. > > So, I'd prefer to find another home for code that splits strings at > comma / delimiter. > >> (and apologies for my long delay in continuing this conversation). > > I'm in no position to take offense there ;) Thanks, that makes it clear. I propo
Re: QEMU virt (arm64) does not honor reserved-memory set in device tree
Hi Gavin, Thanks for the reply. I am new to Linux dev in general and not familiar with the ACPI table, but I will research in the area and give it a try. Sorry for the late response. -Yusuf On Fri, Jun 9, 2023, 8:36 PM Gavin Shan wrote: > Hi Mohd, > > On 6/10/23 10:01 AM, Mohd Yusuf Abdul Hamid wrote: > > I am trying to reserve a portion of the system memory in QEMU (arm64 > virt), v7.2.1 - but the kernel never honors the reserved memory area and > keeps using the area. > > > > Say, I dumped out DTB and added: > > > > reserved-memory { > >#address-cells = <0x02>; > >#size-cells = <0x02>; > > > >rsvdram@5000 { > >no-map; > >reg = <0x00 0x5000 0x00 0x2000>; > >}; > > }; > > > > When booted, /proc/iomem still shows the kernel is using the entire > space - eg 2GB. > > > > Is this a supported feature or I would need to modify the virt.c and > define scratch area for some device driver scratch area. > > > > It relies on the guest kernel to handle the device-tree and the > device-tree node > for the reserved map. I doubt if you had ACPI over device-tree in the > guest kernel's > configuration. In this case, the reserved memory regions need to be > specified in > ACPI tables instead of device-tree. > > Thanks, > Gavin > >
[PATCH V1 0/3] fix migration of suspended runstate
Migration of a guest in the suspended runstate is broken. The incoming migration code automatically tries to wake the guest, which IMO is wrong -- the guest should end migration in the same runstate it started. Further, the automatic wakeup fails. The guest appears to be running, but is not. See the commit messages for the details. Steve Sistare (3): vl: start on wakeup request migration: fix suspended runstate tests/qtest: live migration suspended state include/sysemu/runstate.h| 1 + migration/migration.c| 11 +++- softmmu/runstate.c | 16 +++- tests/migration/i386/Makefile| 5 ++-- tests/migration/i386/a-b-bootblock.S | 49 +--- tests/migration/i386/a-b-bootblock.h | 22 ++-- tests/qtest/migration-helpers.c | 2 +- tests/qtest/migration-test.c | 31 +-- 8 files changed, 112 insertions(+), 25 deletions(-) -- 1.8.3.1
[PATCH V1 2/3] migration: fix suspended runstate
Migration of a guest in the suspended state is broken. The incoming migration code automatically tries to wake the guest, which IMO is wrong -- the guest should end migration in the same state it started. Further, the wakeup is done by calling qemu_system_wakeup_request(), which bypasses vm_start(). The guest appears to be in the running state, but it is not. To fix, leave the guest in the suspended state, but call qemu_system_start_on_wakeup_request() so the guest is properly resumed later, when the client sends a system_wakeup command. Signed-off-by: Steve Sistare --- migration/migration.c | 11 --- softmmu/runstate.c| 1 + 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/migration/migration.c b/migration/migration.c index 17b4b47..851fe6d 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -496,6 +496,10 @@ static void process_incoming_migration_bh(void *opaque) vm_start(); } else { runstate_set(global_state_get_runstate()); +if (runstate_check(RUN_STATE_SUSPENDED)) { +/* Force vm_start to be called later. */ +qemu_system_start_on_wakeup_request(); +} } /* * This must happen after any state changes since as soon as an external @@ -2101,7 +2105,6 @@ static int postcopy_start(MigrationState *ms) qemu_mutex_lock_iothread(); trace_postcopy_start_set_run(); -qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL); global_state_store(); ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); if (ret < 0) { @@ -2307,7 +2310,6 @@ static void migration_completion(MigrationState *s) if (s->state == MIGRATION_STATUS_ACTIVE) { qemu_mutex_lock_iothread(); s->downtime_start = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); -qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL); s->vm_old_state = runstate_get(); global_state_store(); @@ -3102,11 +3104,6 @@ static void *bg_migration_thread(void *opaque) qemu_mutex_lock_iothread(); -/* - * If VM is currently in suspended state, then, to make a valid runstate - * transition in vm_stop_force_state() we need to wakeup it up. - */ -qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, NULL); s->vm_old_state = runstate_get(); global_state_store(); diff --git a/softmmu/runstate.c b/softmmu/runstate.c index e127b21..771896c 100644 --- a/softmmu/runstate.c +++ b/softmmu/runstate.c @@ -159,6 +159,7 @@ static const RunStateTransition runstate_transitions_def[] = { { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED }, { RUN_STATE_SUSPENDED, RUN_STATE_RUNNING }, { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE }, +{ RUN_STATE_SUSPENDED, RUN_STATE_PAUSED }, { RUN_STATE_SUSPENDED, RUN_STATE_PRELAUNCH }, { RUN_STATE_SUSPENDED, RUN_STATE_COLO}, -- 1.8.3.1
[PATCH V1 3/3] tests/qtest: live migration suspended state
Add a test case to verify that the suspended state is handled correctly in live migration. The test suspends the src, migrates, then wakes the dest. Add an option to suspend the src in a-b-bootblock.S, which puts the guest in S3 state after one round of writing to memory. The option is enabled by poking a 1 into the suspend_me word in the boot block prior to starting the src vm. Generate symbol offsets in a-b-bootblock.h so that the suspend_me offset is known. Signed-off-by: Steve Sistare --- tests/migration/i386/Makefile| 5 ++-- tests/migration/i386/a-b-bootblock.S | 49 +--- tests/migration/i386/a-b-bootblock.h | 22 ++-- tests/qtest/migration-helpers.c | 2 +- tests/qtest/migration-test.c | 31 +-- 5 files changed, 92 insertions(+), 17 deletions(-) diff --git a/tests/migration/i386/Makefile b/tests/migration/i386/Makefile index 5c03241..37a72ae 100644 --- a/tests/migration/i386/Makefile +++ b/tests/migration/i386/Makefile @@ -4,9 +4,10 @@ .PHONY: all clean all: a-b-bootblock.h -a-b-bootblock.h: x86.bootsect +a-b-bootblock.h: x86.bootsect x86.o echo "$$__note" > header.tmp xxd -i $< | sed -e 's/.*int.*//' >> header.tmp + nm x86.o | awk '{print "#define SYM_"$$3" 0x"$$1}' >> header.tmp mv header.tmp $@ x86.bootsect: x86.boot @@ -16,7 +17,7 @@ x86.boot: x86.o $(CROSS_PREFIX)objcopy -O binary $< $@ x86.o: a-b-bootblock.S - $(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@ + $(CROSS_PREFIX)gcc -I.. -m32 -march=i486 -c $< -o $@ clean: @rm -rf *.boot *.o *.bootsect diff --git a/tests/migration/i386/a-b-bootblock.S b/tests/migration/i386/a-b-bootblock.S index 3d464c7..63d446f 100644 --- a/tests/migration/i386/a-b-bootblock.S +++ b/tests/migration/i386/a-b-bootblock.S @@ -9,6 +9,21 @@ # # Author: dgilb...@redhat.com +#include "migration-test.h" + +#define ACPI_ENABLE 0xf1 +#define ACPI_PORT_SMI_CMD 0xb2 +#define ACPI_PM_BASE0x600 +#define PM1A_CNT_OFFSET 4 + +#define ACPI_SCI_ENABLE 0x0001 +#define ACPI_SLEEP_TYPE 0x0400 +#define ACPI_SLEEP_ENABLE 0x2000 +#define SLEEP (ACPI_SCI_ENABLE + ACPI_SLEEP_TYPE + ACPI_SLEEP_ENABLE) + +#define LOW_ADDRX86_TEST_MEM_START +#define HIGH_ADDR X86_TEST_MEM_END +#define suspended HIGH_ADDR .code16 .org 0x7c00 @@ -41,12 +56,11 @@ start: # at 0x7c00 ? # bl keeps a counter so we limit the output speed mov $0, %bl mainloop: -# Start from 1MB -mov $(1024*1024),%eax +mov $LOW_ADDR,%eax innerloop: incb (%eax) add $4096,%eax -cmp $(100*1024*1024),%eax +cmp $HIGH_ADDR,%eax jl innerloop inc %bl @@ -57,7 +71,30 @@ innerloop: mov $0x3f8,%dx outb %al,%dx -jmp mainloop +# should this test suspend? +mov (suspend_me),%eax +cmp $0,%eax +je mainloop + +# are we waking after suspend? do not suspend again. +mov $suspended,%eax +mov (%eax),%eax +cmp $1,%eax +je mainloop + +# enable acpi +mov $ACPI_ENABLE,%al +outb %al,$ACPI_PORT_SMI_CMD + +# suspend to ram +mov $suspended,%eax +movl $1,(%eax) +mov $SLEEP,%ax +mov $(ACPI_PM_BASE + PM1A_CNT_OFFSET),%dx +outw %ax,%dx +# not reached. The wakeup causes reset and restart at 0x7c00, and we +# do not save and restore registers as a real kernel would do. + # GDT magic from old (GPLv2) Grub startup.S .p2align2 /* force 4-byte alignment */ @@ -83,6 +120,10 @@ gdtdesc: .word 0x27/* limit */ .long gdt /* addr */ +/* test launcher can poke a 1 here to exercise suspend */ +suspend_me: +.int 0 + /* I'm a bootable disk */ .org 0x7dfe .byte 0x55 diff --git a/tests/migration/i386/a-b-bootblock.h b/tests/migration/i386/a-b-bootblock.h index b7b0fce..b39773f 100644 --- a/tests/migration/i386/a-b-bootblock.h +++ b/tests/migration/i386/a-b-bootblock.h @@ -4,20 +4,20 @@ * the header and the assembler differences in your patch submission. */ unsigned char x86_bootsect[] = { - 0xfa, 0x0f, 0x01, 0x16, 0x78, 0x7c, 0x66, 0xb8, 0x01, 0x00, 0x00, 0x00, + 0xfa, 0x0f, 0x01, 0x16, 0xa4, 0x7c, 0x66, 0xb8, 0x01, 0x00, 0x00, 0x00, 0x0f, 0x22, 0xc0, 0x66, 0xea, 0x20, 0x7c, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xe4, 0x92, 0x0c, 0x02, 0xe6, 0x92, 0xb8, 0x10, 0x00, 0x00, 0x00, 0x8e, 0xd8, 0x66, 0xb8, 0x41, 0x00, 0x66, 0xba, 0xf8, 0x03, 0xee, 0xb3, 0x00, 0xb8, 0x00, 0x00, 0x10, 0x00, 0xfe, 0x00, 0x05, 0x00, 0x10, 0x00, 0x00, 0x3d, 0x00, 0x00, 0x40, 0x06, 0x7c, 0xf2, 0xfe, 0xc3, 0x80, 0xe3, 0x3f, 0x75, 0xe6, 0x66, 0xb8, - 0x42, 0x00, 0x66, 0xba, 0xf8, 0x03, 0xee, 0xeb, 0
[PATCH V1 1/3] vl: start on wakeup request
If qemu starts and loads a VM in the suspended state, then a later wakeup request sets the state to running and tries to start execution, but it bypasses vm_start() and its initialization steps, which is fatal for the guest. See qemu_system_wakeup_request(), and qemu_system_wakeup() in main_loop_should_exit(). Define the start_on_wakeup_requested() hook to cause vm_start() to be called when processing the wakeup request. This will be called in a subsequent migration patch. Signed-off-by: Steve Sistare --- include/sysemu/runstate.h | 1 + softmmu/runstate.c| 15 ++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h index 7beb29c..c12ad7d 100644 --- a/include/sysemu/runstate.h +++ b/include/sysemu/runstate.h @@ -57,6 +57,7 @@ void qemu_system_reset_request(ShutdownCause reason); void qemu_system_suspend_request(void); void qemu_register_suspend_notifier(Notifier *notifier); bool qemu_wakeup_suspend_enabled(void); +void qemu_system_start_on_wakeup_request(void); void qemu_system_wakeup_request(WakeupReason reason, Error **errp); void qemu_system_wakeup_enable(WakeupReason reason, bool enabled); void qemu_register_wakeup_notifier(Notifier *notifier); diff --git a/softmmu/runstate.c b/softmmu/runstate.c index 1957caf..e127b21 100644 --- a/softmmu/runstate.c +++ b/softmmu/runstate.c @@ -343,6 +343,7 @@ void vm_state_notify(bool running, RunState state) } } +static bool start_on_wakeup_requested; static ShutdownCause reset_requested; static ShutdownCause shutdown_requested; static int shutdown_signal; @@ -568,6 +569,11 @@ void qemu_register_suspend_notifier(Notifier *notifier) notifier_list_add(&suspend_notifiers, notifier); } +void qemu_system_start_on_wakeup_request(void) +{ +start_on_wakeup_requested = true; +} + void qemu_system_wakeup_request(WakeupReason reason, Error **errp) { trace_system_wakeup_request(reason); @@ -580,7 +586,14 @@ void qemu_system_wakeup_request(WakeupReason reason, Error **errp) if (!(wakeup_reason_mask & (1 << reason))) { return; } -runstate_set(RUN_STATE_RUNNING); + +if (start_on_wakeup_requested) { +start_on_wakeup_requested = false; +vm_start(); +} else { +runstate_set(RUN_STATE_RUNNING); +} + wakeup_reason = reason; qemu_notify_event(); } -- 1.8.3.1
[PULL 3/6] aspeed: Introduce a boot_rom region at the machine level
This should also avoid Coverity to report a memory leak warning when the QEMU process exits. See CID 1508061. Reviewed-by: Francisco Iglesias Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Cédric Le Goater Reviewed-by: Joel Stanley Signed-off-by: Cédric Le Goater --- hw/arm/aspeed.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index bfc2070bd2ed..76a1e7303de1 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -40,6 +40,7 @@ struct AspeedMachineState { /* Public */ AspeedSoCState soc; +MemoryRegion boot_rom; bool mmio_exec; char *fmc_model; char *spi_model; @@ -275,15 +276,15 @@ static void write_boot_rom(BlockBackend *blk, hwaddr addr, size_t rom_size, * Create a ROM and copy the flash contents at the expected address * (0x0). Boots faster than execute-in-place. */ -static void aspeed_install_boot_rom(AspeedSoCState *soc, BlockBackend *blk, +static void aspeed_install_boot_rom(AspeedMachineState *bmc, BlockBackend *blk, uint64_t rom_size) { -MemoryRegion *boot_rom = g_new(MemoryRegion, 1); +AspeedSoCState *soc = &bmc->soc; -memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom", rom_size, +memory_region_init_rom(&bmc->boot_rom, NULL, "aspeed.boot_rom", rom_size, &error_abort); memory_region_add_subregion_overlap(&soc->spi_boot_container, 0, -boot_rom, 1); +&bmc->boot_rom, 1); write_boot_rom(blk, ASPEED_SOC_SPI_BOOT_ADDR, rom_size, &error_abort); } @@ -431,8 +432,7 @@ static void aspeed_machine_init(MachineState *machine) if (mtd0) { uint64_t rom_size = memory_region_size(&bmc->soc.spi_boot); -aspeed_install_boot_rom(&bmc->soc, blk_by_legacy_dinfo(mtd0), -rom_size); +aspeed_install_boot_rom(bmc, blk_by_legacy_dinfo(mtd0), rom_size); } } -- 2.40.1
[PULL 2/6] aspeed/hace: Initialize g_autofree pointer
As mentioned in docs/devel/style.rst "Automatic memory deallocation": * Variables declared with g_auto* MUST always be initialized, otherwise the cleanup function will use uninitialized stack memory This avoids QEMU to coredump when running the "hash test" command under Zephyr. Cc: Steven Lee Cc: Joel Stanley Cc: qemu-sta...@nongnu.org Fixes: c5475b3f9a ("hw: Model ASPEED's Hash and Crypto Engine") Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alex Bennée Reviewed-by: Thomas Huth Reviewed-by: Francisco Iglesias Message-Id: <20230421131547.2177449-1-...@kaod.org> Signed-off-by: Cédric Le Goater Reviewed-by: Joel Stanley Signed-off-by: Cédric Le Goater --- hw/misc/aspeed_hace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c index 12a761f1f55d..b07506ec04ef 100644 --- a/hw/misc/aspeed_hace.c +++ b/hw/misc/aspeed_hace.c @@ -189,7 +189,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode, bool acc_mode) { struct iovec iov[ASPEED_HACE_MAX_SG]; -g_autofree uint8_t *digest_buf; +g_autofree uint8_t *digest_buf = NULL; size_t digest_len = 0; int niov = 0; int i; -- 2.40.1
[PULL 4/6] aspeed: Use the boot_rom region of the fby35 machine
This change completes commits 5aa281d757 ("aspeed: Introduce a spi_boot region under the SoC") and 8b744a6a47 ("aspeed: Add a boot_rom overlap region in the SoC spi_boot container") which introduced a spi_boot container at the SoC level to map the boot rom region as an overlap. It also fixes a Coverity report (CID 1508061) for a memory leak warning when the QEMU process exits by using an bmc_boot_rom MemoryRegion available at the machine level. Cc: Peter Delevoryas Signed-off-by: Cédric Le Goater Reviewed-by: Joel Stanley Signed-off-by: Cédric Le Goater --- hw/arm/fby35.c | 29 +++-- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c index f4600c290b62..f2ff6c1abfd9 100644 --- a/hw/arm/fby35.c +++ b/hw/arm/fby35.c @@ -70,8 +70,6 @@ static void fby35_bmc_write_boot_rom(DriveInfo *dinfo, MemoryRegion *mr, static void fby35_bmc_init(Fby35State *s) { -DriveInfo *drive0 = drive_get(IF_MTD, 0, 0); - object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3"); memory_region_init(&s->bmc_memory, OBJECT(&s->bmc), "bmc-memory", @@ -95,18 +93,21 @@ static void fby35_bmc_init(Fby35State *s) aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0); /* Install first FMC flash content as a boot rom. */ -if (drive0) { -AspeedSMCFlash *fl = &s->bmc.fmc.flashes[0]; -MemoryRegion *boot_rom = g_new(MemoryRegion, 1); -uint64_t size = memory_region_size(&fl->mmio); - -if (!s->mmio_exec) { -memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom", - size, &error_abort); -memory_region_add_subregion(&s->bmc_memory, FBY35_BMC_FIRMWARE_ADDR, -boot_rom); -fby35_bmc_write_boot_rom(drive0, boot_rom, FBY35_BMC_FIRMWARE_ADDR, - size, &error_abort); +if (!s->mmio_exec) { +DriveInfo *mtd0 = drive_get(IF_MTD, 0, 0); + +if (mtd0) { +AspeedSoCState *bmc = &s->bmc; +uint64_t rom_size = memory_region_size(&bmc->spi_boot); + +memory_region_init_rom(&s->bmc_boot_rom, NULL, "aspeed.boot_rom", + rom_size, &error_abort); +memory_region_add_subregion_overlap(&bmc->spi_boot_container, 0, +&s->bmc_boot_rom, 1); + +fby35_bmc_write_boot_rom(mtd0, &s->bmc_boot_rom, + FBY35_BMC_FIRMWARE_ADDR, + rom_size, &error_abort); } } } -- 2.40.1
[PULL 1/6] hw/arm/aspeed: Add VPD data for Rainier machine
From: Ninad Palsule The current modeling of Rainier machine creates zero filled VPDs(EEPROMs). This makes some services and applications unhappy and causing them to fail. Hence this drop adds some fabricated data for system and BMC FRU so that vpd services are happy and active. Tested: - The system-vpd.service is active. - VPD service related to bmc is active. Signed-off-by: Ninad Palsule Reviewed-by: Cédric Le Goater [ clg: commit title cleanup ] Signed-off-by: Cédric Le Goater --- hw/arm/aspeed_eeprom.h | 5 + hw/arm/aspeed.c| 6 -- hw/arm/aspeed_eeprom.c | 45 +- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/hw/arm/aspeed_eeprom.h b/hw/arm/aspeed_eeprom.h index 86db6f0479b7..bbf9e54365b8 100644 --- a/hw/arm/aspeed_eeprom.h +++ b/hw/arm/aspeed_eeprom.h @@ -22,4 +22,9 @@ extern const size_t fby35_bmc_fruid_len; extern const uint8_t yosemitev2_bmc_fruid[]; extern const size_t yosemitev2_bmc_fruid_len; +extern const uint8_t rainier_bb_fruid[]; +extern const size_t rainier_bb_fruid_len; +extern const uint8_t rainier_bmc_fruid[]; +extern const size_t rainier_bmc_fruid_len; + #endif diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 0b29028fe115..bfc2070bd2ed 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -788,8 +788,10 @@ static void rainier_bmc_i2c_init(AspeedMachineState *bmc) 0x48); i2c_slave_create_simple(aspeed_i2c_get_bus(&soc->i2c, 8), TYPE_TMP105, 0x4a); -at24c_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 8), 0x50, 64 * KiB); -at24c_eeprom_init(aspeed_i2c_get_bus(&soc->i2c, 8), 0x51, 64 * KiB); +at24c_eeprom_init_rom(aspeed_i2c_get_bus(&soc->i2c, 8), 0x50, + 64 * KiB, rainier_bb_fruid, rainier_bb_fruid_len); +at24c_eeprom_init_rom(aspeed_i2c_get_bus(&soc->i2c, 8), 0x51, + 64 * KiB, rainier_bmc_fruid, rainier_bmc_fruid_len); create_pca9552(soc, 8, 0x60); create_pca9552(soc, 8, 0x61); /* Bus 8: ucd90320@11 */ diff --git a/hw/arm/aspeed_eeprom.c b/hw/arm/aspeed_eeprom.c index dc33a88a5466..ace5266cec91 100644 --- a/hw/arm/aspeed_eeprom.c +++ b/hw/arm/aspeed_eeprom.c @@ -119,9 +119,52 @@ const uint8_t yosemitev2_bmc_fruid[] = { 0x6e, 0x66, 0x69, 0x67, 0x20, 0x41, 0xc1, 0x45, }; +const uint8_t rainier_bb_fruid[] = { +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x84, +0x28, 0x00, 0x52, 0x54, 0x04, 0x56, 0x48, 0x44, 0x52, 0x56, 0x44, 0x02, +0x01, 0x00, 0x50, 0x54, 0x0e, 0x56, 0x54, 0x4f, 0x43, 0x00, 0x00, 0x37, +0x00, 0x4a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x46, 0x08, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x46, 0x00, 0x52, 0x54, +0x04, 0x56, 0x54, 0x4f, 0x43, 0x50, 0x54, 0x38, 0x56, 0x49, 0x4e, 0x49, +0x00, 0x00, 0x81, 0x00, 0x3a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x56, 0x53, +0x59, 0x53, 0x00, 0x00, 0xbb, 0x00, 0x27, 0x00, 0x00, 0x00, 0x00, 0x00, +0x56, 0x43, 0x45, 0x4e, 0x00, 0x00, 0xe2, 0x00, 0x27, 0x00, 0x00, 0x00, +0x00, 0x00, 0x56, 0x53, 0x42, 0x50, 0x00, 0x00, 0x09, 0x01, 0x19, 0x00, +0x00, 0x00, 0x00, 0x00, 0x50, 0x46, 0x01, 0x00, 0x00, 0x00, 0x36, 0x00, +0x52, 0x54, 0x04, 0x56, 0x49, 0x4e, 0x49, 0x44, 0x52, 0x04, 0x44, 0x45, +0x53, 0x43, 0x48, 0x57, 0x02, 0x30, 0x31, 0x43, 0x43, 0x04, 0x33, 0x34, +0x35, 0x36, 0x46, 0x4e, 0x04, 0x46, 0x52, 0x34, 0x39, 0x53, 0x4e, 0x04, +0x53, 0x52, 0x31, 0x32, 0x50, 0x4e, 0x04, 0x50, 0x52, 0x39, 0x39, 0x50, +0x46, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x23, 0x00, 0x52, 0x54, +0x04, 0x56, 0x53, 0x59, 0x53, 0x53, 0x45, 0x07, 0x49, 0x42, 0x4d, 0x53, +0x59, 0x53, 0x31, 0x54, 0x4d, 0x08, 0x32, 0x32, 0x32, 0x32, 0x2d, 0x32, +0x32, 0x32, 0x50, 0x46, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x23, +0x00, 0x52, 0x54, 0x04, 0x56, 0x43, 0x45, 0x4e, 0x53, 0x45, 0x07, 0x31, +0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x46, 0x43, 0x08, 0x31, 0x31, 0x31, +0x31, 0x2d, 0x31, 0x31, 0x31, 0x50, 0x46, 0x04, 0x00, 0x00, 0x00, 0x00, +0x00, 0x00, 0x15, 0x00, 0x52, 0x54, 0x04, 0x56, 0x53, 0x42, 0x50, 0x49, +0x4d, 0x04, 0x50, 0x00, 0x10, 0x01, 0x50, 0x46, 0x04, 0x00, 0x00, 0x00, +0x00, 0x00, +}; + +/* Rainier BMC FRU */ +const uint8_t rainier_bmc_fruid[] = { +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x84, +0x28, 0x00, 0x52, 0x54, 0x04, 0x56, 0x48, 0x44, 0x52, 0x56, 0x44, 0x02, +0x01, 0x00, 0x50, 0x54, 0x0e, 0x56, 0x54, 0x4f, 0x43, 0x00, 0x00, 0x37, +0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x46, 0x08, 0x00, 0x00, +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1c, 0x00, 0x52, 0x54, +0x04, 0x56, 0x54, 0x4f, 0x43, 0x50, 0x54, 0x0e, 0x56, 0x49, 0x4e, 0x49, +0x00, 0x00, 0x57, 0x00, 0x1e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x46, +0x01, 0x00, 0x00, 0x00, 0x1a, 0x00, 0x52, 0x54, 0x04, 0x56, 0x49, 0x4e, +0x49, 0x44, 0x52, 0x04, 0x44, 0x45, 0x53, 0x43, 0x48
[PULL 5/6] aspeed: Introduce a "bmc-console" machine option
Most of the Aspeed machines use the UART5 device for the boot console, and QEMU connects the first serial Chardev to this SoC device for this purpose. See routine connect_serial_hds_to_uarts(). Nevertheless, some machines use another boot console, such as the fuji, and commit 5d63d0c76c ("hw/arm/aspeed: Allow machine to set UART default") introduced a SoC class attribute 'uart_default' and property to be able to change the boot console device. It was later changed by commit d2b3eaefb4 ("aspeed: Refactor UART init for multi-SoC machines"). The "bmc-console" machine option goes a step further and lets the user define the UART device from the QEMU command line without introducing a new machine definition. For instance, to use device UART3 (mapped on /dev/ttyS2 under Linux) instead of the default UART5, one would use : -M ast2500-evb,bmc-console=uart3 Cc: Abhishek Singh Dagur Signed-off-by: Cédric Le Goater Reviewed-by: Joel Stanley Signed-off-by: Cédric Le Goater --- docs/system/arm/aspeed.rst | 11 +++ hw/arm/aspeed.c| 40 -- 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/docs/system/arm/aspeed.rst b/docs/system/arm/aspeed.rst index d4e293e7f986..80538422a1a4 100644 --- a/docs/system/arm/aspeed.rst +++ b/docs/system/arm/aspeed.rst @@ -122,6 +122,11 @@ Options specific to Aspeed machines are : * ``spi-model`` to change the SPI Flash model. + * ``bmc-console`` to change the default console device. Most of the + machines use the ``UART5`` device for a boot console, which is + mapped on ``/dev/ttyS4`` under Linux, but it is not always the + case. + For instance, to start the ``ast2500-evb`` machine with a different FMC chip and a bigger (64M) SPI chip, use : @@ -129,6 +134,12 @@ FMC chip and a bigger (64M) SPI chip, use : -M ast2500-evb,fmc-model=mx25l25635e,spi-model=mx66u51235f +To change the boot console and use device ``UART3`` (``/dev/ttyS2`` +under Linux), use : + +.. code-block:: bash + + -M ast2500-evb,bmc-console=uart3 Aspeed minibmc family boards (``ast1030-evb``) == diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 76a1e7303de1..6880998484cd 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -42,6 +42,7 @@ struct AspeedMachineState { AspeedSoCState soc; MemoryRegion boot_rom; bool mmio_exec; +uint32_t uart_chosen; char *fmc_model; char *spi_model; }; @@ -333,10 +334,11 @@ static void connect_serial_hds_to_uarts(AspeedMachineState *bmc) AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc); AspeedSoCState *s = &bmc->soc; AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); +int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : amc->uart_default; -aspeed_soc_uart_set_chr(s, amc->uart_default, serial_hd(0)); +aspeed_soc_uart_set_chr(s, uart_chosen, serial_hd(0)); for (int i = 1, uart = ASPEED_DEV_UART1; i < sc->uarts_num; i++, uart++) { -if (uart == amc->uart_default) { +if (uart == uart_chosen) { continue; } aspeed_soc_uart_set_chr(s, uart, serial_hd(i)); @@ -1078,6 +1080,35 @@ static void aspeed_set_spi_model(Object *obj, const char *value, Error **errp) bmc->spi_model = g_strdup(value); } +static char *aspeed_get_bmc_console(Object *obj, Error **errp) +{ +AspeedMachineState *bmc = ASPEED_MACHINE(obj); +AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc); +int uart_chosen = bmc->uart_chosen ? bmc->uart_chosen : amc->uart_default; + +return g_strdup_printf("uart%d", uart_chosen - ASPEED_DEV_UART1 + 1); +} + +static void aspeed_set_bmc_console(Object *obj, const char *value, Error **errp) +{ +AspeedMachineState *bmc = ASPEED_MACHINE(obj); +AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(bmc); +AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(amc->soc_name)); +int val; + +if (sscanf(value, "uart%u", &val) != 1) { +error_setg(errp, "Bad value for \"uart\" property"); +return; +} + +/* The number of UART depends on the SoC */ +if (val < 1 || val > sc->uarts_num) { +error_setg(errp, "\"uart\" should be in range [1 - %d]", sc->uarts_num); +return; +} +bmc->uart_chosen = ASPEED_DEV_UART1 + val - 1; +} + static void aspeed_machine_class_props_init(ObjectClass *oc) { object_class_property_add_bool(oc, "execute-in-place", @@ -1086,6 +1117,11 @@ static void aspeed_machine_class_props_init(ObjectClass *oc) object_class_property_set_description(oc, "execute-in-place", "boot directly from CE0 flash device"); +object_class_property_add_str(oc, "bmc-console", aspeed_get_bmc_console, + aspeed_set_bmc_console); +object_class_property_set_description(oc, "bmc-console", + "Change the default UART to \"uartX\""); +
[PULL 6/6] target/arm: Allow users to set the number of VFP registers
Cortex A7 CPUs with an FPU implementing VFPv4 without NEON support have 16 64-bit FPU registers and not 32 registers. Let users set the number of VFP registers with a CPU property. The primary use case of this property is for the Cortex A7 of the Aspeed AST2600 SoC. Signed-off-by: Cédric Le Goater Reviewed-by: Joel Stanley Reviewed-by: Peter Maydell Signed-off-by: Cédric Le Goater --- target/arm/cpu.h| 2 ++ hw/arm/aspeed_ast2600.c | 2 ++ target/arm/cpu.c| 32 3 files changed, 36 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 36c608f0e6e1..af0119addfb6 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -924,6 +924,8 @@ struct ArchCPU { bool has_pmu; /* CPU has VFP */ bool has_vfp; +/* CPU has 32 VFP registers */ +bool has_vfp_d32; /* CPU has Neon */ bool has_neon; /* CPU has M-profile DSP extension */ diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c index 1bf12461481c..a8b3a8065a11 100644 --- a/hw/arm/aspeed_ast2600.c +++ b/hw/arm/aspeed_ast2600.c @@ -316,6 +316,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp) &error_abort); object_property_set_bool(OBJECT(&s->cpu[i]), "neon", false, &error_abort); +object_property_set_bool(OBJECT(&s->cpu[i]), "vfp-d32", false, +&error_abort); object_property_set_link(OBJECT(&s->cpu[i]), "memory", OBJECT(s->memory), &error_abort); diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 4d5bb57f0797..353fc4856739 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -1277,6 +1277,9 @@ static Property arm_cpu_cfgend_property = static Property arm_cpu_has_vfp_property = DEFINE_PROP_BOOL("vfp", ARMCPU, has_vfp, true); +static Property arm_cpu_has_vfp_d32_property = +DEFINE_PROP_BOOL("vfp-d32", ARMCPU, has_vfp_d32, true); + static Property arm_cpu_has_neon_property = DEFINE_PROP_BOOL("neon", ARMCPU, has_neon, true); @@ -1408,6 +1411,22 @@ void arm_cpu_post_init(Object *obj) } } +if (cpu->has_vfp && cpu_isar_feature(aa32_simd_r32, cpu)) { +cpu->has_vfp_d32 = true; +if (!kvm_enabled()) { +/* + * The permitted values of the SIMDReg bits [3:0] on + * Armv8-A are either 0b and 0b0010. On such CPUs, + * make sure that has_vfp_d32 can not be set to false. + */ +if (!(arm_feature(&cpu->env, ARM_FEATURE_V8) && + !arm_feature(&cpu->env, ARM_FEATURE_M))) { +qdev_property_add_static(DEVICE(obj), + &arm_cpu_has_vfp_d32_property); +} +} +} + if (arm_feature(&cpu->env, ARM_FEATURE_NEON)) { cpu->has_neon = true; if (!kvm_enabled()) { @@ -1674,6 +1693,19 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) return; } +if (cpu->has_vfp_d32 != cpu->has_neon) { +error_setg(errp, "ARM CPUs must have both VFP-D32 and Neon or neither"); +return; +} + + if (!cpu->has_vfp_d32) { +uint32_t u; + +u = cpu->isar.mvfr0; +u = FIELD_DP32(u, MVFR0, SIMDREG, 1); /* 16 registers */ +cpu->isar.mvfr0 = u; +} + if (!cpu->has_vfp) { uint64_t t; uint32_t u; -- 2.40.1
[PULL 0/6] aspeed queue
The following changes since commit 7efd65423ab22e6f5890ca08ae40c84d6660242f: Merge tag 'pull-riscv-to-apply-20230614' of https://github.com/alistair23/qemu into staging (2023-06-14 05:28:51 +0200) are available in the Git repository at: https://github.com/legoater/qemu/ tags/pull-aspeed-20230615 for you to fetch changes up to 42bea956f6f7477c06186c7add62fa0107a27a9c: target/arm: Allow users to set the number of VFP registers (2023-06-15 18:35:58 +0200) aspeed queue: * extension of the rainier machine with VPD contents * fixes for Coverity issues * new "bmc-console" machine option * new "vfp-d32" ARM CPU property Cédric Le Goater (5): aspeed/hace: Initialize g_autofree pointer aspeed: Introduce a boot_rom region at the machine level aspeed: Use the boot_rom region of the fby35 machine aspeed: Introduce a "bmc-console" machine option target/arm: Allow users to set the number of VFP registers Ninad Palsule (1): hw/arm/aspeed: Add VPD data for Rainier machine docs/system/arm/aspeed.rst | 11 + hw/arm/aspeed_eeprom.h | 5 target/arm/cpu.h | 2 ++ hw/arm/aspeed.c| 58 ++ hw/arm/aspeed_ast2600.c| 2 ++ hw/arm/aspeed_eeprom.c | 45 ++- hw/arm/fby35.c | 29 --- hw/misc/aspeed_hace.c | 2 +- target/arm/cpu.c | 32 + 9 files changed, 160 insertions(+), 26 deletions(-)
Re: [RFC v3] linux-user/riscv: Add syscall riscv_hwprobe
On Thu, 08 Jun 2023 00:55:22 PDT (-0700), r...@rivosinc.com wrote: > This patch adds the new syscall for the > "RISC-V Hardware Probing Interface" > (https://docs.kernel.org/riscv/hwprobe.html). > > Signed-off-by: Robbin Ehn > --- > v1->v2: Moved to syscall.c > v2->v3: Separate function, get/put user > --- > linux-user/riscv/syscall32_nr.h | 1 + > linux-user/riscv/syscall64_nr.h | 1 + > linux-user/syscall.c| 146 > 3 files changed, 148 insertions(+) > > diff --git a/linux-user/riscv/syscall32_nr.h b/linux-user/riscv/syscall32_nr.h > index 1327d7dffa..412e58e5b2 100644 > --- a/linux-user/riscv/syscall32_nr.h > +++ b/linux-user/riscv/syscall32_nr.h > @@ -228,6 +228,7 @@ > #define TARGET_NR_accept4 242 > #define TARGET_NR_arch_specific_syscall 244 > #define TARGET_NR_riscv_flush_icache (TARGET_NR_arch_specific_syscall + 15) > +#define TARGET_NR_riscv_hwprobe (TARGET_NR_arch_specific_syscall + 14) > #define TARGET_NR_prlimit64 261 > #define TARGET_NR_fanotify_init 262 > #define TARGET_NR_fanotify_mark 263 > diff --git a/linux-user/riscv/syscall64_nr.h b/linux-user/riscv/syscall64_nr.h > index 6659751933..29e1eb2075 100644 > --- a/linux-user/riscv/syscall64_nr.h > +++ b/linux-user/riscv/syscall64_nr.h > @@ -251,6 +251,7 @@ > #define TARGET_NR_recvmmsg 243 > #define TARGET_NR_arch_specific_syscall 244 > #define TARGET_NR_riscv_flush_icache (TARGET_NR_arch_specific_syscall + 15) > +#define TARGET_NR_riscv_hwprobe (TARGET_NR_arch_specific_syscall + 14) > #define TARGET_NR_wait4 260 > #define TARGET_NR_prlimit64 261 > #define TARGET_NR_fanotify_init 262 > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 83685f0aa5..e8859cd3be 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -8874,6 +8874,147 @@ static int do_getdents64(abi_long dirfd, abi_long > arg2, abi_long count) > } > #endif /* TARGET_NR_getdents64 */ > > +#if defined(TARGET_NR_riscv_hwprobe) > + > +#define RISCV_HWPROBE_KEY_MVENDORID 0 > +#define RISCV_HWPROBE_KEY_MARCHID 1 > +#define RISCV_HWPROBE_KEY_MIMPID2 > + > +#define RISCV_HWPROBE_KEY_BASE_BEHAVIOR 3 > +#define RISCV_HWPROBE_BASE_BEHAVIOR_IMA (1 << 0) > + > +#define RISCV_HWPROBE_KEY_IMA_EXT_0 4 > +#define RISCV_HWPROBE_IMA_FD (1 << 0) > +#define RISCV_HWPROBE_IMA_C(1 << 1) > + > +#define RISCV_HWPROBE_KEY_CPUPERF_0 5 > +#define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) > +#define RISCV_HWPROBE_MISALIGNED_EMULATED(1 << 0) > +#define RISCV_HWPROBE_MISALIGNED_SLOW(2 << 0) > +#define RISCV_HWPROBE_MISALIGNED_FAST(3 << 0) > +#define RISCV_HWPROBE_MISALIGNED_UNSUPPORTED (4 << 0) > +#define RISCV_HWPROBE_MISALIGNED_MASK(7 << 0) > + > +struct riscv_hwprobe { > +abi_llong key; > +abi_ullong value; > +}; > + > +static void risc_hwprobe_fill_pairs(CPURISCVState *env, > +struct riscv_hwprobe *pair, > +size_t pair_count) > +{ > +const RISCVCPUConfig *cfg = riscv_cpu_cfg(env); > + > +for (; pair_count > 0; pair_count--, pair++) { > +abi_llong key; > +abi_ullong value; > +__put_user(0, &pair->value); > +__get_user(key, &pair->key); > +switch (key) { > +case RISCV_HWPROBE_KEY_MVENDORID: > +__put_user(cfg->mvendorid, &pair->value); > +break; > +case RISCV_HWPROBE_KEY_MARCHID: > +__put_user(cfg->marchid, &pair->value); > +break; > +case RISCV_HWPROBE_KEY_MIMPID: > +__put_user(cfg->mimpid, &pair->value); > +break; > +case RISCV_HWPROBE_KEY_BASE_BEHAVIOR: > +value = riscv_has_ext(env, RVI) && > +riscv_has_ext(env, RVM) && > +riscv_has_ext(env, RVA) ? > +RISCV_HWPROBE_BASE_BEHAVIOR_IMA : 0; > +__put_user(value, &pair->value); > +break; > +case RISCV_HWPROBE_KEY_IMA_EXT_0: > +value = riscv_has_ext(env, RVF) && > +riscv_has_ext(env, RVD) ? > +RISCV_HWPROBE_IMA_FD : 0; > +value |= riscv_has_ext(env, RVC) ? > + RISCV_HWPROBE_IMA_C : pair->value; > +__put_user(value, &pair->value); > +break; > +case RISCV_HWPROBE_KEY_CPUPERF_0: > +__put_user(RISCV_HWPROBE_MISALIGNED_FAST, &pair->value); > +break; > +default: > +__put_user(-1, &pair->key); > +break; > +} > +} > +} > + > +static int cpu_set_valid(abi_long arg3, abi_long arg4) > +{ > +int ret, i, tmp; > +size_t host_mask_size, target_mask_size; > +unsigned long *host_mask; > + > +/* > + * cpu_set_t represent CPU masks as bit masks of type unsigned long *. > + * arg3 contains the cpu count. > + */ > +tmp = (8 * sizeof(abi_ulon
Re: [PATCH 3/4] target/tricore: Honour privilege changes on PSW write
On Thu, Jun 15, 2023 at 05:15:28PM +0200, Bastian Koppelmann wrote: > On Thu, Jun 15, 2023 at 09:37:23AM +0200, Richard Henderson wrote: > > On 6/14/23 18:59, Bastian Koppelmann wrote: > > > void helper_psw_write(CPUTriCoreState *env, uint32_t arg) > > > { > > > +uint32_t old_priv, new_priv; > > > +CPUState *cs; > > > + > > > +old_priv = extract32(env->PSW, 10, 2); > > > psw_write(env, arg); > > > +new_priv = extract32(env->PSW, 10, 2); > > > + > > > +if (old_priv != new_priv) { > > > +cs = env_cpu(env); > > > +env->PC = env->PC + 4; > > > +cpu_loop_exit(cs); > > > +} > > > } > > > > I think you should unconditionally end the TB after write to PSW. I think > > that you should not manipulate the PC here, nor use cpu_loop_exit. > > > > You should add > > > > #define DISAS_EXIT DISAS_TARGET_0 > > #define DISAS_EXIT_UPDATE DISAS_TARGET_1 > > ok. > > > > > > @@ -378,6 +379,7 @@ static inline void gen_mtcr(DisasContext *ctx, TCGv > > > r1, > > > if (ctx->priv == TRICORE_PRIV_SM) { > > > /* since we're caching PSW make this a special case */ > > > if (offset == 0xfe04) { > > > +gen_save_pc(ctx->base.pc_next); > > > gen_helper_psw_write(cpu_env, r1); > > > > Instead set ctx->base.is_jmp = DISAS_EXIT, > > > > and in tricore_tr_tb_stop add > > > > case DISAS_EXIT_UPDATE: > > gen_save_pc(ctx->base.pc_next); > > /* fall through */ > > case DISAS_EXIT: > > tcg_gen_exit_tb(NULL, 0); > > break; > > > > There are a number of places (e.g. rfe), which can then use DISAS_EXIT > > instead of issuing the exit directly. > > ok. > > > > > I'll also say that there are a number of other places using tcg_gen_exit_tb > > which should instead be using tcg_gen_lookup_and_goto_ptr -- all of the > > indirect branches for instance. I would suggest adding > > > > #define DISAS_JUMPDISAS_TARGET_2 > > > > to handle those, again with the code within tricore_tr_tb_stop. > > I'll look into that. However, this is out of scope for this patch series. > > > > > Finally, does JLI really clobber A[11] before branching to A[a]? > > If so, this could use a comment, because it looks like a bug. > > Yes, it does. A[11] is the link register (not only by convention), so it is > hard > coded to save the return address to A[11]. See [1] page 29. Why does it look > like a bug to you? You're right this is a bug. If A[a] = A[11], then we're overwriting the jump address. We have to save A[a] into a temp and then save A[11]. Thanks for finding this! Cheers, Bastian
Re: [PATCH 3/4] target/tricore: Honour privilege changes on PSW write
On Thu, Jun 15, 2023 at 09:37:23AM +0200, Richard Henderson wrote: > On 6/14/23 18:59, Bastian Koppelmann wrote: > > void helper_psw_write(CPUTriCoreState *env, uint32_t arg) > > { > > +uint32_t old_priv, new_priv; > > +CPUState *cs; > > + > > +old_priv = extract32(env->PSW, 10, 2); > > psw_write(env, arg); > > +new_priv = extract32(env->PSW, 10, 2); > > + > > +if (old_priv != new_priv) { > > +cs = env_cpu(env); > > +env->PC = env->PC + 4; > > +cpu_loop_exit(cs); > > +} > > } > > I think you should unconditionally end the TB after write to PSW. I think > that you should not manipulate the PC here, nor use cpu_loop_exit. > > You should add > > #define DISAS_EXIT DISAS_TARGET_0 > #define DISAS_EXIT_UPDATE DISAS_TARGET_1 ok. > > > @@ -378,6 +379,7 @@ static inline void gen_mtcr(DisasContext *ctx, TCGv r1, > > if (ctx->priv == TRICORE_PRIV_SM) { > > /* since we're caching PSW make this a special case */ > > if (offset == 0xfe04) { > > +gen_save_pc(ctx->base.pc_next); > > gen_helper_psw_write(cpu_env, r1); > > Instead set ctx->base.is_jmp = DISAS_EXIT, > > and in tricore_tr_tb_stop add > > case DISAS_EXIT_UPDATE: > gen_save_pc(ctx->base.pc_next); > /* fall through */ > case DISAS_EXIT: > tcg_gen_exit_tb(NULL, 0); > break; > > There are a number of places (e.g. rfe), which can then use DISAS_EXIT > instead of issuing the exit directly. ok. > > I'll also say that there are a number of other places using tcg_gen_exit_tb > which should instead be using tcg_gen_lookup_and_goto_ptr -- all of the > indirect branches for instance. I would suggest adding > > #define DISAS_JUMPDISAS_TARGET_2 > > to handle those, again with the code within tricore_tr_tb_stop. I'll look into that. However, this is out of scope for this patch series. > > Finally, does JLI really clobber A[11] before branching to A[a]? > If so, this could use a comment, because it looks like a bug. Yes, it does. A[11] is the link register (not only by convention), so it is hard coded to save the return address to A[11]. See [1] page 29. Why does it look like a bug to you? Thanks, Bastian [1] https://www.infineon.com/dgdl/Infineon-AURIX_TC3xx_Architecture_vol1-UserManual-v01_00-EN.pdf?fileId=5546d46276fb756a01771bc4c2e33bdd
Re: [PATCH v2 1/6] target/riscv: Add properties for BF16 extensions
On Thu, 2023-06-15 at 21:14 +0800, Weiwei Li wrote: > > On 2023/6/15 20:58, Rob Bradford wrote: > > On Thu, 2023-06-15 at 14:32 +0800, Weiwei Li wrote: > > > Add ext_zfbfmin/zvfbfmin/zvfbfwma properties. > > > Add require check for BF16 extensions. > > > > > > Signed-off-by: Weiwei Li > > > Signed-off-by: Junqiang Wang > > > Reviewed-by: Daniel Henrique Barboza > > > --- > > > target/riscv/cpu.c | 20 > > > target/riscv/cpu_cfg.h | 3 +++ > > > 2 files changed, 23 insertions(+) > > > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > > index 881bddf393..dc6b2f72f6 100644 > > > --- a/target/riscv/cpu.c > > > +++ b/target/riscv/cpu.c > > > @@ -1059,6 +1059,11 @@ void > > > riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > > > return; > > > } > > > > > > + if (cpu->cfg.ext_zfbfmin && !riscv_has_ext(env, RVF)) { > > > + error_setg(errp, "Zfbfmin extension depends on F > > > extension"); > > > + return; > > > + } > > > + > > > if (riscv_has_ext(env, RVD) && !riscv_has_ext(env, RVF)) { > > > error_setg(errp, "D extension requires F extension"); > > > return; > > > @@ -1109,6 +1114,21 @@ void > > > riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > > > return; > > > } > > > > > > + if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zfbfmin) { > > > + error_setg(errp, "Zvfbfmin extension depends on Zfbfmin > > > extension"); > > > + return; > > > + } > > > + > > > + if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zve32f) { > > > + error_setg(errp, "Zvfbfmin extension depends on Zve32f > > > extension"); > > > + return; > > > + } > > I don't think this is correct - from the spec: > > > > "This extension [Zvfbfmin] depends on the Zfbfmin extension and > > either > > the "V" extension or the Zve32f embedded vector extension." > > > > So this should be: > > > > + if (cpu->cfg.ext_zvfbfmin && !(cpu->cfg.ext_zve32f || cpu- > > > cfg.ext_v) { > > + error_setg(errp, "Zvfbfmin extension depends on Zve32f or > > V > > extension"); > > + return; > > + } > > Zve32f will be enabled when V is enabled. So we can simply check > Zve32f > here. Great, thank you for the clarification - I missed that this this enforced directly above. Cheers, Rob > > Regards, > > Weiwei Li > > > Cheers, > > > > Rob > > > > > + > > > + if (cpu->cfg.ext_zvfbfwma && !cpu->cfg.ext_zvfbfmin) { > > > + error_setg(errp, "Zvfbfwma extension depends on Zvfbfmin > > > extension"); > > > + return; > > > + } > > > + > > > /* Set the ISA extensions, checks should have happened > > > above */ > > > if (cpu->cfg.ext_zhinx) { > > > cpu->cfg.ext_zhinxmin = true; > > > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h > > > index c4a627d335..7d16f32720 100644 > > > --- a/target/riscv/cpu_cfg.h > > > +++ b/target/riscv/cpu_cfg.h > > > @@ -75,6 +75,7 @@ struct RISCVCPUConfig { > > > bool ext_svpbmt; > > > bool ext_zdinx; > > > bool ext_zawrs; > > > + bool ext_zfbfmin; > > > bool ext_zfh; > > > bool ext_zfhmin; > > > bool ext_zfinx; > > > @@ -84,6 +85,8 @@ struct RISCVCPUConfig { > > > bool ext_zve64f; > > > bool ext_zve64d; > > > bool ext_zmmul; > > > + bool ext_zvfbfmin; > > > + bool ext_zvfbfwma; > > > bool ext_zvfh; > > > bool ext_zvfhmin; > > > bool ext_smaia; >
Re: [PATCH V2] migration: file URI
Peter Xu writes: > On Wed, Jun 14, 2023 at 02:59:54PM -0300, Fabiano Rosas wrote: >> In this message Daniel mentions virDomainSnapshotXXX which would benefit >> from using the same "file" migration, but being done live: >> >> https://lore.kernel.org/r/zd7mrgq+4qsdb...@redhat.com >> >> And from your response here: >> https://lore.kernel.org/r/ZEA759BSs75ldW6Y@x1n >> >> I had understood that having a new SUSPEND cap to decide whether to do >> it live or non-live would be enough to cover all use-cases. > > Oh, I probably lost some of the contexts there, sorry about that - so it's > about not being able to live snapshot on !LINUX worlds properly, am I > right? > Right, so that gives us for now a reasonable use-case for keeping live migration behavior possible with "file:". > In the ideal world where we can always synchronously tracking guest pages > (like what we do with userfaultfd wr-protections on modern Linux), the > !SUSPEND case should always be covered by CAP_BACKGROUND_SNAPSHOT already > in a more performant way. IOW, !SUSPEND seems to be not useful to Linux, > because whenever we want to set !SUSPEND we should just use BG_SNAPSHOT. > I agree. > But I think indeed the live snapshot support is not good enough. Even on > Linux, it lacks different memory type supports, multi-process support, and > also no-go on very old kernels. So I assume the fallback makes sense, and > then we can't always rely on that. > > Then I agree we can keep "file:" the same as others like proposed here, but > I'd like to double check with all of us so we're on the same page.. +1 > And maybe we should mention some discussions into commit message or > comments where proper in the code, so we can track what has happened > easier. > I'll add some words where appropriate in my series as well. A v2 is already overdue with all the refactorings that have happened in the migration code.
[PATCH v3] imx_serial: set wake bit when we receive a data byte
The Linux kernel added a flood check for RX data recently in commit 496a4471b7c3 ("serial: imx: work-around for hardware RX flood"). This check uses the wake bit in the UART status register 2. The wake bit indicates that the receiver detected a start bit on the RX line. If the kernel sees a number of RX interrupts without the wake bit being set, it treats this as spurious data and resets the UART port. imx_serial does never set the wake bit and triggers the kernel's flood check. This patch adds support for the wake bit. wake is set when we receive a new character (it's not set for break events). It seems that wake is cleared by the kernel driver, the hardware does not have to clear it automatically after data was read. The wake bit can be configured as an interrupt source. Support this mechanism as well. Co-developed-by: Philippe Mathieu-Daudé Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Martin Kaiser --- v3: - fix some spelling mistakes in the commit message - add Philippe's Reviewed-by v2: - support interrupts from wake - clean up the commit message hw/char/imx_serial.c | 5 - include/hw/char/imx_serial.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c index ee1375e26d..1b75a89588 100644 --- a/hw/char/imx_serial.c +++ b/hw/char/imx_serial.c @@ -80,7 +80,7 @@ static void imx_update(IMXSerialState *s) * TCEN and TXDC are both bit 3 * RDR and DREN are both bit 0 */ -mask |= s->ucr4 & (UCR4_TCEN | UCR4_DREN); +mask |= s->ucr4 & (UCR4_WKEN | UCR4_TCEN | UCR4_DREN); usr2 = s->usr2 & mask; @@ -321,6 +321,9 @@ static void imx_put_data(void *opaque, uint32_t value) static void imx_receive(void *opaque, const uint8_t *buf, int size) { +IMXSerialState *s = (IMXSerialState *)opaque; + +s->usr2 |= USR2_WAKE; imx_put_data(opaque, *buf); } diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h index 91c9894ad5..b823f94519 100644 --- a/include/hw/char/imx_serial.h +++ b/include/hw/char/imx_serial.h @@ -71,6 +71,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IMXSerialState, IMX_SERIAL) #define UCR4_DREN BIT(0)/* Receive Data Ready interrupt enable */ #define UCR4_TCEN BIT(3)/* TX complete interrupt enable */ +#define UCR4_WKEN BIT(7)/* WAKE interrupt enable */ #define UTS1_TXEMPTY(1<<6) #define UTS1_RXEMPTY(1<<5) -- 2.30.2
[PATCH] contrib/plugins: add meson build file
Add crossplatform Meson file to build TCG plugins since the Makefile makes wrong assumptions about it being used only on Linux. Tested on Linux and macOS. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1710 Signed-off-by: Anton Kochkov --- contrib/plugins/meson.build | 31 +++ contrib/plugins/meson_options.txt | 1 + 2 files changed, 32 insertions(+) create mode 100644 contrib/plugins/meson.build create mode 100644 contrib/plugins/meson_options.txt diff --git a/contrib/plugins/meson.build b/contrib/plugins/meson.build new file mode 100644 index 00..72c4167461 --- /dev/null +++ b/contrib/plugins/meson.build @@ -0,0 +1,31 @@ +project('qemu-plugins', 'c', meson_version: '>=0.50.0') + +qemu_src = get_option('qemu_path') +if qemu_src == '' + qemu_src = '../..' +endif + +qemu_include = qemu_src + '/include/qemu' +incdir = include_directories(qemu_include) + +plugins = [ + 'execlog', + 'hotblocks', + 'hotpages', + 'howvec', + 'lockstep', + 'hwprofile', + 'cache', + 'drcov', +] + +th = dependency('threads', required: true) +glib = dependency('glib-2.0', required: true) + +foreach p: plugins + library(p, p + '.c', +include_directories: incdir, +dependencies: [th, glib], +override_options: ['b_lundef=false'] + ) +endforeach diff --git a/contrib/plugins/meson_options.txt b/contrib/plugins/meson_options.txt new file mode 100644 index 00..2d76cda496 --- /dev/null +++ b/contrib/plugins/meson_options.txt @@ -0,0 +1 @@ +option('qemu_path', type : 'string', value : '', description : 'Full path to the QEMU sources to build plugins for') -- 2.40.1
[PATCH 0/1] Secondary Cadence GEM IRQs
In testing RTEMS on the ZynqMP platform, I noticed that priority queues were not functioning properly. I tracked this down to an unconnected interrupt source in the Cadence GEM when multiple priority queues are configured. I'm not sure if the Cadence IP can actually be configured for separate interrupt sources per queue, so I opted to leave the multiple IRQ sources intact and connect the additional sources on platforms that need it. If it makes more sense to route all interrutps for this peripheral through a single interrupt source, I can submit that patch instead.
[PATCH] hw/arm/xlnx: Connect secondary CGEM IRQs
The Cadence GEM peripherals as configured for Zynq MPSoC and Versal platforms have two priority queues with separate interrupt sources for each. If the interrupt source for the second priority queue is not connected, they work in polling mode only. This change connects the second interrupt source for platforms where it is available. This patch has been tested using the lwIP stack with a Xilinx-supplied driver from their embeddedsw repository. Signed-off-by: Kinsey Moore --- hw/arm/xlnx-versal.c | 1 + hw/arm/xlnx-zynqmp.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c index 60bf5fe657..a9e06b7fd1 100644 --- a/hw/arm/xlnx-versal.c +++ b/hw/arm/xlnx-versal.c @@ -272,6 +272,7 @@ static void versal_create_gems(Versal *s, qemu_irq *pic) memory_region_add_subregion(&s->mr_ps, addrs[i], mr); sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[irqs[i]]); +sysbus_connect_irq(SYS_BUS_DEVICE(dev), 1, pic[irqs[i]]); g_free(name); } } diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c index 5905a33015..b919b38e91 100644 --- a/hw/arm/xlnx-zynqmp.c +++ b/hw/arm/xlnx-zynqmp.c @@ -635,6 +635,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) sysbus_mmio_map(SYS_BUS_DEVICE(&s->gem[i]), 0, gem_addr[i]); sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem[i]), 0, gic_spi[gem_intr[i]]); +sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem[i]), 1, + gic_spi[gem_intr[i]]); } for (i = 0; i < XLNX_ZYNQMP_NUM_UARTS; i++) { -- 2.30.2
Re: [PATCH v5 1/9] migration: Add switchover ack capability
On 6/15/23 14:38, YangHang Liu wrote: Test in the following two scenarios: [1] Test scenario: Both source VM and target VM (in listening mode) have enabled return-path and switchover-ack capability: Test result : The VFIO migration completed successfully [2] Test scenario : The source VM has enabled return-path and switchover-ack capability while the target VM (in listening mode) not Test result : The VFIO migration fails The detailed error thrown by qemu-kvm when VFIO migration fails: Target VM: :17:00.2: Received INIT_DATA_SENT but switchover ack is not used error while loading state section id 81(:00:02.4:00.0/vfio) load of migration failed: Invalid argument Source VM: failed to save SaveStateEntry with id(name): 2(ram): -5 Unable to write to socket: Connection reset by peer Unable to write to socket: Connection reset by peer Tested-by: YangHang Liu Some more info, Tests were performed with a mainline Linux and a mainline QEMU including this series - patch8. The amount of precopy data for a CX-7 VF is not very large. Any idea how to generate some more initial state with such devices ? I suppose pre-copy will be more important with vGPUs. YangHang, Could you please reply with a Tested-by on the cover letter, so that the whole series is tagged and not only patch 1. Thanks, C. On Wed, May 31, 2023 at 1:46 AM Avihai Horon wrote: Migration downtime estimation is calculated based on bandwidth and remaining migration data. This assumes that loading of migration data in the destination takes a negligible amount of time and that downtime depends only on network speed. While this may be true for RAM, it's not necessarily true for other migrated devices. For example, loading the data of a VFIO device in the destination might require from the device to allocate resources, prepare internal data structures and so on. These operations can take a significant amount of time which can increase migration downtime. This patch adds a new capability "switchover ack" that prevents the source from stopping the VM and completing the migration until an ACK is received from the destination that it's OK to do so. This can be used by migrated devices in various ways to reduce downtime. For example, a device can send initial precopy metadata to pre-allocate resources in the destination and use this capability to make sure that the pre-allocation is completed before the source VM is stopped, so it will have full effect. This new capability relies on the return path capability to communicate from the destination back to the source. The actual implementation of the capability will be added in the following patches. Signed-off-by: Avihai Horon Reviewed-by: Peter Xu Acked-by: Markus Armbruster --- qapi/migration.json | 12 +++- migration/options.h | 1 + migration/options.c | 21 + 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/qapi/migration.json b/qapi/migration.json index 179af0c4d8..061ea512e0 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -487,6 +487,16 @@ # and should not affect the correctness of postcopy migration. # (since 7.1) # +# @switchover-ack: If enabled, migration will not stop the source VM +# and complete the migration until an ACK is received from the +# destination that it's OK to do so. Exactly when this ACK is +# sent depends on the migrated devices that use this feature. +# For example, a device can use it to make sure some of its data +# is sent and loaded in the destination before doing switchover. +# This can reduce downtime if devices that support this capability +# are present. 'return-path' capability must be enabled to use +# it. (since 8.1) +# # Features: # # @unstable: Members @x-colo and @x-ignore-shared are experimental. @@ -502,7 +512,7 @@ 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate', { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] }, 'validate-uuid', 'background-snapshot', - 'zero-copy-send', 'postcopy-preempt'] } + 'zero-copy-send', 'postcopy-preempt', 'switchover-ack'] } ## # @MigrationCapabilityStatus: diff --git a/migration/options.h b/migration/options.h index 45991af3c2..9aaf363322 100644 --- a/migration/options.h +++ b/migration/options.h @@ -40,6 +40,7 @@ bool migrate_postcopy_ram(void); bool migrate_rdma_pin_all(void); bool migrate_release_ram(void); bool migrate_return_path(void); +bool migrate_switchover_ack(void); bool migrate_validate_uuid(void); bool migrate_xbzrle(void); bool migrate_zero_blocks(void); diff --git a/migration/options.c b/migration/options.c index b62ab30cd5..16007afca6 100644 --- a/migration/options.c +++ b/migration/options.c @@ -185,6 +185,8 @@ Property migration_properties[] = { DEFINE_PROP_MIG_CAP("x-zero-copy-send",
[PATCH] chardev/char-win-stdio: Support VT sequences on Windows 11 host
If the monitor or the serial port use STDIO as backend on Windows 11 host, e.g. -nographic options is used, the monitor or the guest Linux do not response to arrow keys. When Windows creates a console, ENABLE_VIRTUAL_PROCESS_INPUT is disabled by default. Arrow keys cannot be retrieved by ReadFile or ReadConsoleInput functions. Add ENABLE_VIRTUAL_PROCESS_INPUT to the flag which is passed to SetConsoleMode, when opening stdio console. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1674 Signed-off-by: Zhang Huasen --- chardev/char-win-stdio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/chardev/char-win-stdio.c b/chardev/char-win-stdio.c index eb830eabd9..1a18999e78 100644 --- a/chardev/char-win-stdio.c +++ b/chardev/char-win-stdio.c @@ -190,7 +190,7 @@ static void qemu_chr_open_stdio(Chardev *chr, } } -dwMode |= ENABLE_LINE_INPUT; +dwMode |= ENABLE_LINE_INPUT | ENABLE_VIRTUAL_TERMINAL_INPUT; if (is_console) { /* set the terminal in raw mode */ -- 2.41.0.windows.1
Re: [PATCH QEMU v5 6/8] migration: Implement dirty-limit convergence algo
On Wed, Jun 14, 2023 at 1:50 AM Juan Quintela wrote: > ~hyman wrote: > > From: Hyman Huang(黄勇) > > To speed thinkng up, 1-5 are included on next Migration PULL request. > OK, I'll post the next version only contain the last 3 commits. > > Implement dirty-limit convergence algo for live migration, > > which is kind of like auto-converge algo but using dirty-limit > > instead of cpu throttle to make migration convergent. > > > > Enable dirty page limit if dirty_rate_high_cnt greater than 2 > > when dirty-limit capability enabled, Disable dirty-limit if > > migration be cancled. > > Nit: canceled. get it. > > > > Note that "set_vcpu_dirty_limit", "cancel_vcpu_dirty_limit" > > commands are not allowed during dirty-limit live migration. > > > > Signed-off-by: Hyman Huang(黄勇) > > Signed-off-by: Markus Armbruster > > > > + * Enable dirty-limit to throttle down the guest > > + */ > > +static void migration_dirty_limit_guest(void) > > +{ > > +static int64_t quota_dirtyrate; > > quota_dirtyrate deserves at least a comment. > > I guess it means the current quota_dirty_rate that is set, but no clue. OK. I'll comment it next version. > > > +MigrationState *s = migrate_get_current(); > > + > > +/* > > + * If dirty limit already enabled and migration parameter > > + * vcpu-dirty-limit untouched. > > + */ > > +if (dirtylimit_in_service() && > > +quota_dirtyrate == s->parameters.vcpu_dirty_limit) { > > +return; > > +} > > + > > +quota_dirtyrate = s->parameters.vcpu_dirty_limit; > > + > > +/* Set or update quota dirty limit */ > > +qmp_set_vcpu_dirty_limit(false, -1, quota_dirtyrate, NULL); > > Care to explain why do we have to "reset" the quota? Or why we can't > set it when the user issues the command, only when we throttle the guest? Indeed, -1 is misleading, the first parameter means the set all vcpu a quota dirtyrate, and the second parameter is meaningless if the first parameter is false. The comment will be like this next version? /* Set all vCPU a quota dirtyrate, note that the second parameter will be ignored if setting all vCPU for a vm. */ > > +trace_migration_dirty_limit_guest(quota_dirtyrate); > > +} > > + > > Split this patch in two: > > a - the logic change > b - the introduction of dirty limit. > > Ok, get it. > > Old code: > > /* During block migration the auto-converge logic incorrectly detects > * that ram migration makes no progress. Avoid this by disabling the > * throttling logic during the bulk phase of block migration. */ > if (blk_mig_bulk_active()) { > return; > } > > if (migrate_auto_converge()) { > /* The following detection logic can be refined later. For now: >Check to see if the ratio between dirtied bytes and the approx. >amount of bytes that just got transferred since the last time >we were in this routine reaches the threshold. If that happens >twice, start or increase throttling. */ > > if ((bytes_dirty_period > bytes_dirty_threshold) && > (++rs->dirty_rate_high_cnt >= 2)) { > trace_migration_throttle(); > rs->dirty_rate_high_cnt = 0; > mig_throttle_guest_down(bytes_dirty_period, > bytes_dirty_threshold); > } > } > > New code: > /* > * The following detection logic can be refined later. For now: > * Check to see if the ratio between dirtied bytes and the approx. > * amount of bytes that just got transferred since the last time > * we were in this routine reaches the threshold. If that happens > * twice, start or increase throttling. > */ > > if ((bytes_dirty_period > bytes_dirty_threshold) && > (++rs->dirty_rate_high_cnt >= 2)) { > rs->dirty_rate_high_cnt = 0; > /* > * During block migration the auto-converge logic incorrectly > detects > * that ram migration makes no progress. Avoid this by disabling > the > * throttling logic during the bulk phase of block migration > */ > if (blk_mig_bulk_active()) { > return; > } > > if (migrate_auto_converge()) { > trace_migration_throttle(); > mig_throttle_guest_down(bytes_dirty_period, > bytes_dirty_threshold); > } else if (migrate_dirty_limit()) { > migration_dirty_limit_guest(); > } > } > > Questions: > > - Why are we changing blk_mig_bulk_active() position? > I think that the old code have it in the right place. Additionally, > you just changefd to this version a couple of patches agon. > Yes, indeed, this modification make no sense, i'll fix it next version. > > > > > > int64_t cpu_index, > > Error **errp) > > { > > +MigrationState *ms = migrate_get_current(); >
Re: [PATCH] hw/riscv/virt.c: fix typo in 'aia' description
On 2023/6/15 17:21, Daniel Henrique Barboza wrote: Cc: qemu-triv...@nongnu.org Signed-off-by: Daniel Henrique Barboza --- Reviewed-by: Weiwei Li Weiwei Li hw/riscv/virt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index 95708d890e..3464abd226 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -1690,7 +1690,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) virt_set_aia); object_class_property_set_description(oc, "aia", "Set type of AIA interrupt " - "conttoller. Valid values are " + "controller. Valid values are " "none, aplic, and aplic-imsic."); object_class_property_add_str(oc, "aia-guests",
Re: [PATCH v2 1/6] target/riscv: Add properties for BF16 extensions
On 2023/6/15 20:58, Rob Bradford wrote: On Thu, 2023-06-15 at 14:32 +0800, Weiwei Li wrote: Add ext_zfbfmin/zvfbfmin/zvfbfwma properties. Add require check for BF16 extensions. Signed-off-by: Weiwei Li Signed-off-by: Junqiang Wang Reviewed-by: Daniel Henrique Barboza --- target/riscv/cpu.c | 20 target/riscv/cpu_cfg.h | 3 +++ 2 files changed, 23 insertions(+) diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 881bddf393..dc6b2f72f6 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1059,6 +1059,11 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) return; } + if (cpu->cfg.ext_zfbfmin && !riscv_has_ext(env, RVF)) { + error_setg(errp, "Zfbfmin extension depends on F extension"); + return; + } + if (riscv_has_ext(env, RVD) && !riscv_has_ext(env, RVF)) { error_setg(errp, "D extension requires F extension"); return; @@ -1109,6 +1114,21 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) return; } + if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zfbfmin) { + error_setg(errp, "Zvfbfmin extension depends on Zfbfmin extension"); + return; + } + + if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zve32f) { + error_setg(errp, "Zvfbfmin extension depends on Zve32f extension"); + return; + } I don't think this is correct - from the spec: "This extension [Zvfbfmin] depends on the Zfbfmin extension and either the "V" extension or the Zve32f embedded vector extension." So this should be: +if (cpu->cfg.ext_zvfbfmin && !(cpu->cfg.ext_zve32f || cpu- cfg.ext_v) { +error_setg(errp, "Zvfbfmin extension depends on Zve32f or V extension"); +return; +} Zve32f will be enabled when V is enabled. So we can simply check Zve32f here. Regards, Weiwei Li Cheers, Rob + + if (cpu->cfg.ext_zvfbfwma && !cpu->cfg.ext_zvfbfmin) { + error_setg(errp, "Zvfbfwma extension depends on Zvfbfmin extension"); + return; + } + /* Set the ISA extensions, checks should have happened above */ if (cpu->cfg.ext_zhinx) { cpu->cfg.ext_zhinxmin = true; diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h index c4a627d335..7d16f32720 100644 --- a/target/riscv/cpu_cfg.h +++ b/target/riscv/cpu_cfg.h @@ -75,6 +75,7 @@ struct RISCVCPUConfig { bool ext_svpbmt; bool ext_zdinx; bool ext_zawrs; + bool ext_zfbfmin; bool ext_zfh; bool ext_zfhmin; bool ext_zfinx; @@ -84,6 +85,8 @@ struct RISCVCPUConfig { bool ext_zve64f; bool ext_zve64d; bool ext_zmmul; + bool ext_zvfbfmin; + bool ext_zvfbfwma; bool ext_zvfh; bool ext_zvfhmin; bool ext_smaia;
Re: [PATCH v2 1/6] target/riscv: Add properties for BF16 extensions
On Thu, 2023-06-15 at 14:32 +0800, Weiwei Li wrote: > Add ext_zfbfmin/zvfbfmin/zvfbfwma properties. > Add require check for BF16 extensions. > > Signed-off-by: Weiwei Li > Signed-off-by: Junqiang Wang > Reviewed-by: Daniel Henrique Barboza > --- > target/riscv/cpu.c | 20 > target/riscv/cpu_cfg.h | 3 +++ > 2 files changed, 23 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 881bddf393..dc6b2f72f6 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1059,6 +1059,11 @@ void > riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > return; > } > > + if (cpu->cfg.ext_zfbfmin && !riscv_has_ext(env, RVF)) { > + error_setg(errp, "Zfbfmin extension depends on F > extension"); > + return; > + } > + > if (riscv_has_ext(env, RVD) && !riscv_has_ext(env, RVF)) { > error_setg(errp, "D extension requires F extension"); > return; > @@ -1109,6 +1114,21 @@ void > riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > return; > } > > + if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zfbfmin) { > + error_setg(errp, "Zvfbfmin extension depends on Zfbfmin > extension"); > + return; > + } > + > + if (cpu->cfg.ext_zvfbfmin && !cpu->cfg.ext_zve32f) { > + error_setg(errp, "Zvfbfmin extension depends on Zve32f > extension"); > + return; > + } I don't think this is correct - from the spec: "This extension [Zvfbfmin] depends on the Zfbfmin extension and either the "V" extension or the Zve32f embedded vector extension." So this should be: +if (cpu->cfg.ext_zvfbfmin && !(cpu->cfg.ext_zve32f || cpu- >cfg.ext_v) { +error_setg(errp, "Zvfbfmin extension depends on Zve32f or V extension"); +return; +} Cheers, Rob > + > + if (cpu->cfg.ext_zvfbfwma && !cpu->cfg.ext_zvfbfmin) { > + error_setg(errp, "Zvfbfwma extension depends on Zvfbfmin > extension"); > + return; > + } > + > /* Set the ISA extensions, checks should have happened above */ > if (cpu->cfg.ext_zhinx) { > cpu->cfg.ext_zhinxmin = true; > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h > index c4a627d335..7d16f32720 100644 > --- a/target/riscv/cpu_cfg.h > +++ b/target/riscv/cpu_cfg.h > @@ -75,6 +75,7 @@ struct RISCVCPUConfig { > bool ext_svpbmt; > bool ext_zdinx; > bool ext_zawrs; > + bool ext_zfbfmin; > bool ext_zfh; > bool ext_zfhmin; > bool ext_zfinx; > @@ -84,6 +85,8 @@ struct RISCVCPUConfig { > bool ext_zve64f; > bool ext_zve64d; > bool ext_zmmul; > + bool ext_zvfbfmin; > + bool ext_zvfbfwma; > bool ext_zvfh; > bool ext_zvfhmin; > bool ext_smaia;
Re: [PATCH v5 1/9] migration: Add switchover ack capability
Test in the following two scenarios: [1] Test scenario: Both source VM and target VM (in listening mode) have enabled return-path and switchover-ack capability: Test result : The VFIO migration completed successfully [2] Test scenario : The source VM has enabled return-path and switchover-ack capability while the target VM (in listening mode) not Test result : The VFIO migration fails The detailed error thrown by qemu-kvm when VFIO migration fails: Target VM: :17:00.2: Received INIT_DATA_SENT but switchover ack is not used error while loading state section id 81(:00:02.4:00.0/vfio) load of migration failed: Invalid argument Source VM: failed to save SaveStateEntry with id(name): 2(ram): -5 Unable to write to socket: Connection reset by peer Unable to write to socket: Connection reset by peer Tested-by: YangHang Liu On Wed, May 31, 2023 at 1:46 AM Avihai Horon wrote: > > Migration downtime estimation is calculated based on bandwidth and > remaining migration data. This assumes that loading of migration data in > the destination takes a negligible amount of time and that downtime > depends only on network speed. > > While this may be true for RAM, it's not necessarily true for other > migrated devices. For example, loading the data of a VFIO device in the > destination might require from the device to allocate resources, prepare > internal data structures and so on. These operations can take a > significant amount of time which can increase migration downtime. > > This patch adds a new capability "switchover ack" that prevents the > source from stopping the VM and completing the migration until an ACK > is received from the destination that it's OK to do so. > > This can be used by migrated devices in various ways to reduce downtime. > For example, a device can send initial precopy metadata to pre-allocate > resources in the destination and use this capability to make sure that > the pre-allocation is completed before the source VM is stopped, so it > will have full effect. > > This new capability relies on the return path capability to communicate > from the destination back to the source. > > The actual implementation of the capability will be added in the > following patches. > > Signed-off-by: Avihai Horon > Reviewed-by: Peter Xu > Acked-by: Markus Armbruster > --- > qapi/migration.json | 12 +++- > migration/options.h | 1 + > migration/options.c | 21 + > 3 files changed, 33 insertions(+), 1 deletion(-) > > diff --git a/qapi/migration.json b/qapi/migration.json > index 179af0c4d8..061ea512e0 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -487,6 +487,16 @@ > # and should not affect the correctness of postcopy migration. > # (since 7.1) > # > +# @switchover-ack: If enabled, migration will not stop the source VM > +# and complete the migration until an ACK is received from the > +# destination that it's OK to do so. Exactly when this ACK is > +# sent depends on the migrated devices that use this feature. > +# For example, a device can use it to make sure some of its data > +# is sent and loaded in the destination before doing switchover. > +# This can reduce downtime if devices that support this capability > +# are present. 'return-path' capability must be enabled to use > +# it. (since 8.1) > +# > # Features: > # > # @unstable: Members @x-colo and @x-ignore-shared are experimental. > @@ -502,7 +512,7 @@ > 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate', > { 'name': 'x-ignore-shared', 'features': [ 'unstable' ] }, > 'validate-uuid', 'background-snapshot', > - 'zero-copy-send', 'postcopy-preempt'] } > + 'zero-copy-send', 'postcopy-preempt', 'switchover-ack'] } > > ## > # @MigrationCapabilityStatus: > diff --git a/migration/options.h b/migration/options.h > index 45991af3c2..9aaf363322 100644 > --- a/migration/options.h > +++ b/migration/options.h > @@ -40,6 +40,7 @@ bool migrate_postcopy_ram(void); > bool migrate_rdma_pin_all(void); > bool migrate_release_ram(void); > bool migrate_return_path(void); > +bool migrate_switchover_ack(void); > bool migrate_validate_uuid(void); > bool migrate_xbzrle(void); > bool migrate_zero_blocks(void); > diff --git a/migration/options.c b/migration/options.c > index b62ab30cd5..16007afca6 100644 > --- a/migration/options.c > +++ b/migration/options.c > @@ -185,6 +185,8 @@ Property migration_properties[] = { > DEFINE_PROP_MIG_CAP("x-zero-copy-send", > MIGRATION_CAPABILITY_ZERO_COPY_SEND), > #endif > +DEFINE_PROP_MIG_CAP("x-switchover-ack", > +MIGRATION_CAPABILITY_SWITCHOVER_ACK), > > DEFINE_PROP_END_OF_LIST(), > }; > @@ -308,6 +310,13 @@ bool migrate_return_path(void) > return s->capabilities[MIGRATION_CAPABILITY_RETURN_PATH]; > } > > +bool migrate
Re: [PATCH 4/4] ppc/spapr: Move spapr nested HV to a new file
On Thu Jun 15, 2023 at 4:30 PM AEST, Harsh Prateek Bora wrote: > > > On 6/8/23 14:43, Nicholas Piggin wrote: > > Create spapr_nested.c for most of the nested HV implementation. > > > > Signed-off-by: Nicholas Piggin > > --- > > hw/ppc/meson.build | 1 + > > hw/ppc/spapr_hcall.c | 415 +- > > hw/ppc/spapr_nested.c | 496 + > > include/hw/ppc/spapr.h | 61 + > > 4 files changed, 499 insertions(+), 474 deletions(-) > > create mode 100644 hw/ppc/spapr_nested.c [snip] > > diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c > > new file mode 100644 > > index 00..c06dd8903c > > --- /dev/null > > +++ b/hw/ppc/spapr_nested.c > > @@ -0,0 +1,496 @@ > > +#include "qemu/osdep.h" > > +#include "qemu/cutils.h" > > +#include "exec/exec-all.h" > > +#include "helper_regs.h" > > +#include "hw/ppc/ppc.h" > > +#include "hw/ppc/spapr.h" > > +#include "hw/ppc/spapr_cpu_core.h" > > + > > +/* > > + * Register state for entering a nested guest with H_ENTER_NESTED. > > + * New member must be added at the end. > > + */ > > +struct kvmppc_hv_guest_state { > > +uint64_t version; /* version of this structure layout, must be > > first */ > > +uint32_t lpid; > > +uint32_t vcpu_token; > > +/* These registers are hypervisor privileged (at least for writing) */ > > +uint64_t lpcr; > > +uint64_t pcr; > > +uint64_t amor; > > +uint64_t dpdes; > > +uint64_t hfscr; > > +int64_t tb_offset; > > +uint64_t dawr0; > > +uint64_t dawrx0; > > +uint64_t ciabr; > > +uint64_t hdec_expiry; > > +uint64_t purr; > > +uint64_t spurr; > > +uint64_t ic; > > +uint64_t vtb; > > +uint64_t hdar; > > +uint64_t hdsisr; > > +uint64_t heir; > > +uint64_t asdr; > > +/* These are OS privileged but need to be set late in guest entry */ > > +uint64_t srr0; > > +uint64_t srr1; > > +uint64_t sprg[4]; > > +uint64_t pidr; > > +uint64_t cfar; > > +uint64_t ppr; > > +/* Version 1 ends here */ > > +uint64_t dawr1; > > +uint64_t dawrx1; > > +/* Version 2 ends here */ > > +}; > > + > > +/* Latest version of hv_guest_state structure */ > > +#define HV_GUEST_STATE_VERSION 2 > > + > > +/* Linux 64-bit powerpc pt_regs struct, used by nested HV */ > > +struct kvmppc_pt_regs { > > +uint64_t gpr[32]; > > +uint64_t nip; > > +uint64_t msr; > > +uint64_t orig_gpr3;/* Used for restarting system calls */ > > +uint64_t ctr; > > +uint64_t link; > > +uint64_t xer; > > +uint64_t ccr; > > +uint64_t softe;/* Soft enabled/disabled */ > > +uint64_t trap; /* Reason for being here */ > > +uint64_t dar; /* Fault registers */ > > +uint64_t dsisr;/* on 4xx/Book-E used for ESR */ > > +uint64_t result; /* Result of a system call */ > > +}; > > Now that we have a separated spapr_nested.c for nested related code, > Can above definitions and other nested related defines (like struct > nested_ppc_state below) be moved to a new spapr_nested.h as well? > Otherwise, looks good to me. They're private to this file, so do we need to? I'm on the fence about it, maybe because they're hcall ABI. I don't object to adding a new spapr_nested.h for nested related things in general though. Thanks, Nick
Re: [PATCH v2 09/10] target/ppc: Simplify syscall exception handlers
On Thu Jun 15, 2023 at 7:25 PM AEST, BALATON Zoltan wrote: > On Thu, 15 Jun 2023, Nicholas Piggin wrote: > > On Thu Jun 15, 2023 at 7:34 AM AEST, BALATON Zoltan wrote: > >> After previous changes the hypercall handling in 7xx and 74xx > >> exception handlers can be folded into one if statement to simpilfy > >> this code. > >> > >> Signed-off-by: BALATON Zoltan > >> --- > >> target/ppc/excp_helper.c | 26 ++ > >> 1 file changed, 10 insertions(+), 16 deletions(-) > >> > >> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > >> index 1682b988ba..662457f342 100644 > >> --- a/target/ppc/excp_helper.c > >> +++ b/target/ppc/excp_helper.c > >> @@ -740,26 +740,23 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int > >> excp) > >> break; > >> case POWERPC_EXCP_SYSCALL: /* System call exception > >>*/ > >> { > >> -int lev = env->error_code; > > > > I would still keep lev. Self documenting and consistent with books > > handler. > > lev is still there in the books version, but probably not really needed in > these 7xx versions which does not really have level parameter. This hack > should likely go away and replaced with something else on the long run as > this won't work with KVM but that needs some support from VOF or compiling > a different version for pegasos2 which wasn't considered so far. I can add > the local back if you really insist but I don't think it really makes much > sense in these cases for 7xx and 74xx. It is using the sc 1 instruction which does have a lev field though? The hardware might not have such a thing but what is being emulatd here does, so I think lev makes sense. Removing this would be fine, but while you have it yes please just leave it as lev. > >> +PowerPCCPU *cpu = env_archcpu(env); > > > > Is this necessary? > > Yes, for cpu->vhyp below. cpu->vhyp was there before your patch... Thanks, Nick
Re: [PATCH v2 05/10] target/ppc: Change parameter of cpu_interrupt_exittb() to an env pointer
On Thu Jun 15, 2023 at 7:19 PM AEST, BALATON Zoltan wrote: > On Thu, 15 Jun 2023, Nicholas Piggin wrote: > > On Thu Jun 15, 2023 at 7:34 AM AEST, BALATON Zoltan wrote: > >> Changing the parameter of cpu_interrupt_exittb() from CPUState to env > >> allows removing some more local CPUState variables in callers. > > > > I think it's more consistent to keep cs, which is same as > > cpu_interrupt(). > > But with this patch it's more consistent with the other functions devlared > in helper_regs.h and gets rid of the #ifdef in hreg_store_msr() so I'd > still like to keep this patch. Callers already have env so it should not > matter. Being consistent with functions of the same file is not important or really makes sense. It's important to be consistent with functions of similar type. cpu_interrupt_exittb() is a helper to call cpu_interrupt() so makes sense to be similar. At best it seems like pointless churn. Thanks, Nick
Re: [PATCH v2] hw/pci: prevent hotplug of devices on pcie-root-ports on the wrong slot
On Thu, 15 Jun 2023 10:46:45 +0530 Ani Sinha wrote: > PCIE root ports and other upstream ports only allow one device on slot 0. > When hotplugging a device on a pcie root port, make sure that the device > address passed always represents slot 0. Any other slot value would be > illegal on a root port. > > CC: jus...@redhat.com > CC: imamm...@redhat.com > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2128929 > Signed-off-by: Ani Sinha > --- > hw/pci/pci.c | 16 > 1 file changed, 16 insertions(+) > > changelog: > v2: feedback from mst included. > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index bf38905b7d..66999352cc 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -64,6 +64,7 @@ bool pci_available = true; > static char *pcibus_get_dev_path(DeviceState *dev); > static char *pcibus_get_fw_dev_path(DeviceState *dev); > static void pcibus_reset(BusState *qbus); > +static bool pcie_has_upstream_port(PCIDevice *dev); > > static Property pci_props[] = { > DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), > @@ -1182,6 +1183,11 @@ static PCIDevice *do_pci_register_device(PCIDevice > *pci_dev, > } else if (dev->hotplugged && > !pci_is_vf(pci_dev) && > pci_get_function_0(pci_dev)) { > +/* > + * populating function 0 triggers a bus scan from the guest that > + * exposes other non-zero functions. Hence we need to ensure that > + * function 0 is available. > + */ > error_setg(errp, "PCI: slot %d function 0 already occupied by %s," > " new func %s cannot be exposed to guest.", > PCI_SLOT(pci_get_function_0(pci_dev)->devfn), > @@ -1189,6 +1195,16 @@ static PCIDevice *do_pci_register_device(PCIDevice > *pci_dev, > name); > > return NULL; > +} else if (dev->hotplugged && > + !pci_is_vf(pci_dev) && > + pcie_has_upstream_port(pci_dev) && PCI_SLOT(devfn)) { > +/* > + * If the device is being plugged into an upstream PCIE port, you probably mixing up downstream port with upstream one, the only thing that could be plugged into upstream port is PCIE switch. Also I'm not sure that we should do this at all. Looking at BZ it seems that QEMU crashes inside backend and tear down/cleanup sequence is broken somewhere. And that is the root cause, so I'd rather fix that 1st and only after that consider adding workarounds if any were necessary. > + * like a pcie root port, we only support one device at slot 0 > + */ > +error_setg(errp, "PCI: slot %d is not valid for %s", > + PCI_SLOT(devfn), name); > +return NULL; > } > > pci_dev->devfn = devfn;
Re: [PATCH] chardev/char-win-stdio: Support VT sequences on Windows 11 host
Hi On Thu, Jun 15, 2023 at 12:36 PM Zhang Huasen wrote: > If the monitor or the serial port use STDIO as backend on Windows 11 host, > e.g. -nographic options is used, the monitor or the guest Linux do not > response to arrow keys. > > When Windows creates a console, ENABLE_VIRTUAL_PROCESS_INPUT is disabled > by default. Arrow keys cannot be retrieved by ReadFile or ReadConsoleInput > functions. > > Add ENABLE_VIRTUAL_PROCESS_INPUT to the flag which is passed to > SetConsoleMode, > when opening stdio console. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1674 > > Signed-off-by: Zhang Huasen > --- > chardev/char-win-stdio.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/chardev/char-win-stdio.c b/chardev/char-win-stdio.c > index eb830eabd9..1a18999e78 100644 > --- a/chardev/char-win-stdio.c > +++ b/chardev/char-win-stdio.c > @@ -190,7 +190,7 @@ static void qemu_chr_open_stdio(Chardev *chr, > } > } > > -dwMode |= ENABLE_LINE_INPUT; > +dwMode |= ENABLE_LINE_INPUT | ENABLE_VIRTUAL_TERMINAL_INPUT; > I think we should set it only when is_console (although that may not make a difference otherwise) thanks > if (is_console) { > /* set the terminal in raw mode */ > -- > 2.41.0.windows.1 > >
Re: [PATCH] hw/riscv/virt.c: fix typo in 'aia' description
On 15/6/23 11:21, Daniel Henrique Barboza wrote: Cc: qemu-triv...@nongnu.org Signed-off-by: Daniel Henrique Barboza --- hw/riscv/virt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2] imx_serial: set wake bit when we receive a data byte
On 15/6/23 11:30, Martin Kaiser wrote: The linux kernel added a flood check for rx data recently in commmit "Linux", "commit" Also maybe s/rx/RX/ s/uart/UART/. 496a4471b7c3 ("serial: imx: work-around for hardware RX flood"). This check uses the wake bit in the uart status register 2. The wake bit indicates that the receiver detected a start bit on the rx line. If the kernel sees a number of rx interrupts without the wake bit being set, it treats this as spurious data and resets the uart port. imx_serial does never set the wake bit and triggers the kernel's flood check. This patch adds support for the wake bit. wake is set when we receive a new character (it's not set for break events). It seems that wake is cleared by the kernel driver, the hardware does not have to clear it automatically after data was read. The wake bit can be configured as an interrupt source. Support this mechanism as well. Co-developed-by: Philippe Mathieu-Daudé Thanks ;) Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Martin Kaiser --- v2: - support interrupts from wake - clean up the commit message hw/char/imx_serial.c | 5 - include/hw/char/imx_serial.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)
Re: [PATCH] vfio/migration: Fix return value of vfio_migration_realize()
On 15/06/2023 10:19, Duan, Zhenzhong wrote: >> -Original Message- >> From: Joao Martins >> Sent: Thursday, June 15, 2023 4:54 PM >> To: Duan, Zhenzhong >> Cc: alex.william...@redhat.com; c...@redhat.com; qemu-devel@nongnu.org; >> Peng, Chao P >> Subject: Re: [PATCH] vfio/migration: Fix return value of >> vfio_migration_realize() >> >> >> >> On 15/06/2023 09:18, Zhenzhong Duan wrote: >>> We should print "Migration disabled" when migration is blocked in >>> vfio_migration_realize(). >>> >>> Fix it by reverting return value of migrate_add_blocker(), meanwhile >>> error out directly once migrate_add_blocker() failed. >>> >> >> It wasn't immediately obvious from commit message that this is mainly about >> just printing an error message when blockers get added and that well >> migrate_add_blocker() returns 0 on success and caller of >> vfio_migration_realize expects the opposite when blockers are added. >> >> Perhaps better wording would be: >> >> migrate_add_blocker() returns 0 when successfully adding the migration >> blocker. However, the caller of vfio_migration_realize() considers that >> migration was blocked when the latter returned an error. Fix it by negating >> the >> return value obtained by migrate_add_blocker(). What matters for migration >> is that the blocker is added in core migration, so this cleans up usability >> such >> that user sees "Migrate disabled" when any of the vfio migration blockers are >> active. > > Great, I'll use your words. > >> >>> Signed-off-by: Zhenzhong Duan >>> --- >>> hw/vfio/common.c| 4 ++-- >>> hw/vfio/migration.c | 6 +++--- >>> 2 files changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c index >>> fa8fd949b1cf..8505385798f3 100644 >>> --- a/hw/vfio/common.c >>> +++ b/hw/vfio/common.c >>> @@ -399,7 +399,7 @@ int vfio_block_multiple_devices_migration(Error >> **errp) >>> multiple_devices_migration_blocker = NULL; >>> } >>> >>> -return ret; >>> +return !ret; >>> } >>> >>> void vfio_unblock_multiple_devices_migration(void) >>> @@ -444,7 +444,7 @@ int vfio_block_giommu_migration(Error **errp) >>> giommu_migration_blocker = NULL; >>> } >>> >>> -return ret; >>> +return !ret; >>> } >>> >>> void vfio_migration_finalize(void) >>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index >>> 6b58dddb8859..0146521d129a 100644 >>> --- a/hw/vfio/migration.c >>> +++ b/hw/vfio/migration.c >>> @@ -646,12 +646,12 @@ int vfio_migration_realize(VFIODevice *vbasedev, >> Error **errp) >>> } >>> >>> ret = vfio_block_multiple_devices_migration(errp); >>> -if (ret) { >>> +if (ret || (errp && *errp)) { >> >> Why do you need this extra clause? > > Now that error happens, no need to add other blockers which will fail for > same reason. > But you don't need the (errp && *errp) for that as that's the whole point of negating the result. And if there's an error set it means migrate_add_blocker failed to add the blocker (e.g. snapshotting IIUC), and you would be failing here unnecessarily? Still it feels strange to use the error pointer to check for that, it's better to return error appropriately. >> >>> return ret; >>> } >>> >>> ret = vfio_block_giommu_migration(errp); >>> -if (ret) { >>> +if (ret || (errp && *errp)) { >> >> Same here? > > To avoid calling into trace_vfio_migration_probe() which hints vfio migration > realize succeed. > That was clear, my question was more related to second clause you are adding.. > Thanks > Zhenzhong > >> >>> return ret; >>> } >>> >>> @@ -667,7 +667,7 @@ add_blocker: >>> error_free(vbasedev->migration_blocker); >>> vbasedev->migration_blocker = NULL; >>> } >>> -return ret; >>> +return !ret; >>> } >>> >>> void vfio_migration_exit(VFIODevice *vbasedev)
Re: [PATCH] udmabuf: revert 'Add support for mapping hugepages (v4)'
Skimming over at shmem_read_mapping_page() users, I assume most of them use a VM_PFNMAP mapping (or don't mmap them at all), where we won't be messing with the struct page at all. (That might even allow you to mmap hugetlb sub-pages, because the struct page -- and mapcount -- will be ignored completely and not touched.) Oh, are you suggesting that if we do vma->vm_flags |= VM_PFNMAP in the mmap handler (mmap_udmabuf) and also do vmf_insert_pfn(vma, vmf->address, page_to_pfn(page)) instead of vmf->page = ubuf->pages[pgoff]; get_page(vmf->page); in the vma fault handler (udmabuf_vm_fault), we can avoid most of the pitfalls you have identified -- including with the usage of hugetlb subpages? Yes, that's my thinking, but I have to do my homework first to see if that would really work for hugetlb. The thing is, I kind-of consider what udmabuf does a layer violation: we have a filesystem (shmem/hugetlb) that should handle mappings to user space. Yet, a driver decides to bypass that and simply map the pages ordinarily to user space. (revealed by the fact that hugetlb does never map sub-pages but udmabuf decides to do so) In an ideal world everybody would simply mmap() the original memfd, but thinking about offset+size configuration within the memfd that might not always be desirable. As a workaround, we could mmap() only the PFNs, leaving the struct page unaffected. I'll have to look closer into that. -- Cheers, David / dhildenb
Re: [PATCH 0/4] ppc/pnv: Add chiptod and core timebase state machine models
[ ... ] Yes it worked with 2 chips. I will give the next series a try. [ ... ] It's difficult to review PATCH 4 without some good knowledge of HW. I know you do but you can not review your own patches ! That said, the impact is limited to PowerNV machines, I guess we are fine. Yeah. I appreciate all the review so far. It's pretty complicated even with the workbook. I might be able to add a simpler and higher-level description of the basic init sequence and states. You would still have to trust me, but it might make it easier to see what's going on. Sure. Patches 2-4 are acked. I am only looking at patch 1 now. Thanks, C.
Re: [PATCH v3] 9pfs: deprecate 'proxy' backend
On Saturday, June 10, 2023 3:39:44 PM CEST Christian Schoenebeck wrote: > As recent CVE-2023-2861 once again showed, the 9p 'proxy' fs driver is in > bad shape. Using the 'proxy' backend was already discouraged for safety > reasons before and we recommended to use the 'local' backend instead, > but now it is time to officially deprecate the 'proxy' backend. > > Signed-off-by: Christian Schoenebeck > --- > v2 -> v3: > * Fix copy wasted typo (-> 'backend'). > > MAINTAINERS| 7 +++ > docs/about/deprecated.rst | 17 + > docs/tools/virtfs-proxy-helper.rst | 3 +++ > fsdev/qemu-fsdev.c | 5 + > fsdev/virtfs-proxy-helper.c| 5 + > hw/9pfs/9p-proxy.c | 5 + > hw/9pfs/9p-proxy.h | 5 + > meson.build| 2 +- > qemu-options.hx| 6 +- > softmmu/vl.c | 5 + > 10 files changed, 58 insertions(+), 2 deletions(-) Or would it be better to split this up, e.g. into 3 separate patches (runtime messages, docs, MAINTAINERS)? > diff --git a/MAINTAINERS b/MAINTAINERS > index 436b3f0afe..185d694b2e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2118,13 +2118,20 @@ S: Odd Fixes > W: https://wiki.qemu.org/Documentation/9p > F: hw/9pfs/ > X: hw/9pfs/xen-9p* > +X: hw/9pfs/9p-proxy* > F: fsdev/ > +X: fsdev/virtfs-proxy-helper.c > F: docs/tools/virtfs-proxy-helper.rst I missed virtfs-proxy-helper.rst here. That should be moved to the new 'proxy' section below as well. > F: tests/qtest/virtio-9p-test.c > F: tests/qtest/libqos/virtio-9p* > T: git https://gitlab.com/gkurz/qemu.git 9p-next > T: git https://github.com/cschoenebeck/qemu.git 9p.next > > +virtio-9p-proxy > +F: hw/9pfs/9p-proxy* > +F: fsdev/virtfs-proxy-helper.c > +S: Obsolete > + > virtio-blk > M: Stefan Hajnoczi > L: qemu-bl...@nongnu.org > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 0743459862..9b2c780365 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -343,6 +343,23 @@ the addition of volatile memory support, it is now > necessary to distinguish > between persistent and volatile memory backends. As such, memdev is > deprecated > in favor of persistent-memdev. > > +``-fsdev proxy`` and ``-virtfs proxy`` (since 8.1) > +^^ > + > +The 9p ``proxy`` filesystem backend driver has been deprecated and will be > +removed in a future version of QEMU. Please use ``-fsdev local`` or > +``-virtfs local`` for using the ``local`` 9p filesystem backend instead. > + > +The 9p ``proxy`` backend was originally developed as an alternative to the 9p > +``local`` backend. The idea was to enhance security by dispatching actual low > +level filesystem operations from 9p server (QEMU process) over to a separate > +process (the virtfs-proxy-helper binary). However this alternative never > gained > +momentum. The proxy backend is much slower than the local backend, hasn't > seen > +any development in years, and showed to be less secure, especially due to the > +fact that its helper daemon must be run as root, whereas with the local > backend > +QEMU is typically run as unprivileged user and allows to tighten behaviour by > +mapping permissions et al. > + > > Block device options > > diff --git a/docs/tools/virtfs-proxy-helper.rst > b/docs/tools/virtfs-proxy-helper.rst > index 6cdeedf8e9..bd310ebb07 100644 > --- a/docs/tools/virtfs-proxy-helper.rst > +++ b/docs/tools/virtfs-proxy-helper.rst > @@ -9,6 +9,9 @@ Synopsis > Description > --- > > +NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > +removed, along with this daemon, in a future version of QEMU! > + > Pass-through security model in QEMU 9p server needs root privilege to do > few file operations (like chown, chmod to any mode/uid:gid). There are two > issues in pass-through security model: > diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c > index 3da64e9f72..242f54ab49 100644 > --- a/fsdev/qemu-fsdev.c > +++ b/fsdev/qemu-fsdev.c > @@ -133,6 +133,11 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp) > } > > if (fsdriver) { > +if (strncmp(fsdriver, "proxy", 5) == 0) { > +warn_report("'-fsdev proxy' is deprecated, use '-fsdev local' " > +"instead"); > +} > + > for (i = 0; i < ARRAY_SIZE(FsDrivers); i++) { > if (strcmp(FsDrivers[i].name, fsdriver) == 0) { > break; > diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c > index d9511f429c..5dd5d99284 100644 > --- a/fsdev/virtfs-proxy-helper.c > +++ b/fsdev/virtfs-proxy-helper.c > @@ -9,6 +9,11 @@ > * the COPYING file in the top-level directory. > */ > > +/* > + * NOTE: The 9p 'proxy' backend is deprecated (since QEMU 8.1) and will be > + * removed in a futu
[PATCH v2] imx_serial: set wake bit when we receive a data byte
The linux kernel added a flood check for rx data recently in commmit 496a4471b7c3 ("serial: imx: work-around for hardware RX flood"). This check uses the wake bit in the uart status register 2. The wake bit indicates that the receiver detected a start bit on the rx line. If the kernel sees a number of rx interrupts without the wake bit being set, it treats this as spurious data and resets the uart port. imx_serial does never set the wake bit and triggers the kernel's flood check. This patch adds support for the wake bit. wake is set when we receive a new character (it's not set for break events). It seems that wake is cleared by the kernel driver, the hardware does not have to clear it automatically after data was read. The wake bit can be configured as an interrupt source. Support this mechanism as well. Co-developed-by: Philippe Mathieu-Daudé Signed-off-by: Martin Kaiser --- v2: - support interrupts from wake - clean up the commit message hw/char/imx_serial.c | 5 - include/hw/char/imx_serial.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c index ee1375e26d..1b75a89588 100644 --- a/hw/char/imx_serial.c +++ b/hw/char/imx_serial.c @@ -80,7 +80,7 @@ static void imx_update(IMXSerialState *s) * TCEN and TXDC are both bit 3 * RDR and DREN are both bit 0 */ -mask |= s->ucr4 & (UCR4_TCEN | UCR4_DREN); +mask |= s->ucr4 & (UCR4_WKEN | UCR4_TCEN | UCR4_DREN); usr2 = s->usr2 & mask; @@ -321,6 +321,9 @@ static void imx_put_data(void *opaque, uint32_t value) static void imx_receive(void *opaque, const uint8_t *buf, int size) { +IMXSerialState *s = (IMXSerialState *)opaque; + +s->usr2 |= USR2_WAKE; imx_put_data(opaque, *buf); } diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h index 91c9894ad5..b823f94519 100644 --- a/include/hw/char/imx_serial.h +++ b/include/hw/char/imx_serial.h @@ -71,6 +71,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IMXSerialState, IMX_SERIAL) #define UCR4_DREN BIT(0)/* Receive Data Ready interrupt enable */ #define UCR4_TCEN BIT(3)/* TX complete interrupt enable */ +#define UCR4_WKEN BIT(7)/* WAKE interrupt enable */ #define UTS1_TXEMPTY(1<<6) #define UTS1_RXEMPTY(1<<5) -- 2.30.2
Re: [PATCH v2 2/2] ui/dbus: Expose a touch device interface
On Thu, Jun 15, 2023 at 11:07 AM Bilal Elmoussaoui wrote: > So that clients making use of the DBus backend could > send touch events through the new org.qemu.Display1.Touch > interface > > Signed-off-by: Bilal Elmoussaoui > Reviewed-by: Marc-André Lureau > --- > ui/dbus-console.c| 59 +++- > ui/dbus-display1.xml | 45 +++-- > ui/trace-events | 1 + > 3 files changed, 102 insertions(+), 3 deletions(-) > > diff --git a/ui/dbus-console.c b/ui/dbus-console.c > index f77bc49..7722f39 100644 > --- a/ui/dbus-console.c > +++ b/ui/dbus-console.c > @@ -32,6 +32,8 @@ > > #include "dbus.h" > > +static struct touch_slot touch_slots[INPUT_EVENT_SLOTS_MAX]; > + > struct _DBusDisplayConsole { > GDBusObjectSkeleton parent_instance; > DisplayChangeListener dcl; > @@ -44,6 +46,7 @@ struct _DBusDisplayConsole { > QKbdState *kbd; > > QemuDBusDisplay1Mouse *iface_mouse; > +QemuDBusDisplay1Touch *iface_touch; > gboolean last_set; > guint last_x; > guint last_y; > @@ -345,6 +348,46 @@ dbus_mouse_rel_motion(DBusDisplayConsole *ddc, > return DBUS_METHOD_INVOCATION_HANDLED; > } > > +static gboolean > +dbus_touch_send_event(DBusDisplayConsole *ddc, > + GDBusMethodInvocation *invocation, > + guint kind, uint64_t num_slot, > + double x, double y) > +{ > +Error *error = NULL; > +int width, height; > +trace_dbus_touch_send_event(kind, num_slot, x, y); > + > +if (kind != INPUT_MULTI_TOUCH_TYPE_BEGIN && > +kind != INPUT_MULTI_TOUCH_TYPE_UPDATE && > +kind != INPUT_MULTI_TOUCH_TYPE_CANCEL && > +kind != INPUT_MULTI_TOUCH_TYPE_END) > +{ > +g_dbus_method_invocation_return_error( > +invocation, DBUS_DISPLAY_ERROR, > +DBUS_DISPLAY_ERROR_INVALID, > +"Invalid touch event kind"); > +return DBUS_METHOD_INVOCATION_HANDLED; > +} > +width = qemu_console_get_width(ddc->dcl.con, 0); > +height = qemu_console_get_height(ddc->dcl.con, 0); > + > +console_handle_touch_event(ddc->dcl.con, touch_slots, > + num_slot, width, height, > + x, y, kind, &error); > +if (error != NULL) { > +g_dbus_method_invocation_return_error( > +invocation, DBUS_DISPLAY_ERROR, > +DBUS_DISPLAY_ERROR_INVALID, > +error_get_pretty(error), NULL); > +error_free(error); > +} else { > +qemu_dbus_display1_touch_complete_send_event(ddc->iface_touch, > + invocation); > +} > +return DBUS_METHOD_INVOCATION_HANDLED; > +} > + > static gboolean > dbus_mouse_set_pos(DBusDisplayConsole *ddc, > GDBusMethodInvocation *invocation, > @@ -440,7 +483,7 @@ dbus_display_console_new(DBusDisplay *display, > QemuConsole *con) > g_autofree char *label = NULL; > char device_addr[256] = ""; > DBusDisplayConsole *ddc; > -int idx; > +int idx, i; > > assert(display); > assert(con); > @@ -495,6 +538,20 @@ dbus_display_console_new(DBusDisplay *display, > QemuConsole *con) > g_dbus_object_skeleton_add_interface(G_DBUS_OBJECT_SKELETON(ddc), > G_DBUS_INTERFACE_SKELETON(ddc->iface_mouse)); > > +ddc->iface_touch = qemu_dbus_display1_touch_skeleton_new(); > +g_object_connect(ddc->iface_touch, > +"swapped-signal::handle-send-event", dbus_touch_send_event, ddc, > +NULL); > +qemu_dbus_display1_touch_set_max_slots(ddc->iface_touch, > + INPUT_EVENT_SLOTS_MAX); > +g_dbus_object_skeleton_add_interface(G_DBUS_OBJECT_SKELETON(ddc), > +G_DBUS_INTERFACE_SKELETON(ddc->iface_touch)); > + > +for (i = 0; i < INPUT_EVENT_SLOTS_MAX; i++) { > +struct touch_slot *slot = &touch_slots[i]; > +slot->tracking_id = -1; > +} > + > register_displaychangelistener(&ddc->dcl); > ddc->mouse_mode_notifier.notify = dbus_mouse_mode_change; > qemu_add_mouse_mode_change_notifier(&ddc->mouse_mode_notifier); > diff --git a/ui/dbus-display1.xml b/ui/dbus-display1.xml > index c3b2293..a98cfd1 100644 > --- a/ui/dbus-display1.xml > +++ b/ui/dbus-display1.xml > @@ -39,8 +39,9 @@ >"Text" (see :dbus:prop:`Type` and other properties). > >Interactions with a console may be done with > - :dbus:iface:`org.qemu.Display1.Keyboard` and > - :dbus:iface:`org.qemu.Display1.Mouse` interfaces when available. > + :dbus:iface:`org.qemu.Display1.Keyboard`, > + :dbus:iface:`org.qemu.Display1.Mouse` and > + :dbus:iface:`org.qemu.Display1.Touch` interfaces when available. >--> > > + org.qemu.Display1.Touch: > + > + This interface in implemented on ``/org/qemu/Display1/Console_$id`` > (see > + :dbus:iface:`~org.qemu.Display1.Console` documentation)
Re: [PATCH] imx_serial: set wake bit when we receive a data byte
Hi Philippe, thanks for reviewing my patch. Philippe Mathieu-Daudé (phi...@linaro.org) wrote: > Shouldn't we mask this bit for interruptions now? yes, we should support interrupts from the wake bit. I'll add your snippet and send a v2. Thanks, Martin > -- >8 -- > diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h > index 91c9894ad5..b823f94519 100644 > --- a/include/hw/char/imx_serial.h > +++ b/include/hw/char/imx_serial.h > @@ -71,6 +71,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IMXSerialState, IMX_SERIAL) > #define UCR4_DREN BIT(0)/* Receive Data Ready interrupt enable */ > #define UCR4_TCEN BIT(3)/* TX complete interrupt enable */ > +#define UCR4_WKEN BIT(7)/* WAKE interrupt enable */ > #define UTS1_TXEMPTY(1<<6) > #define UTS1_RXEMPTY(1<<5) > diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c > index ee1375e26d..c8ec247350 100644 > --- a/hw/char/imx_serial.c > +++ b/hw/char/imx_serial.c > @@ -80,7 +80,7 @@ static void imx_update(IMXSerialState *s) > * TCEN and TXDC are both bit 3 > * RDR and DREN are both bit 0 > */ > -mask |= s->ucr4 & (UCR4_TCEN | UCR4_DREN); > +mask |= s->ucr4 & (UCR4_WKEN | UCR4_TCEN | UCR4_DREN); > usr2 = s->usr2 & mask; > ---
Re: [PATCH v2 1/2] ui/touch: Move event handling to a common helper
On Thu, Jun 15, 2023 at 11:07 AM Bilal Elmoussaoui wrote: > To share code between the GTK and DBus UI bakcends > see the next commit for details > > Signed-off-by: Bilal Elmoussaoui > Reviewed-by: Marc-André Lureau > --- > include/ui/console.h | 15 ++ > ui/console.c | 65 > ui/gtk.c | 61 - > 3 files changed, 85 insertions(+), 56 deletions(-) > > diff --git a/include/ui/console.h b/include/ui/console.h > index ae5ec46..2093e2a 100644 > --- a/include/ui/console.h > +++ b/include/ui/console.h > @@ -5,6 +5,7 @@ > #include "qom/object.h" > #include "qemu/notify.h" > #include "qapi/qapi-types-ui.h" > +#include "ui/input.h" > > #ifdef CONFIG_OPENGL > # include > @@ -95,6 +96,20 @@ bool kbd_put_qcode_console(QemuConsole *s, int qcode, > bool ctrl); > void kbd_put_string_console(QemuConsole *s, const char *str, int len); > void kbd_put_keysym(int keysym); > > +/* Touch devices */ > +typedef struct touch_slot { > +int x; > +int y; > +int tracking_id; > +} touch_slot; > + > +void console_handle_touch_event(QemuConsole *con, > +struct touch_slot > touch_slots[INPUT_EVENT_SLOTS_MAX], > +uint64_t num_slot, > +int width, int height, > +double x, double y, > +InputMultiTouchType type, > +Error **errp); > /* consoles */ > > #define TYPE_QEMU_CONSOLE "qemu-console" > diff --git a/ui/console.c b/ui/console.c > index e173731..63e952e 100644 > --- a/ui/console.c > +++ b/ui/console.c > @@ -1635,6 +1635,71 @@ static bool console_compatible_with(QemuConsole > *con, > return true; > } > > +void console_handle_touch_event(QemuConsole *con, > +struct touch_slot > touch_slots[INPUT_EVENT_SLOTS_MAX], > +uint64_t num_slot, > +int width, int height, > +double x, double y, > +InputMultiTouchType type, > +Error **errp) > +{ > +struct touch_slot *slot; > +bool needs_sync = false; > +int update; > +int i; > + > +if (num_slot >= INPUT_EVENT_SLOTS_MAX) { > +error_setg(errp, > + "Unexpected touch slot number: % " PRId64" >= %d", > + num_slot, INPUT_EVENT_SLOTS_MAX); > +return; > +} > + > +slot = &touch_slots[num_slot]; > +slot->x = x; > +slot->y = y; > + > +if (type == INPUT_MULTI_TOUCH_TYPE_BEGIN) { > +slot->tracking_id = num_slot; > +} > + > +for (i = 0; i < INPUT_EVENT_SLOTS_MAX; ++i) { > +if (i == num_slot) { > +update = type; > +} else { > +update = INPUT_MULTI_TOUCH_TYPE_UPDATE; > +} > + > +slot = &touch_slots[i]; > + > +if (slot->tracking_id == -1) { > +continue; > +} > + > +if (update == INPUT_MULTI_TOUCH_TYPE_END) { > +slot->tracking_id = -1; > +qemu_input_queue_mtt(con, update, i, slot->tracking_id); > +needs_sync = true; > +} else { > +qemu_input_queue_mtt(con, update, i, slot->tracking_id); > +qemu_input_queue_btn(con, INPUT_BUTTON_TOUCH, true); > +qemu_input_queue_mtt_abs(con, > +INPUT_AXIS_X, (int) slot->x, > +0, width, > +i, slot->tracking_id); > +qemu_input_queue_mtt_abs(con, > +INPUT_AXIS_Y, (int) slot->y, > +0, height, > +i, slot->tracking_id); > +needs_sync = true; > +} > +} > + > +if (needs_sync) { > +qemu_input_event_sync(); > +} > +} > + > void qemu_console_set_display_gl_ctx(QemuConsole *con, DisplayGLCtx *gl) > { > /* display has opengl support */ > diff --git a/ui/gtk.c b/ui/gtk.c > index e50f950..e09e164 100644 > --- a/ui/gtk.c > +++ b/ui/gtk.c > @@ -130,11 +130,6 @@ typedef struct VCChardev VCChardev; > DECLARE_INSTANCE_CHECKER(VCChardev, VC_CHARDEV, > TYPE_CHARDEV_VC) > > -struct touch_slot { > -int x; > -int y; > -int tracking_id; > -}; > static struct touch_slot touch_slots[INPUT_EVENT_SLOTS_MAX]; > > bool gtk_use_gl_area; > @@ -1068,27 +1063,12 @@ static gboolean gd_touch_event(GtkWidget *widget, > GdkEventTouch *touch, > void *opaque) > { > VirtualConsole *vc = opaque; > -struct touch_slot *slot; > uint64_t num_slot = GPOINTER_TO_UINT(touch->sequence); > -bool needs_sync = false; > -int update; > int type = -1; > -int i; > - > -if (num_
Re: [PATCH v2 09/10] target/ppc: Simplify syscall exception handlers
On Thu, 15 Jun 2023, Nicholas Piggin wrote: On Thu Jun 15, 2023 at 7:34 AM AEST, BALATON Zoltan wrote: After previous changes the hypercall handling in 7xx and 74xx exception handlers can be folded into one if statement to simpilfy this code. Signed-off-by: BALATON Zoltan --- target/ppc/excp_helper.c | 26 ++ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 1682b988ba..662457f342 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -740,26 +740,23 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp) break; case POWERPC_EXCP_SYSCALL: /* System call exception*/ { -int lev = env->error_code; I would still keep lev. Self documenting and consistent with books handler. lev is still there in the books version, but probably not really needed in these 7xx versions which does not really have level parameter. This hack should likely go away and replaced with something else on the long run as this won't work with KVM but that needs some support from VOF or compiling a different version for pegasos2 which wasn't considered so far. I can add the local back if you really insist but I don't think it really makes much sense in these cases for 7xx and 74xx. +PowerPCCPU *cpu = env_archcpu(env); Is this necessary? Yes, for cpu->vhyp below. Regards, BALATON Zoltan Thanks, Nick -if (lev == 1 && cpu->vhyp) { -dump_hcall(env); -} else { -dump_syscall(env); -} /* * The Virtual Open Firmware (VOF) relies on the 'sc 1' * instruction to communicate with QEMU. The pegasos2 machine * uses VOF and the 7xx CPUs, so although the 7xx don't have * HV mode, we need to keep hypercall support. */ -if (lev == 1 && cpu->vhyp) { +if (unlikely(env->error_code == 1 && cpu->vhyp)) { PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp); +dump_hcall(env); vhc->hypercall(cpu->vhyp, cpu); return; +} else { +dump_syscall(env); } - break; } case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ @@ -884,26 +881,23 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp) break; case POWERPC_EXCP_SYSCALL: /* System call exception*/ { -int lev = env->error_code; +PowerPCCPU *cpu = env_archcpu(env); -if (lev == 1 && cpu->vhyp) { -dump_hcall(env); -} else { -dump_syscall(env); -} /* * The Virtual Open Firmware (VOF) relies on the 'sc 1' * instruction to communicate with QEMU. The pegasos2 machine * uses VOF and the 74xx CPUs, so although the 74xx don't have * HV mode, we need to keep hypercall support. */ -if (lev == 1 && cpu->vhyp) { +if (unlikely(env->error_code == 1 && cpu->vhyp)) { PPCVirtualHypervisorClass *vhc = PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp); +dump_hcall(env); vhc->hypercall(cpu->vhyp, cpu); return; +} else { +dump_syscall(env); } - break; } case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ -- 2.30.9
[PATCH] hw/riscv/virt.c: fix typo in 'aia' description
Cc: qemu-triv...@nongnu.org Signed-off-by: Daniel Henrique Barboza --- hw/riscv/virt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c index 95708d890e..3464abd226 100644 --- a/hw/riscv/virt.c +++ b/hw/riscv/virt.c @@ -1690,7 +1690,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data) virt_set_aia); object_class_property_set_description(oc, "aia", "Set type of AIA interrupt " - "conttoller. Valid values are " + "controller. Valid values are " "none, aplic, and aplic-imsic."); object_class_property_add_str(oc, "aia-guests", -- 2.40.1
RE: [PATCH] vfio/migration: Fix return value of vfio_migration_realize()
>-Original Message- >From: Joao Martins >Sent: Thursday, June 15, 2023 4:54 PM >To: Duan, Zhenzhong >Cc: alex.william...@redhat.com; c...@redhat.com; qemu-devel@nongnu.org; >Peng, Chao P >Subject: Re: [PATCH] vfio/migration: Fix return value of >vfio_migration_realize() > > > >On 15/06/2023 09:18, Zhenzhong Duan wrote: >> We should print "Migration disabled" when migration is blocked in >> vfio_migration_realize(). >> >> Fix it by reverting return value of migrate_add_blocker(), meanwhile >> error out directly once migrate_add_blocker() failed. >> > >It wasn't immediately obvious from commit message that this is mainly about >just printing an error message when blockers get added and that well >migrate_add_blocker() returns 0 on success and caller of >vfio_migration_realize expects the opposite when blockers are added. > >Perhaps better wording would be: > >migrate_add_blocker() returns 0 when successfully adding the migration >blocker. However, the caller of vfio_migration_realize() considers that >migration was blocked when the latter returned an error. Fix it by negating the >return value obtained by migrate_add_blocker(). What matters for migration >is that the blocker is added in core migration, so this cleans up usability >such >that user sees "Migrate disabled" when any of the vfio migration blockers are >active. Great, I'll use your words. > >> Signed-off-by: Zhenzhong Duan >> --- >> hw/vfio/common.c| 4 ++-- >> hw/vfio/migration.c | 6 +++--- >> 2 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c index >> fa8fd949b1cf..8505385798f3 100644 >> --- a/hw/vfio/common.c >> +++ b/hw/vfio/common.c >> @@ -399,7 +399,7 @@ int vfio_block_multiple_devices_migration(Error >**errp) >> multiple_devices_migration_blocker = NULL; >> } >> >> -return ret; >> +return !ret; >> } >> >> void vfio_unblock_multiple_devices_migration(void) >> @@ -444,7 +444,7 @@ int vfio_block_giommu_migration(Error **errp) >> giommu_migration_blocker = NULL; >> } >> >> -return ret; >> +return !ret; >> } >> >> void vfio_migration_finalize(void) >> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index >> 6b58dddb8859..0146521d129a 100644 >> --- a/hw/vfio/migration.c >> +++ b/hw/vfio/migration.c >> @@ -646,12 +646,12 @@ int vfio_migration_realize(VFIODevice *vbasedev, >Error **errp) >> } >> >> ret = vfio_block_multiple_devices_migration(errp); >> -if (ret) { >> +if (ret || (errp && *errp)) { > >Why do you need this extra clause? Now that error happens, no need to add other blockers which will fail for same reason. > >> return ret; >> } >> >> ret = vfio_block_giommu_migration(errp); >> -if (ret) { >> +if (ret || (errp && *errp)) { > >Same here? To avoid calling into trace_vfio_migration_probe() which hints vfio migration realize succeed. Thanks Zhenzhong > >> return ret; >> } >> >> @@ -667,7 +667,7 @@ add_blocker: >> error_free(vbasedev->migration_blocker); >> vbasedev->migration_blocker = NULL; >> } >> -return ret; >> +return !ret; >> } >> >> void vfio_migration_exit(VFIODevice *vbasedev)
Re: [PATCH v2 05/10] target/ppc: Change parameter of cpu_interrupt_exittb() to an env pointer
On Thu, 15 Jun 2023, Nicholas Piggin wrote: On Thu Jun 15, 2023 at 7:34 AM AEST, BALATON Zoltan wrote: Changing the parameter of cpu_interrupt_exittb() from CPUState to env allows removing some more local CPUState variables in callers. I think it's more consistent to keep cs, which is same as cpu_interrupt(). But with this patch it's more consistent with the other functions devlared in helper_regs.h and gets rid of the #ifdef in hreg_store_msr() so I'd still like to keep this patch. Callers already have env so it should not matter. Regards, BALATON Zoltan
[PATCH v2 1/2] ui/touch: Move event handling to a common helper
To share code between the GTK and DBus UI bakcends see the next commit for details Signed-off-by: Bilal Elmoussaoui --- include/ui/console.h | 15 ++ ui/console.c | 65 ui/gtk.c | 61 - 3 files changed, 85 insertions(+), 56 deletions(-) diff --git a/include/ui/console.h b/include/ui/console.h index ae5ec46..2093e2a 100644 --- a/include/ui/console.h +++ b/include/ui/console.h @@ -5,6 +5,7 @@ #include "qom/object.h" #include "qemu/notify.h" #include "qapi/qapi-types-ui.h" +#include "ui/input.h" #ifdef CONFIG_OPENGL # include @@ -95,6 +96,20 @@ bool kbd_put_qcode_console(QemuConsole *s, int qcode, bool ctrl); void kbd_put_string_console(QemuConsole *s, const char *str, int len); void kbd_put_keysym(int keysym); +/* Touch devices */ +typedef struct touch_slot { +int x; +int y; +int tracking_id; +} touch_slot; + +void console_handle_touch_event(QemuConsole *con, +struct touch_slot touch_slots[INPUT_EVENT_SLOTS_MAX], +uint64_t num_slot, +int width, int height, +double x, double y, +InputMultiTouchType type, +Error **errp); /* consoles */ #define TYPE_QEMU_CONSOLE "qemu-console" diff --git a/ui/console.c b/ui/console.c index e173731..63e952e 100644 --- a/ui/console.c +++ b/ui/console.c @@ -1635,6 +1635,71 @@ static bool console_compatible_with(QemuConsole *con, return true; } +void console_handle_touch_event(QemuConsole *con, +struct touch_slot touch_slots[INPUT_EVENT_SLOTS_MAX], +uint64_t num_slot, +int width, int height, +double x, double y, +InputMultiTouchType type, +Error **errp) +{ +struct touch_slot *slot; +bool needs_sync = false; +int update; +int i; + +if (num_slot >= INPUT_EVENT_SLOTS_MAX) { +error_setg(errp, + "Unexpected touch slot number: % " PRId64" >= %d", + num_slot, INPUT_EVENT_SLOTS_MAX); +return; +} + +slot = &touch_slots[num_slot]; +slot->x = x; +slot->y = y; + +if (type == INPUT_MULTI_TOUCH_TYPE_BEGIN) { +slot->tracking_id = num_slot; +} + +for (i = 0; i < INPUT_EVENT_SLOTS_MAX; ++i) { +if (i == num_slot) { +update = type; +} else { +update = INPUT_MULTI_TOUCH_TYPE_UPDATE; +} + +slot = &touch_slots[i]; + +if (slot->tracking_id == -1) { +continue; +} + +if (update == INPUT_MULTI_TOUCH_TYPE_END) { +slot->tracking_id = -1; +qemu_input_queue_mtt(con, update, i, slot->tracking_id); +needs_sync = true; +} else { +qemu_input_queue_mtt(con, update, i, slot->tracking_id); +qemu_input_queue_btn(con, INPUT_BUTTON_TOUCH, true); +qemu_input_queue_mtt_abs(con, +INPUT_AXIS_X, (int) slot->x, +0, width, +i, slot->tracking_id); +qemu_input_queue_mtt_abs(con, +INPUT_AXIS_Y, (int) slot->y, +0, height, +i, slot->tracking_id); +needs_sync = true; +} +} + +if (needs_sync) { +qemu_input_event_sync(); +} +} + void qemu_console_set_display_gl_ctx(QemuConsole *con, DisplayGLCtx *gl) { /* display has opengl support */ diff --git a/ui/gtk.c b/ui/gtk.c index e50f950..e09e164 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -130,11 +130,6 @@ typedef struct VCChardev VCChardev; DECLARE_INSTANCE_CHECKER(VCChardev, VC_CHARDEV, TYPE_CHARDEV_VC) -struct touch_slot { -int x; -int y; -int tracking_id; -}; static struct touch_slot touch_slots[INPUT_EVENT_SLOTS_MAX]; bool gtk_use_gl_area; @@ -1068,27 +1063,12 @@ static gboolean gd_touch_event(GtkWidget *widget, GdkEventTouch *touch, void *opaque) { VirtualConsole *vc = opaque; -struct touch_slot *slot; uint64_t num_slot = GPOINTER_TO_UINT(touch->sequence); -bool needs_sync = false; -int update; int type = -1; -int i; - -if (num_slot >= INPUT_EVENT_SLOTS_MAX) { -warn_report("gtk: unexpected touch slot number: % " PRId64" >= %d\n", -num_slot, INPUT_EVENT_SLOTS_MAX); -return FALSE; -} - -slot = &touch_slots[num_slot]; -slot->x = touch->x; -slot->y = touch->y; switch (touch->type) { case GDK_TOUCH_BEGIN: type = INPUT_MULTI_TOUCH_TYPE_BE
[PATCH v2 2/2] ui/dbus: Expose a touch device interface
So that clients making use of the DBus backend could send touch events through the new org.qemu.Display1.Touch interface Signed-off-by: Bilal Elmoussaoui --- ui/dbus-console.c| 59 +++- ui/dbus-display1.xml | 45 +++-- ui/trace-events | 1 + 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/ui/dbus-console.c b/ui/dbus-console.c index f77bc49..7722f39 100644 --- a/ui/dbus-console.c +++ b/ui/dbus-console.c @@ -32,6 +32,8 @@ #include "dbus.h" +static struct touch_slot touch_slots[INPUT_EVENT_SLOTS_MAX]; + struct _DBusDisplayConsole { GDBusObjectSkeleton parent_instance; DisplayChangeListener dcl; @@ -44,6 +46,7 @@ struct _DBusDisplayConsole { QKbdState *kbd; QemuDBusDisplay1Mouse *iface_mouse; +QemuDBusDisplay1Touch *iface_touch; gboolean last_set; guint last_x; guint last_y; @@ -345,6 +348,46 @@ dbus_mouse_rel_motion(DBusDisplayConsole *ddc, return DBUS_METHOD_INVOCATION_HANDLED; } +static gboolean +dbus_touch_send_event(DBusDisplayConsole *ddc, + GDBusMethodInvocation *invocation, + guint kind, uint64_t num_slot, + double x, double y) +{ +Error *error = NULL; +int width, height; +trace_dbus_touch_send_event(kind, num_slot, x, y); + +if (kind != INPUT_MULTI_TOUCH_TYPE_BEGIN && +kind != INPUT_MULTI_TOUCH_TYPE_UPDATE && +kind != INPUT_MULTI_TOUCH_TYPE_CANCEL && +kind != INPUT_MULTI_TOUCH_TYPE_END) +{ +g_dbus_method_invocation_return_error( +invocation, DBUS_DISPLAY_ERROR, +DBUS_DISPLAY_ERROR_INVALID, +"Invalid touch event kind"); +return DBUS_METHOD_INVOCATION_HANDLED; +} +width = qemu_console_get_width(ddc->dcl.con, 0); +height = qemu_console_get_height(ddc->dcl.con, 0); + +console_handle_touch_event(ddc->dcl.con, touch_slots, + num_slot, width, height, + x, y, kind, &error); +if (error != NULL) { +g_dbus_method_invocation_return_error( +invocation, DBUS_DISPLAY_ERROR, +DBUS_DISPLAY_ERROR_INVALID, +error_get_pretty(error), NULL); +error_free(error); +} else { +qemu_dbus_display1_touch_complete_send_event(ddc->iface_touch, + invocation); +} +return DBUS_METHOD_INVOCATION_HANDLED; +} + static gboolean dbus_mouse_set_pos(DBusDisplayConsole *ddc, GDBusMethodInvocation *invocation, @@ -440,7 +483,7 @@ dbus_display_console_new(DBusDisplay *display, QemuConsole *con) g_autofree char *label = NULL; char device_addr[256] = ""; DBusDisplayConsole *ddc; -int idx; +int idx, i; assert(display); assert(con); @@ -495,6 +538,20 @@ dbus_display_console_new(DBusDisplay *display, QemuConsole *con) g_dbus_object_skeleton_add_interface(G_DBUS_OBJECT_SKELETON(ddc), G_DBUS_INTERFACE_SKELETON(ddc->iface_mouse)); +ddc->iface_touch = qemu_dbus_display1_touch_skeleton_new(); +g_object_connect(ddc->iface_touch, +"swapped-signal::handle-send-event", dbus_touch_send_event, ddc, +NULL); +qemu_dbus_display1_touch_set_max_slots(ddc->iface_touch, + INPUT_EVENT_SLOTS_MAX); +g_dbus_object_skeleton_add_interface(G_DBUS_OBJECT_SKELETON(ddc), +G_DBUS_INTERFACE_SKELETON(ddc->iface_touch)); + +for (i = 0; i < INPUT_EVENT_SLOTS_MAX; i++) { +struct touch_slot *slot = &touch_slots[i]; +slot->tracking_id = -1; +} + register_displaychangelistener(&ddc->dcl); ddc->mouse_mode_notifier.notify = dbus_mouse_mode_change; qemu_add_mouse_mode_change_notifier(&ddc->mouse_mode_notifier); diff --git a/ui/dbus-display1.xml b/ui/dbus-display1.xml index c3b2293..a98cfd1 100644 --- a/ui/dbus-display1.xml +++ b/ui/dbus-display1.xml @@ -39,8 +39,9 @@ "Text" (see :dbus:prop:`Type` and other properties). Interactions with a console may be done with - :dbus:iface:`org.qemu.Display1.Keyboard` and - :dbus:iface:`org.qemu.Display1.Mouse` interfaces when available. + :dbus:iface:`org.qemu.Display1.Keyboard`, + :dbus:iface:`org.qemu.Display1.Mouse` and + :dbus:iface:`org.qemu.Display1.Touch` interfaces when available. --> + + + + + + + + + + + + +
Re: [PATCH] vfio/migration: Fix return value of vfio_migration_realize()
On 15/06/2023 09:18, Zhenzhong Duan wrote: > We should print "Migration disabled" when migration is blocked > in vfio_migration_realize(). > > Fix it by reverting return value of migrate_add_blocker(), > meanwhile error out directly once migrate_add_blocker() failed. > It wasn't immediately obvious from commit message that this is mainly about just printing an error message when blockers get added and that well migrate_add_blocker() returns 0 on success and caller of vfio_migration_realize expects the opposite when blockers are added. Perhaps better wording would be: migrate_add_blocker() returns 0 when successfully adding the migration blocker. However, the caller of vfio_migration_realize() considers that migration was blocked when the latter returned an error. Fix it by negating the return value obtained by migrate_add_blocker(). What matters for migration is that the blocker is added in core migration, so this cleans up usability such that user sees "Migrate disabled" when any of the vfio migration blockers are active. > Signed-off-by: Zhenzhong Duan > --- > hw/vfio/common.c| 4 ++-- > hw/vfio/migration.c | 6 +++--- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c > index fa8fd949b1cf..8505385798f3 100644 > --- a/hw/vfio/common.c > +++ b/hw/vfio/common.c > @@ -399,7 +399,7 @@ int vfio_block_multiple_devices_migration(Error **errp) > multiple_devices_migration_blocker = NULL; > } > > -return ret; > +return !ret; > } > > void vfio_unblock_multiple_devices_migration(void) > @@ -444,7 +444,7 @@ int vfio_block_giommu_migration(Error **errp) > giommu_migration_blocker = NULL; > } > > -return ret; > +return !ret; > } > > void vfio_migration_finalize(void) > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c > index 6b58dddb8859..0146521d129a 100644 > --- a/hw/vfio/migration.c > +++ b/hw/vfio/migration.c > @@ -646,12 +646,12 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error > **errp) > } > > ret = vfio_block_multiple_devices_migration(errp); > -if (ret) { > +if (ret || (errp && *errp)) { Why do you need this extra clause? > return ret; > } > > ret = vfio_block_giommu_migration(errp); > -if (ret) { > +if (ret || (errp && *errp)) { Same here? > return ret; > } > > @@ -667,7 +667,7 @@ add_blocker: > error_free(vbasedev->migration_blocker); > vbasedev->migration_blocker = NULL; > } > -return ret; > +return !ret; > } > > void vfio_migration_exit(VFIODevice *vbasedev)
[PATCH] vfio/migration: Fix return value of vfio_migration_realize()
We should print "Migration disabled" when migration is blocked in vfio_migration_realize(). Fix it by reverting return value of migrate_add_blocker(), meanwhile error out directly once migrate_add_blocker() failed. Signed-off-by: Zhenzhong Duan --- hw/vfio/common.c| 4 ++-- hw/vfio/migration.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index fa8fd949b1cf..8505385798f3 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -399,7 +399,7 @@ int vfio_block_multiple_devices_migration(Error **errp) multiple_devices_migration_blocker = NULL; } -return ret; +return !ret; } void vfio_unblock_multiple_devices_migration(void) @@ -444,7 +444,7 @@ int vfio_block_giommu_migration(Error **errp) giommu_migration_blocker = NULL; } -return ret; +return !ret; } void vfio_migration_finalize(void) diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index 6b58dddb8859..0146521d129a 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -646,12 +646,12 @@ int vfio_migration_realize(VFIODevice *vbasedev, Error **errp) } ret = vfio_block_multiple_devices_migration(errp); -if (ret) { +if (ret || (errp && *errp)) { return ret; } ret = vfio_block_giommu_migration(errp); -if (ret) { +if (ret || (errp && *errp)) { return ret; } @@ -667,7 +667,7 @@ add_blocker: error_free(vbasedev->migration_blocker); vbasedev->migration_blocker = NULL; } -return ret; +return !ret; } void vfio_migration_exit(VFIODevice *vbasedev) -- 2.34.1
Re: Lost partition tables on ide-hd + ahci drive
On Thursday, 15 June 2023 Fiona Ebner wrote: > which version/build of QEMU are you using? Can you correlate the issue > with any block job or was the drive in use by the guest only? I believe this has been seen on a range of releases so that includes QEMU 4.2 and 2.12. We do have custom patches but nothing in the SATA/AHCI code. I’m not familiar with the storage backend but in the RCA for one of the incidents the engineer identified an explicit write that hit the MBR. This seems to suggest QEMU is mistakenly translating a normal guest write to sector 0, probably following an earlier write failure. Regards Simon
Re: [kvm-unit-tests v4 00/12] powerpc: updates, P10, PNV support
On Thu, 15 Jun 2023 at 03:02, Nicholas Piggin wrote: > > On Wed Jun 14, 2023 at 11:09 AM AEST, Joel Stanley wrote: > > On Thu, 8 Jun 2023 at 07:58, Nicholas Piggin wrote: > > > > > > Posting again, a couple of patches were merged and accounted for review > > > comments from last time. > > > > I saw some failures in the spr tests running on a power9 powernv system: > > > > $ TESTNAME=sprs TIMEOUT=90s ACCEL= ./powerpc/run powerpc/sprs.elf -smp > > 1 |grep FAIL > > FAIL: WORT ( 895):0xc0deba80 <==> 0x > > This is just TCG machine? I'm not sure why WORT fails, AFAIKS it's the > same on POWER8 and doesn't do anything just a simple register. I think > on real hardware WORT may not have any bits implemented on POWER9 > though. Yeah, just the TCG machine. Now that you point it out all of the failures I reported are for ACCEL=, so they are running in tcg mode. run_migration timeout -k 1s --foreground 90s /usr/bin/qemu-system-ppc64 -nodefaults -machine pseries,accel=tcg -bios powerpc/boot_rom.bin -display none -serial stdio -kernel powerpc/sprs.elf -smp 1 -append -w # -initrd /tmp/tmp.61XhbvCGcb > > > $ MIGRATION=yes TESTNAME=sprs-migration TIMEOUT=90s ACCEL= > > ./powerpc/run powerpc/sprs.elf -smp 1 -append '-w' | grep FAIL > > FAIL: SRR0 ( 26):0xcafefacec0debabc <==> 0x00402244 > > FAIL: SRR1 ( 27):0xc006409ebab6 <==> 0x80001001 > > FAIL: CTRL ( 136):0x <==> 0x8001 > > FAIL: WORT ( 895):0xc0deba80 <==> 0x > > FAIL: PIR (1023):0x0010 <==> 0x0049 > > > > Linux 6.2.0-20-generic > > QEMU emulator version 7.2.0 (Debian 1:7.2+dfsg-5ubuntu2) > > > > On a power8 powernv: > > > > MIGRATION=yes TESTNAME=sprs-migration TIMEOUT=90s ACCEL= ./powerpc/run > > powerpc/sprs.elf -smp 1 -append '-w' |grep FAIL > > FAIL: SRR0 ( 26):0xcafefacec0debabc <==> 0x00402234 > > FAIL: SRR1 ( 27):0xc006409ebab6 <==> 0x80001000 > > FAIL: CTRL ( 136):0x <==> 0x8001 > > FAIL: PIR (1023):0x0060 <==> 0x0030 > > Hmm, seems we take some interrupt over migration test that is not > accounted for (could check the address in SRR0 to see where it is). > Either need to prevent that interrupt or avoid failing on SRR0/1 on > this test. > > Interesting about CTRL, I wonder if that not migrating correctly. > PIR looks like a migration issue as well, it can't be changed so > destination CPU has got a different PIR. I would be inclined to > leave those as failing to remind us to look into them. > > I'll take a look at the others though. > > Thanks, > Nick