[PATCH v2 1/3] target-arm: delete ARM_FEATURE_A64FX
The ARM_FEATURE_A64FX property was added,
but there is no function that uses this property yet,
so it will be removed until a function that uses it is added.
Signed-off-by: Shuuichirou Ishii
---
target/arm/cpu.h | 1 -
target/arm/cpu64.c | 1 -
2 files changed, 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 1b0c7b91ec..9f0a5f84d5 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2145,7 +2145,6 @@ enum arm_features {
ARM_FEATURE_M_SECURITY, /* M profile Security Extension */
ARM_FEATURE_M_MAIN, /* M profile Main Extension */
ARM_FEATURE_V8_1M, /* M profile extras only in v8.1M and later */
-ARM_FEATURE_A64FX, /* Fujitsu A64FX processor HPC extensions support */
};
static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index a15f9c0c55..dd72300e88 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -856,7 +856,6 @@ static void aarch64_a64fx_initfn(Object *obj)
ARMCPU *cpu = ARM_CPU(obj);
cpu->dtb_compatible = "arm,a64fx";
-set_feature(>env, ARM_FEATURE_A64FX);
set_feature(>env, ARM_FEATURE_V8);
set_feature(>env, ARM_FEATURE_NEON);
set_feature(>env, ARM_FEATURE_GENERIC_TIMER);
--
2.27.0
[PATCH v2 3/3] target-arm: Add A64FX processor support to virt machine
Fix for patch consistency.
https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg06993.html
Signed-off-by: Shuuichirou Ishii
---
docs/system/arm/virt.rst | 1 +
hw/arm/virt.c| 1 +
2 files changed, 2 insertions(+)
diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 27652adfae..5329e952cf 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -55,6 +55,7 @@ Supported guest CPU types:
- ``cortex-a53`` (64-bit)
- ``cortex-a57`` (64-bit)
- ``cortex-a72`` (64-bit)
+- ``a64fx`` (64-bit)
- ``host`` (with KVM only)
- ``max`` (same as ``host`` for KVM; best possible emulation with TCG)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 81eda46b0b..10286d3fd6 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -200,6 +200,7 @@ static const char *valid_cpus[] = {
ARM_CPU_TYPE_NAME("cortex-a53"),
ARM_CPU_TYPE_NAME("cortex-a57"),
ARM_CPU_TYPE_NAME("cortex-a72"),
+ARM_CPU_TYPE_NAME("a64fx"),
ARM_CPU_TYPE_NAME("host"),
ARM_CPU_TYPE_NAME("max"),
};
--
2.27.0
[PATCH v2 0/3] Add support for Fujitsu A64FX processor
This is the v2 patch series. v2: No features have been added or removed from the v1 patch series. Removal of unused definitions that were added in excess, and consolidation of patches for the purpose of functional consistency. For patch 1, ARM_FEATURE_A64FX is not used in the v1 patch series, so it was deleted this time, and will be added again when it is used. For patch 2, since the a64fx_cp_reginfo structure is not used in the v1 patch series, I deleted the empty definition and added the TODO in the aarch64_a64fx_initfn function. Also fixed the appearance, and cleaned up and removed some things for patch consistency. For patch 3, a64fx was added to docs/system/arm/virt.rst and hw/arm/virt.c respectively, as a modification to the patch consistency cleanup done in patch 2. Shuuichirou Ishii (3): target-arm: delete ARM_FEATURE_A64FX target-arm: cpu64: Add support for Fujitsu A64FX target-arm: Add A64FX processor support to virt machine hw/arm/virt.c | 2 +- target/arm/cpu.h | 1 - target/arm/cpu64.c | 10 +++--- 3 files changed, 4 insertions(+), 9 deletions(-) -- 2.27.0
[PATCH v2 2/3] target-arm: cpu64: Add support for Fujitsu A64FX
Remove unused definitions, change of appearance and fix for patch consistency
https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg04789.html
https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg04790.html
Signed-off-by: Shuuichirou Ishii
---
docs/system/arm/virt.rst | 1 -
hw/arm/virt.c| 1 -
target/arm/cpu64.c | 9 +++--
3 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
index 5329e952cf..27652adfae 100644
--- a/docs/system/arm/virt.rst
+++ b/docs/system/arm/virt.rst
@@ -55,7 +55,6 @@ Supported guest CPU types:
- ``cortex-a53`` (64-bit)
- ``cortex-a57`` (64-bit)
- ``cortex-a72`` (64-bit)
-- ``a64fx`` (64-bit)
- ``host`` (with KVM only)
- ``max`` (same as ``host`` for KVM; best possible emulation with TCG)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 3b8a26a420..81eda46b0b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -202,7 +202,6 @@ static const char *valid_cpus[] = {
ARM_CPU_TYPE_NAME("cortex-a72"),
ARM_CPU_TYPE_NAME("host"),
ARM_CPU_TYPE_NAME("max"),
-ARM_CPU_TYPE_NAME("a64fx"),
};
static bool cpu_type_valid(const char *cpu)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index dd72300e88..2b66e30133 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -847,10 +847,6 @@ static void aarch64_max_initfn(Object *obj)
cpu_max_set_sve_max_vq, NULL, NULL);
}
-static const ARMCPRegInfo a64fx_cp_reginfo[] = {
-/* TODO Add A64FX specific HPC extensinos registers */
-REGINFO_SENTINEL
-};
static void aarch64_a64fx_initfn(Object *obj)
{
ARMCPU *cpu = ARM_CPU(obj);
@@ -887,19 +883,20 @@ static void aarch64_a64fx_initfn(Object *obj)
cpu->gic_num_lrs = 4;
cpu->gic_vpribits = 5;
cpu->gic_vprebits = 5;
-define_arm_cp_regs(cpu, a64fx_cp_reginfo);
+/* TODO: Add A64FX specific HPC extension registers */
aarch64_add_sve_properties(obj);
object_property_add(obj, "sve-max-vq", "uint32", cpu_max_get_sve_max_vq,
cpu_max_set_sve_max_vq, NULL, NULL);
+
}
static const ARMCPUInfo aarch64_cpus[] = {
{ .name = "cortex-a57", .initfn = aarch64_a57_initfn },
{ .name = "cortex-a53", .initfn = aarch64_a53_initfn },
{ .name = "cortex-a72", .initfn = aarch64_a72_initfn },
-{ .name = "max",.initfn = aarch64_max_initfn },
{ .name = "a64fx", .initfn = aarch64_a64fx_initfn },
+{ .name = "max",.initfn = aarch64_max_initfn },
};
static bool aarch64_cpu_get_aarch64(Object *obj, Error **errp)
--
2.27.0
[PATCH v2 0/3] Add support for Fujitsu A64FX processor
This is the v2 patch series. v2: No features have been added or removed from the v1 patch series. Removal of unused definitions that were added in excess, and consolidation of patches for the purpose of functional consistency. For patch 1, ARM_FEATURE_A64FX is not used in the v1 patch series, so it was deleted this time, and will be added again when it is used. For patch 2, since the a64fx_cp_reginfo structure is not used in the v1 patch series, I deleted the empty definition and added the TODO in the aarch64_a64fx_initfn function. Also fixed the appearance, and cleaned up and removed some things for patch consistency. For patch 3, a64fx was added to docs/system/arm/virt.rst and hw/arm/virt.c respectively, as a modification to the patch consistency cleanup done in patch 2. Shuuichirou Ishii (3): target-arm: delete ARM_FEATURE_A64FX target-arm: cpu64: Add support for Fujitsu A64FX target-arm: Add A64FX processor support to virt machine hw/arm/virt.c | 2 +- target/arm/cpu.h | 1 - target/arm/cpu64.c | 10 +++--- 3 files changed, 4 insertions(+), 9 deletions(-) -- 2.27.0
[PATCH] hw/char/virtio-serial-bus: fix: Unpop throttled VirtQueueElement to queue before discard vq data
Ports enter a "throttled" state when writing to the chardev would block.
The current output VirtQueueElement is kept around until the chardev
becomes writable again.
Because closing the virtio serial device does not reset the queue, we cannot
directly discard this element, otherwise the control variables of the front
and back ends of the queue are inconsistent such as used_index. We should unpop
the
VirtQueueElement to queue, let discard_vq_data process it.
The test environment:
kernel: linux-5.12
Qemu command:
Qemu-system-x86 -machine pc,accel=kvm \
-cpu host,host-phys-bits \
-smp 4 \
-m 4G \
-kernel ./kernel \
-display none \
-nodefaults \
-serial mon:stdio \
-append "panic=1 no_timer_check noreplace-smp rootflags=data=ordered
rootfstype=ext4 console=ttyS0 reboot=k root=/dev/vda1 rw" \
-drive id=os,file=./disk,if=none \
-device virtio-blk-pci,drive=os \
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x4 \
-chardev socket,id=charchannel0,path=/tmp/char-dev-test,server,nowait \
-device
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0
full up virtio queue after VM started:
Cat /large-file > /dev/vport1p1
Host side:
Open and close character device sockets repeatedly
After awhile we can’t write any request to /dev/vport1p1 at VM side, VM kernel
soft lockup at drivers/char/virtio_console.c: __send_to_port
Signed-off-by: Arafatms
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index dd6bc27b3b..36236defdf 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -150,8 +150,12 @@ static void discard_vq_data(VirtQueue *vq, VirtIODevice
*vdev)
static void discard_throttle_data(VirtIOSerialPort *port)
{
+if (!virtio_queue_ready(port->ovq)) {
+return;
+}
+
if (port->elem) {
-virtqueue_detach_element(port->ovq, port->elem, 0);
+virtqueue_unpop(port->ovq, port->elem, 0);
g_free(port->elem);
port->elem = NULL;
}
[PATCH] intel_iommu: Fix typo in comments
Fix typo: *Unknwon ==> Unknown *futher ==> further *configed ==> configured Signed-off-by: Cai Huoqing --- hw/i386/intel_iommu.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 209b3f5553..75f075547f 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -679,7 +679,7 @@ static inline bool vtd_pe_type_check(X86IOMMUState *x86_iommu, } break; default: -/* Unknwon type */ +/* Unknown type */ return false; } return true; @@ -692,7 +692,7 @@ static inline bool vtd_pdire_present(VTDPASIDDirEntry *pdire) /** * Caller of this function should check present bit if wants - * to use pdir entry for futher usage except for fpd bit check. + * to use pdir entry for further usage except for fpd bit check. */ static int vtd_get_pdire_from_pdir_table(dma_addr_t pasid_dir_base, uint32_t pasid, @@ -746,7 +746,7 @@ static int vtd_get_pe_in_pasid_leaf_table(IntelIOMMUState *s, /** * Caller of this function should check present bit if wants - * to use pasid entry for futher usage except for fpd bit check. + * to use pasid entry for further usage except for fpd bit check. */ static int vtd_get_pe_from_pdire(IntelIOMMUState *s, uint32_t pasid, @@ -1507,7 +1507,7 @@ static int vtd_sync_shadow_page_table(VTDAddressSpace *vtd_as) } /* - * Check if specific device is configed to bypass address + * Check if specific device is configured to bypass address * translation for DMA requests. In Scalable Mode, bypass * 1st-level translation or 2nd-level translation, it depends * on PGTT setting. -- 2.25.1
[PATCH] hw/usb: Fix typo in comments and print
Fix typo:
*informations ==> information
*enougth ==> enough
*enouth ==> enough
*registy ==> registry
*releated ==> related
*Ouptut ==> Output
*manualy ==> manually
*Attemping ==> Attempting
*contine ==> continue
*tranceiver ==> transceiver
*Tranceiver ==> Transceiver
Signed-off-by: Cai Huoqing
---
hw/usb/desc-msos.c | 10 +-
hw/usb/desc.h| 2 +-
hw/usb/dev-audio.c | 4 ++--
hw/usb/host-libusb.c | 2 +-
hw/usb/quirks-ftdi-ids.h | 4 ++--
hw/usb/u2f-emulated.c| 2 +-
6 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/hw/usb/desc-msos.c b/hw/usb/desc-msos.c
index 836e38c67e..c72c65b650 100644
--- a/hw/usb/desc-msos.c
+++ b/hw/usb/desc-msos.c
@@ -5,12 +5,12 @@
/*
* Microsoft OS Descriptors
*
- * Windows tries to fetch some special descriptors with informations
+ * Windows tries to fetch some special descriptors with information
* specifically for windows. Presence is indicated using a special
* string @ index 0xee. There are two kinds of descriptors:
*
* compatid descriptor
- * Used to bind drivers, if usb class isn't specific enougth.
+ * Used to bind drivers, if usb class isn't specific enough.
* Used for PTP/MTP for example (both share the same usb class).
*
* properties descriptor
@@ -23,7 +23,7 @@
* HLM\SYSTEM\CurrentControlSet\Control\usbflags
* HLM\SYSTEM\CurrentControlSet\Enum\USB
* Windows will complain it can't delete entries on the second one.
- * It has deleted everything it had permissions too, which is enouth
+ * It has deleted everything it had permissions too, which is enough
* as this includes "Device Parameters".
*
* http://msdn.microsoft.com/en-us/library/windows/hardware/ff537430.aspx
@@ -192,8 +192,8 @@ static int usb_desc_msos_prop(const USBDesc *desc, uint8_t
*dest)
if (desc->msos->SelectiveSuspendEnabled) {
/*
* Signaling remote wakeup capability in the standard usb
- * descriptors isn't enouth to make windows actually use it.
- * This is the "Yes, we really mean it" registy entry to flip
+ * descriptors isn't enough to make windows actually use it.
+ * This is the "Yes, we really mean it" registry entry to flip
* the switch in the windows drivers.
*/
length += usb_desc_msos_prop_dword(dest+length,
diff --git a/hw/usb/desc.h b/hw/usb/desc.h
index 4d81c68e0e..3ac604ecfa 100644
--- a/hw/usb/desc.h
+++ b/hw/usb/desc.h
@@ -133,7 +133,7 @@ struct USBDescConfig {
const USBDescIface*ifs;
};
-/* conceptually an Interface Association Descriptor, and releated interfaces */
+/* conceptually an Interface Association Descriptor, and related interfaces */
struct USBDescIfaceAssoc {
uint8_t bFirstInterface;
uint8_t bInterfaceCount;
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index f5cb246792..8748c1ba04 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -168,7 +168,7 @@ static const USBDescIface desc_iface[] = {
STRING_FEATURE_UNIT,/* u8 iFeature */
}
},{
-/* Headphone Ouptut Terminal ID3 Descriptor */
+/* Headphone Output Terminal ID3 Descriptor */
.data = (uint8_t[]) {
0x09, /* u8 bLength */
USB_DT_CS_INTERFACE,/* u8 bDescriptorType */
@@ -332,7 +332,7 @@ static const USBDescIface desc_iface_multi[] = {
STRING_FEATURE_UNIT,/* u8 iFeature */
}
},{
-/* Headphone Ouptut Terminal ID3 Descriptor */
+/* Headphone Output Terminal ID3 Descriptor */
.data = (uint8_t[]) {
0x09, /* u8 bLength */
USB_DT_CS_INTERFACE,/* u8 bDescriptorType */
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index c0f314462a..080fb73be6 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1675,7 +1675,7 @@ static void usb_host_free_streams(USBDevice *udev,
USBEndpoint **eps,
/*
* This is *NOT* about restoring state. We have absolutely no idea
* what state the host device is in at the moment and whenever it is
- * still present in the first place. Attemping to contine where we
+ * still present in the first place. Attempting to continue where we
* left off is impossible.
*
* What we are going to do here is emulate a surprise removal of
diff --git a/hw/usb/quirks-ftdi-ids.h b/hw/usb/quirks-ftdi-ids.h
index 01aca55ca7..f3cb157d6f 100644
--- a/hw/usb/quirks-ftdi-ids.h
+++ b/hw/usb/quirks-ftdi-ids.h
@@ -625,9 +625,9 @@
* Definitions for Icom Inc. devices
*/
#define ICOM_VID 0x0C26 /* Icom vendor ID */
-/* Note: ID-1 is a communications tranceiver for HAM-radio operators */
+/* Note: ID-1 is a communications transceiver for HAM-radio
[PATCH] hw/vfio: Fix typo in comments
Fix typo in comments: *programatically ==> programmatically *disconecting ==> disconnecting *mulitple ==> multiple *timout ==> timeout *regsiter ==> register *forumula ==> formula Signed-off-by: Cai Huoqing --- hw/vfio/igd.c| 2 +- hw/vfio/pci-quirks.c | 2 +- hw/vfio/pci.c| 6 +++--- hw/vfio/platform.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c index 470205f487..d4685709a3 100644 --- a/hw/vfio/igd.c +++ b/hw/vfio/igd.c @@ -557,7 +557,7 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) * must allocate a 1MB aligned reserved memory region below 4GB with * the requested size (in bytes) for use by the Intel PCI class VGA * device at VM address 00:02.0. The base address of this reserved - * memory region must be written to the device BDSM regsiter at PCI + * memory region must be written to the device BDSM register at PCI * config offset 0x5C. */ bdsm_size = g_malloc(sizeof(*bdsm_size)); diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c index e21a6ede11..0cf69a8c6d 100644 --- a/hw/vfio/pci-quirks.c +++ b/hw/vfio/pci-quirks.c @@ -1356,7 +1356,7 @@ static bool vfio_radeon_smc_is_running(VFIOPCIDevice *vdev) /* * The scope of a config reset is controlled by a mode bit in the misc register * and a fuse, exposed as a bit in another register. The fuse is the default - * (0 = GFX, 1 = whole GPU), the misc bit is a toggle, with the forumula + * (0 = GFX, 1 = whole GPU), the misc bit is a toggle, with the formula * scope = !(misc ^ fuse), where the resulting scope is defined the same as * the fuse. A truth table therefore tells us that if misc == fuse, we need * to flip the value of the bit in the misc register. diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index e1ea1d8a23..4feaa1cb68 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -1364,7 +1364,7 @@ static void vfio_pci_relocate_msix(VFIOPCIDevice *vdev, Error **errp) * TODO: Lookup table for known devices. * * Logically we might use an algorithm here to select the BAR adding - * the least additional MMIO space, but we cannot programatically + * the least additional MMIO space, but we cannot programmatically * predict the driver dependency on BAR ordering or sizing, therefore * 'auto' becomes a lookup for combinations reported to work. */ @@ -2158,7 +2158,7 @@ static void vfio_pci_pre_reset(VFIOPCIDevice *vdev) } /* - * Stop any ongoing DMA by disconecting I/O, MMIO, and bus master. + * Stop any ongoing DMA by disconnecting I/O, MMIO, and bus master. * Also put INTx Disable in known state. */ cmd = vfio_pci_read_config(pdev, PCI_COMMAND, 2); @@ -2384,7 +2384,7 @@ out_single: } /* - * We want to differentiate hot reset of mulitple in-use devices vs hot reset + * We want to differentiate hot reset of multiple in-use devices vs hot reset * of a single in-use device. VFIO_DEVICE_RESET will already handle the case * of doing hot resets when there is only a single device per bus. The in-use * here refers to how many VFIODevices are affected. A hot reset that affects diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c index cc3f66f7e4..f8f08a0f36 100644 --- a/hw/vfio/platform.c +++ b/hw/vfio/platform.c @@ -156,7 +156,7 @@ static void vfio_mmap_set_enabled(VFIOPlatformDevice *vdev, bool enabled) * if there is no more active IRQ * @opaque: actually points to the VFIO platform device * - * Called on mmap timer timout, this function checks whether the + * Called on mmap timer timeout, this function checks whether the * IRQ is still active and if not, restores the fast path. * by construction a single eventfd is handled at a time. * if the IRQ is still active, the timer is re-programmed. -- 2.25.1
Re: [PATCH v4 2/3] hw/net: e1000e: Correct the initial value of VET register
Hi Jason, On Fri, Jul 23, 2021 at 3:55 PM Bin Meng wrote: > > From: Christina Wang > > The initial value of VLAN Ether Type (VET) register is 0x8100, as per > the manual and real hardware. > > While Linux e1000e driver always writes VET register to 0x8100, it is > not always the case for everyone. Drivers relying on the reset value > of VET won't be able to transmit and receive VLAN frames in QEMU. > > Unlike e1000 in QEMU, e1000e uses a field 'vet' in "struct E1000Core" > to cache the value of VET register, but the cache only gets updated > when VET register is written. To always get a consistent VET value > no matter VET is written or remains its reset value, drop the 'vet' > field and use 'core->mac[VET]' directly. > > Reported-by: Markus Carlstedt > Signed-off-by: Christina Wang > Signed-off-by: Bin Meng > > --- > > Changes in v4: > - Only keep hw_compat_6_0[] changes > > Changes in v3: > - add a "init-vet" property for versioned machines > > Changes in v2: > - keep the 'vet' field in "struct E1000Core" for migration compatibility > > hw/core/machine.c| 1 + > hw/net/e1000e.c | 8 +++- > hw/net/e1000e_core.c | 9 - > 3 files changed, 12 insertions(+), 6 deletions(-) > Will this series be in 6.1? Regards, Bin
Re: [PATCH for-6.2 12/43] target/sh4: Implement do_unaligned_access for user-only
On 7/29/21 3:52 AM, Peter Maydell wrote: sh4 kernel default for unaligned accesses seems to be "warn and fixup", not SIGBUS, unless the user changes that by writing to /proc/cpu/alignment or the process changes it via prctl(). We will still need this for load-locked/store-conditional (MOVLI/MOVCO). It appears that the sh4 kernel fails to decode these properly, and will do something ugly, like interpreting MOVLI as a multiple-store instead of a load. There are also other instructions that the kernel does not attempt to handle, such as MAC. I suppose we could begin with turning off TARGET_ALIGNED_ONLY for sh4-linux-user, then re-enabling MO_ALIGN for the atomics (at least). r~
Re: [PATCH 1/3] ui/gtk: adds status bar for expressing ups and fps
On Mon, Jul 26, 2021 at 03:25:49PM -0700, Dongwon Kim wrote: [meta-comment] When sending a patch series, remember to include a 0/3 cover letter, as it makes it easier to distinguish replies about the series as a whole from review of the first patch. > With a display option, "show-fps=on", qemu adds a status bar and print > following performance numbers on the bar, > > ups = update per seconds - the rate the guest scanout is updated. > fps = frame per seconds - the frame rate of VC's GL drawing area > > One function, gd_gl_count_frame is added to count # frames > and calculate ups and fps every 100 frames or guest scanout updates. > > Signed-off-by: Dongwon Kim > --- > include/ui/console.h | 4 +++- > include/ui/gtk.h | 2 ++ > qapi/ui.json | 6 +- > ui/console.c | 6 ++ > ui/gtk.c | 51 > 5 files changed, 67 insertions(+), 2 deletions(-) > +++ b/qapi/ui.json > @@ -1035,13 +1035,17 @@ > # assuming the guest will resize the display to match > # the window size then. Otherwise it defaults to "off". > # Since 3.1 > +# @show-fps:Enable showing Guest Scanout's update rate (UPS) and > +# Surface render swap rate (FPS) on a status bar (default: > off). > +# Since 6.0 This is too late to make the 6.1 release; at best, it should read 'since 6.2' -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Re: [PATCH v2 0/6] migration/ram: Optimize for virtio-mem via RamDiscardManager
On Thu, Jul 29, 2021 at 10:06:16PM +0200, David Hildenbrand wrote: > On 29.07.21 22:00, Peter Xu wrote: > > On Thu, Jul 29, 2021 at 09:39:24PM +0200, David Hildenbrand wrote: > > > > > > > > In the meantime I adjusted the code but it does the clearing under the > > > > > iothread lock, which should not be what we want ... I'll have a look. > > > > > > > > Thanks; if it takes more changes than expected we can still start from > > > > simple, > > > > IMHO, by taking bql and timely yield it. > > > > > > > > At the meantime, I found two things in ram_init_bitmaps() that I'm not > > > > sure we > > > > need them of not: > > > > > > > > 1. Do we need WITH_RCU_READ_LOCK_GUARD() if with both bql and > > > > ramlist lock? > > > >(small question) > > > > > > Good question, I'm not sure if we need it. > > > > > > > > > > > 2. Do we need migration_bitmap_sync_precopy() even if dirty bmap is > > > > all 1's? > > > >(bigger question) > > > > > > IIRC, the bitmap sync will fetch the proper dirty bitmap from KVM and set > > > the proper bits in the clear_bitmap. So once we call > > > migration_clear_memory_region_dirty_bitmap_range() etc. later we will > > > actually clear dirty bits. > > > > Good point, however.. then I'm wondering whether we should just init > > clear_bmap > > to all 1's too when init just like dirty bmap. :) > > Yes, but ... I'm not sure if we have to get the dirty bits into > KVMSlot->dirty_bmap as well in order to clear them. Yes, so far it's closely bound to kvm's dirty_bmap, so sounds needed indeed (in kvm_slot_init_dirty_bitmap). > > It could work with "manual_dirty_log_protect". For !manual_dirty_log_protect > we might have to keep it that way ... which means we might have to expose > some ugly details up to migration/ram.c . > Might require some thought :) We should make sure clear_log() hooks always work, so the memory api should be able to call the memory region clear log api without knowing whether it's enabled underneath in either kvm or other future clear_log() hooks. KVM currently should be fine as kvm_physical_log_clear() checks manual protect at the entry, and it returns directly otherwise. Thanks, -- Peter Xu
Re: [PATCH-for-6.1? v2] target/nios2: Mark raise_exception() as noreturn
On 7/29/21 9:13 AM, Richard Henderson wrote:
On 7/29/21 12:13 AM, Philippe Mathieu-Daudé wrote:
Raised exceptions don't return, so mark the helper with
noreturn.
Fixes: 032c76bc6f9 ("nios2: Add architecture emulation support")
Signed-off-by: Philippe Mathieu-Daudé
---
Reviewed-by: Richard Henderson
I'm going to queue this for 6.1, since I have another bug that wants pulling and this is
trivial.
r~
Re: [PATCH v2 0/6] migration/ram: Optimize for virtio-mem via RamDiscardManager
On 29.07.21 22:00, Peter Xu wrote: On Thu, Jul 29, 2021 at 09:39:24PM +0200, David Hildenbrand wrote: In the meantime I adjusted the code but it does the clearing under the iothread lock, which should not be what we want ... I'll have a look. Thanks; if it takes more changes than expected we can still start from simple, IMHO, by taking bql and timely yield it. At the meantime, I found two things in ram_init_bitmaps() that I'm not sure we need them of not: 1. Do we need WITH_RCU_READ_LOCK_GUARD() if with both bql and ramlist lock? (small question) Good question, I'm not sure if we need it. 2. Do we need migration_bitmap_sync_precopy() even if dirty bmap is all 1's? (bigger question) IIRC, the bitmap sync will fetch the proper dirty bitmap from KVM and set the proper bits in the clear_bitmap. So once we call migration_clear_memory_region_dirty_bitmap_range() etc. later we will actually clear dirty bits. Good point, however.. then I'm wondering whether we should just init clear_bmap to all 1's too when init just like dirty bmap. :) Yes, but ... I'm not sure if we have to get the dirty bits into KVMSlot->dirty_bmap as well in order to clear them. It could work with "manual_dirty_log_protect". For !manual_dirty_log_protect we might have to keep it that way ... which means we might have to expose some ugly details up to migration/ram.c . Might require some thought :) Besides: let's not be affected with these details as they should be more suitable to be handled separately; maybe I'll follow this up. It could be a place to discuss things, but shouldn't be a burden to block this series. Agreed :) -- Thanks, David / dhildenb
Re: [PATCH v2 0/6] migration/ram: Optimize for virtio-mem via RamDiscardManager
On Thu, Jul 29, 2021 at 09:39:24PM +0200, David Hildenbrand wrote: > > > > In the meantime I adjusted the code but it does the clearing under the > > > iothread lock, which should not be what we want ... I'll have a look. > > > > Thanks; if it takes more changes than expected we can still start from > > simple, > > IMHO, by taking bql and timely yield it. > > > > At the meantime, I found two things in ram_init_bitmaps() that I'm not sure > > we > > need them of not: > > > >1. Do we need WITH_RCU_READ_LOCK_GUARD() if with both bql and ramlist > > lock? > > (small question) > > Good question, I'm not sure if we need it. > > > > >2. Do we need migration_bitmap_sync_precopy() even if dirty bmap is all > > 1's? > > (bigger question) > > IIRC, the bitmap sync will fetch the proper dirty bitmap from KVM and set > the proper bits in the clear_bitmap. So once we call > migration_clear_memory_region_dirty_bitmap_range() etc. later we will > actually clear dirty bits. Good point, however.. then I'm wondering whether we should just init clear_bmap to all 1's too when init just like dirty bmap. :) Besides: let's not be affected with these details as they should be more suitable to be handled separately; maybe I'll follow this up. It could be a place to discuss things, but shouldn't be a burden to block this series. Thanks, -- Peter Xu
Re: [PATCH for-6.2 17/43] accel/tcg: Report unaligned atomics for user-only
On 7/29/21 5:02 PM, Peter Maydell wrote:
> On Thu, 29 Jul 2021 at 02:09, Richard Henderson
> wrote:
>>
>> Use the newly exposed do_unaligned_access hook from atomic_mmu_lookup,
>> which has access to complete alignment info from the TCGMemOpIdx arg.
>>
>> Signed-off-by: Richard Henderson
>> ---
>> accel/tcg/user-exec.c | 23 ++-
>> 1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
>> index 90d1a2d327..dd77e90789 100644
>> --- a/accel/tcg/user-exec.c
>> +++ b/accel/tcg/user-exec.c
>> @@ -852,6 +852,16 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>>
>> /* The softmmu versions of these helpers are in cputlb.c. */
>>
>> +static void cpu_unaligned_access(CPUState *cpu, vaddr addr,
>> + MMUAccessType access_type,
>> + int mmu_idx, uintptr_t ra)
>> +{
>> +CPUClass *cc = CPU_GET_CLASS(cpu);
>> +
>> +cc->tcg_ops->do_unaligned_access(cpu, addr, access_type, mmu_idx, ra);
>> +g_assert_not_reached();
>> +}
>
> The softmmu version doesn't g_assert_not_reached(), I think
> perhaps with the intent that a CPU implementation could
> in some cases return without raising an exception to
> mean "continue with the unaligned access". We should decide
> whether we want the API to permit that, or else consistently
> have both softmmu and useronly versions be marked noreturn
> and with an assert, and we should document whichever we choose.
Agreed. I'd rather use noreturn, which exposed these bugs:
- "target/xtensa: clean up unaligned access"
https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg05890.html
- "target/nios2: Mark raise_exception() as noreturn"
https://lists.gnu.org/archive/html/qemu-devel/2021-07/msg07001.html
Re: [PATCH v2 0/6] migration/ram: Optimize for virtio-mem via RamDiscardManager
In the meantime I adjusted the code but it does the clearing under the iothread lock, which should not be what we want ... I'll have a look. Thanks; if it takes more changes than expected we can still start from simple, IMHO, by taking bql and timely yield it. At the meantime, I found two things in ram_init_bitmaps() that I'm not sure we need them of not: 1. Do we need WITH_RCU_READ_LOCK_GUARD() if with both bql and ramlist lock? (small question) Good question, I'm not sure if we need it. 2. Do we need migration_bitmap_sync_precopy() even if dirty bmap is all 1's? (bigger question) IIRC, the bitmap sync will fetch the proper dirty bitmap from KVM and set the proper bits in the clear_bitmap. So once we call migration_clear_memory_region_dirty_bitmap_range() etc. later we will actually clear dirty bits. Without that, migration_clear_memory_region_dirty_bitmap_range() is a nop and we might migrate stuff unnecessarily twice as dirty bits are not cleared: I certainly need that, otherwise the migration_clear_memory_region_dirty_bitmap_range() is a nop and it all won't work as you proposed. -- Thanks, David / dhildenb
migration-test hang, s390x host, aarch64 guest
migration-test hung again during 'make check'.
Process tree:
ubuntu 42067 0.0 0.0 5460 3156 ?S13:55 0:00
\_ bash -o pipefail -c echo
'MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_IMG=./qemu-img
G_TEST_DBUS_DAEMON=/home/ubuntu/qemu/tests/dbus-vmstate-daemon.sh
QTEST_QEMU_BINARY=./qemu-system-aarch64
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon
tests/qtest/migration-test --tap -k' &&
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_IMG=./qemu-img
G_TEST_DBUS_DAEMON=/home/ubuntu/qemu/tests/dbus-vmstate-daemon.sh
QTEST_QEMU_BINARY=./qemu-system-aarch64
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon
tests/qtest/migration-test --tap -k -m quick < /dev/null |
./scripts/tap-driver.pl --test-name="qtest-aarch64/migration-test"
ubuntu 42070 0.0 0.0 78844 3184 ?Sl 13:55 0:01
\_ tests/qtest/migration-test --tap -k -m quick
ubuntu 43248 0.0 4.1 1401368 160852 ? Sl 13:55 0:03
| \_ ./qemu-system-aarch64 -qtest
unix:/tmp/qtest-42070.sock -qtest-log /dev/null -chardev
socket,path=/tmp/qtest-42070.qmp,id=char0 -mon
chardev=char0,mode=control -display none -accel kvm -accel tcg
-machine virt,gic-version=max -name source,debug-threads=on -m 150M
-serial file:/tmp/migration-test-SL7EkN/src_serial -cpu max -kernel
/tmp/migration-test-SL7EkN/bootsect -accel qtest
ubuntu 43256 0.0 1.4 1394208 57648 ? Sl 13:55 0:00
| \_ ./qemu-system-aarch64 -qtest
unix:/tmp/qtest-42070.sock -qtest-log /dev/null -chardev
socket,path=/tmp/qtest-42070.qmp,id=char0 -mon
chardev=char0,mode=control -display none -accel kvm -accel tcg
-machine virt,gic-version=max -name target,debug-threads=on -m 150M
-serial file:/tmp/migration-test-SL7EkN/dest_serial -incoming
unix:/tmp/migration-test-SL7EkN/migsocket -cpu max -kernel
/tmp/migration-test-SL7EkN/bootsect -accel qtest
ubuntu 42071 0.0 0.2 14116 11372 ?S13:55 0:00
\_ perl ./scripts/tap-driver.pl
--test-name=qtest-aarch64/migration-test
Backtrace, migration-test process:
Thread 2 (Thread 0x3ff8ff7f910 (LWP 42075)):
#0 syscall () at ../sysdeps/unix/sysv/linux/s390/s390-64/syscall.S:53
#1 0x02aa1d0d1c1c in qemu_futex_wait (val=,
f=) at /home/ubuntu/qemu/include/qemu/futex.h:29
#2 qemu_event_wait (ev=ev@entry=0x2aa1d0fda58 )
at ../../util/qemu-thread-posix.c:480
#3 0x02aa1d0d0884 in call_rcu_thread (opaque=opaque@entry=0x0) at
../../util/rcu.c:258
#4 0x02aa1d0d0c32 in qemu_thread_start (args=) at
../../util/qemu-thread-posix.c:541
#5 0x03ff90207aa8 in start_thread (arg=0x3ff8ff7f910) at
pthread_create.c:463
#6 0x03ff900fa11e in thread_start () at
../sysdeps/unix/sysv/linux/s390/s390-64/clone.S:65
Thread 1 (Thread 0x3ff905f7c00 (LWP 42070)):
#0 0x03ff90212978 in __waitpid (pid=,
stat_loc=stat_loc@entry=0x2aa1d5e06bc, options=options@entry=0)
at ../sysdeps/unix/sysv/linux/waitpid.c:30
#1 0x02aa1d0a2176 in qtest_kill_qemu (s=0x2aa1d5e06b0) at
../../tests/qtest/libqtest.c:144
#2 0x03ff903c0de6 in g_hook_list_invoke () from
/usr/lib/s390x-linux-gnu/libglib-2.0.so.0
#3
#4 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#5 0x03ff9003f3ca in __GI_abort () at abort.c:79
#6 0x03ff903fcbb2 in g_assertion_message () from
/usr/lib/s390x-linux-gnu/libglib-2.0.so.0
#7 0x03ff903fd2b4 in g_assertion_message_cmpstr () from
/usr/lib/s390x-linux-gnu/libglib-2.0.so.0
#8 0x02aa1d0a10f6 in check_migration_status
(ungoals=0x3dfe198, goal=0x2aa1d0d75c0 "postcopy-paused",
who=0x2aa1d5dee90)
at ../../tests/qtest/migration-helpers.c:146
#9 wait_for_migration_status (who=0x2aa1d5dee90, goal=, ungoals=0x3dfe198)
at ../../tests/qtest/migration-helpers.c:156
#10 0x02aa1d09f610 in test_postcopy_recovery () at
../../tests/qtest/migration-test.c:773
#11 0x03ff903fc70c in ?? () from /usr/lib/s390x-linux-gnu/libglib-2.0.so.0
#12 0x03ff903fc648 in ?? () from /usr/lib/s390x-linux-gnu/libglib-2.0.so.0
#13 0x03ff903fc648 in ?? () from /usr/lib/s390x-linux-gnu/libglib-2.0.so.0
#14 0x03ff903fc648 in ?? () from /usr/lib/s390x-linux-gnu/libglib-2.0.so.0
#15 0x03ff903fc8ee in g_test_run_suite () from
/usr/lib/s390x-linux-gnu/libglib-2.0.so.0
#16 0x03ff903fc928 in g_test_run () from
/usr/lib/s390x-linux-gnu/libglib-2.0.so.0
#17 0x02aa1d09d8f2 in main (argc=, argv=) at ../../tests/qtest/migration-test.c:1495
Backtrace, QEMU process 43248:
Thread 5 (Thread 0x3ff8d7fe910 (LWP 43361)):
#0 0x03ff9d110582 in futex_abstimed_wait_cancelable (private=0,
abstime=0x0, expected=0, futex_word=0x2aa1e27dfac)
at ../sysdeps/unix/sysv/linux/futex-internal.h:205
#1 0x03ff9d110582 in do_futex_wait (sem=sem@entry=0x2aa1e27dfa8,
abstime=0x0) at sem_waitcommon.c:111
#2 0x03ff9d11068c in __new_sem_wait_slow
(sem=sem@entry=0x2aa1e27dfa8, abstime=0x0)
Re: [PATCH v2 0/6] migration/ram: Optimize for virtio-mem via RamDiscardManager
On Thu, Jul 29, 2021 at 06:19:31PM +0200, David Hildenbrand wrote: > On 29.07.21 18:12, Peter Xu wrote: > > On Thu, Jul 29, 2021 at 10:14:47AM +0200, David Hildenbrand wrote: > > > > > > > > The thing is I still think this extra operation during sync() > > > > > > > > can be ignored by > > > > > > > > simply clear dirty log during bitmap init, then.. why not? :) > > > > > > > > > > > > > > I guess clearing the dirty log (especially in KVM) might be more > > > > > > > expensive. > > > > > > > > > > > > If we send one ioctl per cb that'll be expensive for sure. I think > > > > > > it'll be > > > > > > fine if we send one clear ioctl to kvm, summarizing the whole > > > > > > bitmap to clear. > > > > > > > > > > > > The other thing is imho having overhead during bitmap init is > > > > > > always better > > > > > > than having that during sync(). :) > > > > > > > > > > Oh, right, so you're saying, after we set the dirty bmap to all ones > > > > > and > > > > > excluded the discarded parts, setting the respective bits to 0, we > > > > > simply > > > > > issue clearing of the whole area? > > > > > > > > > > For now I assumed we would have to clear per cb. > > > > > > > > Hmm when I replied I thought we can pass in a bitmap to ->log_clear() > > > > but I > > > > just remembered memory API actually hides the bitmap interface.. > > > > > > > > Reset the whole region works, but it'll slow down migration starts, more > > > > importantly that'll be with mmu write lock so we will lose most > > > > clear-log > > > > benefit for the initial round of migration and stuck the guest #pf at > > > > the > > > > meantime... > > > > > > > > Let's try do that in cb()s as you mentioned; I think that'll still be > > > > okay, > > > > because so far the clear log block size is much larger (1gb), 1tb is > > > > worst case > > > > 1000 ioctls during bitmap init, slightly better than 250k calls during > > > > sync(), > > > > maybe? :) > > > > > > Just to get it right, what you propose is calling > > > migration_clear_memory_region_dirty_bitmap_range() from each cb(). > > > > Right. We can provide a more complicated memory api for passing in bitmap > > but > > I think that can be an overkill and tricky. > > > > > Due to the clear_bmap, we will end up clearing each chunk (e.g., 1GB) at > > > most > > > once. > > > > > > But if our layout is fragmented, we can actually end up clearing all > > > chunks > > > (1024 ioctls for 1TB), resulting in a slower migration start. > > > > > > Any gut feeling how much slower migration start could be with largish > > > (e.g., > > > 1 TiB) regions? > > > > I had a vague memory of KVM_GET_DIRTY_LOG that I used to measure which took > > ~10ms for 1g guest mem, supposing that's mostly used to protect the pages or > > clearing dirties in the EPT pgtables. Then the worst case is ~1 second for > > 1tb. > > > > But note that it's still during setup phase, so we should expect to see a > > somehow large setup time and longer period that migration stays in SETUP > > state, > > but I think it's fine. Reasons: > > > >- We don't care too much about guest dirtying pages during the setup > > process > > because we haven't migrated anything yet, meanwhile we should not > > block any > > other thread either (e.g., we don't hold BQL). > > > >- We don't block guest execution too. Unlike KVM_GET_DIRTY_LOG without > > CLEAR > > we won't hold the mmu lock for a huge long time but do it only in 1g > > chunk, > > so guest page faults can still be serviced. It'll be affected somehow > > since we'll still run with the mmu write lock critical sections for > > each > > single ioctl(), but we do that for 1gb each time so we frequently > > yield it. > > > > Please note that we are holding the iothread lock while setting up the > bitmaps + syncing the dirty log. I'll have to make sure that that code runs > outside of the BQL, otherwise we'll block guest execution. Oh right. > > In the meantime I adjusted the code but it does the clearing under the > iothread lock, which should not be what we want ... I'll have a look. Thanks; if it takes more changes than expected we can still start from simple, IMHO, by taking bql and timely yield it. At the meantime, I found two things in ram_init_bitmaps() that I'm not sure we need them of not: 1. Do we need WITH_RCU_READ_LOCK_GUARD() if with both bql and ramlist lock? (small question) 2. Do we need migration_bitmap_sync_precopy() even if dirty bmap is all 1's? (bigger question) I feel like those can be dropped. Dave/Juan? -- Peter Xu
Re: [PATCH v3 0/2] x86/sev: Measured Linux SEV guest with kernel/initrd/cmdline
Hi Paolo, Phil, Connor, On 08/07/2021 20:16, Connor Kuehl wrote: > On 7/8/21 10:03 AM, Philippe Mathieu-Daudé wrote: >> On 7/8/21 6:41 PM, Connor Kuehl wrote: >>> Hi Paolo, >>> >>> Please consider this series[1] for inclusion into your next pull request. >>> >>> Just a note that this series has a companion series that is getting >>> upstreamed into OVMF[2] >> >> Shouldn't we get the OVMF part merged first? The OVMF companion series has been reviewed by the new OVMF maintainer and merged to edk2 master branch as of edk2 commit 514b3aa08ece [1]. [1] https://github.com/tianocore/edk2/commit/514b3aa08ece Thanks, Dov > > The approach taken in the OVMF series doesn't seem very controversial, > so I don't anticipate any breaking changes with the current state of > those patches as far as QEMU is concerned. > > However, I'm fine with erring on the side of caution. > > Connor >
Re: [PATCH v2 5/6] migration/postcopy: Handle RAMBlocks with a RamDiscardManager on the destination
On 29.07.21 21:20, Peter Xu wrote:
On Thu, Jul 29, 2021 at 06:15:58PM +0200, David Hildenbrand wrote:
On 29.07.21 17:52, Peter Xu wrote:
On Thu, Jul 29, 2021 at 02:14:41PM +0200, David Hildenbrand wrote:
On 24.07.21 00:10, Peter Xu wrote:
On Fri, Jul 23, 2021 at 09:01:42PM +0200, David Hildenbrand wrote:
It can happen in corner cases and is valid: with the current virtio-mem
spec, guests are allowed to read unplugged memory. This will, for example,
happen on older Linux guests when reading /proc/kcore or (with even older
guests) when dumping guest memory via kdump. These corner cases were the
main reason why the spec allows for it -- until we have guests properly
adjusted such that it won't happen even in corner cases.
A future feature bit will disallow it for the guest: required for supporting
shmem/hugetlb cleanly. With that in place, I agree that we would want to
warn in this case!
OK that makes sense; with the page_size change, feel free to add:
I just realized that relying on the page_size would be wrong.
We migrate TARGET_PAGE_SIZE chunks and the offset might not be page_size
aligned. So if we were to replace TARGET_PAGE_SIZE by rb->page_size, we
might accidentally cover a "too big" range.
I'm wondering whether we should make the offset page size aligned instead. For
example, note that postcopy_place_page_zero() should only take page_size
aligned host addr or UFFDIO_COPY could fail (hugetlb doesn't support
UFFDIO_ZEROPAGE yet).
That is true indeed. I'd assume in that case that we would get called with
the proper offset already, right? Because uffd would only report properly
aligned pages IIRC.
Nop; it should return the faulted address. So postcopy_request_page() may need
some alignment work, as it was handled in migrate_send_rp_req_pages().
Right, figured that out myself just now:
static int postcopy_request_page(MigrationIncomingState *mis, RAMBlock *rb,
ram_addr_t start, uint64_t haddr)
{
void *aligned = (void *)(uintptr_t)(haddr & -qemu_ram_pagesize(rb));
/*
* Discarded pages (via RamDiscardManager) are never migrated. On unlikely
* access, place a zeropage, which will also set the relevant bits in the
* recv_bitmap accordingly, so we won't try placing a zeropage twice.
*
* Checking a single bit is sufficient to handle pagesize > TPS as either
* all relevant bits are set or not.
*/
assert(QEMU_IS_ALIGNED(start, qemu_ram_pagesize(rb)));
if (ramblock_page_is_discarded(rb, start)) {
bool received = ramblock_recv_bitmap_test_byte_offset(rb, start);
return received ? 0 : postcopy_place_page_zero(mis, aligned, rb);
}
return migrate_send_rp_req_pages(mis, rb, start, haddr);
}
--
Thanks,
David / dhildenb
[PATCH 1/2] virtio: add a way to disable a queue
Add virtio_queue_disable()/virtio_queue_enable() to disable/enable a queue
by setting vring.num to 0 (or num_default).
This is needed to be able to disable a guest driver from the host side
Signed-off-by: Laurent Vivier
---
include/hw/virtio/virtio.h | 2 ++
hw/virtio/virtio.c | 10 ++
2 files changed, 12 insertions(+)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 8bab9cfb7507..6a3f71b4cd88 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -251,6 +251,8 @@ void virtio_config_modern_writel(VirtIODevice *vdev,
uint32_t addr, uint32_t data);
void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
+void virtio_queue_enable(VirtIODevice *vdev, int n);
+void virtio_queue_disable(VirtIODevice *vdev, int n);
void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
int virtio_queue_get_num(VirtIODevice *vdev, int n);
int virtio_queue_get_max_num(VirtIODevice *vdev, int n);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 874377f37a70..fa5228c1a2d6 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2244,6 +2244,16 @@ void virtio_queue_set_rings(VirtIODevice *vdev, int n,
hwaddr desc,
virtio_init_region_cache(vdev, n);
}
+void virtio_queue_disable(VirtIODevice *vdev, int n)
+{
+vdev->vq[n].vring.num = 0;
+}
+
+void virtio_queue_enable(VirtIODevice *vdev, int n)
+{
+vdev->vq[n].vring.num = vdev->vq[n].vring.num_default;
+}
+
void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
{
/* Don't allow guest to flip queue between existent and
--
2.31.1
Re: [PATCH v2 5/6] migration/postcopy: Handle RAMBlocks with a RamDiscardManager on the destination
On Thu, Jul 29, 2021 at 06:15:58PM +0200, David Hildenbrand wrote: > On 29.07.21 17:52, Peter Xu wrote: > > On Thu, Jul 29, 2021 at 02:14:41PM +0200, David Hildenbrand wrote: > > > On 24.07.21 00:10, Peter Xu wrote: > > > > On Fri, Jul 23, 2021 at 09:01:42PM +0200, David Hildenbrand wrote: > > > > > It can happen in corner cases and is valid: with the current > > > > > virtio-mem > > > > > spec, guests are allowed to read unplugged memory. This will, for > > > > > example, > > > > > happen on older Linux guests when reading /proc/kcore or (with even > > > > > older > > > > > guests) when dumping guest memory via kdump. These corner cases were > > > > > the > > > > > main reason why the spec allows for it -- until we have guests > > > > > properly > > > > > adjusted such that it won't happen even in corner cases. > > > > > > > > > > A future feature bit will disallow it for the guest: required for > > > > > supporting > > > > > shmem/hugetlb cleanly. With that in place, I agree that we would want > > > > > to > > > > > warn in this case! > > > > > > > > OK that makes sense; with the page_size change, feel free to add: > > > > > > I just realized that relying on the page_size would be wrong. > > > > > > We migrate TARGET_PAGE_SIZE chunks and the offset might not be page_size > > > aligned. So if we were to replace TARGET_PAGE_SIZE by rb->page_size, we > > > might accidentally cover a "too big" range. > > > > I'm wondering whether we should make the offset page size aligned instead. > > For > > example, note that postcopy_place_page_zero() should only take page_size > > aligned host addr or UFFDIO_COPY could fail (hugetlb doesn't support > > UFFDIO_ZEROPAGE yet). > > That is true indeed. I'd assume in that case that we would get called with > the proper offset already, right? Because uffd would only report properly > aligned pages IIRC. Nop; it should return the faulted address. So postcopy_request_page() may need some alignment work, as it was handled in migrate_send_rp_req_pages(). > > > > > Btw, does virtio-mem supports hugetlbfs now? When with it, the smallest > > unit > > to plug/unplug would the huge page size (e.g., for 1g huge page, sounds not > > helpful to unplug 2M memory), am I right? > > It supports it to 99.9 % I'd say (especially with the dump/tpm/migration > fixes upstream). The units are handled properly: the unit is at least as big > as the page size. > > So with 1 GiB pages, you have a unit of 1 GiB. I see, thanks for confirming. Then fixing up the offset seems to be the right thing to do. Thanks, -- Peter Xu
[PATCH 2/2] virtio: failover: define the default device to use in case of error
If the guest driver doesn't support the STANDBY feature, by default
we keep the virtio-net device and don't hotplug the VFIO device,
but in some cases, user can prefer to use the VFIO device rather
than the virtio-net one. We can't unplug the virtio-net device
(because on migration it is expected on the destination side)
but we can force the guest driver to be disabled. Then, we can
hotplug the VFIO device that will be unplugged before the migration
like in the normal failover migration but without the failover device.
This patch adds a new property to virtio-net device: "failover-default".
By default, "failover-default" is set to true and thus the default NIC
to use if the failover cannot be enabled is the virtio-net device
(this is what is done until now with the virtio-net failover).
If "failover-default" is set to false, in case of error, the virtio-net
device is not the default anymore and the failover primary device
is used instead.
If the STANDBY feature is supported by guest and host, the virtio-net
failover acts as usual.
Signed-off-by: Laurent Vivier
---
include/hw/virtio/virtio-net.h | 1 +
hw/net/virtio-net.c| 34 ++
2 files changed, 35 insertions(+)
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 824a69c23f06..ab77930a327e 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -208,6 +208,7 @@ struct VirtIONet {
/* primary failover device is hidden*/
bool failover_primary_hidden;
bool failover;
+bool failover_default;
DeviceListener primary_listener;
Notifier migration_state;
VirtioNetRssData rss_data;
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 16d20cdee52a..6fe0a09a263b 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -891,6 +891,39 @@ static void virtio_net_set_features(VirtIODevice *vdev,
uint64_t features)
Error *err = NULL;
int i;
+/*
+ * If the guest driver doesn't support the STANDBY feature, by default
+ * we keep the virtio-net device and don't hotplug the VFIO device,
+ * but in some cases, user can prefer to use the VFIO device rather
+ * than the virtio-net one. We can't unplug the virtio-net device
+ * (because on migration it is expected on the destination side)
+ * but we can force the guest driver to be disabled. Then, we can
+ * hotplug the VFIO device that will be unplugged before the migration
+ * like in the normal failover migration but without the failover device.
+ */
+if (n->failover && !n->failover_default) {
+if (!virtio_has_feature(features, VIRTIO_NET_F_STANDBY)) {
+/* disable the first queue to disable the driver */
+virtio_queue_disable(vdev, 0);
+/*
+ * as the virtio-net driver is disable we can plug back the
+ * failover primary device
+ */
+qatomic_set(>failover_primary_hidden, false);
+failover_add_primary(n, );
+if (err) {
+warn_report_err(err);
+}
+return;
+} else {
+ /*
+ * if the driver renegotiates features, we need to re-enable
+ * the queue
+ */
+ virtio_queue_enable(vdev, 0);
+}
+}
+
if (n->mtu_bypass_backend &&
!virtio_has_feature(vdev->backend_features, VIRTIO_NET_F_MTU)) {
features &= ~(1ULL << VIRTIO_NET_F_MTU);
@@ -3625,6 +3658,7 @@ static Property virtio_net_properties[] = {
DEFINE_PROP_INT32("speed", VirtIONet, net_conf.speed, SPEED_UNKNOWN),
DEFINE_PROP_STRING("duplex", VirtIONet, net_conf.duplex_str),
DEFINE_PROP_BOOL("failover", VirtIONet, failover, false),
+DEFINE_PROP_BOOL("failover-default", VirtIONet, failover_default, true),
DEFINE_PROP_END_OF_LIST(),
};
--
2.31.1
[PATCH 0/2] virtio: failover: allow to keep the VFIO device rather than the virtio-net one
With failover, when the guest virtio-net driver doesn't support the STANDBY feature, the primary device is not plugged and only the virtio-net device is kept. Doing like that we can migrate the machine and keep the network connection. But in some cases, when performance is more important than availability we would prefer to keep the VFIO device rather than the virtio-net one, even if it means to lose the network connection during the migration of the machine. To do that we can't simply unplug the virtio-net device and plug the VFIO one because for the migration the initial state must be kept (virtio-net plugged, VFIO unplugged) but we can try to disable the virtio-net driver and plug the VFIO card, so the initial state is correct (the virtio-net card is plugged, but disabled in guest, and the VFIO card is unplugged before migration). A way to disable the virtio-net driver at startup is to trigger an error in the kernel probing function. We can do that by disabling the RX queues. I tried to add a function to disable the queue that does that by setting the queue vring "num" value to 0 (and re-enable the queue by setting back to the default value). This change doesn't impact the case when guest and host support the STANDBY feature. I've introduced the "failover-default" property to virtio-net device to set which device to keep (failover-default=true keeps the virtio-net device, =off the other one). For example, with a guest driver that doesn't support STANDBY: ... -device virtio-net-pci,id=virtio0,failover=on,failover-default=on \ -device vfio-pci,host=$PCI,id=hostdev0,failover_pair_id=virtio0 \ ... [root@localhost ~]# ip a 1: lo: mtu 65536 qdisc noqueue state UNKNOWN qlen 1 link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 inet 127.0.0.1/8 scope host lo valid_lft forever preferred_lft forever inet6 ::1/128 scope host valid_lft forever preferred_lft forever 2: eth0: mtu 1500 qdisc pfifo_fast state UP q0 link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff inet 192.168.20.2/24 brd 192.168.20.255 scope global eth0 valid_lft forever preferred_lft forever inet6 fe80::2428:c5ff:fe7f:1424/64 scope link valid_lft forever preferred_lft forever # ethtool -i eth0 driver: virtio_net version: 1.0.0 firmware-version: expansion-rom-version: bus-info: :04:00.0 supports-statistics: no supports-test: no supports-eeprom-access: no supports-register-dump: no supports-priv-flags: no ... -device virtio-net-pci,id=virtio0,failover=on,failover-default=off \ -device vfio-pci,host=$PCI,id=hostdev0,failover_pair_id=virtio0 \ ... [root@localhost ~]# ip a 1: lo: mtu 65536 qdisc noqueue state UNKNOWN qlen 1 link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 inet 127.0.0.1/8 scope host lo valid_lft forever preferred_lft forever inet6 ::1/128 scope host valid_lft forever preferred_lft forever 2: enp2s0: mtu 1500 qdisc mq state UP qlen 100 link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff inet 192.168.20.2/24 brd 192.168.20.255 scope global enp2s0 valid_lft forever preferred_lft forever inet6 fe80::2428:c5ff:fe7f:1424/64 scope link valid_lft forever preferred_lft forever [root@localhost ~]# ethtool -i enp2s0 driver: i40evf version: 1.6.27-k firmware-version: N/A expansion-rom-version: bus-info: :02:00.0 supports-statistics: yes supports-test: no supports-eeprom-access: no supports-register-dump: no supports-priv-flags: no With guest driver that supports STANDBY, we would always have: [root@localhost ~]# ip a 1: lo: mtu 65536 qdisc noqueue state UNKNOWN group defau0 link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 inet 127.0.0.1/8 scope host lo valid_lft forever preferred_lft forever inet6 ::1/128 scope host valid_lft forever preferred_lft forever 2: enp4s0: mtu 1500 qdisc noqueue state UP gr0 link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff inet 192.168.20.2/24 brd 192.168.20.255 scope global noprefixroute enp4s0 valid_lft forever preferred_lft forever inet6 fe80::2428:c5ff:fe7f:1424/64 scope link valid_lft forever preferred_lft forever 3: enp4s0nsby: mtu 1500 qdisc fq_codel master0 link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff 4: enp2s0: mtu 1500 qdisc mq master enp4s0 st0 link/ether 26:28:c5:7f:14:24 brd ff:ff:ff:ff:ff:ff [root@localhost ~]# ethtool -i enp4s0 driver: net_failover version: 0.1 firmware-version: expansion-rom-version: bus-info: supports-statistics: no supports-test: no supports-eeprom-access: no supports-register-dump: no supports-priv-flags: no [root@localhost ~]# ethtool -i enp4s0nsby driver: virtio_net version: 1.0.0 firmware-version: expansion-rom-version: bus-info: :04:00.0 supports-statistics: yes supports-test: no
Re: [PATCH-for-6.1? v2] target/nios2: Mark raise_exception() as noreturn
On 7/29/21 12:13 AM, Philippe Mathieu-Daudé wrote:
Raised exceptions don't return, so mark the helper with
noreturn.
Fixes: 032c76bc6f9 ("nios2: Add architecture emulation support")
Signed-off-by: Philippe Mathieu-Daudé
---
Reviewed-by: Richard Henderson
r~
Re: [PATCH for-6.2 03/43] target/arm: Implement do_unaligned_access for user-only
On 7/29/21 3:14 AM, Peter Maydell wrote:
On Thu, 29 Jul 2021 at 01:47, Richard Henderson
wrote:
Cc: qemu-...@nongnu.org
Signed-off-by: Richard Henderson
---
linux-user/aarch64/cpu_loop.c | 4
linux-user/arm/cpu_loop.c | 43 +++
target/arm/cpu.c | 2 +-
target/arm/cpu_tcg.c | 2 +-
4 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index ee72a1c20f..998831f87f 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -137,6 +137,10 @@ void cpu_loop(CPUARMState *env)
case 0x11: /* Synchronous Tag Check Fault */
info.si_code = TARGET_SEGV_MTESERR;
break;
+case 0x21: /* Alignment fault */
+info.si_signo = TARGET_SIGBUS;
+info.si_code = TARGET_BUS_ADRALN;
+break;
default:
g_assert_not_reached();
}
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 69632d15be..da7da6a0c1 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -23,6 +23,7 @@
#include "elf.h"
#include "cpu_loop-common.h"
#include "semihosting/common-semi.h"
+#include "target/arm/syndrome.h"
Not a huge fan of linux-user files pulling in target/arm headers, but
I guess we do it already in aarch64/cpu_loop.c. (Though that is afaict
the only other place ATM...)
#define get_user_code_u32(x, gaddr, env)\
({ abi_long __r = get_user_u32((x), (gaddr)); \
@@ -286,9 +287,8 @@ void cpu_loop(CPUARMState *env)
{
CPUState *cs = env_cpu(env);
int trapnr;
-unsigned int n, insn;
+unsigned int n, insn, ec, fsc;
target_siginfo_t info;
-uint32_t addr;
abi_ulong ret;
for(;;) {
@@ -437,15 +437,40 @@ void cpu_loop(CPUARMState *env)
break;
case EXCP_PREFETCH_ABORT:
case EXCP_DATA_ABORT:
-addr = env->exception.vaddress;
-{
-info.si_signo = TARGET_SIGSEGV;
-info.si_errno = 0;
-/* XXX: check env->error_code */
+info.si_signo = TARGET_SIGSEGV;
+info.si_errno = 0;
+info._sifields._sigfault._addr = env->exception.vaddress;
+/*
+ * We should only arrive here with EC in {DATAABORT, INSNABORT},
+ * and short-form FSC, which then tells us to look at the FSR.
+ * ??? arm_cpu_reset never sets TTBCR_EAE, so we always get
+ * short-form FSC.
+ */
+ec = syn_get_ec(env->exception.syndrome);
+assert(ec == EC_DATAABORT || ec == EC_INSNABORT);
+fsc = extract32(env->exception.syndrome, 0, 6);
+assert(fsc == 0x3f);
+switch (env->exception.fsr & 0x1f) {
+case 0x1: /* Alignment */
+info.si_signo = TARGET_SIGBUS;
+info.si_code = TARGET_BUS_ADRALN;
+break;
+case 0x3: /* Access flag fault, level 1 */
+case 0x6: /* Access flag fault, level 2 */
+case 0x9: /* Domain fault, level 1 */
+case 0xb: /* Domain fault, level 2 */
+case 0xd: /* Permision fault, level 1 */
+case 0xf: /* Permision fault, level 2 */
+info.si_code = TARGET_SEGV_ACCERR;
+break;
+case 0x5: /* Translation fault, level 1 */
+case 0x7: /* Translation fault, level 2 */
info.si_code = TARGET_SEGV_MAPERR;
-info._sifields._sigfault._addr = addr;
-queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
+break;
+default:
+g_assert_not_reached();
}
+queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
break;
It's slightly sad that we start off with a nicely symbolic
ArmMMUFaultInfo type enum value, carefully encode it into a
numeric value and then have to switch on the numeric value here,
but I can see why we end up this way...
We don't have to leave it that way.
We could move the ARMMMUFaultInfo out of internals.h, create special user-only copies of
arm_cpu_tlb_fill and arm_cpu_do_unaligned_access, create a new function to raise the MTE
exception, and place the proper enumeraor into env->error_code instead of the hw syndrome.
What we have seemed cleaner on the target/arm/ side at the time.
r~
Re: [PATCH for-6.2 05/43] target/microblaze: Implement do_unaligned_access for user-only
On Thu, Jul 29, 2021 at 08:00:50AM -1000, Richard Henderson wrote:
> On 7/29/21 3:26 AM, Peter Maydell wrote:
> > On Thu, 29 Jul 2021 at 01:54, Richard Henderson
> > wrote:
> > >
> > > Cc: Edgar E. Iglesias
> > > Signed-off-by: Richard Henderson
> > > ---
> > > target/microblaze/cpu.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> > > index 72d8f2a0da..cbec062ed7 100644
> > > --- a/target/microblaze/cpu.c
> > > +++ b/target/microblaze/cpu.c
> > > @@ -367,11 +367,11 @@ static const struct TCGCPUOps mb_tcg_ops = {
> > > .synchronize_from_tb = mb_cpu_synchronize_from_tb,
> > > .cpu_exec_interrupt = mb_cpu_exec_interrupt,
> > > .tlb_fill = mb_cpu_tlb_fill,
> > > +.do_unaligned_access = mb_cpu_do_unaligned_access,
> > >
> > > #ifndef CONFIG_USER_ONLY
> > > .do_interrupt = mb_cpu_do_interrupt,
> > > .do_transaction_failed = mb_cpu_transaction_failed,
> > > -.do_unaligned_access = mb_cpu_do_unaligned_access,
> > > #endif /* !CONFIG_USER_ONLY */
> > > };
> >
> > If I'm reading the kernel sources correctly, for Microblaze it always
> > fixes up unaligned accesses, so for our linux-user code we want
> > "ignore unaligned access errors" rather than reporting them up
> > to cpu-loop.c, I think ?
Yes, I think so.
>
> Ah, in that case we should not be setting MO_ALIGN for some -cpu xxx, I
> think? Or does the MSR_EE bit cover that? Anyway, it looked reachable at
> first glance.
Hmm yeah, perhaps we shouldn't be setting MO_ALIGN for linux-user...
Best regards,
Edgar
Re: [PATCH for-6.1? 23/43] accel/tcg: Remove double bswap for helper_atomic_sto_*_mmu
On 7/28/21 8:29 PM, Philippe Mathieu-Daudé wrote: On 7/29/21 2:46 AM, Richard Henderson wrote: This crept in as either a cut-and-paste error, or rebase error. Fixes: cfec388518d Signed-off-by: Richard Henderson --- accel/tcg/atomic_template.h | 1 - 1 file changed, 1 deletion(-) diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h index 4427fab6df..4230ff2957 100644 --- a/accel/tcg/atomic_template.h +++ b/accel/tcg/atomic_template.h @@ -251,7 +251,6 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val, PAGE_WRITE, retaddr); uint16_t info = atomic_trace_st_pre(env, addr, oi); -val = BSWAP(val); val = BSWAP(val); atomic16_set(haddr, val); ATOMIC_MMU_CLEANUP; Why not merge this for 6.1? Because old bug, no regression? Probably should. r~
Re: [PATCH for-6.2 16/43] target/xtensa: Implement do_unaligned_access for user-only
On 7/29/21 4:55 AM, Peter Maydell wrote: On Thu, 29 Jul 2021 at 02:03, Richard Henderson wrote: Cc: Max Filippov Signed-off-by: Richard Henderson --- target/xtensa/cpu.c| 2 +- target/xtensa/helper.c | 30 +++--- 2 files changed, 16 insertions(+), 16 deletions(-) The xtensa kernel has a CONFIG_XTENSA_UNALIGNED_USER option to make the kernel silently fix up unaligned userspace accesses, and most of the defconfigs for xtensa set it to 'y'. I believe the load/store-exclusive instructions should still fault. r~
Re: [PATCH for-6.2 15/43] target/sparc: Implement do_unaligned_access for user-only
On 7/28/21 11:40 PM, Philippe Mathieu-Daudé wrote:
On 7/29/21 2:46 AM, Richard Henderson wrote:
Cc: Mark Cave-Ayland
Signed-off-by: Richard Henderson
---
linux-user/sparc/cpu_loop.c | 11 +++
target/sparc/cpu.c | 2 +-
target/sparc/ldst_helper.c | 2 --
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/linux-user/sparc/cpu_loop.c b/linux-user/sparc/cpu_loop.c
index 02532f198d..612e77807e 100644
--- a/linux-user/sparc/cpu_loop.c
+++ b/linux-user/sparc/cpu_loop.c
@@ -272,6 +272,17 @@ void cpu_loop (CPUSPARCState *env)
queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
}
break;
+case TT_UNALIGNED:
+info.si_signo = TARGET_SIGBUS;
+info.si_errno = 0;
+info.si_code = TARGET_BUS_ADRALN;
+#ifdef TARGET_SPARC64
+info._sifields._sigfault._addr = env->dmmu.sfar;
+#else
+info._sifields._sigfault._addr = env->mmuregs[4];
+#endif
+queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
+break;
case EXCP_DEBUG:
info.si_signo = TARGET_SIGTRAP;
info.si_errno = 0;
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index da6b30ec74..d33d41e837 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -865,11 +865,11 @@ static const struct TCGCPUOps sparc_tcg_ops = {
.synchronize_from_tb = sparc_cpu_synchronize_from_tb,
.cpu_exec_interrupt = sparc_cpu_exec_interrupt,
.tlb_fill = sparc_cpu_tlb_fill,
+.do_unaligned_access = sparc_cpu_do_unaligned_access,
#ifndef CONFIG_USER_ONLY
.do_interrupt = sparc_cpu_do_interrupt,
.do_transaction_failed = sparc_cpu_do_transaction_failed,
-.do_unaligned_access = sparc_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
};
#endif /* CONFIG_TCG */
diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index 7367b48c8b..69b812e68c 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -1954,7 +1954,6 @@ void sparc_cpu_do_transaction_failed(CPUState *cs, hwaddr
physaddr,
}
#endif
-#if !defined(CONFIG_USER_ONLY)
void QEMU_NORETURN sparc_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
MMUAccessType access_type,
int mmu_idx,
@@ -1973,4 +1972,3 @@ void QEMU_NORETURN sparc_cpu_do_unaligned_access(CPUState
*cs, vaddr addr,
cpu_raise_exception_ra(env, TT_UNALIGNED, retaddr);
}
-#endif
Somewhere around this patch I get:
SKIPPED signals on sparc64 because BROKEN awaiting sigframe clean-ups
and vdso support
TESTtest-mmap (default) on sparc64
timeout: the monitored command dumped core
Bus error
make[2]: *** [tests/tcg/multiarch/Makefile.target:49: run-test-mmap]
Error 135
make[1]: *** [tests/tcg/Makefile.qemu:102: run-guest-tests] Error 2
make: *** [tests/Makefile.include:63: run-tcg-tests-sparc64-linux-user]
That's really surprising, since the do_unaligned_access hook is not yet used? Oh, but
then target/sparc/ does some of its own manual TT_UNALIGNED exceptions for some ASI
access. I'll have a look, but you should have been seeing a different assert for that case.
r~
Re: [PATCH for-6.2 07/43] target/ppc: Set fault address in ppc_cpu_do_unaligned_access
On 7/29/21 3:44 AM, Peter Maydell wrote: On Thu, 29 Jul 2021 at 01:51, Richard Henderson wrote: We ought to have been recording the virtual address for reporting to the guest trap handler. Cc: qemu-...@nongnu.org Signed-off-by: Richard Henderson --- target/ppc/excp_helper.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index a79a0ed465..0b2c6de442 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -1503,6 +1503,8 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, CPUPPCState *env = cs->env_ptr; uint32_t insn; +env->spr[SPR_DAR] = vaddr; + Is this the right SPR for all PPC variants? For instance the kernel's code in arch/powerpc/kernel/exceptions-64e.S looks in SPRN_DEAR, which is our SPR_BOOKE_DEAR or SPR_40x_DEAR. I have no idea. I glanced through a handful of the mmu's, and looked at the current BookS docs, but that's certainly not all. I'll note that if we do need to set different regs for different mmus, we'll probably want to standardize on this one for user-only, like we did for the user-only copy of ppc_cpu_tlb_fill. r~
Re: [PATCH for-6.2 05/43] target/microblaze: Implement do_unaligned_access for user-only
On 7/29/21 3:26 AM, Peter Maydell wrote:
On Thu, 29 Jul 2021 at 01:54, Richard Henderson
wrote:
Cc: Edgar E. Iglesias
Signed-off-by: Richard Henderson
---
target/microblaze/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
index 72d8f2a0da..cbec062ed7 100644
--- a/target/microblaze/cpu.c
+++ b/target/microblaze/cpu.c
@@ -367,11 +367,11 @@ static const struct TCGCPUOps mb_tcg_ops = {
.synchronize_from_tb = mb_cpu_synchronize_from_tb,
.cpu_exec_interrupt = mb_cpu_exec_interrupt,
.tlb_fill = mb_cpu_tlb_fill,
+.do_unaligned_access = mb_cpu_do_unaligned_access,
#ifndef CONFIG_USER_ONLY
.do_interrupt = mb_cpu_do_interrupt,
.do_transaction_failed = mb_cpu_transaction_failed,
-.do_unaligned_access = mb_cpu_do_unaligned_access,
#endif /* !CONFIG_USER_ONLY */
};
If I'm reading the kernel sources correctly, for Microblaze it always
fixes up unaligned accesses, so for our linux-user code we want
"ignore unaligned access errors" rather than reporting them up
to cpu-loop.c, I think ?
Ah, in that case we should not be setting MO_ALIGN for some -cpu xxx, I think? Or does
the MSR_EE bit cover that? Anyway, it looked reachable at first glance.
r~
[PATCH for-6.2 10/10] MAINTAINERS: Add qom.rst to QOM section
Add qom.rst to the QOM section of MAINTAINERS. Signed-off-by: Eduardo Habkost --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 42ac45c3e50..dc3f04242eb 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2651,6 +2651,7 @@ M: Paolo Bonzini R: Daniel P. Berrange R: Eduardo Habkost S: Supported +F: docs/devel/qom.rst F: docs/qdev-device-use.txt F: hw/core/qdev* F: hw/core/bus.c -- 2.31.1
[PATCH for-6.2 09/10] docs: qom: Remove OBJECT_CHECK macro examples
We shouldn't encourage people to keep defining typecast macros
manually, when we have the OBJECT_DECLARE* macros. Remove the
section showing how to define them, and replace with a section
explaining how typecasting works.
Signed-off-by: Eduardo Habkost
---
docs/devel/qom.rst | 46 +++---
1 file changed, 31 insertions(+), 15 deletions(-)
diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
index 3ae6f75a1a2..0f222555019 100644
--- a/docs/devel/qom.rst
+++ b/docs/devel/qom.rst
@@ -69,21 +69,10 @@ are instantiated dynamically but there is only ever one
instance for any
given type. The `ObjectClass` typically holds a table of function pointers
for the virtual methods implemented by this type.
-Using `object_new()`, a new `Object` derivative will be instantiated. You can
-cast an `Object` to a subclass (or base-class) type using
-`object_dynamic_cast()`. You typically want to define macro wrappers around
-`OBJECT_CHECK()` and `OBJECT_CLASS_CHECK()` to make it easier to convert to a
-specific type:
-
-.. code-block:: c
- :caption: Typecasting macros
-
- #define MY_DEVICE_GET_CLASS(obj) \
- OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
- #define MY_DEVICE_CLASS(klass) \
- OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
- #define MY_DEVICE(obj) \
- OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
+Using `object_new()`, a new `Object` derivative will be
+instantiated. You can cast an `Object` to a subclass (or
+base-class) type using `object_dynamic_cast()` or :ref:`typecast
+wrappers `.
In case the ObjectClass implementation can be built as module a
module_obj() line must be added to make sure qemu loads the module
@@ -93,6 +82,31 @@ when the object is needed.
module_obj(TYPE_MY_DEVICE);
+.. _typecasting:
+
+Typecasting
+===
+
+The `OBJECT_DECLARE macros ` automatically define
+typecasting functions having signatures like these:
+
+.. code-block:: c
+
+ static inline MyDevice *MY_DEVICE(const void *obj);
+ static inline MyDeviceClass *MY_DEVICE_GET_CLASS(const void *obj);
+ static inline MyDeviceClass *MY_DEVICE_CLASS(const void *klass);
+
+These typecasting functions are wrappers around `OBJECT_CHECK`,
+`OBJECT_GET_CLASS`, and `OBJECT_CLASS_CHECK`. Example usage:
+
+.. code-block:: c
+
+Object *obj = object_new("my-device");
+MyDevice *my_dev = MY_DEVICE(obj);
+DeviceState *dev = DEVICE(my_dev);
+MyDeviceClass *mdc = MY_DEVICE_GET_CLASS(my_dev);
+DeviceClass *dc = DEVICE_CLASS(mdc);
+
Class Initialization
@@ -282,6 +296,8 @@ convention. To reduce the amount of boilerplate code that
needs to be
written for a new type there are two sets of macros to generate the
common parts in a standard format.
+.. _OBJECT_DECLARE:
+
Type declaration macros
---
--
2.31.1
[PATCH for-6.2 08/10] docs: qom: Show actual typecast functions in examples
For clarity and to avoid encouraging people to copy the examples,
show the actual typecast functions being defined by
OBJECT_DECLARE* macros in the examples.
Signed-off-by: Eduardo Habkost
---
docs/devel/qom.rst | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
index aa1f672efbe..3ae6f75a1a2 100644
--- a/docs/devel/qom.rst
+++ b/docs/devel/qom.rst
@@ -303,8 +303,10 @@ This is equivalent to the following:
typedef struct MyDevice MyDevice;
G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDevice, object_unref)
- #define MY_DEVICE(void *obj)
- OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
+ static inline MyDevice *MY_DEVICE(void *obj)
+ {
+ return OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE);
+ }
The 'struct MyDevice' needs to be declared separately.
@@ -327,12 +329,18 @@ This is equivalent to the following:
G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDevice, object_unref)
- #define MY_DEVICE_GET_CLASS(void *obj) \
- OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
- #define MY_DEVICE_CLASS(void *klass) \
- OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
- #define MY_DEVICE(void *obj)
- OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
+ static inline MyDeviceClass *MY_DEVICE_GET_CLASS(void *obj)
+ {
+ return OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE);
+ }
+ static inline MyDeviceClass *MY_DEVICE_CLASS(void *klass)
+ {
+ return OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE);
+ }
+ static inline MyDevice *MY_DEVICE(void *obj)
+ {
+ return OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE);
+ }
Type definition macros
--
--
2.31.1
[PATCH for-6.2 07/10] docs: qom: Fix OBJECT_DECLARE_SIMPLE_TYPE documentation
The OBJECT_DECLARE_SIMPLE_TYPE documentation was inaccurate: it
doesn't define a class struct or class type checking helpers.
OBJECT_DECLARE_TYPE expansion looks very similar to the existing
example, though. Rewrite that section to show both both
OBJECT_DECLARE_SIMPLE_TYPE and OBJECT_DECLARE_TYPE.
Signed-off-by: Eduardo Habkost
---
docs/devel/qom.rst | 31 +--
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
index dee60a64c0a..aa1f672efbe 100644
--- a/docs/devel/qom.rst
+++ b/docs/devel/qom.rst
@@ -301,6 +301,27 @@ This is equivalent to the following:
.. code-block:: c
:caption: Expansion from declaring a simple type
+ typedef struct MyDevice MyDevice;
+ G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDevice, object_unref)
+ #define MY_DEVICE(void *obj)
+ OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
+
+The 'struct MyDevice' needs to be declared separately.
+
+If the type requires virtual functions to be declared in a class
+struct, then the alternative `OBJECT_DECLARE_TYPE()` macro can be
+used:
+
+.. code-block:: c
+ :caption: Declaring a type with custom class struct
+
+ OBJECT_DECLARE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
+
+This is equivalent to the following:
+
+.. code-block:: c
+ :caption: Expansion from declaring a type with custom class struct
+
typedef struct MyDevice MyDevice;
typedef struct MyDeviceClass MyDeviceClass;
@@ -313,16 +334,6 @@ This is equivalent to the following:
#define MY_DEVICE(void *obj)
OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
- struct MyDeviceClass {
- DeviceClass parent_class;
- };
-
-The 'struct MyDevice' needs to be declared separately.
-If the type requires virtual functions to be declared in the class
-struct, then the alternative `OBJECT_DECLARE_TYPE()` macro can be
-used. This does the same as `OBJECT_DECLARE_SIMPLE_TYPE()`, but without
-the 'struct MyDeviceClass' definition.
-
Type definition macros
--
--
2.31.1
[PATCH for-6.2 05/10] docs: qom: Add subsection headings to declaration/definition macros section
Add two new subsection headings to make the separation between "declaration macros" and "definition macros" more visible. Signed-off-by: Eduardo Habkost --- docs/devel/qom.rst | 6 ++ 1 file changed, 6 insertions(+) diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst index 3f48016aa8f..05d045bf570 100644 --- a/docs/devel/qom.rst +++ b/docs/devel/qom.rst @@ -285,6 +285,9 @@ convention. To reduce the amount of boilerplate code that needs to be written for a new type there are two sets of macros to generate the common parts in a standard format. +Type declaration macros +--- + A type is declared using the ``OBJECT_DECLARE`` macro family. In types which do not require any virtual functions in the class, the `OBJECT_DECLARE_SIMPLE_TYPE` macro is suitable, and is commonly placed @@ -323,6 +326,9 @@ struct, then the alternative `OBJECT_DECLARE_TYPE()` macro can be used. This does the same as `OBJECT_DECLARE_SIMPLE_TYPE()`, but without the 'struct MyDeviceClass' definition. +Type definition macros +-- + To implement the type, the ``OBJECT_DEFINE`` macro family is available. In the simple case the `OBJECT_DEFINE_TYPE()` macro is suitable: -- 2.31.1
[PATCH for-6.2 06/10] docs: qom: Remove unnecessary class typedefs from example
When there's no specific class struct used for a QOM type, we
normally don't define a typedef for it. Remove the typedef from
the minimal example, as it is unnecessary.
Signed-off-by: Eduardo Habkost
---
docs/devel/qom.rst | 3 ---
1 file changed, 3 deletions(-)
diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
index 05d045bf570..dee60a64c0a 100644
--- a/docs/devel/qom.rst
+++ b/docs/devel/qom.rst
@@ -20,9 +20,6 @@ features:
#define TYPE_MY_DEVICE "my-device"
- // No new virtual functions: we can reuse the typedef for the
- // superclass.
- typedef DeviceClass MyDeviceClass;
typedef struct MyDevice
{
DeviceState parent;
--
2.31.1
[PATCH for-6.2 03/10] docs: qom: Fix autoptr expansion example
The wrong type name was being used. The autoptr cleanup function will be declared for the instance type, not the class type. Signed-off-by: Eduardo Habkost --- docs/devel/qom.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst index 3499a8ca3b6..7ef16d92ca6 100644 --- a/docs/devel/qom.rst +++ b/docs/devel/qom.rst @@ -304,7 +304,7 @@ This is equivalent to the following: typedef struct MyDevice MyDevice; typedef struct MyDeviceClass MyDeviceClass; - G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDeviceClass, object_unref) + G_DEFINE_AUTOPTR_CLEANUP_FUNC(MyDevice, object_unref) #define MY_DEVICE_GET_CLASS(void *obj) \ OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE) -- 2.31.1
[PATCH for-6.2 01/10] docs: qom: Replace old GTK-Doc #symbol syntax with `symbol`
Replace leftover of GTK-Doc #name syntax with `name`, and use default-role:: any, so we can add references to other functions, types, and macros. There are 3 cases that required extra care: - #TypeInfo.class_init: kernel-doc doesn't generate c:member:: directives, so references to C struct members are not possible yet. This was replaced with `TypeInfo`.class_init. - #CPUClass.reset and #DeviceClass.realize: cpu.h and qdev docs are not rendered using Sphinx yet, so use ``code`` syntax for those. Signed-off-by: Eduardo Habkost --- docs/devel/qom.rst | 25 + 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst index e5fe3597cd8..9c1be5d7fc2 100644 --- a/docs/devel/qom.rst +++ b/docs/devel/qom.rst @@ -3,6 +3,7 @@ The QEMU Object Model (QOM) === .. highlight:: c +.. default-role:: any The QEMU Object Model provides a framework for registering user creatable types and instantiating objects from those types. QOM provides the following @@ -42,8 +43,8 @@ features: type_init(my_device_register_types) -In the above example, we create a simple type that is described by #TypeInfo. -#TypeInfo describes information about the type including what it inherits +In the above example, we create a simple type that is described by `TypeInfo`. +`TypeInfo` describes information about the type including what it inherits from, the instance and class size, and constructor/destructor hooks. Alternatively several static types could be registered using helper macro @@ -66,13 +67,13 @@ DEFINE_TYPES() DEFINE_TYPES(device_types_info) -Every type has an #ObjectClass associated with it. #ObjectClass derivatives +Every type has an `ObjectClass` associated with it. `ObjectClass` derivatives are instantiated dynamically but there is only ever one instance for any -given type. The #ObjectClass typically holds a table of function pointers +given type. The `ObjectClass` typically holds a table of function pointers for the virtual methods implemented by this type. -Using object_new(), a new #Object derivative will be instantiated. You can -cast an #Object to a subclass (or base-class) type using +Using object_new(), a new `Object` derivative will be instantiated. You can +cast an `Object` to a subclass (or base-class) type using object_dynamic_cast(). You typically want to define macro wrappers around OBJECT_CHECK() and OBJECT_CLASS_CHECK() to make it easier to convert to a specific type: @@ -111,7 +112,7 @@ The effect of this is that classes automatically inherit any virtual function pointers that the parent class has already initialized. All other fields will be zero filled. -Once all of the parent classes have been initialized, #TypeInfo::class_init +Once all of the parent classes have been initialized, `TypeInfo`::class_init is called to let the class being instantiated provide default initialize for its virtual functions. Here is how the above example might be modified to introduce an overridden virtual function: @@ -135,7 +136,7 @@ to introduce an overridden virtual function: }; Introducing new virtual methods requires a class to define its own -struct and to add a .class_size member to the #TypeInfo. Each method +struct and to add a .class_size member to the `TypeInfo`. Each method will also have a wrapper function to call it easily: .. code-block:: c @@ -188,12 +189,12 @@ strongly-typed first argument. If it does not operate on an object instance, it is dubbed *class method*. -Methods cannot be overloaded. That is, the #ObjectClass and method name +Methods cannot be overloaded. That is, the `ObjectClass` and method name uniquely identity the function to be called; the signature does not vary except for trailing varargs. Methods are always *virtual*. Overriding a method in -#TypeInfo.class_init of a subclass leads to any user of the class obtained +`TypeInfo`.class_init of a subclass leads to any user of the class obtained via OBJECT_GET_CLASS() accessing the overridden function. The original function is not automatically invoked. It is the responsibility of the overriding class to determine whether and when to invoke the method @@ -273,8 +274,8 @@ Alternatively, object_class_by_name() can be used to obtain the class and its non-overridden methods for a specific type. This would correspond to ``MyClass::method(...)`` in C++. -The first example of such a QOM method was #CPUClass.reset, -another example is #DeviceClass.realize. +The first example of such a QOM method was ``CPUClass.reset``, +another example is ``DeviceClass.realize``. Standard type declaration and definition macros === -- 2.31.1
[PATCH for-6.2 02/10] docs: qom: Use Sphinx cross references more often
To make the document easier to navigate, use `reference` syntax
more often when mentioning other types, functions, and macros.
Signed-off-by: Eduardo Habkost
---
docs/devel/qom.rst | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
index 9c1be5d7fc2..3499a8ca3b6 100644
--- a/docs/devel/qom.rst
+++ b/docs/devel/qom.rst
@@ -48,7 +48,7 @@ In the above example, we create a simple type that is
described by `TypeInfo`.
from, the instance and class size, and constructor/destructor hooks.
Alternatively several static types could be registered using helper macro
-DEFINE_TYPES()
+`DEFINE_TYPES()`:
.. code-block:: c
@@ -72,10 +72,10 @@ are instantiated dynamically but there is only ever one
instance for any
given type. The `ObjectClass` typically holds a table of function pointers
for the virtual methods implemented by this type.
-Using object_new(), a new `Object` derivative will be instantiated. You can
+Using `object_new()`, a new `Object` derivative will be instantiated. You can
cast an `Object` to a subclass (or base-class) type using
-object_dynamic_cast(). You typically want to define macro wrappers around
-OBJECT_CHECK() and OBJECT_CLASS_CHECK() to make it easier to convert to a
+`object_dynamic_cast()`. You typically want to define macro wrappers around
+`OBJECT_CHECK()` and `OBJECT_CLASS_CHECK()` to make it easier to convert to a
specific type:
.. code-block:: c
@@ -195,7 +195,7 @@ except for trailing varargs.
Methods are always *virtual*. Overriding a method in
`TypeInfo`.class_init of a subclass leads to any user of the class obtained
-via OBJECT_GET_CLASS() accessing the overridden function.
+via `OBJECT_GET_CLASS()` accessing the overridden function.
The original function is not automatically invoked. It is the responsibility
of the overriding class to determine whether and when to invoke the method
being overridden.
@@ -270,7 +270,7 @@ class, which someone might choose to change at some point.
.class_init = derived_class_init,
};
-Alternatively, object_class_by_name() can be used to obtain the class and
+Alternatively, `object_class_by_name()` can be used to obtain the class and
its non-overridden methods for a specific type. This would correspond to
``MyClass::method(...)`` in C++.
@@ -285,9 +285,9 @@ convention. To reduce the amount of boilerplate code that
needs to be
written for a new type there are two sets of macros to generate the
common parts in a standard format.
-A type is declared using the OBJECT_DECLARE macro family. In types
+A type is declared using the ``OBJECT_DECLARE`` macro family. In types
which do not require any virtual functions in the class, the
-OBJECT_DECLARE_SIMPLE_TYPE macro is suitable, and is commonly placed
+`OBJECT_DECLARE_SIMPLE_TYPE` macro is suitable, and is commonly placed
in the header file:
.. code-block:: c
@@ -319,12 +319,12 @@ This is equivalent to the following:
The 'struct MyDevice' needs to be declared separately.
If the type requires virtual functions to be declared in the class
-struct, then the alternative OBJECT_DECLARE_TYPE() macro can be
-used. This does the same as OBJECT_DECLARE_SIMPLE_TYPE(), but without
+struct, then the alternative `OBJECT_DECLARE_TYPE()` macro can be
+used. This does the same as `OBJECT_DECLARE_SIMPLE_TYPE()`, but without
the 'struct MyDeviceClass' definition.
-To implement the type, the OBJECT_DEFINE macro family is available.
-In the simple case the OBJECT_DEFINE_TYPE macro is suitable:
+To implement the type, the ``OBJECT_DEFINE`` macro family is available.
+In the simple case the `OBJECT_DEFINE_TYPE()` macro is suitable:
.. code-block:: c
:caption: Defining a simple type
@@ -362,7 +362,7 @@ system, and the three standard methods now need to be
implemented
along with any other logic required for the type.
If the type needs to implement one or more interfaces, then the
-OBJECT_DEFINE_TYPE_WITH_INTERFACES() macro can be used instead.
+`OBJECT_DEFINE_TYPE_WITH_INTERFACES()` macro can be used instead.
This accepts an array of interface type names.
.. code-block:: c
@@ -374,7 +374,7 @@ This accepts an array of interface type names.
{ NULL })
If the type is not intended to be instantiated, then then
-the OBJECT_DEFINE_ABSTRACT_TYPE() macro can be used instead:
+the `OBJECT_DEFINE_ABSTRACT_TYPE()` macro can be used instead:
.. code-block:: c
:caption: Defining a simple abstract type
--
2.31.1
[PATCH for-6.2 04/10] docs: qom: Fix "API Reference" heading level
The API reference section was being rendered as a subsection of the "Standard type declaration and definition macros" subsection. Fix that. Signed-off-by: Eduardo Habkost --- docs/devel/qom.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst index 7ef16d92ca6..3f48016aa8f 100644 --- a/docs/devel/qom.rst +++ b/docs/devel/qom.rst @@ -385,6 +385,6 @@ the `OBJECT_DEFINE_ABSTRACT_TYPE()` macro can be used instead: API Reference -- += .. kernel-doc:: include/qom/object.h -- 2.31.1
[PATCH for-6.2 00/10] QOM documentation updates
This series includes a few fixes to the QOM documentation, and add some changes to avoid encouraging people from defining typecast macros manually. Eduardo Habkost (10): docs: qom: Replace old GTK-Doc #symbol syntax with `symbol` docs: qom: Use Sphinx cross references more often docs: qom: Fix autoptr expansion example docs: qom: Fix "API Reference" heading level docs: qom: Add subsection headings to declaration/definition macros section docs: qom: Remove unnecessary class typedefs from example docs: qom: Fix OBJECT_DECLARE_SIMPLE_TYPE documentation docs: qom: Show actual typecast functions in examples docs: qom: Remove OBJECT_CHECK macro examples MAINTAINERS: Add qom.rst to QOM section MAINTAINERS| 1 + docs/devel/qom.rst | 147 - 2 files changed, 94 insertions(+), 54 deletions(-) -- 2.31.1
Re: [PATCH for-6.2 04/43] target/hppa: Implement do_unaligned_access for user-only
On 7/29/21 3:15 AM, Peter Maydell wrote:
On Thu, 29 Jul 2021 at 01:57, Richard Henderson
wrote:
Signed-off-by: Richard Henderson
---
linux-user/hppa/cpu_loop.c | 2 +-
target/hppa/cpu.c | 8 +---
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
index 82d8183821..5ce30fec8b 100644
--- a/linux-user/hppa/cpu_loop.c
+++ b/linux-user/hppa/cpu_loop.c
@@ -161,7 +161,7 @@ void cpu_loop(CPUHPPAState *env)
case EXCP_UNALIGN:
info.si_signo = TARGET_SIGBUS;
info.si_errno = 0;
-info.si_code = 0;
+info.si_code = TARGET_BUS_ADRALN;
info._sifields._sigfault._addr = env->cr[CR_IOR];
queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
break;
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 2eace4ee12..55c0d81046 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -71,7 +71,6 @@ static void hppa_cpu_disas_set_info(CPUState *cs,
disassemble_info *info)
info->print_insn = print_insn_hppa;
}
-#ifndef CONFIG_USER_ONLY
static void hppa_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
MMUAccessType access_type,
int mmu_idx, uintptr_t retaddr)
@@ -80,15 +79,18 @@ static void hppa_cpu_do_unaligned_access(CPUState *cs,
vaddr addr,
CPUHPPAState *env = >env;
cs->exception_index = EXCP_UNALIGN;
+#ifdef CONFIG_USER_ONLY
+env->cr[CR_IOR] = addr;
Do we not need the top 32 bits of the address, which the softmmu
version is recording in CR_ISR ?
No. That's the "space" number, which for Linux user is always 0.
In system mode we use 64-bit addresses to make sure we can represent that; in user-only,
we use 32-bit addresses.
r~
+#else
if (env->psw & PSW_Q) {
/* ??? Needs tweaking for hppa64. */
env->cr[CR_IOR] = addr;
env->cr[CR_ISR] = addr >> 32;
}
+#endif
cpu_loop_exit_restore(cs, retaddr);
}
-#endif /* CONFIG_USER_ONLY */
-- PMM
Re: [PATCH for-6.2 01/43] hw/core: Make do_unaligned_access available to user-only
On 7/28/21 8:19 PM, Philippe Mathieu-Daudé wrote:
On 7/29/21 2:46 AM, Richard Henderson wrote:
We shouldn't be ignoring SIGBUS for user-only.
Move our existing TCGCPUOps hook out from CONFIG_SOFTMMU.
Signed-off-by: Richard Henderson
---
include/hw/core/tcg-cpu-ops.h | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index eab27d0c03..513d6bfe72 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -60,6 +60,13 @@ struct TCGCPUOps {
/** @debug_excp_handler: Callback for handling debug exceptions */
void (*debug_excp_handler)(CPUState *cpu);
+/**
+ * @do_unaligned_access: Callback for unaligned access handling
+ */
+void (*do_unaligned_access)(CPUState *cpu, vaddr addr,
+MMUAccessType access_type,
+int mmu_idx, uintptr_t retaddr);
Shouldn't it be QEMU_NORETURN?
I think in system mode we're allowed to return, letting the unaligned access continue.
But I'm not sure about that, and it may not even be used.
r~
Re: [PULL 0/7] Misc patches for QEMU 6.1-rc2
On Thu, 29 Jul 2021 at 13:49, Paolo Bonzini wrote: > > The following changes since commit 69ea12b19a15ae006521cd5cc0f627f27f738746: > > Merge remote-tracking branch 'remotes/armbru/tags/pull-misc-2021-07-27' > into staging (2021-07-28 13:32:12 +0100) > > are available in the Git repository at: > > https://gitlab.com/bonzini/qemu.git tags/for-upstream > > for you to fetch changes up to 4fe29344bef6c54a6eff7aa0343754f8a9df5715: > > libvhost-user: fix -Werror=format= warnings with __u64 fields (2021-07-29 > 10:15:52 +0200) > > > Bugfixes. > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1 for any user-visible changes. -- PMM
Re: [PULL 6/7] meson: fix meson 0.58 warning with libvhost-user subproject
Relative symlink should work on Windows. Only symlink that points to non-exist file would file. On Thu, Jul 29, 2021 at 11:09 PM Peter Maydell wrote: > > On Thu, 29 Jul 2021 at 15:05, Thomas Huth wrote: > > > > On 29/07/2021 14.58, Peter Maydell wrote: > > > On Thu, 29 Jul 2021 at 13:56, Paolo Bonzini wrote: > > >> > > >> From: Marc-André Lureau > > >> > > >> Meson now checks that subprojects do not access files from parent > > >> project. While we all agree this is best practice, libvhost-user also > > >> want to share a few headers with QEMU, and libvhost-user isn't really a > > >> standalone project at this point (although this is making the dependency > > >> a bit more explicit). > > >> > > >> Signed-off-by: Marc-André Lureau > > >> Message-Id: <20210505151313.203258-1-marcandre.lur...@redhat.com> > > >> Signed-off-by: Paolo Bonzini > > >> --- > > >> subprojects/libvhost-user/include/atomic.h | 1 + > > >> subprojects/libvhost-user/libvhost-user.c| 2 +- > > >> subprojects/libvhost-user/meson.build| 6 +- > > >> subprojects/libvhost-user/standard-headers/linux | 1 + > > > > > >> diff --git a/subprojects/libvhost-user/include/atomic.h b/subprojects/libvhost-user/include/atomic.h > > >> new file mode 12 > > >> index 00..8c2be64f7b > > >> --- /dev/null > > >> +++ b/subprojects/libvhost-user/include/atomic.h > > >> @@ -0,0 +1 @@ > > >> +../../../include/qemu/atomic.h > > >> \ No newline at end of file > > > > > >> diff --git a/subprojects/libvhost-user/standard-headers/linux b/subprojects/libvhost-user/standard-headers/linux > > >> new file mode 12 > > >> index 00..15a2378139 > > >> --- /dev/null > > >> +++ b/subprojects/libvhost-user/standard-headers/linux > > >> @@ -0,0 +1 @@ > > >> +../../../include/standard-headers/linux > > >> \ No newline at end of file > > > > > > > > > Should these really be missing the trailing newline ? > > > > It's a symlink, so yes, there does not need to be a newline in here. > > Interesting. How does it work on Windows hosts ? > > -- PMM > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
Re: modular tcg (was: Re: [PATCH v2 1/1] modules: Improve error message when module is not) found
On 29/07/21 11:14, Gerd Hoffmann wrote:
The common functions could be added TCGCPUOps to allow them being called via
CPUClass->tcg_ops->$name instead of a direct symbol reference. Not sure this
is a good idea though. Experimental patch covering tcg_exec_realizefn +
tcg_exec_unrealizefn below.
A lot of these (though probably not all) are already stubbed out as
"static inline" in include/exec/exec-all.h. I think they can be changed
to function pointers in the style of ui/spice-module.c
(accel/tcg/tcg-module.c?).
No idea yet how to handle arch-specific bits best. Seems there is no existing
infrastructure to untangle target-specific code and tcg, so this probably needs
something new.
If they are few enough, I would just move them out of target/i386/tcg
and into target/i386/cpu-sysemu.c.
Noticed softmmu/physmem.c has lots of CONFIG_TCG #ifdefs, splitting this into
softmmu/physmem-{common,tcg}.c is probably a good idea.
I only count one, and those function should be easily moved to
accel/tcg/cputlb.c (after all both physmem.c and cputlb.c used to be a
single file, exec.c, so this is just an oversight).
Paolo
Re: modular tcg
On 7/29/21 4:59 PM, Philippe Mathieu-Daudé wrote: > On 7/29/21 4:22 PM, Claudio Fontana wrote: >> On 7/29/21 1:34 PM, Philippe Mathieu-Daudé wrote: >>> On 7/29/21 12:44 PM, Claudio Fontana wrote: On 7/29/21 12:29 PM, Gerd Hoffmann wrote: > Hi, > >> And another comment: I think we should have some progress on ARM with >> the kvm/tcg split and with the KConfig of boards, before we continue >> here. > > Why? This can easily be tacked in parallel. We can flip the switch > for modular tcg per target in meson.build. > > take care, > Gerd > Because in the end we need to do this for ARM too and for the other archs too (s390 is already ok), and in order to be sure not to end up in a dead-end, I think it would be good to have at least a sketch for the other archs as well.. Just my 2c ofc, I think really here still ARM is behind, and we should help it catch up. If I had more time I would have pushed more on the ARM series, but.. yeah. >>> >>> IIUC Alex is waiting 6.2 release to respin. >>> >> >> How does the Kconfig for ARM improvements go? I mean I think those >> improvements (enabling only compatible boards with the chosen accelerators) >> are important for both tcg-kvm split and possibly for modularization of ARM >> accelerators too right? > > I think we all (Alex/you/me) reached the same point where builds work > but current the testing framework isn't ready for non-TCG or > modularized-TCG so the CI ends failing. > > I don't want to push for 'CI build-only' because most of the annoying > problems were from runtime (interfaces not resolved, ... which are > important when using modules or board with unavailable devices). > > I tried to address that with a QMP command to query accelerators but > there is a disagreement whether we should query for available/built-in/ > loaded/modularized-but-not-installed/...). At this point I think I > fairly understand the technical problems but misunderstand the big > picture here, in particular w.r.t. management apps. I spent too many > time on this to appear enthusiastic, sorry. > The Kconfig thing also depended on the querying the accelerator part, or not? I mean, was there the idea was "I want to build only the ARM boards that are compatible with the accel options I chose", which is totally sensible, and removes all the need for workarounds in the tcg/kvm split series Alex is now maintaining. I think you actually had a solution for quering the accelerators btw, and the problem was just caching the result to improve the performance? Thanks, Claudio
Re: [PATCH v2] block/io_uring: resubmit when result is -EAGAIN
On Thu, Jul 29, 2021 at 11:10:29AM +0200, Fabian Ebner wrote: > Linux SCSI can throw spurious -EAGAIN in some corner cases in its > completion path, which will end up being the result in the completed > io_uring request. > > Resubmitting such requests should allow block jobs to complete, even > if such spurious errors are encountered. > > Co-authored-by: Stefan Hajnoczi > Reviewed-by: Stefano Garzarella > Signed-off-by: Fabian Ebner > --- > > Changes from v1: > * Focus on what's relevant for the patch itself in the commit > message. > * Add Stefan's comment. > * Add Stefano's R-b tag (I hope that's fine, since there was no > change code-wise). > > block/io_uring.c | 16 +++- > 1 file changed, 15 insertions(+), 1 deletion(-) Thanks, applied to my block tree: https://gitlab.com/stefanha/qemu/commits/block Stefan signature.asc Description: PGP signature
Re: [PATCH v2 5/6] migration/postcopy: Handle RAMBlocks with a RamDiscardManager on the destination
On 29.07.21 17:52, Peter Xu wrote: On Thu, Jul 29, 2021 at 02:14:41PM +0200, David Hildenbrand wrote: On 24.07.21 00:10, Peter Xu wrote: On Fri, Jul 23, 2021 at 09:01:42PM +0200, David Hildenbrand wrote: It can happen in corner cases and is valid: with the current virtio-mem spec, guests are allowed to read unplugged memory. This will, for example, happen on older Linux guests when reading /proc/kcore or (with even older guests) when dumping guest memory via kdump. These corner cases were the main reason why the spec allows for it -- until we have guests properly adjusted such that it won't happen even in corner cases. A future feature bit will disallow it for the guest: required for supporting shmem/hugetlb cleanly. With that in place, I agree that we would want to warn in this case! OK that makes sense; with the page_size change, feel free to add: I just realized that relying on the page_size would be wrong. We migrate TARGET_PAGE_SIZE chunks and the offset might not be page_size aligned. So if we were to replace TARGET_PAGE_SIZE by rb->page_size, we might accidentally cover a "too big" range. I'm wondering whether we should make the offset page size aligned instead. For example, note that postcopy_place_page_zero() should only take page_size aligned host addr or UFFDIO_COPY could fail (hugetlb doesn't support UFFDIO_ZEROPAGE yet). That is true indeed. I'd assume in that case that we would get called with the proper offset already, right? Because uffd would only report properly aligned pages IIRC. Btw, does virtio-mem supports hugetlbfs now? When with it, the smallest unit to plug/unplug would the huge page size (e.g., for 1g huge page, sounds not helpful to unplug 2M memory), am I right? It supports it to 99.9 % I'd say (especially with the dump/tpm/migration fixes upstream). The units are handled properly: the unit is at least as big as the page size. So with 1 GiB pages, you have a unit of 1 GiB. -- Thanks, David / dhildenb
Re: [PATCH RFC 0/3] mirror: rework soft-cancelling READY mirror
29.07.2021 16:47, Max Reitz wrote: On 29.07.21 13:35, Vladimir Sementsov-Ogievskiy wrote: 29.07.2021 13:38, Max Reitz wrote: On 29.07.21 12:02, Vladimir Sementsov-Ogievskiy wrote: 28.07.2021 10:00, Max Reitz wrote: On 27.07.21 18:47, Vladimir Sementsov-Ogievskiy wrote: Hi all! That's an alternative to (part of) Max's "[PATCH for-6.1? v2 0/7] mirror: Handle errors after READY cancel" and shows' my idea of handling soft-cancelling READY mirror case directly in qmp_block_job_cancel. And cleanup all other job cancelling functions. That's untested draft, don't take it to heart :) Well, I would have preferred it if you’d rebased this on top of that series, precisely because it’s an alternative to only part of it. And if it’s just an untested draft, that would have been even better, because it would’ve given a better idea on what the cleanup looks like. There are also things like this series making cancel internally always a force-cancel, where I’m not sure whether we want that in the replication driver or not[1]. With my series, we add an explicit parameter, so we’re forced to think about it, and then in this series on top we can just drop the parameter for all force-cancel invocations again, and for all non-force-cancel invocations we would have to think a bit more. I now don't sure that patch 5 of your series is correct (see my last answer to it), that's why I decided to not base on it. Well, we can always take patch 5 from v1. (Where I changed any job_is_cancelled() to job_cancel_requested() when it influenced the external interface.) My series has the benefit of handling soft-mirror-cancel case the other way and handles mirror finalization in case of soft-cancel properly. Specifically as for this series, I don’t like job_complete_ex() very much, I think the parameter should be part of job_complete() itself. That was my idea. But job_complete is passed as function pointer, so changing its prototype would be more work.. But I think it's possible. If we think that’s too specific of a mirror parameter to include in normal job_complete(), well, then there shouldn’t be a job_complete_ex() either, and do_graph_change should be a property of the mirror job (perhaps as pivot_on_completion) that’s cleared by qmp_block_job_cancel() before invoking job_complete(). This way users will lose a way to make a decision during job running.. On the contrary, it would be a precursor to letting the user change this property explicitly with a new QMP command. But probably they don't need actually. Moving the option to mirror job parameter seems a good option to me. Max [1] Although looking at it again now, it probably wants force-cancel. What do you think of my idea to keep old bugs as is and just deprecate block-job-cancel and add a new interface for "no-graph-change mirror" case? I don’t see a reason for that. The fix isn’t that complicated. Also, honestly, I don’t see a good reason for deprecating anything. Current interface lead to mess in the code, that's bad. Cancellation mode that is actually a kind of completion (and having comments in many places about that) - that shows for me that interface is not good.. It's a question of terminology, what to call "cancel". Also, that's not the first time this question arise. Remember my recent cancel-in-flight-requests series, when I thought that "cancel is cancel" and didn't consider soft-cancel of mirror.. And reviewers didn't caught it. I don't think that interface is good, it will always confuse new developers and users. But that's just my opinion, I don't impose it ) If not deprecate, i.e. if we consider old interface to be good, than no reason for this my series and for introducing new interface :) I’m not against a better interface, I’m against using this current bug as an excuse to improve the interface. We’ve known we want to improve the interface for quite a long time now, we don’t need an excuse for that. If we use this bug as an excuse, I’m afraid of becoming hung up on interface discussions instead of just getting the bug fixed. And we must get the bug fixed, it’s real, it’s kind of bad, and saying “it won’t appear with the new interface, let’s not worry about the old one” is not something I like. OTOH, if we use this bug as an excuse, I’m also afraid of trying to rush the design instead of actually implementing the interface that we’ve always desired, i.e. where the user gets to choose the completion mode via yet-to-be-implemented some job property setter function. As a final note (but this is precisely the interface discussion that I want to avoid for now), I said I don’t see a good reason for deprecating anything, because `job-cancel force=false` can just internally do `set-job-property .pivot_on_completion=false; job-complete`. From an implementation perspective, that should be simple. I understand that for users the existence of the `force` flag may still be
Re: [PATCH v2 0/6] migration/ram: Optimize for virtio-mem via RamDiscardManager
On 29.07.21 18:12, Peter Xu wrote: On Thu, Jul 29, 2021 at 10:14:47AM +0200, David Hildenbrand wrote: The thing is I still think this extra operation during sync() can be ignored by simply clear dirty log during bitmap init, then.. why not? :) I guess clearing the dirty log (especially in KVM) might be more expensive. If we send one ioctl per cb that'll be expensive for sure. I think it'll be fine if we send one clear ioctl to kvm, summarizing the whole bitmap to clear. The other thing is imho having overhead during bitmap init is always better than having that during sync(). :) Oh, right, so you're saying, after we set the dirty bmap to all ones and excluded the discarded parts, setting the respective bits to 0, we simply issue clearing of the whole area? For now I assumed we would have to clear per cb. Hmm when I replied I thought we can pass in a bitmap to ->log_clear() but I just remembered memory API actually hides the bitmap interface.. Reset the whole region works, but it'll slow down migration starts, more importantly that'll be with mmu write lock so we will lose most clear-log benefit for the initial round of migration and stuck the guest #pf at the meantime... Let's try do that in cb()s as you mentioned; I think that'll still be okay, because so far the clear log block size is much larger (1gb), 1tb is worst case 1000 ioctls during bitmap init, slightly better than 250k calls during sync(), maybe? :) Just to get it right, what you propose is calling migration_clear_memory_region_dirty_bitmap_range() from each cb(). Right. We can provide a more complicated memory api for passing in bitmap but I think that can be an overkill and tricky. Due to the clear_bmap, we will end up clearing each chunk (e.g., 1GB) at most once. But if our layout is fragmented, we can actually end up clearing all chunks (1024 ioctls for 1TB), resulting in a slower migration start. Any gut feeling how much slower migration start could be with largish (e.g., 1 TiB) regions? I had a vague memory of KVM_GET_DIRTY_LOG that I used to measure which took ~10ms for 1g guest mem, supposing that's mostly used to protect the pages or clearing dirties in the EPT pgtables. Then the worst case is ~1 second for 1tb. But note that it's still during setup phase, so we should expect to see a somehow large setup time and longer period that migration stays in SETUP state, but I think it's fine. Reasons: - We don't care too much about guest dirtying pages during the setup process because we haven't migrated anything yet, meanwhile we should not block any other thread either (e.g., we don't hold BQL). - We don't block guest execution too. Unlike KVM_GET_DIRTY_LOG without CLEAR we won't hold the mmu lock for a huge long time but do it only in 1g chunk, so guest page faults can still be serviced. It'll be affected somehow since we'll still run with the mmu write lock critical sections for each single ioctl(), but we do that for 1gb each time so we frequently yield it. Please note that we are holding the iothread lock while setting up the bitmaps + syncing the dirty log. I'll have to make sure that that code runs outside of the BQL, otherwise we'll block guest execution. In the meantime I adjusted the code but it does the clearing under the iothread lock, which should not be what we want ... I'll have a look. -- Thanks, David / dhildenb
Re: [PATCH] MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver
On Wed, Jul 28, 2021 at 08:33:40PM +0200, Philippe Mathieu-Daudé wrote: > I'm interested in following the activity around the NVMe bdrv. > > Signed-off-by: Philippe Mathieu-Daudé > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) Thanks, applied to my block tree: https://gitlab.com/stefanha/qemu/commits/block Stefan signature.asc Description: PGP signature
[PULL for-6.1 0/3] Block patches
The following changes since commit 3521ade3510eb5cefb2e27a101667f25dad89935: Merge remote-tracking branch 'remotes/thuth-gitlab/tags/pull-request-2021-07-29' into staging (2021-07-29 13:17:20 +0100) are available in the Git repository at: https://gitlab.com/stefanha/qemu.git tags/block-pull-request for you to fetch changes up to cc8eecd7f105a1dff5876adeb238a14696061a4a: MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver (2021-07-29 17:17:34 +0100) Pull request The main fix here is for io_uring. Spurious -EAGAIN errors can happen and the request needs to be resubmitted. The MAINTAINERS changes carry no risk and we might as well include them in QEMU 6.1. Fabian Ebner (1): block/io_uring: resubmit when result is -EAGAIN Philippe Mathieu-Daudé (1): MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver Stefano Garzarella (1): MAINTAINERS: add Stefano Garzarella as io_uring reviewer MAINTAINERS | 2 ++ block/io_uring.c | 16 +++- 2 files changed, 17 insertions(+), 1 deletion(-) -- 2.31.1
[PULL for-6.1 1/3] MAINTAINERS: add Stefano Garzarella as io_uring reviewer
From: Stefano Garzarella I've been working with io_uring for a while so I'd like to help with reviews. Signed-off-by: Stefano Garzarella Message-Id: <20210728131515.131045-1-sgarz...@redhat.com> Signed-off-by: Stefan Hajnoczi --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 42ac45c3e5..1776d0950b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3257,6 +3257,7 @@ Linux io_uring M: Aarushi Mehta M: Julia Suvorova M: Stefan Hajnoczi +R: Stefano Garzarella L: qemu-bl...@nongnu.org S: Maintained F: block/io_uring.c -- 2.31.1
[PULL for-6.1 2/3] block/io_uring: resubmit when result is -EAGAIN
From: Fabian Ebner
Linux SCSI can throw spurious -EAGAIN in some corner cases in its
completion path, which will end up being the result in the completed
io_uring request.
Resubmitting such requests should allow block jobs to complete, even
if such spurious errors are encountered.
Co-authored-by: Stefan Hajnoczi
Reviewed-by: Stefano Garzarella
Signed-off-by: Fabian Ebner
Message-id: 20210729091029.65369-1-f.eb...@proxmox.com
Signed-off-by: Stefan Hajnoczi
---
block/io_uring.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/block/io_uring.c b/block/io_uring.c
index 00a3ee9fb8..dfa475cc87 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -165,7 +165,21 @@ static void luring_process_completions(LuringState *s)
total_bytes = ret + luringcb->total_read;
if (ret < 0) {
-if (ret == -EINTR) {
+/*
+ * Only writev/readv/fsync requests on regular files or host block
+ * devices are submitted. Therefore -EAGAIN is not expected but
it's
+ * known to happen sometimes with Linux SCSI. Submit again and hope
+ * the request completes successfully.
+ *
+ * For more information, see:
+ *
https://lore.kernel.org/io-uring/20210727165811.284510-3-ax...@kernel.dk/T/#u
+ *
+ * If the code is changed to submit other types of requests in the
+ * future, then this workaround may need to be extended to deal
with
+ * genuine -EAGAIN results that should not be resubmitted
+ * immediately.
+ */
+if (ret == -EINTR || ret == -EAGAIN) {
luring_resubmit(s, luringcb);
continue;
}
--
2.31.1
[PULL for-6.1 3/3] MAINTAINERS: Added myself as a reviewer for the NVMe Block Driver
From: Philippe Mathieu-Daudé I'm interested in following the activity around the NVMe bdrv. Signed-off-by: Philippe Mathieu-Daudé Message-id: 20210728183340.2018313-1-phi...@redhat.com Signed-off-by: Stefan Hajnoczi --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 1776d0950b..a3d598459a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3178,6 +3178,7 @@ F: block/null.c NVMe Block Driver M: Stefan Hajnoczi R: Fam Zheng +R: Philippe Mathieu-Daudé L: qemu-bl...@nongnu.org S: Supported F: block/nvme* -- 2.31.1
Re: [PING][PING][PATCH v2] vhost: make SET_VRING_ADDR, SET_FEATURES send replies
On Thu, Jul 29, 2021 at 02:53:53PM +0200, Philippe Mathieu-Daudé wrote:
> Cc more ppl.
This needs to go through Michael Tsirkin's tree.
Stefan
>
> On 7/29/21 12:56 PM, Denis Plotnikov wrote:
> >
> > On 23.07.2021 12:59, Denis Plotnikov wrote:
> >>
> >> ping!
> >>
> >> On 19.07.2021 17:21, Denis Plotnikov wrote:
> >>> On vhost-user-blk migration, qemu normally sends a number of commands
> >>> to enable logging if VHOST_USER_PROTOCOL_F_LOG_SHMFD is negotiated.
> >>> Qemu sends VHOST_USER_SET_FEATURES to enable buffers logging and
> >>> VHOST_USER_SET_VRING_ADDR per each started ring to enable "used ring"
> >>> data logging.
> >>> The issue is that qemu doesn't wait for reply from the vhost daemon
> >>> for these commands which may result in races between qemu expectation
> >>> of logging starting and actual login starting in vhost daemon.
> >>>
> >>> The race can appear as follows: on migration setup, qemu enables dirty
> >>> page
> >>> logging by sending VHOST_USER_SET_FEATURES. The command doesn't arrive to
> >>> a
> >>> vhost-user-blk daemon immediately and the daemon needs some time to turn
> >>> the
> >>> logging on internally. If qemu doesn't wait for reply, after sending the
> >>> command, qemu may start migrate memory pages to a destination. At this
> >>> time,
> >>> the logging may not be actually turned on in the daemon but some guest
> >>> pages,
> >>> which the daemon is about to write to, may have already been transferred
> >>> without logging to the destination. Since the logging wasn't turned on,
> >>> those pages won't be transferred again as dirty. So we may end up with
> >>> corrupted data on the destination.
> >>> The same scenario is applicable for "used ring" data logging, which is
> >>> turned on with VHOST_USER_SET_VRING_ADDR command.
> >>>
> >>> To resolve this issue, this patch makes qemu wait for the commands result
> >>> explicilty if VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated and
> >>> logging is enabled.
> >>>
> >>> Signed-off-by: Denis Plotnikov
> >>> ---
> >>> v1 -> v2:
> >>> * send reply only when logging is enabled [mst]
> >>>
> >>> v0 -> v1:
> >>> * send reply for SET_VRING_ADDR, SET_FEATURES only [mst]
> >>>
> >>> hw/virtio/vhost-user.c | 37 ++---
> >>> 1 file changed, 34 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> >>> index ee57abe04526..133588b3961e 100644
> >>> --- a/hw/virtio/vhost-user.c
> >>> +++ b/hw/virtio/vhost-user.c
> >>> @@ -1095,6 +1095,11 @@ static int vhost_user_set_mem_table(struct
> >>> vhost_dev *dev,
> >>> return 0;
> >>> }
> >>>
> >>> +static bool log_enabled(uint64_t features)
> >>> +{
> >>> +return !!(features & (0x1ULL << VHOST_F_LOG_ALL));
> >>> +}
> >>> +
> >>> static int vhost_user_set_vring_addr(struct vhost_dev *dev,
> >>> struct vhost_vring_addr *addr)
> >>> {
> >>> @@ -1105,10 +1110,21 @@ static int vhost_user_set_vring_addr(struct
> >>> vhost_dev *dev,
> >>> .hdr.size = sizeof(msg.payload.addr),
> >>> };
> >>>
> >>> +bool reply_supported = virtio_has_feature(dev->protocol_features,
> >>> +
> >>> VHOST_USER_PROTOCOL_F_REPLY_ACK);
> >>> +
> >>> +if (reply_supported && log_enabled(msg.hdr.flags)) {
> >>> +msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
> >>> +}
> >>> +
> >>> if (vhost_user_write(dev, , NULL, 0) < 0) {
> >>> return -1;
> >>> }
> >>>
> >>> +if (msg.hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
> >>> +return process_message_reply(dev, );
> >>> +}
> >>> +
> >>> return 0;
> >>> }
> >>>
> >>> @@ -1288,7 +1304,8 @@ static int vhost_user_set_vring_call(struct
> >>> vhost_dev *dev,
> >>> return vhost_set_vring_file(dev, VHOST_USER_SET_VRING_CALL, file);
> >>> }
> >>>
> >>> -static int vhost_user_set_u64(struct vhost_dev *dev, int request,
> >>> uint64_t u64)
> >>> +static int vhost_user_set_u64(struct vhost_dev *dev, int request,
> >>> uint64_t u64,
> >>> + bool need_reply)
> >>> {
> >>> VhostUserMsg msg = {
> >>> .hdr.request = request,
> >>> @@ -1297,23 +1314,37 @@ static int vhost_user_set_u64(struct vhost_dev
> >>> *dev, int request, uint64_t u64)
> >>> .hdr.size = sizeof(msg.payload.u64),
> >>> };
> >>>
> >>> +if (need_reply) {
> >>> +bool reply_supported = virtio_has_feature(dev->protocol_features,
> >>> +
> >>> VHOST_USER_PROTOCOL_F_REPLY_ACK);
> >>> +if (reply_supported) {
> >>> +msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
> >>> +}
> >>> +}
> >>> +
> >>> if (vhost_user_write(dev, , NULL, 0) < 0) {
> >>> return -1;
> >>> }
> >>>
> >>> +if (msg.hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
> >>> +return process_message_reply(dev, );
> >>> +}
> >>> +
> >>>
Re: [PATCH v2 0/6] migration/ram: Optimize for virtio-mem via RamDiscardManager
On Thu, Jul 29, 2021 at 10:14:47AM +0200, David Hildenbrand wrote: > > > > > > The thing is I still think this extra operation during sync() can > > > > > > be ignored by > > > > > > simply clear dirty log during bitmap init, then.. why not? :) > > > > > > > > > > I guess clearing the dirty log (especially in KVM) might be more > > > > > expensive. > > > > > > > > If we send one ioctl per cb that'll be expensive for sure. I think > > > > it'll be > > > > fine if we send one clear ioctl to kvm, summarizing the whole bitmap to > > > > clear. > > > > > > > > The other thing is imho having overhead during bitmap init is always > > > > better > > > > than having that during sync(). :) > > > > > > Oh, right, so you're saying, after we set the dirty bmap to all ones and > > > excluded the discarded parts, setting the respective bits to 0, we simply > > > issue clearing of the whole area? > > > > > > For now I assumed we would have to clear per cb. > > > > Hmm when I replied I thought we can pass in a bitmap to ->log_clear() but I > > just remembered memory API actually hides the bitmap interface.. > > > > Reset the whole region works, but it'll slow down migration starts, more > > importantly that'll be with mmu write lock so we will lose most clear-log > > benefit for the initial round of migration and stuck the guest #pf at the > > meantime... > > > > Let's try do that in cb()s as you mentioned; I think that'll still be okay, > > because so far the clear log block size is much larger (1gb), 1tb is worst > > case > > 1000 ioctls during bitmap init, slightly better than 250k calls during > > sync(), > > maybe? :) > > Just to get it right, what you propose is calling > migration_clear_memory_region_dirty_bitmap_range() from each cb(). Right. We can provide a more complicated memory api for passing in bitmap but I think that can be an overkill and tricky. > Due to the clear_bmap, we will end up clearing each chunk (e.g., 1GB) at most > once. > > But if our layout is fragmented, we can actually end up clearing all chunks > (1024 ioctls for 1TB), resulting in a slower migration start. > > Any gut feeling how much slower migration start could be with largish (e.g., > 1 TiB) regions? I had a vague memory of KVM_GET_DIRTY_LOG that I used to measure which took ~10ms for 1g guest mem, supposing that's mostly used to protect the pages or clearing dirties in the EPT pgtables. Then the worst case is ~1 second for 1tb. But note that it's still during setup phase, so we should expect to see a somehow large setup time and longer period that migration stays in SETUP state, but I think it's fine. Reasons: - We don't care too much about guest dirtying pages during the setup process because we haven't migrated anything yet, meanwhile we should not block any other thread either (e.g., we don't hold BQL). - We don't block guest execution too. Unlike KVM_GET_DIRTY_LOG without CLEAR we won't hold the mmu lock for a huge long time but do it only in 1g chunk, so guest page faults can still be serviced. It'll be affected somehow since we'll still run with the mmu write lock critical sections for each single ioctl(), but we do that for 1gb each time so we frequently yield it. Comparing to the other solution, it'll be different if we spend time during sync() because during migration guest vcpu threads are dirtying pages that we migrated so page dirtying is aggregating, it should make the migration to converge harder just like when sync() is slower. Thanks, -- Peter Xu
Re: aarch64 efi boot failures with qemu 6.0+
On Thu, Jul 29, 2021 at 09:42:52AM -0500, Bjorn Helgaas wrote: > On Thu, Jul 29, 2021 at 3:08 AM Philippe Mathieu-Daudé > wrote: > > > Michael, if describing the issue in the revert is too complex, could you > > include a link to this thread in the revert description? > > (Message-Id: <20210724185234.ga2265...@roeck-us.net> or > > https://www.mail-archive.com/qemu-devel@nongnu.org/msg826392.html) > > Or https://lore.kernel.org/r/20210724185234.ga2265...@roeck-us.net/, > which is a convenient, ad-free, long-term, text-only, tools-friendly > archive maintained by the Linux Foundation. OK, thanks! -- MST
Re: [PATCH v2 5/6] migration/postcopy: Handle RAMBlocks with a RamDiscardManager on the destination
On Thu, Jul 29, 2021 at 02:14:41PM +0200, David Hildenbrand wrote: > On 24.07.21 00:10, Peter Xu wrote: > > On Fri, Jul 23, 2021 at 09:01:42PM +0200, David Hildenbrand wrote: > > > It can happen in corner cases and is valid: with the current virtio-mem > > > spec, guests are allowed to read unplugged memory. This will, for example, > > > happen on older Linux guests when reading /proc/kcore or (with even older > > > guests) when dumping guest memory via kdump. These corner cases were the > > > main reason why the spec allows for it -- until we have guests properly > > > adjusted such that it won't happen even in corner cases. > > > > > > A future feature bit will disallow it for the guest: required for > > > supporting > > > shmem/hugetlb cleanly. With that in place, I agree that we would want to > > > warn in this case! > > > > OK that makes sense; with the page_size change, feel free to add: > > I just realized that relying on the page_size would be wrong. > > We migrate TARGET_PAGE_SIZE chunks and the offset might not be page_size > aligned. So if we were to replace TARGET_PAGE_SIZE by rb->page_size, we > might accidentally cover a "too big" range. I'm wondering whether we should make the offset page size aligned instead. For example, note that postcopy_place_page_zero() should only take page_size aligned host addr or UFFDIO_COPY could fail (hugetlb doesn't support UFFDIO_ZEROPAGE yet). Btw, does virtio-mem supports hugetlbfs now? When with it, the smallest unit to plug/unplug would the huge page size (e.g., for 1g huge page, sounds not helpful to unplug 2M memory), am I right? -- Peter Xu
Re: [PATCH] tests: Fix migration-test build failure for sparc
On Thu, Jul 29, 2021 at 01:48:57AM +0200, Philippe Mathieu-Daudé wrote:
> On 7/28/21 11:41 PM, Peter Xu wrote:
> > Even if seems to exist for all archs on linux, however
> > including
> > it with __linux__ defined seems to be not working yet as it'll try to
> > include
> > asm/kvm.h and that can be missing for archs that do not support kvm.
> >
> > To fix this (instead of any attempt to fix linux headers..), we can mark the
> > header to be x86_64 only, because it's so far only service for adding the
> > kvm
> > dirty ring test.
> >
> > No need to have "Fixes" as the issue is just introduced very recently.
>
> Personally I find it very useful to navigate in gitk without having
> to use git-blame.
Makes sense.
>
> Fixes: 1f546b709d6 ("tests: migration-test: Add dirty ring test")
> Reviewed-by: Philippe Mathieu-Daudé
Thanks all!
--
Peter Xu
Re: [PULL 6/7] meson: fix meson 0.58 warning with libvhost-user subproject
On Thu, 29 Jul 2021 at 15:05, Thomas Huth wrote: > > On 29/07/2021 14.58, Peter Maydell wrote: > > On Thu, 29 Jul 2021 at 13:56, Paolo Bonzini wrote: > >> > >> From: Marc-André Lureau > >> > >> Meson now checks that subprojects do not access files from parent > >> project. While we all agree this is best practice, libvhost-user also > >> want to share a few headers with QEMU, and libvhost-user isn't really a > >> standalone project at this point (although this is making the dependency > >> a bit more explicit). > >> > >> Signed-off-by: Marc-André Lureau > >> Message-Id: <20210505151313.203258-1-marcandre.lur...@redhat.com> > >> Signed-off-by: Paolo Bonzini > >> --- > >> subprojects/libvhost-user/include/atomic.h | 1 + > >> subprojects/libvhost-user/libvhost-user.c| 2 +- > >> subprojects/libvhost-user/meson.build| 6 +- > >> subprojects/libvhost-user/standard-headers/linux | 1 + > > > >> diff --git a/subprojects/libvhost-user/include/atomic.h > >> b/subprojects/libvhost-user/include/atomic.h > >> new file mode 12 > >> index 00..8c2be64f7b > >> --- /dev/null > >> +++ b/subprojects/libvhost-user/include/atomic.h > >> @@ -0,0 +1 @@ > >> +../../../include/qemu/atomic.h > >> \ No newline at end of file > > > >> diff --git a/subprojects/libvhost-user/standard-headers/linux > >> b/subprojects/libvhost-user/standard-headers/linux > >> new file mode 12 > >> index 00..15a2378139 > >> --- /dev/null > >> +++ b/subprojects/libvhost-user/standard-headers/linux > >> @@ -0,0 +1 @@ > >> +../../../include/standard-headers/linux > >> \ No newline at end of file > > > > > > Should these really be missing the trailing newline ? > > It's a symlink, so yes, there does not need to be a newline in here. Interesting. How does it work on Windows hosts ? -- PMM
Re: [PULL 0/9] Gitlab-CI improvements and some other fixes
On Thu, 29 Jul 2021 at 09:22, Thomas Huth wrote: > > Hi Peter! > > The following changes since commit 69ea12b19a15ae006521cd5cc0f627f27f738746: > > Merge remote-tracking branch 'remotes/armbru/tags/pull-misc-2021-07-27' > into staging (2021-07-28 13:32:12 +0100) > > are available in the Git repository at: > > https://gitlab.com/thuth/qemu.git tags/pull-request-2021-07-29 > > for you to fetch changes up to b8ee198d21c4bab41b8cb8d1729a956d9f648997: > > configure script fix for Haiku (2021-07-29 08:09:32 +0200) > > > * Document GitLab custom CI/CD variables > * Fix 'when:' condition in gitlab-CI jobs > * Disable tests in the gitlab-CI that fail due to out-of-memory conditions > * Allow pushing to "staging" again for maintainers without s390x access > * Fix migration-test build failure on SPARC > * Compile without "pie" on Haiku Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1 for any user-visible changes. -- PMM
Re: [PATCH for-6.2 17/43] accel/tcg: Report unaligned atomics for user-only
On Thu, 29 Jul 2021 at 02:09, Richard Henderson
wrote:
>
> Use the newly exposed do_unaligned_access hook from atomic_mmu_lookup,
> which has access to complete alignment info from the TCGMemOpIdx arg.
>
> Signed-off-by: Richard Henderson
> ---
> accel/tcg/user-exec.c | 23 ++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 90d1a2d327..dd77e90789 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -852,6 +852,16 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>
> /* The softmmu versions of these helpers are in cputlb.c. */
>
> +static void cpu_unaligned_access(CPUState *cpu, vaddr addr,
> + MMUAccessType access_type,
> + int mmu_idx, uintptr_t ra)
> +{
> +CPUClass *cc = CPU_GET_CLASS(cpu);
> +
> +cc->tcg_ops->do_unaligned_access(cpu, addr, access_type, mmu_idx, ra);
> +g_assert_not_reached();
> +}
The softmmu version doesn't g_assert_not_reached(), I think
perhaps with the intent that a CPU implementation could
in some cases return without raising an exception to
mean "continue with the unaligned access". We should decide
whether we want the API to permit that, or else consistently
have both softmmu and useronly versions be marked noreturn
and with an assert, and we should document whichever we choose.
-- PMM
Re: modular tcg
On 7/29/21 4:22 PM, Claudio Fontana wrote: > On 7/29/21 1:34 PM, Philippe Mathieu-Daudé wrote: >> On 7/29/21 12:44 PM, Claudio Fontana wrote: >>> On 7/29/21 12:29 PM, Gerd Hoffmann wrote: Hi, > And another comment: I think we should have some progress on ARM with > the kvm/tcg split and with the KConfig of boards, before we continue > here. Why? This can easily be tacked in parallel. We can flip the switch for modular tcg per target in meson.build. take care, Gerd >>> >>> Because in the end we need to do this for ARM too and for the other archs >>> too (s390 is already ok), >>> >>> and in order to be sure not to end up in a dead-end, I think it would be >>> good to have at least a sketch for the other archs as well.. >>> >>> Just my 2c ofc, I think really here still ARM is behind, and we should help >>> it catch up. >>> >>> If I had more time I would have pushed more on the ARM series, but.. yeah. >> >> IIUC Alex is waiting 6.2 release to respin. >> > > How does the Kconfig for ARM improvements go? I mean I think those > improvements (enabling only compatible boards with the chosen accelerators) > are important for both tcg-kvm split and possibly for modularization of ARM > accelerators too right? I think we all (Alex/you/me) reached the same point where builds work but current the testing framework isn't ready for non-TCG or modularized-TCG so the CI ends failing. I don't want to push for 'CI build-only' because most of the annoying problems were from runtime (interfaces not resolved, ... which are important when using modules or board with unavailable devices). I tried to address that with a QMP command to query accelerators but there is a disagreement whether we should query for available/built-in/ loaded/modularized-but-not-installed/...). At this point I think I fairly understand the technical problems but misunderstand the big picture here, in particular w.r.t. management apps. I spent too many time on this to appear enthusiastic, sorry.
Re: [PATCH for-6.2 16/43] target/xtensa: Implement do_unaligned_access for user-only
On Thu, 29 Jul 2021 at 02:03, Richard Henderson wrote: > > Cc: Max Filippov > Signed-off-by: Richard Henderson > --- > target/xtensa/cpu.c| 2 +- > target/xtensa/helper.c | 30 +++--- > 2 files changed, 16 insertions(+), 16 deletions(-) The xtensa kernel has a CONFIG_XTENSA_UNALIGNED_USER option to make the kernel silently fix up unaligned userspace accesses, and most of the defconfigs for xtensa set it to 'y'. -- PMM
Re: [PATCH for-6.2 14/43] target/sparc: Set fault address in sparc_cpu_do_unaligned_access
On Thu, 29 Jul 2021 at 02:01, Richard Henderson wrote: > > We ought to have been recording the virtual address for reporting > to the guest trap handler. Mirror the SFSR FIXME from the sparc64 > version of get_physical_address_data. > > Cc: Mark Cave-Ayland > Signed-off-by: Richard Henderson > --- > target/sparc/ldst_helper.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c > index 974afea041..7367b48c8b 100644 > --- a/target/sparc/ldst_helper.c > +++ b/target/sparc/ldst_helper.c > @@ -1963,6 +1963,14 @@ void QEMU_NORETURN > sparc_cpu_do_unaligned_access(CPUState *cs, vaddr addr, > SPARCCPU *cpu = SPARC_CPU(cs); > CPUSPARCState *env = >env; > > +#ifdef TARGET_SPARC64 > +/* FIXME: ASI field in SFSR must be set */ > +env->dmmu.sfsr = SFSR_VALID_BIT; /* Fault status register */ > +env->dmmu.sfar = addr; /* Fault address register */ > +#else > +env->mmuregs[4] = addr; > +#endif > + > cpu_raise_exception_ra(env, TT_UNALIGNED, retaddr); > } > #endif The architecture manual seems to be gratuitously opaque about whether and where the fault address for an alignment fault gets recorded, but Linux at least for 64-bit seems to pull it out of the sfar, so I guess that's right. We probably ought to have the "if SFSR_VALID_BIT already set, set the OW bit". MMUAccessType and mmu_idx give us enough to set the CT bits and the WRITE bit in the same way we do at the start of get_physical_address_data(). -- PMM
Re: aarch64 efi boot failures with qemu 6.0+
On Thu, Jul 29, 2021 at 3:08 AM Philippe Mathieu-Daudé wrote: > Michael, if describing the issue in the revert is too complex, could you > include a link to this thread in the revert description? > (Message-Id: <20210724185234.ga2265...@roeck-us.net> or > https://www.mail-archive.com/qemu-devel@nongnu.org/msg826392.html) Or https://lore.kernel.org/r/20210724185234.ga2265...@roeck-us.net/, which is a convenient, ad-free, long-term, text-only, tools-friendly archive maintained by the Linux Foundation.
Re: modular tcg
On 7/29/21 1:34 PM, Philippe Mathieu-Daudé wrote: > On 7/29/21 12:44 PM, Claudio Fontana wrote: >> On 7/29/21 12:29 PM, Gerd Hoffmann wrote: >>> Hi, >>> And another comment: I think we should have some progress on ARM with the kvm/tcg split and with the KConfig of boards, before we continue here. >>> >>> Why? This can easily be tacked in parallel. We can flip the switch >>> for modular tcg per target in meson.build. >>> >>> take care, >>> Gerd >>> >> >> Because in the end we need to do this for ARM too and for the other archs >> too (s390 is already ok), >> >> and in order to be sure not to end up in a dead-end, I think it would be >> good to have at least a sketch for the other archs as well.. >> >> Just my 2c ofc, I think really here still ARM is behind, and we should help >> it catch up. >> >> If I had more time I would have pushed more on the ARM series, but.. yeah. > > IIUC Alex is waiting 6.2 release to respin. > How does the Kconfig for ARM improvements go? I mean I think those improvements (enabling only compatible boards with the chosen accelerators) are important for both tcg-kvm split and possibly for modularization of ARM accelerators too right? Ciao, Claudio
[PATCH 2/2] vl: stop recording -smp in QemuOpts
-readconfig is still recording SMP options in QemuOpts instead of
using machine_opts_dict. This means that SMP options from -readconfig
are ignored.
Just stop using QemuOpts for -smp, making it return false for
is_qemuopts_group. Configuration files will merge the values in
machine_opts_dict using the new function machine_merge_property.
At the same time, fix -mem-prealloc which looked at QemuOpts to find the
number of guest CPUs, which it used as the default number of preallocation
threads.
Signed-off-by: Paolo Bonzini
---
softmmu/vl.c | 22 ++
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 93aef8e747..5ca11e7469 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -31,6 +31,7 @@
#include "qapi/compat-policy.h"
#include "qapi/error.h"
#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qstring.h"
#include "qapi/qmp/qjson.h"
#include "qemu-version.h"
#include "qemu/cutils.h"
@@ -2166,7 +2167,8 @@ static int global_init_func(void *opaque, QemuOpts *opts,
Error **errp)
static bool is_qemuopts_group(const char *group)
{
if (g_str_equal(group, "object") ||
-g_str_equal(group, "machine")) {
+g_str_equal(group, "machine") ||
+g_str_equal(group, "smp-opts")) {
return false;
}
return true;
@@ -2186,6 +2188,8 @@ static void qemu_record_config_group(const char *group,
QDict *dict,
*/
assert(!from_json);
keyval_merge(machine_opts_dict, dict, errp);
+} else if (g_str_equal(group, "smp-opts")) {
+machine_merge_property("smp", dict, _fatal);
} else {
abort();
}
@@ -2452,13 +2456,15 @@ static void qemu_validate_options(const QDict
*machine_opts)
static void qemu_process_sugar_options(void)
{
if (mem_prealloc) {
-char *val;
-
-val = g_strdup_printf("%d",
- (uint32_t)
qemu_opt_get_number(qemu_find_opts_singleton("smp-opts"), "cpus", 1));
-object_register_sugar_prop("memory-backend", "prealloc-threads", val,
- false);
-g_free(val);
+QObject *smp = qdict_get(machine_opts_dict, "smp");
+if (smp && qobject_type(smp) == QTYPE_QDICT) {
+QObject *cpus = qdict_get(qobject_to(QDict, smp), "cpus");
+if (cpus && qobject_type(cpus) == QTYPE_QSTRING) {
+const char *val = qstring_get_str(qobject_to(QString, cpus));
+object_register_sugar_prop("memory-backend",
"prealloc-threads",
+ val, false);
+}
+}
object_register_sugar_prop("memory-backend", "prealloc", "on", false);
}
--
2.31.1
[PATCH 1/2] vl: introduce machine_merge_property
It will be used to parse smp-opts config groups from configuration
files. The point to note is that it does not steal a reference
from the caller. This is better because this function will be called
from qemu_config_foreach's callback; qemu_config_foreach does not cede
its reference to the qdict to the callback, and wants to free it. To
balance that extra reference, machine_parse_property_opt now needs
a qobject_unref.
Signed-off-by: Paolo Bonzini
---
softmmu/vl.c | 25 +++--
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 4dee472c79..93aef8e747 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1534,23 +1534,36 @@ static void machine_help_func(const QDict *qdict)
}
}
+static void
+machine_merge_property(const char *propname, QDict *prop, Error **errp)
+{
+QDict *opts;
+
+opts = qdict_new();
+/* Preserve the caller's reference to prop. */
+qobject_ref(prop);
+qdict_put(opts, propname, prop);
+keyval_merge(machine_opts_dict, opts, errp);
+qobject_unref(opts);
+}
+
static void
machine_parse_property_opt(QemuOptsList *opts_list, const char *propname,
const char *arg, Error **errp)
{
-QDict *opts, *prop;
+QDict *prop = NULL;
bool help = false;
-ERRP_GUARD();
prop = keyval_parse(arg, opts_list->implied_opt_name, , errp);
if (help) {
qemu_opts_print_help(opts_list, true);
exit(0);
}
-opts = qdict_new();
-qdict_put(opts, propname, prop);
-keyval_merge(machine_opts_dict, opts, errp);
-qobject_unref(opts);
+if (!prop) {
+return;
+}
+machine_merge_property(propname, prop, errp);
+qobject_unref(prop);
}
static const char *pid_file;
--
2.31.1
[PATCH 0/2] vl: fix passing smp options via -readconfig
Even though [smp] does not work in config files, [smp-opts] does and lxd is using it. So, fix it. Paolo Bonzini (2): vl: introduce machine_merge_property vl: stop recording -smp in QemuOpts softmmu/vl.c | 47 +-- 1 file changed, 33 insertions(+), 14 deletions(-) -- 2.31.1
Re: [PULL 6/7] meson: fix meson 0.58 warning with libvhost-user subproject
On 29/07/2021 14.58, Peter Maydell wrote: On Thu, 29 Jul 2021 at 13:56, Paolo Bonzini wrote: From: Marc-André Lureau Meson now checks that subprojects do not access files from parent project. While we all agree this is best practice, libvhost-user also want to share a few headers with QEMU, and libvhost-user isn't really a standalone project at this point (although this is making the dependency a bit more explicit). Signed-off-by: Marc-André Lureau Message-Id: <20210505151313.203258-1-marcandre.lur...@redhat.com> Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/include/atomic.h | 1 + subprojects/libvhost-user/libvhost-user.c| 2 +- subprojects/libvhost-user/meson.build| 6 +- subprojects/libvhost-user/standard-headers/linux | 1 + diff --git a/subprojects/libvhost-user/include/atomic.h b/subprojects/libvhost-user/include/atomic.h new file mode 12 index 00..8c2be64f7b --- /dev/null +++ b/subprojects/libvhost-user/include/atomic.h @@ -0,0 +1 @@ +../../../include/qemu/atomic.h \ No newline at end of file diff --git a/subprojects/libvhost-user/standard-headers/linux b/subprojects/libvhost-user/standard-headers/linux new file mode 12 index 00..15a2378139 --- /dev/null +++ b/subprojects/libvhost-user/standard-headers/linux @@ -0,0 +1 @@ +../../../include/standard-headers/linux \ No newline at end of file Should these really be missing the trailing newline ? It's a symlink, so yes, there does not need to be a newline in here. Thomas
Re: [PATCH for-6.2 00/43] Unaligned accesses for user-only
On 7/29/21 8:14 AM, Philippe Mathieu-Daudé wrote: > On 7/29/21 2:46 AM, Richard Henderson wrote: >> This began with Peter wanting a cpu_ldst.h interface that can handle >> alignment info for Arm M-profile system mode, which will also compile >> for user-only without ifdefs. This is patch 32. >> >> Once I had that interface, I thought I might as well enforce the >> requested alignment in user-only. There are plenty of cases where >> we ought to have been doing that for quite a while. This took rather >> more work than I imagined to start. >> >> So far only x86 host has been fully converted to handle unaligned >> operations in user-only mode. I'll get to the others later. But >> the added testcase is fairly broad, and caught lots of bugs and/or >> missing code between target/ and linux-user/. >> >> Notes: >> * For target/i386 we have no way to signal SIGBUS from user-only. >> In theory we could go through do_unaligned_access in system mode, >> via #AC. But we don't even implement that control in tcg, probably >> because no one ever sets it. The cmpxchg16b insn requires alignment, >> but raises #GP, which maps to SIGSEGV. >> >> * For target/s390x we have no way to signal SIGBUS from user-only. >> The atomic operations raise PGM_SPECIFICATION, which the linux >> kernel maps to SIGILL. >> >> * I think target/hexagon should be setting TARGET_ALIGNED_ONLY=y. >> In the meantime, all memory accesses are allowed to be unaligned. > > Now I better understand what you tried to explain me last with > TCGCPUOps. Since Claudio was also involved, Cc'ing him (not asking > for a review, just in case he wants to follow up). > Thanks, what I understand from glancing through the thread is that at the time we made handling of unaligned access a sysmmu-only TCGCPUOps, while this series corrects that and implements unaligned accesses for user-only, so seems good to me. Ciao, Claudio
Re: [PATCH for-6.2 12/43] target/sh4: Implement do_unaligned_access for user-only
On Thu, 29 Jul 2021 at 02:01, Richard Henderson wrote: > > Cc: Yoshinori Sato > Signed-off-by: Richard Henderson > --- > linux-user/sh4/cpu_loop.c | 8 > target/sh4/cpu.c | 2 +- > target/sh4/op_helper.c| 3 --- > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/linux-user/sh4/cpu_loop.c b/linux-user/sh4/cpu_loop.c > index 222ed1c670..21d97250a8 100644 > --- a/linux-user/sh4/cpu_loop.c > +++ b/linux-user/sh4/cpu_loop.c > @@ -71,6 +71,14 @@ void cpu_loop(CPUSH4State *env) > info._sifields._sigfault._addr = env->tea; > queue_signal(env, info.si_signo, QEMU_SI_FAULT, ); > break; > +case 0xe0: > +case 0x100: > +info.si_signo = TARGET_SIGBUS; > +info.si_errno = 0; > +info.si_code = TARGET_BUS_ADRALN; > +info._sifields._sigfault._addr = env->tea; > +queue_signal(env, info.si_signo, QEMU_SI_FAULT, ); > +break; sh4 kernel default for unaligned accesses seems to be "warn and fixup", not SIGBUS, unless the user changes that by writing to /proc/cpu/alignment or the process changes it via prctl(). -- PMM
[PATCH] hyperv: Fix struct hv_message_header ordering
According to Hyper-V TLFS Version 6.0b, struct hv_message_header members
should be defined in the order:
message_type, _reserved, message_flags, payload_size
but we have it defined in the order:
message_type, payload_size, message_flags, _reserved
that is, the payload_size and _reserved members swapped. Due to this mix
up, we were inadvertently causing two issues:
- The payload_size field has invalid data; it didn't cause an issue
so far because we are delivering only timer messages which has fixed
size payloads the guest probably did a sizeof(payload) instead
relying on the value of payload_size member.
- The message_flags was always delivered as 0 to the guest;
fortunately, according to section 13.3.1 message_flags is also
treated as a reserved field.
Although this is not causing an issue now, it might in future (we are
adding more message types in our VSM implementation) so fix it to
reflect the specification.
Signed-off-by: Siddharth Chandrasekaran
---
include/hw/hyperv/hyperv-proto.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/hw/hyperv/hyperv-proto.h b/include/hw/hyperv/hyperv-proto.h
index 21dc28aee9..f578a60e78 100644
--- a/include/hw/hyperv/hyperv-proto.h
+++ b/include/hw/hyperv/hyperv-proto.h
@@ -101,9 +101,9 @@ struct hyperv_signal_event_input {
*/
struct hyperv_message_header {
uint32_t message_type;
-uint8_t payload_size;
-uint8_t message_flags; /* HV_MESSAGE_FLAG_XX */
uint8_t _reserved[2];
+uint8_t message_flags; /* HV_MESSAGE_FLAG_XX */
+uint8_t payload_size;
uint64_t sender;
};
--
2.17.1
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Re: [PATCH RFC 0/3] mirror: rework soft-cancelling READY mirror
On 29.07.21 13:35, Vladimir Sementsov-Ogievskiy wrote: 29.07.2021 13:38, Max Reitz wrote: On 29.07.21 12:02, Vladimir Sementsov-Ogievskiy wrote: 28.07.2021 10:00, Max Reitz wrote: On 27.07.21 18:47, Vladimir Sementsov-Ogievskiy wrote: Hi all! That's an alternative to (part of) Max's "[PATCH for-6.1? v2 0/7] mirror: Handle errors after READY cancel" and shows' my idea of handling soft-cancelling READY mirror case directly in qmp_block_job_cancel. And cleanup all other job cancelling functions. That's untested draft, don't take it to heart :) Well, I would have preferred it if you’d rebased this on top of that series, precisely because it’s an alternative to only part of it. And if it’s just an untested draft, that would have been even better, because it would’ve given a better idea on what the cleanup looks like. There are also things like this series making cancel internally always a force-cancel, where I’m not sure whether we want that in the replication driver or not[1]. With my series, we add an explicit parameter, so we’re forced to think about it, and then in this series on top we can just drop the parameter for all force-cancel invocations again, and for all non-force-cancel invocations we would have to think a bit more. I now don't sure that patch 5 of your series is correct (see my last answer to it), that's why I decided to not base on it. Well, we can always take patch 5 from v1. (Where I changed any job_is_cancelled() to job_cancel_requested() when it influenced the external interface.) My series has the benefit of handling soft-mirror-cancel case the other way and handles mirror finalization in case of soft-cancel properly. Specifically as for this series, I don’t like job_complete_ex() very much, I think the parameter should be part of job_complete() itself. That was my idea. But job_complete is passed as function pointer, so changing its prototype would be more work.. But I think it's possible. If we think that’s too specific of a mirror parameter to include in normal job_complete(), well, then there shouldn’t be a job_complete_ex() either, and do_graph_change should be a property of the mirror job (perhaps as pivot_on_completion) that’s cleared by qmp_block_job_cancel() before invoking job_complete(). This way users will lose a way to make a decision during job running.. On the contrary, it would be a precursor to letting the user change this property explicitly with a new QMP command. But probably they don't need actually. Moving the option to mirror job parameter seems a good option to me. Max [1] Although looking at it again now, it probably wants force-cancel. What do you think of my idea to keep old bugs as is and just deprecate block-job-cancel and add a new interface for "no-graph-change mirror" case? I don’t see a reason for that. The fix isn’t that complicated. Also, honestly, I don’t see a good reason for deprecating anything. Current interface lead to mess in the code, that's bad. Cancellation mode that is actually a kind of completion (and having comments in many places about that) - that shows for me that interface is not good.. It's a question of terminology, what to call "cancel". Also, that's not the first time this question arise. Remember my recent cancel-in-flight-requests series, when I thought that "cancel is cancel" and didn't consider soft-cancel of mirror.. And reviewers didn't caught it. I don't think that interface is good, it will always confuse new developers and users. But that's just my opinion, I don't impose it ) If not deprecate, i.e. if we consider old interface to be good, than no reason for this my series and for introducing new interface :) I’m not against a better interface, I’m against using this current bug as an excuse to improve the interface. We’ve known we want to improve the interface for quite a long time now, we don’t need an excuse for that. If we use this bug as an excuse, I’m afraid of becoming hung up on interface discussions instead of just getting the bug fixed. And we must get the bug fixed, it’s real, it’s kind of bad, and saying “it won’t appear with the new interface, let’s not worry about the old one” is not something I like. OTOH, if we use this bug as an excuse, I’m also afraid of trying to rush the design instead of actually implementing the interface that we’ve always desired, i.e. where the user gets to choose the completion mode via yet-to-be-implemented some job property setter function. As a final note (but this is precisely the interface discussion that I want to avoid for now), I said I don’t see a good reason for deprecating anything, because `job-cancel force=false` can just internally do `set-job-property .pivot_on_completion=false; job-complete`. From an implementation perspective, that should be simple. I understand that for users the existence of the `force` flag may still be confusing and so we might
Re: [PATCH for-6.2 07/43] target/ppc: Set fault address in ppc_cpu_do_unaligned_access
On Thu, 29 Jul 2021 at 01:51, Richard Henderson wrote: > > We ought to have been recording the virtual address for reporting > to the guest trap handler. > > Cc: qemu-...@nongnu.org > Signed-off-by: Richard Henderson > --- > target/ppc/excp_helper.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index a79a0ed465..0b2c6de442 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -1503,6 +1503,8 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr > vaddr, > CPUPPCState *env = cs->env_ptr; > uint32_t insn; > > +env->spr[SPR_DAR] = vaddr; > + Is this the right SPR for all PPC variants? For instance the kernel's code in arch/powerpc/kernel/exceptions-64e.S looks in SPRN_DEAR, which is our SPR_BOOKE_DEAR or SPR_40x_DEAR. -- PMM
Re: [PATCH for-6.2 05/43] target/microblaze: Implement do_unaligned_access for user-only
On Thu, 29 Jul 2021 at 01:54, Richard Henderson
wrote:
>
> Cc: Edgar E. Iglesias
> Signed-off-by: Richard Henderson
> ---
> target/microblaze/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> index 72d8f2a0da..cbec062ed7 100644
> --- a/target/microblaze/cpu.c
> +++ b/target/microblaze/cpu.c
> @@ -367,11 +367,11 @@ static const struct TCGCPUOps mb_tcg_ops = {
> .synchronize_from_tb = mb_cpu_synchronize_from_tb,
> .cpu_exec_interrupt = mb_cpu_exec_interrupt,
> .tlb_fill = mb_cpu_tlb_fill,
> +.do_unaligned_access = mb_cpu_do_unaligned_access,
>
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = mb_cpu_do_interrupt,
> .do_transaction_failed = mb_cpu_transaction_failed,
> -.do_unaligned_access = mb_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> };
If I'm reading the kernel sources correctly, for Microblaze it always
fixes up unaligned accesses, so for our linux-user code we want
"ignore unaligned access errors" rather than reporting them up
to cpu-loop.c, I think ?
-- PMM
Re: [PATCH 00/16] Various error handling fixes and cleanups
Markus Armbruster writes: > I doubt the fixes are 6.1 material at this late stage. If you > disagree, let me know. PATCH 16 has become commit 3e61a13af3. Remainder queued for 6.2; additional review is welcome all the same. Thanks, guys!
[PATCH] hw/acpi: use existing references to pci device struct within functions
There is no need to use fresh typecasts to get references to pci device structs
when there is an existing reference to pci device struct. Use existing
reference.
Minor cleanup.
Signed-off-by: Ani Sinha
---
hw/acpi/pcihp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Make and make check passed.
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 054ee8cbc5..e0f68c4fcf 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -291,7 +291,7 @@ void acpi_pcihp_device_pre_plug_cb(HotplugHandler
*hotplug_dev,
/* Only hotplugged devices need the hotplug capability. */
if (dev->hotplugged &&
-acpi_pcihp_get_bsel(pci_get_bus(PCI_DEVICE(dev))) < 0) {
+acpi_pcihp_get_bsel(pci_get_bus(pdev)) < 0) {
error_setg(errp, "Unsupported bus. Bus doesn't have property '"
ACPI_PCIHP_PROP_BSEL "' set");
return;
@@ -371,8 +371,8 @@ void acpi_pcihp_device_unplug_cb(HotplugHandler
*hotplug_dev, AcpiPciHpState *s,
{
PCIDevice *pdev = PCI_DEVICE(dev);
-trace_acpi_pci_unplug(PCI_SLOT(PCI_DEVICE(dev)->devfn),
- acpi_pcihp_get_bsel(pci_get_bus(PCI_DEVICE(dev;
+trace_acpi_pci_unplug(PCI_SLOT(pdev->devfn),
+ acpi_pcihp_get_bsel(pci_get_bus(pdev)));
/*
* clean up acpi-index so it could reused by another device
--
2.25.1
Re: [PATCH for-6.2 04/43] target/hppa: Implement do_unaligned_access for user-only
On Thu, 29 Jul 2021 at 01:57, Richard Henderson
wrote:
>
> Signed-off-by: Richard Henderson
> ---
> linux-user/hppa/cpu_loop.c | 2 +-
> target/hppa/cpu.c | 8 +---
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
> index 82d8183821..5ce30fec8b 100644
> --- a/linux-user/hppa/cpu_loop.c
> +++ b/linux-user/hppa/cpu_loop.c
> @@ -161,7 +161,7 @@ void cpu_loop(CPUHPPAState *env)
> case EXCP_UNALIGN:
> info.si_signo = TARGET_SIGBUS;
> info.si_errno = 0;
> -info.si_code = 0;
> +info.si_code = TARGET_BUS_ADRALN;
> info._sifields._sigfault._addr = env->cr[CR_IOR];
> queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
> break;
> diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
> index 2eace4ee12..55c0d81046 100644
> --- a/target/hppa/cpu.c
> +++ b/target/hppa/cpu.c
> @@ -71,7 +71,6 @@ static void hppa_cpu_disas_set_info(CPUState *cs,
> disassemble_info *info)
> info->print_insn = print_insn_hppa;
> }
>
> -#ifndef CONFIG_USER_ONLY
> static void hppa_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> MMUAccessType access_type,
> int mmu_idx, uintptr_t retaddr)
> @@ -80,15 +79,18 @@ static void hppa_cpu_do_unaligned_access(CPUState *cs,
> vaddr addr,
> CPUHPPAState *env = >env;
>
> cs->exception_index = EXCP_UNALIGN;
> +#ifdef CONFIG_USER_ONLY
> +env->cr[CR_IOR] = addr;
Do we not need the top 32 bits of the address, which the softmmu
version is recording in CR_ISR ?
> +#else
> if (env->psw & PSW_Q) {
> /* ??? Needs tweaking for hppa64. */
> env->cr[CR_IOR] = addr;
> env->cr[CR_ISR] = addr >> 32;
> }
> +#endif
>
> cpu_loop_exit_restore(cs, retaddr);
> }
> -#endif /* CONFIG_USER_ONLY */
-- PMM
Re: [PATCH for-6.2 03/43] target/arm: Implement do_unaligned_access for user-only
On Thu, 29 Jul 2021 at 01:47, Richard Henderson
wrote:
>
> Cc: qemu-...@nongnu.org
> Signed-off-by: Richard Henderson
> ---
> linux-user/aarch64/cpu_loop.c | 4
> linux-user/arm/cpu_loop.c | 43 +++
> target/arm/cpu.c | 2 +-
> target/arm/cpu_tcg.c | 2 +-
> 4 files changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
> index ee72a1c20f..998831f87f 100644
> --- a/linux-user/aarch64/cpu_loop.c
> +++ b/linux-user/aarch64/cpu_loop.c
> @@ -137,6 +137,10 @@ void cpu_loop(CPUARMState *env)
> case 0x11: /* Synchronous Tag Check Fault */
> info.si_code = TARGET_SEGV_MTESERR;
> break;
> +case 0x21: /* Alignment fault */
> +info.si_signo = TARGET_SIGBUS;
> +info.si_code = TARGET_BUS_ADRALN;
> +break;
> default:
> g_assert_not_reached();
> }
> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
> index 69632d15be..da7da6a0c1 100644
> --- a/linux-user/arm/cpu_loop.c
> +++ b/linux-user/arm/cpu_loop.c
> @@ -23,6 +23,7 @@
> #include "elf.h"
> #include "cpu_loop-common.h"
> #include "semihosting/common-semi.h"
> +#include "target/arm/syndrome.h"
Not a huge fan of linux-user files pulling in target/arm headers, but
I guess we do it already in aarch64/cpu_loop.c. (Though that is afaict
the only other place ATM...)
>
> #define get_user_code_u32(x, gaddr, env)\
> ({ abi_long __r = get_user_u32((x), (gaddr)); \
> @@ -286,9 +287,8 @@ void cpu_loop(CPUARMState *env)
> {
> CPUState *cs = env_cpu(env);
> int trapnr;
> -unsigned int n, insn;
> +unsigned int n, insn, ec, fsc;
> target_siginfo_t info;
> -uint32_t addr;
> abi_ulong ret;
>
> for(;;) {
> @@ -437,15 +437,40 @@ void cpu_loop(CPUARMState *env)
> break;
> case EXCP_PREFETCH_ABORT:
> case EXCP_DATA_ABORT:
> -addr = env->exception.vaddress;
> -{
> -info.si_signo = TARGET_SIGSEGV;
> -info.si_errno = 0;
> -/* XXX: check env->error_code */
> +info.si_signo = TARGET_SIGSEGV;
> +info.si_errno = 0;
> +info._sifields._sigfault._addr = env->exception.vaddress;
> +/*
> + * We should only arrive here with EC in {DATAABORT, INSNABORT},
> + * and short-form FSC, which then tells us to look at the FSR.
> + * ??? arm_cpu_reset never sets TTBCR_EAE, so we always get
> + * short-form FSC.
> + */
> +ec = syn_get_ec(env->exception.syndrome);
> +assert(ec == EC_DATAABORT || ec == EC_INSNABORT);
> +fsc = extract32(env->exception.syndrome, 0, 6);
> +assert(fsc == 0x3f);
> +switch (env->exception.fsr & 0x1f) {
> +case 0x1: /* Alignment */
> +info.si_signo = TARGET_SIGBUS;
> +info.si_code = TARGET_BUS_ADRALN;
> +break;
> +case 0x3: /* Access flag fault, level 1 */
> +case 0x6: /* Access flag fault, level 2 */
> +case 0x9: /* Domain fault, level 1 */
> +case 0xb: /* Domain fault, level 2 */
> +case 0xd: /* Permision fault, level 1 */
> +case 0xf: /* Permision fault, level 2 */
> +info.si_code = TARGET_SEGV_ACCERR;
> +break;
> +case 0x5: /* Translation fault, level 1 */
> +case 0x7: /* Translation fault, level 2 */
> info.si_code = TARGET_SEGV_MAPERR;
> -info._sifields._sigfault._addr = addr;
> -queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
> +break;
> +default:
> +g_assert_not_reached();
> }
> +queue_signal(env, info.si_signo, QEMU_SI_FAULT, );
> break;
It's slightly sad that we start off with a nicely symbolic
ArmMMUFaultInfo type enum value, carefully encode it into a
numeric value and then have to switch on the numeric value here,
but I can see why we end up this way...
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [PATCH] hw/i386/ich9: add comment explaining an argument to acpi_pcihp_reset call
ping ...
On Tue, 27 Jul 2021, Ani Sinha wrote:
> acpi_pcihp_reset() call from ich9/pm_reset() passes an unconditional truth
> value
> as the second argument. Added a commnet here to explain the reason why the
> argument is being passed unconditionally.
>
> Signed-off-by: Ani Sinha
> ---
> hw/acpi/ich9.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 778e27b659..b2e3c46075 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -281,6 +281,11 @@ static void pm_reset(void *opaque)
> pm->smi_en_wmask = ~0;
>
> if (pm->use_acpi_hotplug_bridge) {
> +/*
> + * PCI Express root buses do not support hot-plug, for
> + * details see docs/pcie.txt. Hence, the second argument
> + * is unconditionally true.
> + */
> acpi_pcihp_reset(>acpi_pci_hotplug, true);
> }
>
> --
> 2.25.1
>
>
Re: [PATCH for-6.2 02/43] target/alpha: Implement do_unaligned_access for user-only
On Thu, 29 Jul 2021 at 01:50, Richard Henderson
wrote:
>
> Signed-off-by: Richard Henderson
> ---
> target/alpha/cpu.c| 2 +-
> target/alpha/mem_helper.c | 8 +++-
> 2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/target/alpha/cpu.c b/target/alpha/cpu.c
> index 4871ad0c0a..cb7e5261bd 100644
> --- a/target/alpha/cpu.c
> +++ b/target/alpha/cpu.c
> @@ -220,11 +220,11 @@ static const struct TCGCPUOps alpha_tcg_ops = {
> .initialize = alpha_translate_init,
> .cpu_exec_interrupt = alpha_cpu_exec_interrupt,
> .tlb_fill = alpha_cpu_tlb_fill,
> +.do_unaligned_access = alpha_cpu_do_unaligned_access,
>
> #ifndef CONFIG_USER_ONLY
> .do_interrupt = alpha_cpu_do_interrupt,
> .do_transaction_failed = alpha_cpu_do_transaction_failed,
> -.do_unaligned_access = alpha_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */
> };
>
> diff --git a/target/alpha/mem_helper.c b/target/alpha/mem_helper.c
> index 75e72bc337..e3cf98b270 100644
> --- a/target/alpha/mem_helper.c
> +++ b/target/alpha/mem_helper.c
> @@ -23,30 +23,28 @@
> #include "exec/exec-all.h"
> #include "exec/cpu_ldst.h"
>
> -/* Softmmu support */
> -#ifndef CONFIG_USER_ONLY
> void alpha_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> MMUAccessType access_type,
> int mmu_idx, uintptr_t retaddr)
> {
> AlphaCPU *cpu = ALPHA_CPU(cs);
> CPUAlphaState *env = >env;
> -uint64_t pc;
> uint32_t insn;
>
> cpu_restore_state(cs, retaddr, true);
>
> -pc = env->pc;
> -insn = cpu_ldl_code(env, pc);
> +insn = cpu_ldl_code(env, env->pc);
>
> env->trap_arg0 = addr;
> env->trap_arg1 = insn >> 26;/* opcode */
> env->trap_arg2 = (insn >> 21) & 31; /* dest regno */
> +
> cs->exception_index = EXCP_UNALIGN;
> env->error_code = 0;
> cpu_loop_exit(cs);
> }
The code changes here seem unnecessary ?
Anyway
Reviewed-by: Peter Maydell
thanks
-- PMM
Re: [PATCH for-6.2 01/43] hw/core: Make do_unaligned_access available to user-only
On Thu, 29 Jul 2021 at 01:50, Richard Henderson wrote: > > We shouldn't be ignoring SIGBUS for user-only. > Move our existing TCGCPUOps hook out from CONFIG_SOFTMMU. > > Signed-off-by: Richard Henderson Reviewed-by: Peter Maydell thanks -- PMM
[PULL 7/7] docs: Fold usb2.txt passthrough information into usb.rst
From: Peter Maydell
Fold the usb2.txt information on device passthrough into usb.rst;
since this is the last part of the .txt file we can delete it now.
Signed-off-by: Peter Maydell
Message-Id: <20210728141457.14825-5-peter.mayd...@linaro.org>
Signed-off-by: Gerd Hoffmann
---
docs/usb2.txt | 58 -
MAINTAINERS | 1 -
docs/system/devices/usb.rst | 49 +++
3 files changed, 49 insertions(+), 59 deletions(-)
delete mode 100644 docs/usb2.txt
diff --git a/docs/usb2.txt b/docs/usb2.txt
deleted file mode 100644
index 6a88d5314f9c..
--- a/docs/usb2.txt
+++ /dev/null
@@ -1,58 +0,0 @@
-
-More USB tips & tricks
-==
-
-Recently the USB pass through driver (also known as usb-host) and the
-QEMU USB subsystem gained a few capabilities which are available only
-via qdev properties, i,e. when using '-device'.
-
-USB pass through hints
---
-
-The usb-host driver has a bunch of properties to specify the device
-which should be passed to the guest:
-
- hostbus= -- Specifies the bus number the device must be attached
- to.
-
- hostaddr= -- Specifies the device address the device got
- assigned by the guest os.
-
- hostport= -- Specifies the physical port the device is attached
- to.
-
- vendorid= -- Specifies the vendor ID of the device.
- productid= -- Specifies the product ID of the device.
-
-In theory you can combine all these properties as you like. In
-practice only a few combinations are useful:
-
- (1) vendorid+productid -- match for a specific device, pass it to
- the guest when it shows up somewhere in the host.
-
- (2) hostbus+hostport -- match for a specific physical port in the
- host, any device which is plugged in there gets passed to the
- guest.
-
- (3) hostbus+hostaddr -- most useful for ad-hoc pass through as the
- hostaddr isn't stable, the next time you plug in the device it
- gets a new one ...
-
-Note that USB 1.1 devices are handled by UHCI/OHCI and USB 2.0 by
-EHCI. That means a device plugged into the very same physical port
-may show up on different buses depending on the speed. The port I'm
-using for testing is bus 1 + port 1 for 2.0 devices and bus 3 + port 1
-for 1.1 devices. Passing through any device plugged into that port
-and also assign them to the correct bus can be done this way:
-
-qemu -M pc ${otheroptions} \
--usb \
--device usb-ehci,id=ehci \
--device usb-host,bus=usb-bus.0,hostbus=3,hostport=1 \
--device usb-host,bus=ehci.0,hostbus=1,hostport=1
-
-enjoy,
- Gerd
-
---
-Gerd Hoffmann
diff --git a/MAINTAINERS b/MAINTAINERS
index b1f8e82befc6..2089e71007d6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1837,7 +1837,6 @@ F: hw/usb/*
F: stubs/usb-dev-stub.c
F: tests/qtest/usb-*-test.c
F: docs/system/devices/usb.rst
-F: docs/usb-storage.txt
F: include/hw/usb.h
F: include/hw/usb/
diff --git a/docs/system/devices/usb.rst b/docs/system/devices/usb.rst
index bab0cd3fdfd1..afb7d6c2268d 100644
--- a/docs/system/devices/usb.rst
+++ b/docs/system/devices/usb.rst
@@ -300,3 +300,52 @@ are not supported yet.
When relaunching QEMU, you may have to unplug and plug again the USB
device to make it work again (this is a bug).
+
+``usb-host`` properties for specifying the host device
+^^
+
+The example above uses the ``vendorid`` and ``productid`` to
+specify which host device to pass through, but this is not
+the only way to specify the host device. ``usb-host`` supports
+the following properties:
+
+``hostbus=``
+ Specifies the bus number the device must be attached to
+``hostaddr=``
+ Specifies the device address the device got assigned by the guest os
+``hostport=``
+ Specifies the physical port the device is attached to
+``vendorid=``
+ Specifies the vendor ID of the device
+``productid=``
+ Specifies the product ID of the device.
+
+In theory you can combine all these properties as you like. In
+practice only a few combinations are useful:
+
+- ``vendorid`` and ``productid`` -- match for a specific device, pass it to
+ the guest when it shows up somewhere in the host.
+
+- ``hostbus`` and ``hostport`` -- match for a specific physical port in the
+ host, any device which is plugged in there gets passed to the
+ guest.
+
+- ``hostbus`` and ``hostaddr`` -- most useful for ad-hoc pass through as the
+ hostaddr isn't stable. The next time you plug the device into the host it
+ will get a new hostaddr.
+
+Note that on the host USB 1.1 devices are handled by UHCI/OHCI and USB
+2.0 by EHCI. That means different USB devices plugged into the very
+same physical port on the host may show up on different host buses
+depending on the speed. Supposing that devices plugged into a given
+physical port appear as
Re: [PULL 6/7] meson: fix meson 0.58 warning with libvhost-user subproject
On Thu, 29 Jul 2021 at 13:56, Paolo Bonzini wrote: > > From: Marc-André Lureau > > Meson now checks that subprojects do not access files from parent > project. While we all agree this is best practice, libvhost-user also > want to share a few headers with QEMU, and libvhost-user isn't really a > standalone project at this point (although this is making the dependency > a bit more explicit). > > Signed-off-by: Marc-André Lureau > Message-Id: <20210505151313.203258-1-marcandre.lur...@redhat.com> > Signed-off-by: Paolo Bonzini > --- > subprojects/libvhost-user/include/atomic.h | 1 + > subprojects/libvhost-user/libvhost-user.c| 2 +- > subprojects/libvhost-user/meson.build| 6 +- > subprojects/libvhost-user/standard-headers/linux | 1 + > diff --git a/subprojects/libvhost-user/include/atomic.h > b/subprojects/libvhost-user/include/atomic.h > new file mode 12 > index 00..8c2be64f7b > --- /dev/null > +++ b/subprojects/libvhost-user/include/atomic.h > @@ -0,0 +1 @@ > +../../../include/qemu/atomic.h > \ No newline at end of file > diff --git a/subprojects/libvhost-user/standard-headers/linux > b/subprojects/libvhost-user/standard-headers/linux > new file mode 12 > index 00..15a2378139 > --- /dev/null > +++ b/subprojects/libvhost-user/standard-headers/linux > @@ -0,0 +1 @@ > +../../../include/standard-headers/linux > \ No newline at end of file Should these really be missing the trailing newline ? -- PMM
[PULL 4/7] docs: Incorporate information in usb-storage.txt into rST manual
From: Peter Maydell
We already have a section on USB in the rST manual; fold
the information in docs/usb-storage.txt into it.
We add 'format=raw' to the various -drive options in the code
examples, because QEMU will print warnings these days if you
omit it.
Signed-off-by: Peter Maydell
Message-Id: <20210728141457.14825-2-peter.mayd...@linaro.org>
Signed-off-by: Gerd Hoffmann
---
docs/usb-storage.txt| 59 -
MAINTAINERS | 2 +-
docs/system/devices/usb.rst | 57 ++-
3 files changed, 51 insertions(+), 67 deletions(-)
delete mode 100644 docs/usb-storage.txt
diff --git a/docs/usb-storage.txt b/docs/usb-storage.txt
deleted file mode 100644
index 551af6f88bb1..
--- a/docs/usb-storage.txt
+++ /dev/null
@@ -1,59 +0,0 @@
-
-qemu usb storage emulation
---
-
-QEMU has three devices for usb storage emulation.
-
-Number one emulates the classic bulk-only transport protocol which is
-used by 99% of the usb sticks on the market today and is called
-"usb-storage". Usage (hooking up to xhci, other host controllers work
-too):
-
- qemu ${other_vm_args}\
- -drive if=none,id=stick,file=/path/to/file.img \
- -device nec-usb-xhci,id=xhci\
- -device usb-storage,bus=xhci.0,drive=stick
-
-
-Number two is the newer usb attached scsi transport. This one doesn't
-automagically create a scsi disk, so you have to explicitly attach one
-manually. Multiple logical units are supported. Here is an example
-with tree logical units:
-
- qemu ${other_vm_args}\
- -drive if=none,id=uas-disk1,file=/path/to/file1.img \
- -drive if=none,id=uas-disk2,file=/path/to/file2.img \
- -drive if=none,id=uas-cdrom,media=cdrom,file=/path/to/image.iso \
- -device nec-usb-xhci,id=xhci\
- -device usb-uas,id=uas,bus=xhci.0 \
- -device scsi-hd,bus=uas.0,scsi-id=0,lun=0,drive=uas-disk1 \
- -device scsi-hd,bus=uas.0,scsi-id=0,lun=1,drive=uas-disk2 \
- -device scsi-cd,bus=uas.0,scsi-id=0,lun=5,drive=uas-cdrom
-
-
-Number three emulates the classic bulk-only transport protocol too.
-It's called "usb-bot". It shares most code with "usb-storage", and
-the guest will not be able to see the difference. The qemu command
-line interface is similar to usb-uas though, i.e. no automatic scsi
-disk creation. It also features support for up to 16 LUNs. The LUN
-numbers must be continuous, i.e. for three devices you must use 0+1+2.
-The 0+1+5 numbering from the "usb-uas" example isn't going to work
-with "usb-bot".
-
-Starting with qemu version 2.7 usb-bot and usb-uas devices can be
-hotplugged. In the hotplug case they are added with "attached =
-false" so the guest will not see the device until the "attached"
-property is explicitly set to true. That allows to attach one or more
-scsi devices before making the device visible to the guest, i.e. the
-workflow looks like this:
-
- (1) device-add usb-bot,id=foo
- (2) device-add scsi-{hd,cd},bus=foo.0,lun=0
- (2b) optionally add more devices (luns 1 ... 15).
- (3) scripts/qmp/qom-set foo.attached = true
-
-enjoy,
- Gerd
-
---
-Gerd Hoffmann
diff --git a/MAINTAINERS b/MAINTAINERS
index 42ac45c3e502..b1f8e82befc6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1836,7 +1836,7 @@ S: Maintained
F: hw/usb/*
F: stubs/usb-dev-stub.c
F: tests/qtest/usb-*-test.c
-F: docs/usb2.txt
+F: docs/system/devices/usb.rst
F: docs/usb-storage.txt
F: include/hw/usb.h
F: include/hw/usb/
diff --git a/docs/system/devices/usb.rst b/docs/system/devices/usb.rst
index eeab78dcfbee..7da142ecbb9f 100644
--- a/docs/system/devices/usb.rst
+++ b/docs/system/devices/usb.rst
@@ -28,17 +28,46 @@ option or the ``device_add`` monitor command. Available
devices are:
``usb-storage,drive=drive_id``
Mass storage device backed by drive_id (see the :ref:`disk images`
- chapter in the System Emulation Users Guide)
+ chapter in the System Emulation Users Guide). This is the classic
+ bulk-only transport protocol used by 99% of USB sticks. This
+ example shows it connected to an XHCI USB controller and with
+ a drive backed by a raw format disk image:
+
+ .. parsed-literal::
+
+ |qemu_system| [...] \\
+-drive if=none,id=stick,format=raw,file=/path/to/file.img \\
+-device nec-usb-xhci,id=xhci \\
+-device usb-storage,bus=xhci.0,drive=stick
``usb-uas``
- USB attached SCSI device, see
- `usb-storage.txt
`__
- for details
+ USB attached SCSI device. This does not create a SCSI disk, so
+ you need to explicitly create a ``scsi-hd`` or ``scsi-cd`` device
+ on the
[PULL 0/7] Usb 20210729 patches
The following changes since commit f2da205cb4142259d9bc6b9d4596ebbe2426fe49: Update version for v6.1.0-rc1 release (2021-07-27 18:07:52 +0100) are available in the Git repository at: git://git.kraxel.org/qemu tags/usb-20210729-pull-request for you to fetch changes up to 30a20f2c5a9cf8f01ffcc918a7a5751dfe956524: docs: Fold usb2.txt passthrough information into usb.rst (2021-07-29 11:18:55 +0200) usb: fixes for 6.1: usbredir, usb-host for windows, docs. Gerd Hoffmann (3): usb-host: wire up timer for windows ci: add libusb for windows builds usbredir: fix free call Peter Maydell (4): docs: Incorporate information in usb-storage.txt into rST manual docs: Fold usb2.txt USB controller information into usb.rst docs: Fold usb2.txt physical port addressing info into usb.rst docs: Fold usb2.txt passthrough information into usb.rst docs/usb-storage.txt | 59 - docs/usb2.txt | 172 - hw/usb/host-libusb.c | 33 ++- hw/usb/redirect.c | 2 +- MAINTAINERS | 3 +- docs/system/devices/usb.rst | 225 +- .../dockerfiles/fedora-win32-cross.docker | 1 + .../dockerfiles/fedora-win64-cross.docker | 1 + 8 files changed, 254 insertions(+), 242 deletions(-) delete mode 100644 docs/usb-storage.txt delete mode 100644 docs/usb2.txt -- 2.31.1
Re: [PATCH v2] hw/net/can: sja1000 fix buff2frame_bas and buff2frame_pel when dlc is out of std CAN 8 bytes
"hw/net/can: sja1000 fix buff2frame* when dlc is out of std CAN 8 bytes"
On 7/29/21 2:33 PM, Pavel Pisa wrote:
> Problem reported by openEuler fuzz-sig group.
>
> The buff2frame_bas function (hw\net\can\can_sja1000.c)
> infoleak(qemu5.x~qemu6.x) or stack-overflow(qemu 4.x).
Cc: qemu-sta...@nongnu.org
> Reported-by: Qiang Ning
> Signed-off-by: Pavel Pisa
Reviewed-by: Philippe Mathieu-Daudé
> ---
> hw/net/can/can_sja1000.c | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/hw/net/can/can_sja1000.c b/hw/net/can/can_sja1000.c
> index 42d2f99dfb..34eea684ce 100644
> --- a/hw/net/can/can_sja1000.c
> +++ b/hw/net/can/can_sja1000.c
> @@ -275,6 +275,10 @@ static void buff2frame_pel(const uint8_t *buff,
> qemu_can_frame *frame)
> }
> frame->can_dlc = buff[0] & 0x0f;
>
> +if (frame->can_dlc > 8) {
> +frame->can_dlc = 8;
> +}
> +
> if (buff[0] & 0x80) { /* Extended */
> frame->can_id |= QEMU_CAN_EFF_FLAG;
> frame->can_id |= buff[1] << 21; /* ID.28~ID.21 */
> @@ -311,6 +315,10 @@ static void buff2frame_bas(const uint8_t *buff,
> qemu_can_frame *frame)
> }
> frame->can_dlc = buff[1] & 0x0f;
>
> +if (frame->can_dlc > 8) {
> +frame->can_dlc = 8;
> +}
> +
> for (i = 0; i < frame->can_dlc; i++) {
> frame->data[i] = buff[2 + i];
> }
>
[PULL 5/7] docs: Fold usb2.txt USB controller information into usb.rst
From: Peter Maydell
Fold the information in docs/usb2.txt about the different
kinds of supported USB controller into the main rST manual.
Signed-off-by: Peter Maydell
Message-Id: <20210728141457.14825-3-peter.mayd...@linaro.org>
Signed-off-by: Gerd Hoffmann
---
docs/usb2.txt | 82 ---
docs/system/devices/usb.rst | 86 +
2 files changed, 86 insertions(+), 82 deletions(-)
diff --git a/docs/usb2.txt b/docs/usb2.txt
index 172614d3a7e0..adf4ba3f2a0c 100644
--- a/docs/usb2.txt
+++ b/docs/usb2.txt
@@ -1,86 +1,4 @@
-USB Quick Start
-===
-
-XHCI controller support
-
-QEMU has XHCI host adapter support. The XHCI hardware design is much
-more virtualization-friendly when compared to EHCI and UHCI, thus XHCI
-emulation uses less resources (especially cpu). So if your guest
-supports XHCI (which should be the case for any operating system
-released around 2010 or later) we recommend using it:
-
-qemu -device qemu-xhci
-
-XHCI supports USB 1.1, USB 2.0 and USB 3.0 devices, so this is the
-only controller you need. With only a single USB controller (and
-therefore only a single USB bus) present in the system there is no
-need to use the bus= parameter when adding USB devices.
-
-
-EHCI controller support
-
-The QEMU EHCI Adapter supports USB 2.0 devices. It can be used either
-standalone or with companion controllers (UHCI, OHCI) for USB 1.1
-devices. The companion controller setup is more convenient to use
-because it provides a single USB bus supporting both USB 2.0 and USB
-1.1 devices. See next section for details.
-
-When running EHCI in standalone mode you can add UHCI or OHCI
-controllers for USB 1.1 devices too. Each controller creates its own
-bus though, so there are two completely separate USB buses: One USB
-1.1 bus driven by the UHCI controller and one USB 2.0 bus driven by
-the EHCI controller. Devices must be attached to the correct
-controller manually.
-
-The easiest way to add a UHCI controller to a 'pc' machine is the
-'-usb' switch. QEMU will create the UHCI controller as function of
-the PIIX3 chipset. The USB 1.1 bus will carry the name "usb-bus.0".
-
-You can use the standard -device switch to add a EHCI controller to
-your virtual machine. It is strongly recommended to specify an ID for
-the controller so the USB 2.0 bus gets an individual name, for example
-'-device usb-ehci,id=ehci". This will give you a USB 2.0 bus named
-"ehci.0".
-
-When adding USB devices using the -device switch you can specify the
-bus they should be attached to. Here is a complete example:
-
-qemu -M pc ${otheroptions} \
--drive if=none,id=usbstick,file=/path/to/image \
--usb \
--device usb-ehci,id=ehci \
--device usb-tablet,bus=usb-bus.0 \
--device usb-storage,bus=ehci.0,drive=usbstick
-
-This attaches a USB tablet to the UHCI adapter and a USB mass storage
-device to the EHCI adapter.
-
-
-Companion controller support
-
-
-The UHCI and OHCI controllers can attach to a USB bus created by EHCI
-as companion controllers. This is done by specifying the masterbus
-and firstport properties. masterbus specifies the bus name the
-controller should attach to. firstport specifies the first port the
-controller should attach to, which is needed as usually one EHCI
-controller with six ports has three UHCI companion controllers with
-two ports each.
-
-There is a config file in docs which will do all this for
-you, just try ...
-
-qemu -readconfig docs/config/ich9-ehci-uhci.cfg
-
-... then use "bus=ehci.0" to assign your USB devices to that bus.
-
-Using the '-usb' switch for 'q35' machines will create a similar
-USB controller configuration.
-
-
More USB tips & tricks
==
diff --git a/docs/system/devices/usb.rst b/docs/system/devices/usb.rst
index 7da142ecbb9f..9f0e613dcc7c 100644
--- a/docs/system/devices/usb.rst
+++ b/docs/system/devices/usb.rst
@@ -8,6 +8,92 @@ plug virtual USB devices or real host USB devices (only works
with
certain host operating systems). QEMU will automatically create and
connect virtual USB hubs as necessary to connect multiple USB devices.
+USB controllers
+~~~
+
+XHCI controller support
+^^^
+
+QEMU has XHCI host adapter support. The XHCI hardware design is much
+more virtualization-friendly when compared to EHCI and UHCI, thus XHCI
+emulation uses less resources (especially CPU). So if your guest
+supports XHCI (which should be the case for any operating system
+released around 2010 or later) we recommend using it:
+
+qemu -device qemu-xhci
+
+XHCI supports USB 1.1, USB 2.0 and USB 3.0 devices, so this is the
+only controller you need. With only a single USB controller (and
[PULL 6/7] meson: fix meson 0.58 warning with libvhost-user subproject
From: Marc-André Lureau
Meson now checks that subprojects do not access files from parent
project. While we all agree this is best practice, libvhost-user also
want to share a few headers with QEMU, and libvhost-user isn't really a
standalone project at this point (although this is making the dependency
a bit more explicit).
Signed-off-by: Marc-André Lureau
Message-Id: <20210505151313.203258-1-marcandre.lur...@redhat.com>
Signed-off-by: Paolo Bonzini
---
subprojects/libvhost-user/include/atomic.h | 1 +
subprojects/libvhost-user/libvhost-user.c| 2 +-
subprojects/libvhost-user/meson.build| 6 +-
subprojects/libvhost-user/standard-headers/linux | 1 +
4 files changed, 4 insertions(+), 6 deletions(-)
create mode 12 subprojects/libvhost-user/include/atomic.h
create mode 12 subprojects/libvhost-user/standard-headers/linux
diff --git a/subprojects/libvhost-user/include/atomic.h
b/subprojects/libvhost-user/include/atomic.h
new file mode 12
index 00..8c2be64f7b
--- /dev/null
+++ b/subprojects/libvhost-user/include/atomic.h
@@ -0,0 +1 @@
+../../../include/qemu/atomic.h
\ No newline at end of file
diff --git a/subprojects/libvhost-user/libvhost-user.c
b/subprojects/libvhost-user/libvhost-user.c
index fab7ca17ee..2971ba0112 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -40,7 +40,7 @@
#endif
-#include "qemu/atomic.h"
+#include "include/atomic.h"
#include "libvhost-user.h"
diff --git a/subprojects/libvhost-user/meson.build
b/subprojects/libvhost-user/meson.build
index b03446e7cd..39825d9404 100644
--- a/subprojects/libvhost-user/meson.build
+++ b/subprojects/libvhost-user/meson.build
@@ -4,21 +4,17 @@ project('libvhost-user', 'c',
threads = dependency('threads')
glib = dependency('glib-2.0')
-inc = include_directories('../../include', '../../linux-headers')
vhost_user = static_library('vhost-user',
files('libvhost-user.c'),
-include_directories: inc,
dependencies: threads,
c_args: '-D_GNU_SOURCE')
executable('link-test', files('link-test.c'),
- link_whole: vhost_user,
- include_directories: inc)
+ link_whole: vhost_user)
vhost_user_glib = static_library('vhost-user-glib',
files('libvhost-user-glib.c'),
- include_directories: inc,
link_with: vhost_user,
dependencies: glib)
diff --git a/subprojects/libvhost-user/standard-headers/linux
b/subprojects/libvhost-user/standard-headers/linux
new file mode 12
index 00..15a2378139
--- /dev/null
+++ b/subprojects/libvhost-user/standard-headers/linux
@@ -0,0 +1 @@
+../../../include/standard-headers/linux
\ No newline at end of file
--
2.31.1
Re: [PING][PING][PATCH v2] vhost: make SET_VRING_ADDR, SET_FEATURES send replies
Cc more ppl.
On 7/29/21 12:56 PM, Denis Plotnikov wrote:
>
> On 23.07.2021 12:59, Denis Plotnikov wrote:
>>
>> ping!
>>
>> On 19.07.2021 17:21, Denis Plotnikov wrote:
>>> On vhost-user-blk migration, qemu normally sends a number of commands
>>> to enable logging if VHOST_USER_PROTOCOL_F_LOG_SHMFD is negotiated.
>>> Qemu sends VHOST_USER_SET_FEATURES to enable buffers logging and
>>> VHOST_USER_SET_VRING_ADDR per each started ring to enable "used ring"
>>> data logging.
>>> The issue is that qemu doesn't wait for reply from the vhost daemon
>>> for these commands which may result in races between qemu expectation
>>> of logging starting and actual login starting in vhost daemon.
>>>
>>> The race can appear as follows: on migration setup, qemu enables dirty page
>>> logging by sending VHOST_USER_SET_FEATURES. The command doesn't arrive to a
>>> vhost-user-blk daemon immediately and the daemon needs some time to turn the
>>> logging on internally. If qemu doesn't wait for reply, after sending the
>>> command, qemu may start migrate memory pages to a destination. At this time,
>>> the logging may not be actually turned on in the daemon but some guest
>>> pages,
>>> which the daemon is about to write to, may have already been transferred
>>> without logging to the destination. Since the logging wasn't turned on,
>>> those pages won't be transferred again as dirty. So we may end up with
>>> corrupted data on the destination.
>>> The same scenario is applicable for "used ring" data logging, which is
>>> turned on with VHOST_USER_SET_VRING_ADDR command.
>>>
>>> To resolve this issue, this patch makes qemu wait for the commands result
>>> explicilty if VHOST_USER_PROTOCOL_F_REPLY_ACK is negotiated and
>>> logging is enabled.
>>>
>>> Signed-off-by: Denis Plotnikov
>>> ---
>>> v1 -> v2:
>>> * send reply only when logging is enabled [mst]
>>>
>>> v0 -> v1:
>>> * send reply for SET_VRING_ADDR, SET_FEATURES only [mst]
>>>
>>> hw/virtio/vhost-user.c | 37 ++---
>>> 1 file changed, 34 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>>> index ee57abe04526..133588b3961e 100644
>>> --- a/hw/virtio/vhost-user.c
>>> +++ b/hw/virtio/vhost-user.c
>>> @@ -1095,6 +1095,11 @@ static int vhost_user_set_mem_table(struct vhost_dev
>>> *dev,
>>> return 0;
>>> }
>>>
>>> +static bool log_enabled(uint64_t features)
>>> +{
>>> +return !!(features & (0x1ULL << VHOST_F_LOG_ALL));
>>> +}
>>> +
>>> static int vhost_user_set_vring_addr(struct vhost_dev *dev,
>>> struct vhost_vring_addr *addr)
>>> {
>>> @@ -1105,10 +1110,21 @@ static int vhost_user_set_vring_addr(struct
>>> vhost_dev *dev,
>>> .hdr.size = sizeof(msg.payload.addr),
>>> };
>>>
>>> +bool reply_supported = virtio_has_feature(dev->protocol_features,
>>> +
>>> VHOST_USER_PROTOCOL_F_REPLY_ACK);
>>> +
>>> +if (reply_supported && log_enabled(msg.hdr.flags)) {
>>> +msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
>>> +}
>>> +
>>> if (vhost_user_write(dev, , NULL, 0) < 0) {
>>> return -1;
>>> }
>>>
>>> +if (msg.hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
>>> +return process_message_reply(dev, );
>>> +}
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -1288,7 +1304,8 @@ static int vhost_user_set_vring_call(struct vhost_dev
>>> *dev,
>>> return vhost_set_vring_file(dev, VHOST_USER_SET_VRING_CALL, file);
>>> }
>>>
>>> -static int vhost_user_set_u64(struct vhost_dev *dev, int request, uint64_t
>>> u64)
>>> +static int vhost_user_set_u64(struct vhost_dev *dev, int request, uint64_t
>>> u64,
>>> + bool need_reply)
>>> {
>>> VhostUserMsg msg = {
>>> .hdr.request = request,
>>> @@ -1297,23 +1314,37 @@ static int vhost_user_set_u64(struct vhost_dev
>>> *dev, int request, uint64_t u64)
>>> .hdr.size = sizeof(msg.payload.u64),
>>> };
>>>
>>> +if (need_reply) {
>>> +bool reply_supported = virtio_has_feature(dev->protocol_features,
>>> + VHOST_USER_PROTOCOL_F_REPLY_ACK);
>>> +if (reply_supported) {
>>> +msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
>>> +}
>>> +}
>>> +
>>> if (vhost_user_write(dev, , NULL, 0) < 0) {
>>> return -1;
>>> }
>>>
>>> +if (msg.hdr.flags & VHOST_USER_NEED_REPLY_MASK) {
>>> +return process_message_reply(dev, );
>>> +}
>>> +
>>> return 0;
>>> }
>>>
>>> static int vhost_user_set_features(struct vhost_dev *dev,
>>> uint64_t features)
>>> {
>>> -return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features);
>>> +return vhost_user_set_u64(dev, VHOST_USER_SET_FEATURES, features,
>>> + log_enabled(features));
>>> }
>>>
>>> static int
[PULL 2/7] ci: add libusb for windows builds
Add CI coverage for usb passthrough on windows. Signed-off-by: Gerd Hoffmann Reviewed-by: Willian Rampazzo Message-Id: <20210623085249.1151901-3-kra...@redhat.com> Signed-off-by: Gerd Hoffmann --- tests/docker/dockerfiles/fedora-win32-cross.docker | 1 + tests/docker/dockerfiles/fedora-win64-cross.docker | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/docker/dockerfiles/fedora-win32-cross.docker b/tests/docker/dockerfiles/fedora-win32-cross.docker index 5a03e1af43ac..aad39dd97ff4 100644 --- a/tests/docker/dockerfiles/fedora-win32-cross.docker +++ b/tests/docker/dockerfiles/fedora-win32-cross.docker @@ -23,6 +23,7 @@ ENV PACKAGES \ mingw32-libjpeg-turbo \ mingw32-libpng \ mingw32-libtasn1 \ +mingw32-libusbx \ mingw32-nettle \ mingw32-nsis \ mingw32-pixman \ diff --git a/tests/docker/dockerfiles/fedora-win64-cross.docker b/tests/docker/dockerfiles/fedora-win64-cross.docker index d3f13666e82e..9a224a619bd4 100644 --- a/tests/docker/dockerfiles/fedora-win64-cross.docker +++ b/tests/docker/dockerfiles/fedora-win64-cross.docker @@ -23,6 +23,7 @@ ENV PACKAGES \ mingw64-libjpeg-turbo \ mingw64-libpng \ mingw64-libtasn1 \ +mingw64-libusbx \ mingw64-pixman \ mingw64-pkg-config \ perl \ -- 2.31.1
[PULL 3/7] usbredir: fix free call
data might point into the middle of a larger buffer, there is a separate
free_on_destroy pointer passed into bufp_alloc() to handle that. It is
only used in the normal workflow though, not when dropping packets due
to the queue being full. Fix that.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/491
Signed-off-by: Gerd Hoffmann
Reviewed-by: Marc-André Lureau
Message-Id: <20210722072756.647673-1-kra...@redhat.com>
---
hw/usb/redirect.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 4ec9326e0582..1ec909a63a80 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -476,7 +476,7 @@ static int bufp_alloc(USBRedirDevice *dev, uint8_t *data,
uint16_t len,
if (dev->endpoint[EP2I(ep)].bufpq_dropping_packets) {
if (dev->endpoint[EP2I(ep)].bufpq_size >
dev->endpoint[EP2I(ep)].bufpq_target_size) {
-free(data);
+free(free_on_destroy);
return -1;
}
dev->endpoint[EP2I(ep)].bufpq_dropping_packets = 0;
--
2.31.1
