[PATCH v3 2/4] target/i386: add fast short REP MOV support
For CPUs support fast short REP MOV[CPUID.(EAX=7,ECX=0):EDX(bit4)], e.g
Icelake and Tigerlake, expose it to the guest VM.
Signed-off-by: Chenyi Qiang
---
target/i386/cpu.c | 2 +-
target/i386/cpu.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 3ed7502e80..4bbe9f0948 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -984,7 +984,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
.type = CPUID_FEATURE_WORD,
.feat_names = {
NULL, NULL, "avx512-4vnniw", "avx512-4fmaps",
-NULL, NULL, NULL, NULL,
+"fsrm", NULL, NULL, NULL,
NULL, NULL, "md-clear", NULL,
NULL, NULL, NULL, NULL,
NULL, NULL, NULL /* pconfig */, NULL,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index e818fc712a..d320265b92 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -770,6 +770,8 @@ typedef uint64_t FeatureWordArray[FEATURE_WORDS];
#define CPUID_7_0_EDX_AVX512_4VNNIW (1U << 2)
/* AVX512 Multiply Accumulation Single Precision */
#define CPUID_7_0_EDX_AVX512_4FMAPS (1U << 3)
+/* Fast Short Rep Mov */
+#define CPUID_7_0_EDX_FSRM (1U << 4)
/* Speculation Control */
#define CPUID_7_0_EDX_SPEC_CTRL (1U << 26)
/* Single Thread Indirect Branch Predictors */
--
2.17.1
[PATCH v3 0/4] modify CPU model info
Add the missing VMX features in Skylake-Server, Cascadelake-Server and Icelake-Server CPU models. In Icelake-Server CPU model, it lacks sha_ni, avx512ifma, rdpid and fsrm. The model number of Icelake-Server also needs to be fixed. The modification on Icelake-Client CPU model will be gathered and sent in anothor patch set. Changes in v3: - remove the modification on Icelake-Client model number - change the missing features of Icelake-Server from v3 to v4 Changes in v2: - add missing features as a new version of CPU model - add the support of FSRM - add New CPUID of FSRM and RDPID in Icelake-Server CPU model Chenyi Qiang (4): target/i386: add missing vmx features for several CPU models target/i386: add fast short REP MOV support target/i386: add the missing features for Icelake-Server CPU model target/i386: modify Icelake-Server CPU model number target/i386/cpu.c | 20 +--- target/i386/cpu.h | 2 ++ 2 files changed, 19 insertions(+), 3 deletions(-) -- 2.17.1
[PATCH v3 4/4] target/i386: modify Icelake-Server CPU model number
According to the Intel Icelake family list, Icelake-Server uses model
number 106(0x6A).
Signed-off-by: Chenyi Qiang
---
target/i386/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d2f8a276c4..04bcf01b5a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3364,7 +3364,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
.level = 0xd,
.vendor = CPUID_VENDOR_INTEL,
.family = 6,
-.model = 134,
+.model = 106,
.stepping = 0,
.features[FEAT_1_EDX] =
CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
--
2.17.1
[PATCH v3 1/4] target/i386: add missing vmx features for several CPU models
Add some missing VMX features in Skylake-Server, Cascadelake-Server and
Icelake-Server CPU models based on the output of Paolo's script.
Signed-off-by: Chenyi Qiang
---
target/i386/cpu.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 90ffc5f3b1..3ed7502e80 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2982,6 +2982,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
VMX_SECONDARY_EXEC_RDRAND_EXITING |
VMX_SECONDARY_EXEC_ENABLE_INVPCID |
VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS
|
VMX_SECONDARY_EXEC_RDSEED_EXITING | VMX_SECONDARY_EXEC_ENABLE_PML,
+.features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
.xlevel = 0x8008,
.model_id = "Intel Xeon Processor (Skylake)",
.versions = (X86CPUVersionDefinition[]) {
@@ -3110,6 +3111,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
VMX_SECONDARY_EXEC_RDRAND_EXITING |
VMX_SECONDARY_EXEC_ENABLE_INVPCID |
VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS
|
VMX_SECONDARY_EXEC_RDSEED_EXITING | VMX_SECONDARY_EXEC_ENABLE_PML,
+.features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
.xlevel = 0x8008,
.model_id = "Intel Xeon Processor (Cascadelake)",
.versions = (X86CPUVersionDefinition[]) {
@@ -3457,7 +3459,9 @@ static X86CPUDefinition builtin_x86_defs[] = {
VMX_SECONDARY_EXEC_APIC_REGISTER_VIRT |
VMX_SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
VMX_SECONDARY_EXEC_RDRAND_EXITING |
VMX_SECONDARY_EXEC_ENABLE_INVPCID |
- VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS,
+ VMX_SECONDARY_EXEC_ENABLE_VMFUNC | VMX_SECONDARY_EXEC_SHADOW_VMCS
|
+ VMX_SECONDARY_EXEC_RDSEED_EXITING | VMX_SECONDARY_EXEC_ENABLE_PML,
+.features[FEAT_VMX_VMFUNC] = MSR_VMX_VMFUNC_EPT_SWITCHING,
.xlevel = 0x8008,
.model_id = "Intel Xeon Processor (Icelake)",
.versions = (X86CPUVersionDefinition[]) {
--
2.17.1
[PATCH v3 3/4] target/i386: add the missing features for Icelake-Server CPU model
Add the SHA_NI and AVX512IFMA feature bits in FEAT_7_0_EBX, RDPID
feature bit in FEAT_7_0_ECX and FSRM feature bit in FEAT_7_0_EDX.
Signed-off-by: Chenyi Qiang
---
target/i386/cpu.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 4bbe9f0948..d2f8a276c4 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3488,6 +3488,16 @@ static X86CPUDefinition builtin_x86_defs[] = {
{ /* end of list */ }
},
},
+{
+.version = 4,
+.props = (PropValue[]) {
+{ "sha-ni", "on" },
+{ "avx512ifma", "on" },
+{ "rdpid", "on" },
+{ "fsrm", "on" },
+{ /* end of list */ }
+},
+},
{ /* end of list */ }
}
},
--
2.17.1
Re: [PATCH v12 2/3] qcow2: Allow writing compressed data of multiple clusters
10.04.2020 3:12, Andrey Shinkevich wrote:
We could assign indices to the clusters/chunks and improve the algorithm to
write them down on the disk in the same order adjacently. If you find it
feasible for QEMU, I'd like to create a task for doing that, shall I?
Compressed cluster occupy different size chunks in the image. How are you going
to preallocate? Anyway, I don't see any benefit in ordering compressed
clusters, I think it's not worth doing.
--
*From:* Vladimir Sementsov-Ogievskiy
*Sent:* Thursday, April 9, 2020 9:39 PM
*To:* Alberto Garcia ; Andrey Shinkevich
; qemu-devel@nongnu.org ;
qemu-bl...@nongnu.org
*Cc:* kw...@redhat.com ; arm...@redhat.com ;
mre...@redhat.com ; Denis Lunev
*Subject:* Re: [PATCH v12 2/3] qcow2: Allow writing compressed data of multiple
clusters
09.04.2020 19:50, Alberto Garcia wrote:
On Mon 02 Dec 2019 01:15:05 PM CET, Andrey Shinkevich wrote:
+static coroutine_fn int
+qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset)
+{
+ BDRVQcow2State *s = bs->opaque;
+ AioTaskPool *aio = NULL;
+ int ret = 0;
+
+ if (has_data_file(bs)) {
+ return -ENOTSUP;
+ }
+
+ if (bytes == 0) {
+ /*
+ * align end of file to a sector boundary to ease reading with
+ * sector based I/Os
+ */
+ int64_t len = bdrv_getlength(bs->file->bs);
+ if (len < 0) {
+ return len;
+ }
+ return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, NULL);
+ }
+
+ if (offset_into_cluster(s, offset)) {
+ return -EINVAL;
+ }
+
+ while (bytes && aio_task_pool_status(aio) == 0) {
+ uint64_t chunk_size = MIN(bytes, s->cluster_size);
+
+ if (!aio && chunk_size != bytes) {
+ aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
+ }
+
+ ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_compressed_task_entry,
+ 0, 0, offset, chunk_size, qiov, qiov_offset,
NULL);
+ if (ret < 0) {
+ break;
+ }
+ qiov_offset += chunk_size;
+ offset += chunk_size;
+ bytes -= chunk_size;
+ }
This patch allows the user to write more than one cluster of compressed
data at a time, and it does so by splitting the request into many
cluster-sized requests and using qcow2_add_task() for each one of them.
What happens however is that there's no guarantee that the requests are
processed in the same order that they were added.
One consequence is that running on an empty qcow2 file a command as
simple as this one:
qemu-io -c 'write -c 0 256k' image.qcow2
does not always produce the same results.
This does not have any user-visible consequences for the guest. In all
cases the data is correctly written, it's just that the ordering of the
compressed clusters (and therefore the contents of the L2 entries) will
be different each time.
Because of this a test cannot expect that running the same commands on
an empty image produces always the same results.
Is this something that we should be concerned about?
Parallel writing compressed clusters is significant improvement, as it allow
compressing in really parallel threads.
Generally, async parallel issuing of several requests gives more performance
than handling peaces one-by-one, mirror works on this basis and it is fast.
I've already moved qcow2 to this idea (aio tasks in qcow2 code), and in
progress of moving backup job. So, I think that asynchrony and ambiguity would
be native for block-layer anyway.
Hmm. Still, what about cluster sequence? For normal clusters there may be
simple thing to do: preallocation (at least of metadata). So, we can pre-create
cluster sequence.. But what to do with compressed clusters if we want specific
order for them, I don't know. On the other hand, ordering of normal cluster may
make sence: it should increase p
[PATCH] hax: Add hax max vcpu IOCTL and support 64 vcpu
This commit tried to obtain max vcpu of haxm driver by calling
HAX_IOCTL_CAP_MAX_VCPU before creating the vm so that if using hax as
the accelerator and the smp value is larger than the haxm driver
supported max value, the program will terminate. Previously, it will
create vm and vcpu one by one until haxm reports error. Also, the
maximum vcpu value in qemu for haxm is updated from 0x10 to 0x40 in
hax-i386.h.
This patch resolves the issue by calling hax device ioctl
HAX_IOCTL_CAP_MAX_VCPU in hax_init and store the min(haxm max, qemu max)
in hax_state structure. The value will be compared with smp value in
vm_create. (ioctl naming credit to KVM)
This commit results in if ioctl doesn't exist or occur error, it will
continue running but output warning, if smp value is larger than the
min(hax max,qemu max), it will terminate and output error message.
Signed-off-by: WangBowen
---
target/i386/hax-all.c | 7 +--
target/i386/hax-i386.h| 4 +++-
target/i386/hax-posix.c | 29 +
target/i386/hax-posix.h | 1 +
target/i386/hax-windows.c | 32
target/i386/hax-windows.h | 2 ++
6 files changed, 72 insertions(+), 3 deletions(-)
diff --git a/target/i386/hax-all.c b/target/i386/hax-all.c
index a22adec5da..eadfa7c881 100644
--- a/target/i386/hax-all.c
+++ b/target/i386/hax-all.c
@@ -259,8 +259,9 @@ struct hax_vm *hax_vm_create(struct hax_state *hax, int
max_cpus)
goto error;
}
-if (max_cpus > HAX_MAX_VCPU) {
-fprintf(stderr, "Maximum VCPU number QEMU supported is %d\n",
HAX_MAX_VCPU);
+if (max_cpus > hax->hax_max_vcpu) {
+fprintf(stderr, "Maximum VCPU number QEMU and HAXM driver supported is
%d\n",
+hax->hax_max_vcpu);
goto error;
}
@@ -332,6 +333,8 @@ static int hax_init(ram_addr_t ram_size, int max_cpus)
goto error;
}
+hax->hax_max_vcpu = hax_max_vcpus_support(hax);
+
if (!hax_version_support(hax)) {
ret = -EINVAL;
goto error;
diff --git a/target/i386/hax-i386.h b/target/i386/hax-i386.h
index 7d988f81da..1ffa8df63a 100644
--- a/target/i386/hax-i386.h
+++ b/target/i386/hax-i386.h
@@ -38,9 +38,10 @@ struct hax_state {
struct hax_vm *vm;
uint64_t mem_quota;
bool supports_64bit_ramblock;
+int hax_max_vcpu;
};
-#define HAX_MAX_VCPU 0x10
+#define HAX_MAX_VCPU 0x40
#define MAX_VM_ID 0x40
#define MAX_VCPU_ID 0x40
@@ -74,6 +75,7 @@ int hax_notify_qemu_version(hax_fd vm_fd, struct
hax_qemu_version *qversion);
int hax_set_ram(uint64_t start_pa, uint32_t size, uint64_t host_va, int flags);
/* Common host function */
+int hax_max_vcpus_support(struct hax_state *hax);
int hax_host_create_vm(struct hax_state *hax, int *vm_id);
hax_fd hax_host_open_vm(struct hax_state *hax, int vm_id);
int hax_host_create_vcpu(hax_fd vm_fd, int vcpuid);
diff --git a/target/i386/hax-posix.c b/target/i386/hax-posix.c
index a5426a6dac..a4f9dce55e 100644
--- a/target/i386/hax-posix.c
+++ b/target/i386/hax-posix.c
@@ -163,6 +163,35 @@ int hax_host_create_vm(struct hax_state *hax, int *vmid)
return ret;
}
+int hax_max_vcpus_support(struct hax_state *hax)
+{
+int ret;
+int vcpu_num = 0;
+
+if (hax_invalid_fd(hax->fd)) {
+return vcpu_num;
+}
+
+ret = ioctl(hax->fd, HAX_IOCTL_CAP_MAX_VCPU, &vcpu_num);
+
+if (ret == 0 && vcpu_num > 0) {
+if (vcpu_num != HAX_MAX_VCPU) {
+fprintf(stderr, "Warning: HAXM driver and QEMU are inconsistent"
+" in max vcpu number, HAXM driver: %d, QEMU: %d,"
+" refers to the smaller one.\n", vcpu_num, HAX_MAX_VCPU);
+if (vcpu_num > HAX_MAX_VCPU) {
+vcpu_num = HAX_MAX_VCPU;
+}
+}
+} else {
+vcpu_num = HAX_MAX_VCPU;
+fprintf(stderr, "Warning: HAXM driver doesn't support
HAX_IOCTL_CAP_MAX_VCPU,"
+" will refer to max value defined in QEMU\n");
+}
+
+return vcpu_num;
+}
+
hax_fd hax_host_open_vm(struct hax_state *hax, int vm_id)
{
hax_fd fd;
diff --git a/target/i386/hax-posix.h b/target/i386/hax-posix.h
index fb7c64426d..42e58f6fa5 100644
--- a/target/i386/hax-posix.h
+++ b/target/i386/hax-posix.h
@@ -38,6 +38,7 @@ static inline void hax_close_fd(hax_fd fd)
#define HAX_IOCTL_CREATE_VM _IOWR(0, 0x21, uint32_t)
#define HAX_IOCTL_DESTROY_VM _IOW(0, 0x22, uint32_t)
#define HAX_IOCTL_CAPABILITY _IOR(0, 0x23, struct hax_capabilityinfo)
+#define HAX_IOCTL_CAP_MAX_VCPU _IOR(0, 0x25, uint32_t)
#define HAX_VM_IOCTL_VCPU_CREATE _IOWR(0, 0x80, uint32_t)
#define HAX_VM_IOCTL_ALLOC_RAM _IOWR(0, 0x81, struct hax_alloc_ram_info)
diff --git a/target/i386/hax-windows.c b/target/i386/hax-windows.c
index 5729ad9b48..c7816e1950 100644
--- a/target/i386/hax-windows.c
+++ b/target/i386/hax-windows.c
@@ -249,6 +249,38 @@ int hax_host_create_vm(struct hax_state *hax, int *vmid)
return 0;
}
+int hax_max_vcpus_support(struct
[PATCH v19 QEMU 1/4] virtio-balloon: Implement support for page poison tracking feature
From: Alexander Duyck
We need to make certain to advertise support for page poison tracking if
we want to actually get data on if the guest will be poisoning pages. So
if free page hinting is active we should add page poisoning support and
let the guest disable it if it isn't using it.
Page poisoning will result in a page being dirtied on free. As such we
cannot really avoid having to copy the page at least one more time since
we will need to write the poison value to the destination. As such we can
just ignore free page hinting if page poisoning is enabled as it will
actually reduce the work we have to do.
Signed-off-by: Alexander Duyck
---
hw/virtio/virtio-balloon.c | 26 ++
include/hw/virtio/virtio-balloon.h |1 +
2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index a4729f7fc930..1c6d36a29a04 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -531,6 +531,15 @@ static void virtio_balloon_free_page_start(VirtIOBalloon
*s)
return;
}
+/*
+ * If page poisoning is enabled then we probably shouldn't bother with
+ * the hinting since the poisoning will dirty the page and invalidate
+ * the work we are doing anyway.
+ */
+if (virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON)) {
+return;
+}
+
if (s->free_page_report_cmd_id == UINT_MAX) {
s->free_page_report_cmd_id =
VIRTIO_BALLOON_FREE_PAGE_REPORT_CMD_ID_MIN;
@@ -618,12 +627,10 @@ static size_t virtio_balloon_config_size(VirtIOBalloon *s)
if (s->qemu_4_0_config_size) {
return sizeof(struct virtio_balloon_config);
}
-if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON)) {
+if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON) ||
+virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
return sizeof(struct virtio_balloon_config);
}
-if (virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
-return offsetof(struct virtio_balloon_config, poison_val);
-}
return offsetof(struct virtio_balloon_config, free_page_report_cmd_id);
}
@@ -634,6 +641,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev,
uint8_t *config_data)
config.num_pages = cpu_to_le32(dev->num_pages);
config.actual = cpu_to_le32(dev->actual);
+config.poison_val = cpu_to_le32(dev->poison_val);
if (dev->free_page_report_status == FREE_PAGE_REPORT_S_REQUESTED) {
config.free_page_report_cmd_id =
@@ -697,6 +705,9 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
qapi_event_send_balloon_change(vm_ram_size -
((ram_addr_t) dev->actual <<
VIRTIO_BALLOON_PFN_SHIFT));
}
+dev->poison_val = virtio_vdev_has_feature(vdev,
+ VIRTIO_BALLOON_F_PAGE_POISON) ?
+ le32_to_cpu(config.poison_val) : 0;
trace_virtio_balloon_set_config(dev->actual, oldactual);
}
@@ -706,6 +717,9 @@ static uint64_t virtio_balloon_get_features(VirtIODevice
*vdev, uint64_t f,
VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
f |= dev->host_features;
virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ);
+if (virtio_has_feature(f, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+virtio_add_feature(&f, VIRTIO_BALLOON_F_PAGE_POISON);
+}
return f;
}
@@ -854,6 +868,8 @@ static void virtio_balloon_device_reset(VirtIODevice *vdev)
g_free(s->stats_vq_elem);
s->stats_vq_elem = NULL;
}
+
+s->poison_val = 0;
}
static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
@@ -916,6 +932,8 @@ static Property virtio_balloon_properties[] = {
VIRTIO_BALLOON_F_DEFLATE_ON_OOM, false),
DEFINE_PROP_BIT("free-page-hint", VirtIOBalloon, host_features,
VIRTIO_BALLOON_F_FREE_PAGE_HINT, false),
+DEFINE_PROP_BIT("x-page-poison", VirtIOBalloon, host_features,
+VIRTIO_BALLOON_F_PAGE_POISON, false),
/* QEMU 4.0 accidentally changed the config size even when free-page-hint
* is disabled, resulting in QEMU 3.1 migration incompatibility. This
* property retains this quirk for QEMU 4.1 machine types.
diff --git a/include/hw/virtio/virtio-balloon.h
b/include/hw/virtio/virtio-balloon.h
index d1c968d2376e..7fe78e5c14d7 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -70,6 +70,7 @@ typedef struct VirtIOBalloon {
uint32_t host_features;
bool qemu_4_0_config_size;
+uint32_t poison_val;
} VirtIOBalloon;
#endif
Re: [PATCH v1] nrf51: Fix last GPIO CNF address
On Tue, 7 Apr 2020, at 18:20, Peter Maydell wrote: > On Tue, 7 Apr 2020 at 09:45, Joel Stanley wrote: > > On Tue, 7 Apr 2020 at 08:41, Peter Maydell wrote: > > > Do you have a link to this patch, please? I had a quick search through > > > my mailing list articles but couldn't see anything obviously relevant. > > > > There is a reference in this thread: > > > > https://lore.kernel.org/qemu-devel/dd8fc1f7-56d9-4d9f-96a4-0fdcafdc8...@www.fastmail.com/ > > > > The patch is here: > > > > https://lore.kernel.org/qemu-devel/20170630030058.28943-1-and...@aj.id.au/ > > Oh, that's from 2017, no wonder I couldn't find it! Yeah, I never quite got back to finishing it :( It's development was driven by development of the ASPEED ADC model, which I hacked up in the interest of getting the ASPEED SDK booting under qemu (the SDK kernel had an infinite spin waiting for the ADC-ready bit). IIRC Phil wanted to enable sub-word accesses to the sample value registers but I didn't want to spread logic dealing with different access widths through the model. We already had a mechanism to describe the model's supported access sizes (as opposed to the valid access sizes allowed by hardware) in the `impl` member of the MemoryRegionOps, so I was trying to use that, but it didn't work as I needed. The accesses generated at the point of the guest MMIO need to be expanded to the access width supported by the model and then the resulting data trimmed again upon returning the data (in the case of a read) via the MMIO operation. So the intent was less about unaligned accesses than enabling models implementations that only have to handle certain-sized access widths. It happens to help the unaligned access case as well. > > Does somebody who's already reviewed the patch want to summarize > what the effects on devices are -- i.e. what calls the device's read/write > methods used to get if the guest did an unaligned access, including an > unaligned access half off-the-end of the memory region, and what > calls the read/write methods get after the patch ? The patch's commit > message doesn't really describe what it's doing... Honestly any of that information has well left my memory at this point, I'd have to analyse the patch to recover it. I was hoping that my turn-around time would be shorter than 3 years but there hasn't been a shortage of fires to put out in the mean time. Andrew
[PATCH v19 QEMU 0/4] virtio-balloon: add support for free page reporting
This series provides an asynchronous means of reporting free guest pages
to QEMU through virtio-balloon so that the memory associated with those
pages can be dropped and reused by other processes and/or guests on the
host. Using this it is possible to avoid unnecessary I/O to disk and
greatly improve performance in the case of memory overcommit on the host.
I originally submitted this patch series back on February 11th 2020[1],
but at that time I was focused primarily on the kernel portion of this
patch set. However as of April 7th those patches are now included in
Linus's kernel tree[2] and so I am submitting the QEMU pieces for
inclusion.
[1]:
https://lore.kernel.org/lkml/20200211224416.29318.44077.stgit@localhost.localdomain/
[2]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b0c504f154718904ae49349147e3b7e6ae91ffdc
Changes from v17:
Fixed typo in patch 1 title
Addressed white-space issues reported via checkpatch
Added braces {} for two if statements to match expected coding style
Changes from v18:
Updated patches 2 and 3 based on input from dhildenb
Added comment to patch 2 describing what keeps us from reporting a bad page
Added patch to address issue with ROM devices being directly writable
---
Alexander Duyck (4):
virtio-balloon: Implement support for page poison tracking feature
linux-headers: update to contain virito-balloon free page reporting
virtio-balloon: Provide an interface for free page reporting
memory: Do not allow direct write access to rom_device regions
hw/virtio/virtio-balloon.c | 85 ++-
include/exec/memory.h |4 +
include/hw/virtio/virtio-balloon.h |3 +
include/standard-headers/linux/virtio_balloon.h |1
4 files changed, 86 insertions(+), 7 deletions(-)
--
[PATCH v19 QEMU 3/4] virtio-balloon: Provide an interface for free page reporting
From: Alexander Duyck
Add support for free page reporting. The idea is to function very similar
to how the balloon works in that we basically end up madvising the page as
not being used. However we don't really need to bother with any deflate
type logic since the page will be faulted back into the guest when it is
read or written to.
This provides a new way of letting the guest proactively report free
pages to the hypervisor, so the hypervisor can reuse them. In contrast to
inflate/deflate that is triggered via the hypervisor explicitly.
Signed-off-by: Alexander Duyck
---
hw/virtio/virtio-balloon.c | 63 +++-
include/hw/virtio/virtio-balloon.h |2 +
2 files changed, 62 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 1c6d36a29a04..86d8b48a8e3a 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -321,6 +321,57 @@ static void balloon_stats_set_poll_interval(Object *obj,
Visitor *v,
balloon_stats_change_timer(s, 0);
}
+static void virtio_balloon_handle_report(VirtIODevice *vdev, VirtQueue *vq)
+{
+VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
+VirtQueueElement *elem;
+
+while ((elem = virtqueue_pop(vq, sizeof(VirtQueueElement {
+unsigned int i;
+
+for (i = 0; i < elem->in_num; i++) {
+void *addr = elem->in_sg[i].iov_base;
+size_t size = elem->in_sg[i].iov_len;
+ram_addr_t ram_offset;
+size_t rb_page_size;
+RAMBlock *rb;
+
+if (qemu_balloon_is_inhibited() || dev->poison_val) {
+continue;
+}
+
+/*
+ * There is no need to check the memory section to see if
+ * it is ram/readonly/romd like there is for handle_output
+ * below. If the region is not meant to be written to then
+ * address_space_map will have allocated a bounce buffer
+ * and it will be freed in address_space_unmap and trigger
+ * and unassigned_mem_write before failing to copy over the
+ * buffer. If more than one bad descriptor is provided it
+ * will return NULL after the first bounce buffer and fail
+ * to map any resources.
+ */
+rb = qemu_ram_block_from_host(addr, false, &ram_offset);
+if (!rb) {
+trace_virtio_balloon_bad_addr(elem->in_addr[i]);
+continue;
+}
+
+/* For now we will simply ignore unaligned memory regions */
+rb_page_size = qemu_ram_pagesize(rb);
+if (!QEMU_IS_ALIGNED(ram_offset | size, rb_page_size)) {
+continue;
+}
+
+ram_block_discard_range(rb, ram_offset, size);
+}
+
+virtqueue_push(vq, elem, 0);
+virtio_notify(vdev, vq);
+g_free(elem);
+}
+}
+
static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
{
VirtIOBalloon *s = VIRTIO_BALLOON(vdev);
@@ -628,7 +679,8 @@ static size_t virtio_balloon_config_size(VirtIOBalloon *s)
return sizeof(struct virtio_balloon_config);
}
if (virtio_has_feature(features, VIRTIO_BALLOON_F_PAGE_POISON) ||
-virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+virtio_has_feature(features, VIRTIO_BALLOON_F_FREE_PAGE_HINT) ||
+virtio_has_feature(features, VIRTIO_BALLOON_F_REPORTING)) {
return sizeof(struct virtio_balloon_config);
}
return offsetof(struct virtio_balloon_config, free_page_report_cmd_id);
@@ -717,7 +769,8 @@ static uint64_t virtio_balloon_get_features(VirtIODevice
*vdev, uint64_t f,
VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
f |= dev->host_features;
virtio_add_feature(&f, VIRTIO_BALLOON_F_STATS_VQ);
-if (virtio_has_feature(f, VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
+if (virtio_has_feature(f, VIRTIO_BALLOON_F_FREE_PAGE_HINT) ||
+virtio_has_feature(f, VIRTIO_BALLOON_F_REPORTING)) {
virtio_add_feature(&f, VIRTIO_BALLOON_F_PAGE_POISON);
}
@@ -807,6 +860,10 @@ static void virtio_balloon_device_realize(DeviceState
*dev, Error **errp)
s->dvq = virtio_add_queue(vdev, 128, virtio_balloon_handle_output);
s->svq = virtio_add_queue(vdev, 128, virtio_balloon_receive_stats);
+if (virtio_has_feature(s->host_features, VIRTIO_BALLOON_F_REPORTING)) {
+s->rvq = virtio_add_queue(vdev, 32, virtio_balloon_handle_report);
+}
+
if (virtio_has_feature(s->host_features,
VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
s->free_page_vq = virtio_add_queue(vdev, VIRTQUEUE_MAX_SIZE,
@@ -940,6 +997,8 @@ static Property virtio_balloon_properties[] = {
*/
DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon,
qemu_4_0_config_size, false),
+DEFINE_PROP_BIT("free-page-reporting", VirtIOBalloon, host_features,
+
[PATCH v19 QEMU 2/4] linux-headers: update to contain virito-balloon free page reporting
From: Alexander Duyck Sync the latest upstream changes for free page reporting. To be replaced by a full linux header sync. Signed-off-by: Alexander Duyck --- include/standard-headers/linux/virtio_balloon.h |1 + 1 file changed, 1 insertion(+) diff --git a/include/standard-headers/linux/virtio_balloon.h b/include/standard-headers/linux/virtio_balloon.h index 9375ca2a70de..1c5f6d6f2de6 100644 --- a/include/standard-headers/linux/virtio_balloon.h +++ b/include/standard-headers/linux/virtio_balloon.h @@ -36,6 +36,7 @@ #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM2 /* Deflate balloon on OOM */ #define VIRTIO_BALLOON_F_FREE_PAGE_HINT3 /* VQ to report free pages */ #define VIRTIO_BALLOON_F_PAGE_POISON 4 /* Guest is using page poisoning */ +#define VIRTIO_BALLOON_F_REPORTING 5 /* Page reporting virtqueue */ /* Size of a PFN in the balloon interface. */ #define VIRTIO_BALLOON_PFN_SHIFT 12
[PATCH v19 QEMU 4/4] memory: Do not allow direct write access to rom_device regions
From: Alexander Duyck
According to the documentation in memory.h a ROM memory region will be
backed by RAM for reads, but is supposed to go through a callback for
writes. Currently we were not checking for the existence of the rom_device
flag when determining if we could perform a direct write or not.
To correct that add a check to memory_region_is_direct so that if the
memory region has the rom_device flag set we will return false for all
checks where is_write is set.
Signed-off-by: Alexander Duyck
---
include/exec/memory.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 1614d9a02c0c..e000bd2f97b2 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2351,8 +2351,8 @@ void address_space_write_cached_slow(MemoryRegionCache
*cache,
static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
{
if (is_write) {
-return memory_region_is_ram(mr) &&
- !mr->readonly && !memory_region_is_ram_device(mr);
+return memory_region_is_ram(mr) && !mr->readonly &&
+ !mr->rom_device && !memory_region_is_ram_device(mr);
} else {
return (memory_region_is_ram(mr) && !memory_region_is_ram_device(mr))
||
memory_region_is_romd(mr);
Re: [PATCH v18 QEMU 3/3] virtio-balloon: Provide a interface for free page reporting
On Thu, Apr 9, 2020 at 10:35 AM David Hildenbrand wrote:
> On 09.04.20 19:34, Alexander Duyck wrote:
> >>> hw/virtio/virtio-balloon.c | 48
> >>> +++-
> >>> include/hw/virtio/virtio-balloon.h |2 +-
> >>> 2 files changed, 47 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> >>> index 1c6d36a29a04..297b267198ac 100644
> >>> --- a/hw/virtio/virtio-balloon.c
> >>> +++ b/hw/virtio/virtio-balloon.c
> >>> @@ -321,6 +321,42 @@ static void balloon_stats_set_poll_interval(Object
> >>> *obj, Visitor *v,
> >>> balloon_stats_change_timer(s, 0);
> >>> }
> >>>
> >>> +static void virtio_balloon_handle_report(VirtIODevice *vdev, VirtQueue
> >>> *vq)
> >>> +{
> >>> +VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
> >>> +VirtQueueElement *elem;
> >>> +
> >>> +while ((elem = virtqueue_pop(vq, sizeof(VirtQueueElement {
> >>> +unsigned int i;
> >>> +
> >>> +for (i = 0; i < elem->in_num; i++) {
> >>> +void *addr = elem->in_sg[i].iov_base;
> >>> +size_t size = elem->in_sg[i].iov_len;
> >>> +ram_addr_t ram_offset;
> >>> +size_t rb_page_size;
> >>> +RAMBlock *rb;
> >>> +
> >>> +if (qemu_balloon_is_inhibited() || dev->poison_val) {
> >>> +continue;
> >>> +}
> >>
> >> These checks are not sufficient. See virtio_balloon_handle_output(),
> >> where we e.g., check that somebody doesn't try to discard the bios.
> >>
> >> Maybe we can factor our/unify the handling in both code paths.
> >
> > I am going to have to look at this further. If I understand correctly
> > you are asking me to add all of the memory section checks that are in
> > virtio_balloon_handle_output correct?
> >
> > I'm not sure it makes sense with the scatterlist approach I have taken
> > here. All of the mappings were created as a scatterlist of writable
> > DMA mappings unlike the regular balloon which is just stuffing PFN
> > numbers into an array and then sending the array across. As such I
> > would think there are already such protections in place. For instance,
> > you wouldn't want to let virtio-net map the BIOS and request data be
> > written into that either, correct? So I am assuming there is something
> > there to prevent that already isn't there?
>
> Good point, let's find out :)
Okay, so I believe I figured it out. From what I can tell there is a
call in address_space_map that will determine if we can directly write
to the page or not. However it looks like there might be one minor
issue as it is assuming it can write directly to ROM devices and that
isn't correct. I will add a patch to my set to address that.
Other than that the behavior seems to be that a bounce buffer will be
allocated on the first instance of a page backed by ROM or anything
other than RAM, and after that it will return NULL until the bounce
buffer is freed. If we start getting NULLs the mapping will be aborted
and we wouldn't even get into this code path. After we unmap the
region it will attempt to use address_space_write which should fail
for any region that isn't meant to be written to, or it will copy
zeros out of the bounce buffer into the region if it is writable via
address_space_write.
So the DMA mapping API in virtqueue_pop/virtqueue_push will take care
of doing the right stuff for the mappings in the case that the guest
is trying to do something really stupid, at least after I address the
direct write access to rom_device regions.
- Alex
[Bug 1863441] Re: cmd_mode_sense always reports 0x70, no CDROM present
Roger that, thanks! I don't think we're very careful about which version we try to emulate. In practice it's "Whatever seems to work across the largest swatch of guest operating systems simultaneously". If this comes up again, feel free to file against a specific guest OS that appears to be non-functioning. Thanks! --js -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1863441 Title: cmd_mode_sense always reports 0x70, no CDROM present Status in QEMU: New Bug description: cmd_mode_sense https://git.qemu.org/?p=qemu.git;a=blob;f=hw/ide/atapi.c;hb=refs/heads/master#l852 always reports 0x70 in byte 2 returned, indicating no CD-ROM present. If CD-ROM is present, should report 0x01 (or 0x11). If CD-ROM absent, should report 0x70. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1863441/+subscriptions
Re: [PATCH] Fixed IPv6 payload lenght without jumbo option
On 2020/4/9 下午9:35, Andrew Melnichenko wrote:
> Do we support ipv6 segmentation then?
No, but - if the backend supports big frames there is an issue that
IPv6 plen doesn't have valid value.
Actually, It's a good idea to add IPv6 fragmentation - I would do it
later.
Right, another question.
E.g for virtio-net, we will do the following check:
if (!peer_has_vnet_hdr(n)) {
virtio_clear_feature(&features, VIRTIO_NET_F_CSUM);
virtio_clear_feature(&features, VIRTIO_NET_F_HOST_TSO4);
virtio_clear_feature(&features, VIRTIO_NET_F_HOST_TSO6);
virtio_clear_feature(&features, VIRTIO_NET_F_HOST_ECN);
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_CSUM);
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO4);
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6);
virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN);
}
I think we should do something similar in e1000e. But I can only see
disable_vnet parameter but not a checking of the ability of its peer.
1) when peer has vnet hdr, it supports receiving GSO packets, we don't
need software segmentation.
2) when peer does not have vnet hdr, we need to use software path
segmentation.
This means we need:
1) check peer_has_vnet_hdr() and disable_vnet in net_pkt_send() before
calling net_tx_pkt_sendv() and fallback to software segmentation
2) fix the ipv6 payload len
3) add the ipv6 software segmentation support
It would be better if we can fix all of these issue in one series.
Thanks
On Thu, Apr 9, 2020 at 6:15 AM Jason Wang > wrote:
On 2020/4/6 上午3:19, and...@daynix.com
wrote:
> From: Andrew Melnychenko mailto:and...@daynix.com>>
>
> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1708065
> e1000e driver doesn't sets 'plen' field for IPv6 for big packets
> if TSO is enabled. Jumbo option isn't added yet, until
> qemu supports packets greater than 64K.
>
> Signed-off-by: Andrew Melnychenko mailto:and...@daynix.com>>
> ---
> hw/net/e1000e_core.c | 1 +
> hw/net/net_tx_pkt.c | 31 +++
> hw/net/net_tx_pkt.h | 7 +++
> include/net/eth.h | 1 +
> 4 files changed, 40 insertions(+)
>
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index d5676871fa..a1ec55598b 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -656,6 +656,7 @@ e1000e_tx_pkt_send(E1000ECore *core, struct
e1000e_tx *tx, int queue_index)
> NetClientState *queue = qemu_get_subqueue(core->owner_nic,
target_queue);
>
> e1000e_setup_tx_offloads(core, tx);
> + net_tx_pkt_fix_ip6_payload_len(tx->tx_pkt);
A question here:
I don't see any code that set ip6_plen during
net_tx_pkt_do_sw_fragmentation(). This is described in 7.3.6.2.1 and
7.3.6.2.2 in the datasheet.
And:
1) eth_setup_ip4_fragmentation() only deal with ipv4
2) eth_fix_ip4_checksum() assumes a ipv4 header
Do we support ipv6 segmentation then?
Thanks
>
> net_tx_pkt_dump(tx->tx_pkt);
>
> diff --git a/hw/net/net_tx_pkt.c b/hw/net/net_tx_pkt.c
> index 162f802dd7..b05d554ac3 100644
> --- a/hw/net/net_tx_pkt.c
> +++ b/hw/net/net_tx_pkt.c
> @@ -635,3 +635,34 @@ bool net_tx_pkt_send_loopback(struct
NetTxPkt *pkt, NetClientState *nc)
>
> return res;
> }
> +
> +void net_tx_pkt_fix_ip6_payload_len(struct NetTxPkt *pkt)
> +{
> + /*
> + * If ipv6 payload length field is 0 - then there should be
Hop-by-Hop
> + * option for packets greater than 65,535.
> + * For packets with payload less than 65,535: fix 'plen' field.
> + * For now, qemu drops every packet with size greater 64K
> + * (see net_tx_pkt_send()) so, there is no reason to add
jumbo option to ip6
> + * hop-by-hop extension if it's missed
> + */
> +
> + struct iovec *l2 = &pkt->vec[NET_TX_PKT_L2HDR_FRAG];
> + if (eth_get_l3_proto(l2, 1, l2->iov_len) == ETH_P_IPV6) {
> + struct ip6_header *ip6 = (struct ip6_header *) pkt->l3_hdr;
> + /*
> + * TODO: if qemu would support >64K packets - add jumbo
option check
> + * something like that:
> + * 'if (ip6->ip6_plen == 0 && !has_jumbo_option(ip6)) {'
> + */
> + if (ip6->ip6_plen == 0) {
> + if (pkt->payload_len <= ETH_MAX_IP_DGRAM_LEN) {
> + ip6->ip6_plen = htons(pkt->payload_len);
> + }
> + /*
> + * TODO: if qemu would support >64K packets
> + * add jumbo option for packets greater then 65,535
bytes
> + */
> + }
> + }
> +}
> diff --git a/hw/net/net_tx_pkt.h b/hw/net/net_tx_pkt.h
[PATCH v5 1/1] colo-compare: Fix memory leak in packet_enqueue()
The patch is to fix the "pkt" memory leak in packet_enqueue().
The allocated "pkt" needs to be freed if the colo compare
primary or secondary queue is too big.
Replace the error_report of full queue with a trace event.
Signed-off-by: Derek Su
---
net/colo-compare.c | 23 +++
net/trace-events | 1 +
2 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 10c0239f9d..035e11d4d3 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -122,6 +122,10 @@ enum {
SECONDARY_IN,
};
+static const char *colo_mode[] = {
+[PRIMARY_IN] = "primary",
+[SECONDARY_IN] = "secondary",
+};
static int compare_chr_send(CompareState *s,
const uint8_t *buf,
@@ -217,6 +221,7 @@ static int packet_enqueue(CompareState *s, int mode,
Connection **con)
ConnectionKey key;
Packet *pkt = NULL;
Connection *conn;
+int ret;
if (mode == PRIMARY_IN) {
pkt = packet_new(s->pri_rs.buf,
@@ -245,16 +250,18 @@ static int packet_enqueue(CompareState *s, int mode,
Connection **con)
}
if (mode == PRIMARY_IN) {
-if (!colo_insert_packet(&conn->primary_list, pkt, &conn->pack)) {
-error_report("colo compare primary queue size too big,"
- "drop packet");
-}
+ret = colo_insert_packet(&conn->primary_list, pkt, &conn->pack);
} else {
-if (!colo_insert_packet(&conn->secondary_list, pkt, &conn->sack)) {
-error_report("colo compare secondary queue size too big,"
- "drop packet");
-}
+ret = colo_insert_packet(&conn->secondary_list, pkt, &conn->sack);
}
+
+if (!ret) {
+trace_colo_compare_drop_packet(colo_mode[mode],
+"queue size too big, drop packet");
+packet_destroy(pkt, NULL);
+pkt = NULL;
+}
+
*con = conn;
return 0;
diff --git a/net/trace-events b/net/trace-events
index 02c13fd0ba..fa49c71533 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -12,6 +12,7 @@ colo_proxy_main(const char *chr) ": %s"
# colo-compare.c
colo_compare_main(const char *chr) ": %s"
+colo_compare_drop_packet(const char *queue, const char *chr) ": %s: %s"
colo_compare_udp_miscompare(const char *sta, int size) ": %s = %d"
colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize,
const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s,
spkt size = %d, ip_src = %s, ip_dst = %s"
--
2.17.1
[PATCH v5 0/1] COLO: handling of the full primary or secondary queue
The series is to handle the full primary or secondary queue in colo-compare. Fix the "pkt" memory leak in packet_enqueue(). Reproduce steps: 1. Setup PVM and SVM both with NIC e1000 by the steps descripted in the wiki qemu/COLO 2. Run "iperf3 -s" in PVM 3. Run "iperf3 -c -t 7200" in client The memory usage of qemu-system-x86_64 increases as the PVM's QMP shows "qemu-system-x86_64: colo compare secondary queue size too big, drop packet". Please help to review, thanks. V5: - Replace the error_report of full queue with a trace event - Remove handling of the full primary or secondary queue which hurt network throughput too much V4: - Remove redundant flush of packets V3: - handling of the full primary or secondary queue according to the suggestion from Zhang Chen V2: - Fix incorrect patch format Derek Su (1): colo-compare: Fix memory leak in packet_enqueue() net/colo-compare.c | 23 +++ net/trace-events | 1 + 2 files changed, 16 insertions(+), 8 deletions(-) -- 2.17.1
Re: [PATCH] exec: fetch the alignment of Linux devdax pmem character device nodes
On 4/10/2020 12:46 AM, Dan Williams wrote:
On Thu, Apr 9, 2020 at 7:33 AM Liu, Jingqi wrote:
On 4/8/2020 5:42 PM, Joao Martins wrote:
On 4/8/20 3:25 AM, Liu, Jingqi wrote:
On 4/8/2020 2:28 AM, Joao Martins wrote:
On 4/7/20 5:55 PM, Dan Williams wrote:
On Tue, Apr 7, 2020 at 4:01 AM Joao Martins wrote:
Perhaps, you meant instead:
/sys/dev/char/%d:%d/align
Hmm, are you sure that's working?
It is, except that I made the slight mistake of testing with a bunch of wip
patches on top which one of them actually adds the 'align' to child dax device.
Argh, my apologies - and thanks for noticing.
I expect the alignment to be found
in the region device:
/sys/class/dax:
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus1/region1/dax1.1/dax1.0
$(readlink -f /sys/dev/char/253\:263)/../align
$(readlink -f /sys/dev/char/253\:263)/device/align
/sys/bus/dax:
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus1/region1/dax1.0/dax1.0
$(readlink -f /sys/dev/char/253\:265)/../align
$(readlink -f /sys/dev/char/253\:265)/device/align <-- No such file
The use of the /sys/dev/char/%d:%d/device is only supported by the
deprecated /sys/class/dax.
Hi Dan,
Thanks for your comments.
Seems it is a mistake.
It should be: $(readlink -f /sys/dev/char/253\:263)/../../align
Hmm, perhaps you have an extra '../' in the path? This works for me:
# ls $(readlink -f /sys/dev/char/252\:0/../align)
/sys/devices/platform/e820_pmem/ndbus0/region0/dax0.0/dax0.0/../align
# cat $(readlink -f /sys/dev/char/252\:0)/../align
2097152
# cat /sys/dev/char/252\:0/../align
2097152
Hi Joao,
Hmm, I need to have an extra '../' in the path. The details are as follows:
# ll /dev/dax2.0
crw--- 1 root root 251, 5 Mar 20 13:35 /dev/dax2.0
# uname -r
5.6.0-rc1-00044-gb19e8c684703
# readlink -f /sys/dev/char/251\:5/
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region2/dax2.1/dax/dax2.0
# ls $(readlink -f /sys/dev/char/251\:5)/../align
ls: cannot access
'/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region2/dax2.1/dax/dax2.0/../align':
No such file or directory
# ls $(readlink -f /sys/dev/char/251\:5)/../dax_region/align
ls: cannot access
'/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region2/dax2.1/dax/dax2.0/../dax_region/align':
No such file or directory
# ls $(readlink -f /sys/dev/char/251\:5)/../../align
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region2/dax2.1/dax/dax2.0/../../align
# ls $(readlink -f /sys/dev/char/251\:5)/../../dax_region/align
/sys/devices/LNXSYSTM:00/LNXSYBUS:00/ACPI0012:00/ndbus0/region2/dax2.1/dax/dax2.0/../../dax_region/align
# lsmod|grep pmem
dax_pmem_compat16384 0
device_dax 20480 1 dax_pmem_compat
dax_pmem_core 16384 1 dax_pmem_compat
# lsmod|grep dax
dax_pmem_compat16384 0
device_dax 20480 1 dax_pmem_compat
dax_pmem_core 16384 1 dax_pmem_compat
Seems some configurations are different ?
Can you share your info as above ? Thanks.
Alternatively maybe you can use libdaxctl that automatically handles
the ABI differences between compat-dax-class and dax-bus layouts? I
didn't recommend it before because I was not sure about qemu's policy
about taking on new library dependencies, but with libdaxctl you could
do something like (untested):
path = g_strdup_printf("/sys/dev/char/%d:%d", major(st.st_rdev),
minor(st.st_rdev));
rpath = realpath(path, NULL);
daxctl_region_foreach(ctx, region)
if (strstr(daxctl_region_get_path(region), rpath)) {
align = daxctl_region_get_align(region);
break;
}
Thanks for your suggestion. I'll test it.
Thanks,
Jingqi
Re: [RFC][PATCH v2 0/3] IVSHMEM version 2 device for QEMU
On 4/9/20 10:11 AM, Jan Kiszka wrote: > On 09.04.20 15:52, Liang Yan wrote: >> >> >> On 1/7/20 9:36 AM, Jan Kiszka wrote: >>> Overdue update of the ivshmem 2.0 device model as presented at [1]. >>> >>> Changes in v2: >>> - changed PCI device ID to Siemens-granted one, >>> adjusted PCI device revision to 0 >>> - removed unused feature register from device >>> - addressed feedback on specification document >>> - rebased over master >>> >>> This version is now fully in sync with the implementation for Jailhouse >>> that is currently under review [2][3], UIO and virtio-ivshmem drivers >>> are shared. Jailhouse will very likely pick up this revision of the >>> device in order to move forward with stressing it. >>> >>> More details on the usage with QEMU were in the original cover letter >>> (with adjustements to the new device ID): >>> >>> If you want to play with this, the basic setup of the shared memory >>> device is described in patch 1 and 3. UIO driver and also the >>> virtio-ivshmem prototype can be found at >>> >>> >>> http://git.kiszka.org/?p=linux.git;a=shortlog;h=refs/heads/queues/ivshmem2 >>> >>> >>> Accessing the device via UIO is trivial enough. If you want to use it >>> for virtio, this is additionally to the description in patch 3 needed on >>> the virtio console backend side: >>> >>> modprobe uio_ivshmem >>> echo "110a 4106 1af4 1100 ffc003 ff" > >>> /sys/bus/pci/drivers/uio_ivshmem/new_id >>> linux/tools/virtio/virtio-ivshmem-console /dev/uio0 >>> >>> And for virtio block: >>> >>> echo "110a 4106 1af4 1100 ffc002 ff" > >>> /sys/bus/pci/drivers/uio_ivshmem/new_id >>> linux/tools/virtio/virtio-ivshmem-console /dev/uio0 >>> /path/to/disk.img >>> >>> After that, you can start the QEMU frontend instance with the >>> virtio-ivshmem driver installed which can use the new /dev/hvc* or >>> /dev/vda* as usual. >>> >> Hi, Jan, >> >> Nice work. >> >> I did a full test for your this new version. QEMU device part looks >> good, virtio console worked as expected. Just had some issue with >> virtio-ivshmem-block tests here. >> >> I suppose you mean "linux/tools/virtio/virtio-ivshmem-block"? > > Yes, copy&paste mistake, had the same issue over in > https://github.com/siemens/jailhouse/blob/master/Documentation/inter-cell-communication.md > > >> >> Noticed "ffc002" is the main difference, however I saw nothing response >> when run echo command here, are there anything I need to prepare? >> >> I build the driver in guest kernel already. >> >> Do I need a new protocol or anything for below command line? >> ivshmem2-server -F -l 64K -n 2 -V 3 -P 0x8003 > > Yes, you need to adjust that command line - didn't I document that > somewhere? Looks like I didn't: > > ivshmem2-server -F -l 1M -n 2 -V 2 -P 0x8002 > > i.e. a bit more memory is good (but this isn't speed-optimized anyway), > you only need 2 vectors here (but more do not harm), and the protocol > indeed needs adjustment (that is the key). > Thanks for the reply. I just confirmed that virtio-ivshmem-block worked with the new configruation, a "vdb" disk is mounted to fronted VM. I will send out a full test summary later. Best, Liang > Jan >
Re: [PATCH v12 2/3] qcow2: Allow writing compressed data of multiple clusters
We could assign indices to the clusters/chunks and improve the algorithm to
write them down on the disk in the same order adjacently. If you find it
feasible for QEMU, I'd like to create a task for doing that, shall I?
Andrey
From: Vladimir Sementsov-Ogievskiy
Sent: Thursday, April 9, 2020 9:39 PM
To: Alberto Garcia ; Andrey Shinkevich
; qemu-devel@nongnu.org
; qemu-bl...@nongnu.org
Cc: kw...@redhat.com ; arm...@redhat.com ;
mre...@redhat.com ; Denis Lunev
Subject: Re: [PATCH v12 2/3] qcow2: Allow writing compressed data of multiple
clusters
09.04.2020 19:50, Alberto Garcia wrote:
> On Mon 02 Dec 2019 01:15:05 PM CET, Andrey Shinkevich wrote:
>> +static coroutine_fn int
>> +qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
>> + uint64_t offset, uint64_t bytes,
>> + QEMUIOVector *qiov, size_t qiov_offset)
>> +{
>> +BDRVQcow2State *s = bs->opaque;
>> +AioTaskPool *aio = NULL;
>> +int ret = 0;
>> +
>> +if (has_data_file(bs)) {
>> +return -ENOTSUP;
>> +}
>> +
>> +if (bytes == 0) {
>> +/*
>> + * align end of file to a sector boundary to ease reading with
>> + * sector based I/Os
>> + */
>> +int64_t len = bdrv_getlength(bs->file->bs);
>> +if (len < 0) {
>> +return len;
>> +}
>> +return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF,
>> NULL);
>> +}
>> +
>> +if (offset_into_cluster(s, offset)) {
>> +return -EINVAL;
>> +}
>> +
>> +while (bytes && aio_task_pool_status(aio) == 0) {
>> +uint64_t chunk_size = MIN(bytes, s->cluster_size);
>> +
>> +if (!aio && chunk_size != bytes) {
>> +aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
>> +}
>> +
>> +ret = qcow2_add_task(bs, aio,
>> qcow2_co_pwritev_compressed_task_entry,
>> + 0, 0, offset, chunk_size, qiov, qiov_offset,
>> NULL);
>> +if (ret < 0) {
>> +break;
>> +}
>> +qiov_offset += chunk_size;
>> +offset += chunk_size;
>> +bytes -= chunk_size;
>> +}
>
> This patch allows the user to write more than one cluster of compressed
> data at a time, and it does so by splitting the request into many
> cluster-sized requests and using qcow2_add_task() for each one of them.
>
> What happens however is that there's no guarantee that the requests are
> processed in the same order that they were added.
>
> One consequence is that running on an empty qcow2 file a command as
> simple as this one:
>
> qemu-io -c 'write -c 0 256k' image.qcow2
>
> does not always produce the same results.
>
> This does not have any user-visible consequences for the guest. In all
> cases the data is correctly written, it's just that the ordering of the
> compressed clusters (and therefore the contents of the L2 entries) will
> be different each time.
>
> Because of this a test cannot expect that running the same commands on
> an empty image produces always the same results.
>
> Is this something that we should be concerned about?
>
Parallel writing compressed clusters is significant improvement, as it allow
compressing in really parallel threads.
Generally, async parallel issuing of several requests gives more performance
than handling peaces one-by-one, mirror works on this basis and it is fast.
I've already moved qcow2 to this idea (aio tasks in qcow2 code), and in
progress of moving backup job. So, I think that asynchrony and ambiguity would
be native for block-layer anyway.
Hmm. Still, what about cluster sequence? For normal clusters there may be
simple thing to do: preallocation (at least of metadata). So, we can pre-create
cluster sequence.. But what to do with compressed clusters if we want specific
order for them, I don't know. On the other hand, ordering of normal cluster may
make sence: it should increase performnace of following IO. But for compressed
clusters it's not the case.
So, I don't think we should make specific workaround for testing... What
exactly is the case?
--
Best regards,
Vladimir
Re: [Bug 1871842] [NEW] AMD CPUID leaf 0x8000'0008 reported number of cores inconsistent with ACPI.MADT
Philipp,
Can you please check if this patch works for you.
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 90ffc5f..e467fee 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5831,10 +5831,17 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
index, uint32_t count,
}
*ebx = env->features[FEAT_8000_0008_EBX];
*ecx = 0;
-*edx = 0;
if (cs->nr_cores * cs->nr_threads > 1) {
-*ecx |= (cs->nr_cores * cs->nr_threads) - 1;
+unsigned long max_apicids, bits_required;
+
+max_apicids = (cs->nr_cores * cs->nr_threads) - 1;
+if (max_apicids) {
+/* Find out the number of bits to represent all the
apicids */
+bits_required = find_last_bit(&max_apicids,
BITS_PER_BYTE) + 1;
+*ecx |= bits_required << 12 | max_apicids;
+}
}
+*edx = 0;
break;
case 0x800A:
if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_SVM) {
On 4/9/20 9:00 AM, Igor Mammedov wrote:
> On Thu, 09 Apr 2020 12:58:11 -
> Philipp Eppelt <1871...@bugs.launchpad.net> wrote:
>
>> Public bug reported:
>>
>> Setup:
>> CPU: AMD EPYC-v2 or host's EPYC cpu
>> Linux 64-bit fedora host; Kernel version 5.5.15-200.fc31
>> qemu version: self build
>> git-head: f3bac27cc1e303e1860cc55b9b6889ba39dee587
>> config: Configured with: '../configure'
>> '--target-list=x86_64-softmmu,mips64el-softmmu,mips64-softmmu,mipsel-softmmu,mips-softmmu,i386-softmmu,aarch64-softmmu,arm-softmmu'
>> '--prefix=/opt/qemu-master'
>>
>> Cmdline:
>> qemu-system-x86_64 -kernel
>> /home/peppelt/code/l4/internal/.build-x86_64/bin/amd64_gen/bootstrap -append
>> "" -initrd "./fiasco/.build-x86_64/fiasco , ... " -serial stdio -nographic
>> -monitor none -nographic -monitor none -cpu EPYC-v2 -m 4G -smp 4
>>
>> Issue:
>> We are developing an microkernel operating system called L4Re. We recently
>> got an AMD EPYC server for testing and we couldn't execute SMP tests of our
>> system when running Linux + qemu + VM w/ L4Re.
>> In fact, the kernel did not recognize any APs at all. On AMD CPUs the kernel
>> checks for the number of cores reported in CPUID leaf 0x8000_0008.ECX[NC] or
>> [ApicIdSize]. [0][1]
>>
>> The physical machine reports for leaf 0x8000_0008: EAX: 0x3030 EBX:
>> 0x18cf757 ECX: 0x703f EDX: 0x1000
>> The lower four bits of ECX are the [NC] field and all set.
>>
>> When querying inside qemu with -enable-kvm -cpu host -smp 4 (basically as
>> replacement and addition to the above cmdline) the CPUID leaf shows: EAX:
>> 0x3024, EBX: 0x1001000, ECX: 0x0, EDX: 0x0
>> Note, ECX is zero. Indicating that this is no SMP capabale CPU.
>>
>> I'm debugging it using my local machine and the QEMU provided EPYC-v2
>> CPU model and it is reproducible there as well and reports: EAX:
>> 0x3028, EBX: 0x0, ECX: 0x0, EDX: 0x0
>>
>> I checked other AMD based CPU models (phenom, opteron_g3/g5) and they behave
>> the same. [2] shows the CPUID 0x8000'0008 handling in the QEMU source.
>> I believe that behavior here is wrong as ECX[NC] should report the number of
>> cores per processor, as stated in the AMD manual [2] p.584. In my
>> understanding -smp 4 should then lead to ECX[NC] = 0x3.
>>
>> The following table shows my findings with the -smp option:
>> Option | Qemu guest observed ECX value
>> -smp 4 | 0x0
>> -smp 4,cores=4 | 0x3
>> -smp 4,cores=2,thread=2 | 0x3
>> -smp 4,cores=4,threads=2 | QEMU boot error: topology false.
>>
>> Now, I'm asking myself how the terminology of the AMD manual maps to QEMU's
>> -smp option.
>> Obviously, nr_cores and nr_threads correspond to the cores and threads
>> options on the cmdline and cores * threads <= 4 (in this example), but what
>> corresponds the X in -smp X to?
> I'd say X corresponds to number of logical CPUs.
> Depending on presence of other options these are distributed among them in
> magical manner
> (see pc_smp_parse() for starters)
>
>> Querying 0x8000'0008 on the physical processor results in different
>> reports than quering QEMU's model as does it with -enable-kvm -cpu host.
>>
>> Furthermore, the ACPI.MADT shows 4 local APICs to be present while the
>> CPU leave reports a single core processor.
> it matches -smp X as it should be.
>
>>
>> This leads me to the conclusion that CPUID 0x8000'0008.ECX reports the
>> wrong number.
> CCed author of recent epyc patches who might know how AMD should work better
> than me.
>
>>
>> Please let me know, if you need more information from my side.
>>
>>
>> [0]
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fkernkonzept%2Ffiasco%2Fblob%2F522ccc5f29ab120213cf02d71328e2b879cbbd19%2Fsrc%2Fkern%2Fia32%2Fkernel_thread-ia32.cpp%23L109&data=02%7C01%7Cbabu.moger%40amd.com%7C57569f7959744399655b08d7dc8e6e24%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637220379083511672&sdata=hcFJzLAVQoIh5IN9CP%2F9cUQNOZoBnpRA6FliJur1wzQ%3D&reserved=0
>> [1]
>> https://nam11.sa
Re: [PATCH for 5.0-rc3 v1 00/11] more random fixes
Patchew URL: https://patchew.org/QEMU/20200409211529.5269-1-alex.ben...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH for 5.0-rc3 v1 00/11] more random fixes Message-id: 20200409211529.5269-1-alex.ben...@linaro.org Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Switched to a new branch 'test' 2779bae .travis.yml: Build OSX 10.14 with Xcode 10.0 ffa69ee linux-user: fix /proc/self/stat handling da73496 gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb a7392db target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray 6cdbe4b configure: disable PIE for Windows builds e70b41a configure: redirect sphinx-build check to config.log 84b0026 tests/docker: add docs FEATURE flag and use for test-misc e991630 linux-user/ppc: Fix padding in mcontext_t for ppc64 3fe0015 accel/tcg: Relax va restrictions on 64-bit guests 3a50414 exec/cpu-all: Use bool for have_guest_base 8327d01 linux-user: completely re-write init_guest_space === OUTPUT BEGIN === 1/11 Checking commit 8327d0183d84 (linux-user: completely re-write init_guest_space) ERROR: trailing whitespace #715: FILE: linux-user/qemu.h:229: + * $ total: 1 errors, 0 warnings, 681 lines checked Patch 1/11 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 2/11 Checking commit 3a504143c770 (exec/cpu-all: Use bool for have_guest_base) 3/11 Checking commit 3fe00151c0e3 (accel/tcg: Relax va restrictions on 64-bit guests) ERROR: Macros with complex values should be enclosed in parenthesis #91: FILE: include/exec/cpu-all.h:182: +# define GUEST_ADDR_MAX_ ~0ul total: 1 errors, 0 warnings, 88 lines checked Patch 3/11 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 4/11 Checking commit e99163099022 (linux-user/ppc: Fix padding in mcontext_t for ppc64) 5/11 Checking commit 84b002680ba0 (tests/docker: add docs FEATURE flag and use for test-misc) 6/11 Checking commit e70b41a4ac1e (configure: redirect sphinx-build check to config.log) 7/11 Checking commit 6cdbe4bcdb6b (configure: disable PIE for Windows builds) 8/11 Checking commit a7392db3e091 (target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray) 9/11 Checking commit da73496e5afd (gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb) 10/11 Checking commit ffa69ee03e63 (linux-user: fix /proc/self/stat handling) 11/11 Checking commit 2779baef4f14 (.travis.yml: Build OSX 10.14 with Xcode 10.0) === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200409211529.5269-1-alex.ben...@linaro.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
[Bug 1871798] Re: Fails to start on Windows host without explicit --disable-pie
Tested and working. Thank you! -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1871798 Title: Fails to start on Windows host without explicit --disable-pie Status in QEMU: Incomplete Bug description: Since commit d2cd29e30736afd4a1e8cac3cf4da360bbc65978, which removed the x86 conditional around PIE, QEMU completely fails to start on a Windows host unless --disable-pie is explicitly given at build time. Even just requesting the help text doesn't work. To make testing easier, this can be replicated with Wine. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1871798/+subscriptions
Re: [PATCH v1 07/11] configure: disable PIE for Windows builds
On Thu, Apr 9, 2020 at 11:18 PM Alex Bennée wrote: > It seems on some compilers the test can pass but still give you > broken binaries. > > [AJB untested - please could windows users test] > > Fixes: d2cd29e30736 > Fixes: https://bugs.launchpad.net/qemu/+bug/1871798 > Cc: Bug 1871798 <1871...@bugs.launchpad.net> > Cc: James Le Cuirot > Signed-off-by: Alex Bennée > --- > configure | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/configure b/configure > index a207cce82bc..e9c5f630c14 100755 > --- a/configure > +++ b/configure > @@ -807,6 +807,7 @@ MINGW32*) > audio_drv_list="" >fi >supported_os="yes" > + pie="no" > ;; > GNU/kFreeBSD) >bsd="yes" > -- > 2.20.1 > Solves my issue! So, Tested-by: Howard Spoelstra
[PATCH v1 10/11] linux-user: fix /proc/self/stat handling
In the original bug report long files names in Guix caused
/proc/self/stat be truncated without the trailing ") " as specified in
proc manpage which says:
(2) comm %s
The filename of the executable, in parentheses. This
is visible whether or not the executable is swapped
out.
Additionally it should only be reporting the executable name rather
than the full path. Fix both these failings while cleaning up the code
to use GString to build up the reported values. As the whole function
is cleaned up also adjust the white space to the current coding style.
Message-ID:
Reported-by: Brice Goglin
Cc: Philippe_Mathieu-Daudé
Signed-off-by: Alex Bennée
---
linux-user/syscall.c | 43 +++
1 file changed, 19 insertions(+), 24 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 6495ddc4cda..674f70e70a5 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7295,34 +7295,29 @@ static int open_self_stat(void *cpu_env, int fd)
{
CPUState *cpu = env_cpu((CPUArchState *)cpu_env);
TaskState *ts = cpu->opaque;
-abi_ulong start_stack = ts->info->start_stack;
+g_autoptr(GString) buf = g_string_new(NULL);
int i;
for (i = 0; i < 44; i++) {
- char buf[128];
- int len;
- uint64_t val = 0;
-
- if (i == 0) {
-/* pid */
-val = getpid();
-snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
- } else if (i == 1) {
-/* app name */
-snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
- } else if (i == 27) {
-/* stack bottom */
-val = start_stack;
-snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
- } else {
-/* for the rest, there is MasterCard */
-snprintf(buf, sizeof(buf), "0%c", i == 43 ? '\n' : ' ');
- }
+if (i == 0) {
+/* pid */
+g_string_printf(buf, FMT_pid " ", getpid());
+} else if (i == 1) {
+/* app name */
+gchar *bin = g_strrstr(ts->bprm->argv[0], "/");
+bin = bin ? bin + 1 : ts->bprm->argv[0];
+g_string_printf(buf, "(%.15s) ", bin);
+} else if (i == 27) {
+/* stack bottom */
+g_string_printf(buf, TARGET_ABI_FMT_ld " ", ts->info->start_stack);
+} else {
+/* for the rest, there is MasterCard */
+g_string_printf(buf, "0%c", i == 43 ? '\n' : ' ');
+}
- len = strlen(buf);
- if (write(fd, buf, len) != len) {
- return -1;
- }
+if (write(fd, buf->str, buf->len) != buf->len) {
+return -1;
+}
}
return 0;
--
2.20.1
[PATCH v1 06/11] configure: redirect sphinx-build check to config.log
Otherwise it's hard to debug whats going on.
Signed-off-by: Alex Bennée
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 233c671aaa9..a207cce82bc 100755
--- a/configure
+++ b/configure
@@ -4936,7 +4936,7 @@ has_sphinx_build() {
# sphinx-build doesn't exist at all or if it is too old.
mkdir -p "$TMPDIR1/sphinx"
touch "$TMPDIR1/sphinx/index.rst"
-"$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx"
"$TMPDIR1/sphinx/out" >/dev/null 2>&1
+"$sphinx_build" -c "$source_path/docs" -b html "$TMPDIR1/sphinx"
"$TMPDIR1/sphinx/out" >> config.log 2>&1
}
# Check if tools are available to build documentation.
--
2.20.1
[PATCH v1 09/11] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
From: Peter Xu
We should only pass in gdb_get_reg16() with the GByteArray* object
itself, no need to shift. Without this patch, gdb remote attach will
crash QEMU.
Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Signed-off-by: Peter Xu
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Alex Bennée
Message-Id: <20200409164954.36902-3-pet...@redhat.com>
---
target/i386/gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index f3d23b614ee..b98a99500ae 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray
*mem_buf, int n)
} else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
-len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
+len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
return len;
} else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
n -= IDX_XMM_REGS;
--
2.20.1
[PATCH v1 03/11] accel/tcg: Relax va restrictions on 64-bit guests
From: Richard Henderson
We cannot at present limit a 64-bit guest to a virtual address
space smaller than the host. It will mostly work to ignore this
limitation, except if the guest uses high bits of the address
space for tags. But it will certainly work better, as presently
we can wind up failing to allocate the guest stack.
Widen our user-only page tree to the host or abi pointer width.
Remove the workaround for this problem from target/alpha.
Always validate guest addresses vs reserved_va, as there we
control allocation ourselves.
Signed-off-by: Richard Henderson
Signed-off-by: Alex Bennée
---
include/exec/cpu-all.h| 23 +++
target/alpha/cpu-param.h | 15 ++-
accel/tcg/translate-all.c | 15 +--
3 files changed, 30 insertions(+), 23 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index b4fb5832c4a..c0c2fa3cc56 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -162,12 +162,27 @@ extern unsigned long guest_base;
extern bool have_guest_base;
extern unsigned long reserved_va;
-#if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
-#define GUEST_ADDR_MAX (~0ul)
+/*
+ * Limit the guest addresses as best we can.
+ *
+ * When not using -R reserved_va, we cannot really limit the guest
+ * to less address space than the host. For 32-bit guests, this
+ * acts as a sanity check that we're not giving the guest an address
+ * that it cannot even represent. For 64-bit guests... the address
+ * might not be what the real kernel would give, but it is at least
+ * representable in the guest.
+ *
+ * TODO: Improve address allocation to avoid this problem, and to
+ * avoid setting bits at the top of guest addresses that might need
+ * to be used for tags.
+ */
+#if MIN(TARGET_VIRT_ADDR_SPACE_BITS, TARGET_ABI_BITS) <= 32
+# define GUEST_ADDR_MAX_ UINT32_MAX
#else
-#define GUEST_ADDR_MAX (reserved_va ? reserved_va - 1 : \
-(1ul << TARGET_VIRT_ADDR_SPACE_BITS) - 1)
+# define GUEST_ADDR_MAX_ ~0ul
#endif
+#define GUEST_ADDR_MAX(reserved_va ? reserved_va - 1 : GUEST_ADDR_MAX_)
+
#else
#include "exec/hwaddr.h"
diff --git a/target/alpha/cpu-param.h b/target/alpha/cpu-param.h
index 692aee27ca9..1153992e42a 100644
--- a/target/alpha/cpu-param.h
+++ b/target/alpha/cpu-param.h
@@ -10,22 +10,11 @@
#define TARGET_LONG_BITS 64
#define TARGET_PAGE_BITS 13
-#ifdef CONFIG_USER_ONLY
-/*
- * ??? The kernel likes to give addresses in high memory. If the host has
- * more virtual address space than the guest, this can lead to impossible
- * allocations. Honor the long-standing assumption that only kernel addrs
- * are negative, but otherwise allow allocations anywhere. This could lead
- * to tricky emulation problems for programs doing tagged addressing, but
- * that's far fewer than encounter the impossible allocation problem.
- */
-#define TARGET_PHYS_ADDR_SPACE_BITS 63
-#define TARGET_VIRT_ADDR_SPACE_BITS 63
-#else
+
/* ??? EV4 has 34 phys addr bits, EV5 has 40, EV6 has 44. */
#define TARGET_PHYS_ADDR_SPACE_BITS 44
#define TARGET_VIRT_ADDR_SPACE_BITS (30 + TARGET_PAGE_BITS)
-#endif
+
#define NB_MMU_MODES 3
#endif
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 9924e66d1f7..e4f703a7e6d 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -173,8 +173,13 @@ struct page_collection {
#define TB_FOR_EACH_JMP(head_tb, tb, n) \
TB_FOR_EACH_TAGGED((head_tb)->jmp_list_head, tb, n, jmp_list_next)
-/* In system mode we want L1_MAP to be based on ram offsets,
- while in user mode we want it to be based on virtual addresses. */
+/*
+ * In system mode we want L1_MAP to be based on ram offsets,
+ * while in user mode we want it to be based on virtual addresses.
+ *
+ * TODO: For user mode, see the caveat re host vs guest virtual
+ * address spaces near GUEST_ADDR_MAX.
+ */
#if !defined(CONFIG_USER_ONLY)
#if HOST_LONG_BITS < TARGET_PHYS_ADDR_SPACE_BITS
# define L1_MAP_ADDR_SPACE_BITS HOST_LONG_BITS
@@ -182,7 +187,7 @@ struct page_collection {
# define L1_MAP_ADDR_SPACE_BITS TARGET_PHYS_ADDR_SPACE_BITS
#endif
#else
-# define L1_MAP_ADDR_SPACE_BITS TARGET_VIRT_ADDR_SPACE_BITS
+# define L1_MAP_ADDR_SPACE_BITS MIN(HOST_LONG_BITS, TARGET_ABI_BITS)
#endif
/* Size of the L2 (and L3, etc) page tables. */
@@ -2497,9 +2502,7 @@ void page_set_flags(target_ulong start, target_ulong end,
int flags)
/* This function should never be called with addresses outside the
guest address space. If this assert fires, it probably indicates
a missing call to h2g_valid. */
-#if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS
-assert(end <= ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
-#endif
+assert(end - 1 <= GUEST_ADDR_MAX);
assert(start < end);
assert_memory_lock();
--
2.20.1
[PATCH v1 07/11] configure: disable PIE for Windows builds
It seems on some compilers the test can pass but still give you broken binaries. [AJB untested - please could windows users test] Fixes: d2cd29e30736 Fixes: https://bugs.launchpad.net/qemu/+bug/1871798 Cc: Bug 1871798 <1871...@bugs.launchpad.net> Cc: James Le Cuirot Signed-off-by: Alex Bennée --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index a207cce82bc..e9c5f630c14 100755 --- a/configure +++ b/configure @@ -807,6 +807,7 @@ MINGW32*) audio_drv_list="" fi supported_os="yes" + pie="no" ;; GNU/kFreeBSD) bsd="yes" -- 2.20.1
[PATCH v1 02/11] exec/cpu-all: Use bool for have_guest_base
From: Richard Henderson
Signed-off-by: Richard Henderson
Signed-off-by: Alex Bennée
---
include/exec/cpu-all.h | 2 +-
bsd-user/main.c| 4 ++--
linux-user/main.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 49384bb66a5..b4fb5832c4a 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -159,7 +159,7 @@ static inline void tswap64s(uint64_t *s)
* This allows the guest address space to be offset to a convenient location.
*/
extern unsigned long guest_base;
-extern int have_guest_base;
+extern bool have_guest_base;
extern unsigned long reserved_va;
#if HOST_LONG_BITS <= TARGET_VIRT_ADDR_SPACE_BITS
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 770c2b267ad..aef5531628a 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -42,7 +42,7 @@
int singlestep;
unsigned long mmap_min_addr;
unsigned long guest_base;
-int have_guest_base;
+bool have_guest_base;
unsigned long reserved_va;
static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
@@ -828,7 +828,7 @@ int main(int argc, char **argv)
}
} else if (!strcmp(r, "B")) {
guest_base = strtol(argv[optind++], NULL, 0);
- have_guest_base = 1;
+ have_guest_base = true;
} else if (!strcmp(r, "drop-ld-preload")) {
(void) envlist_unsetenv(envlist, "LD_PRELOAD");
} else if (!strcmp(r, "bsd")) {
diff --git a/linux-user/main.c b/linux-user/main.c
index 1d20a83d4e8..90ad365b439 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -59,7 +59,7 @@ static const char *cpu_type;
static const char *seed_optarg;
unsigned long mmap_min_addr;
unsigned long guest_base;
-int have_guest_base;
+bool have_guest_base;
/*
* Used to implement backwards-compatibility for the `-strace`, and
@@ -334,7 +334,7 @@ static void handle_arg_cpu(const char *arg)
static void handle_arg_guest_base(const char *arg)
{
guest_base = strtol(arg, NULL, 0);
-have_guest_base = 1;
+have_guest_base = true;
}
static void handle_arg_reserved_va(const char *arg)
--
2.20.1
[PATCH v1 11/11] .travis.yml: Build OSX 10.14 with Xcode 10.0
From: Philippe Mathieu-Daudé Travis recently made a change which generates various warnings such [*]: CC utils.o In file included from cs.c:11: In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/stdio.h:64: /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/_stdio.h:93:16: warning: pointer is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Wnullability-completeness] unsigned char *_base; ^ /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/_stdio.h:93:16: note: insert '_Nullable' if the pointer may be null unsigned char *_base; ^ _Nullable We only aim to support MacOS 10.14 and 10.15. 10.14 comes with Xcode 10.0. These warnings are not emitted with this Xcode version, so switch back to it. [*] https://travis-ci.org/github/qemu/qemu/jobs/673000302#L1387 Reported-by: Daniel P. Berrangé Signed-off-by: Philippe Mathieu-Daudé Signed-off-by: Alex Bennée Message-Id: <20200409190618.7402-1-phi...@redhat.com> --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2fd63eceaac..7c92206ea33 100644 --- a/.travis.yml +++ b/.travis.yml @@ -272,12 +272,12 @@ jobs: # MacOSX builds - cirrus.yml also tests some MacOS builds including latest Xcode -- name: "OSX Xcode 10.3" +- name: "OSX 10.14 (Xcode 10.0)" env: - BASE_CONFIG="--disable-docs --enable-tools" - CONFIG="--target-list=i386-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,x86_64-softmmu" os: osx - osx_image: xcode10.3 + osx_image: xcode10 compiler: clang addons: homebrew: -- 2.20.1
[PATCH v1 05/11] tests/docker: add docs FEATURE flag and use for test-misc
The test-misc docker test fails on a number of images which don't have the prerequisites to build the docs. Use the FEATURES flag so we can skip those tests. As the sphinx test fails to detect whatever feature we need to get hxtool to work we drop them from debian9 so the windows build doesn't attempt to build the docs. Signed-off-by: Alex Bennée --- tests/docker/dockerfiles/debian10.docker | 2 ++ tests/docker/dockerfiles/debian9.docker| 2 -- tests/docker/dockerfiles/fedora.docker | 2 +- tests/docker/dockerfiles/travis.docker | 2 +- tests/docker/dockerfiles/ubuntu.docker | 2 +- tests/docker/dockerfiles/ubuntu1804.docker | 2 +- tests/docker/test-misc | 2 ++ 7 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/docker/dockerfiles/debian10.docker b/tests/docker/dockerfiles/debian10.docker index 2fcdc406e83..0769700a416 100644 --- a/tests/docker/dockerfiles/debian10.docker +++ b/tests/docker/dockerfiles/debian10.docker @@ -34,3 +34,5 @@ RUN apt update && \ python3-sphinx \ texinfo \ $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\ -f2) + +ENV FEATURES docs diff --git a/tests/docker/dockerfiles/debian9.docker b/tests/docker/dockerfiles/debian9.docker index 92edbbf0f48..08cc970feb1 100644 --- a/tests/docker/dockerfiles/debian9.docker +++ b/tests/docker/dockerfiles/debian9.docker @@ -30,6 +30,4 @@ RUN apt update && \ pkg-config \ psmisc \ python3 \ -python3-sphinx \ -texinfo \ $(apt-get -s build-dep qemu | egrep ^Inst | fgrep '[all]' | cut -d\ -f2) diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index 4bd2c953af8..179575ecaaa 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -103,4 +103,4 @@ ENV QEMU_CONFIGURE_OPTS --python=/usr/bin/python3 RUN dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt ENV PATH $PATH:/usr/libexec/python3-sphinx/ -ENV FEATURES mingw clang pyyaml asan +ENV FEATURES mingw clang pyyaml asan docs diff --git a/tests/docker/dockerfiles/travis.docker b/tests/docker/dockerfiles/travis.docker index e8eb48dccfd..591282561bc 100644 --- a/tests/docker/dockerfiles/travis.docker +++ b/tests/docker/dockerfiles/travis.docker @@ -13,5 +13,5 @@ RUN apt-get -y install device-tree-compiler python3 python3-yaml dh-autoreconf g # Travis tools require PhantomJS / Neo4j / Maven accessible # in their PATH (QEMU build won't access them). ENV PATH /usr/local/phantomjs/bin:/usr/local/phantomjs:/usr/local/neo4j-3.2.7/bin:/usr/local/maven-3.5.2/bin:/usr/local/cmake-3.9.2/bin:/usr/local/clang-5.0.0/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin -ENV FEATURES clang pyyaml +ENV FEATURES clang pyyaml docs USER travis diff --git a/tests/docker/dockerfiles/ubuntu.docker b/tests/docker/dockerfiles/ubuntu.docker index b6c7b41..eeb3b22bf20 100644 --- a/tests/docker/dockerfiles/ubuntu.docker +++ b/tests/docker/dockerfiles/ubuntu.docker @@ -68,4 +68,4 @@ ENV PACKAGES flex bison \ RUN apt-get update && \ DEBIAN_FRONTEND=noninteractive apt-get -y install $PACKAGES RUN dpkg -l $PACKAGES | sort > /packages.txt -ENV FEATURES clang pyyaml sdl2 +ENV FEATURES clang pyyaml sdl2 docs diff --git a/tests/docker/dockerfiles/ubuntu1804.docker b/tests/docker/dockerfiles/ubuntu1804.docker index 1efedeef995..f66b06f4cff 100644 --- a/tests/docker/dockerfiles/ubuntu1804.docker +++ b/tests/docker/dockerfiles/ubuntu1804.docker @@ -54,7 +54,7 @@ ENV PACKAGES flex bison \ RUN apt-get update && \ DEBIAN_FRONTEND=noninteractive apt-get -y install $PACKAGES RUN dpkg -l $PACKAGES | sort > /packages.txt -ENV FEATURES clang pyyaml sdl2 +ENV FEATURES clang pyyaml sdl2 docs # https://bugs.launchpad.net/qemu/+bug/1838763 ENV QEMU_CONFIGURE_OPTS --disable-libssh diff --git a/tests/docker/test-misc b/tests/docker/test-misc index d480afedca7..cc94a738dd0 100755 --- a/tests/docker/test-misc +++ b/tests/docker/test-misc @@ -14,6 +14,8 @@ . common.rc +requires docs + cd "$BUILD_DIR" # build everything else but QEMU -- 2.20.1
[PATCH for 5.0-rc3 v1 00/11] more random fixes
Hi, Here are some more random fixes for the tree. In no particular order we have: - A couple of bugs found in the gdbstub GByteArray conversion - A trivial fix to /proc/self/stat output - An attempt to fix broken PIE builds for Windows (please test!) - Some fixes to get "make docker-all-tests" running again - Some travis MacOSX tweaks I've also included the guest base re-factoring patches as it makes it easier for me to soak test the tree with the sanitiser although those actual fixes won't go into 5.0 at this late stage. The following patches need review: - linux-user: fix /proc/self/stat handling - configure: disable PIE for Windows builds - configure: redirect sphinx-build check to config.log - tests/docker: add docs FEATURE flag and use for test-misc - linux-user: completely re-write init_guest_space Alex Bennée (5): linux-user: completely re-write init_guest_space tests/docker: add docs FEATURE flag and use for test-misc configure: redirect sphinx-build check to config.log configure: disable PIE for Windows builds linux-user: fix /proc/self/stat handling Peter Xu (1): gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb Philippe Mathieu-Daudé (2): target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray .travis.yml: Build OSX 10.14 with Xcode 10.0 Richard Henderson (3): exec/cpu-all: Use bool for have_guest_base accel/tcg: Relax va restrictions on 64-bit guests linux-user/ppc: Fix padding in mcontext_t for ppc64 configure | 3 +- include/exec/cpu-all.h | 25 +- linux-user/qemu.h | 31 +- target/alpha/cpu-param.h | 15 +- accel/tcg/translate-all.c | 15 +- bsd-user/main.c| 4 +- linux-user/elfload.c | 503 ++--- linux-user/flatload.c | 6 + linux-user/main.c | 27 +- linux-user/ppc/signal.c| 69 ++- linux-user/syscall.c | 43 +- target/i386/gdbstub.c | 2 +- target/m68k/helper.c | 4 +- .travis.yml| 4 +- tests/docker/dockerfiles/debian10.docker | 2 + tests/docker/dockerfiles/debian9.docker| 2 - tests/docker/dockerfiles/fedora.docker | 2 +- tests/docker/dockerfiles/travis.docker | 2 +- tests/docker/dockerfiles/ubuntu.docker | 2 +- tests/docker/dockerfiles/ubuntu1804.docker | 2 +- tests/docker/test-misc | 2 + 21 files changed, 375 insertions(+), 390 deletions(-) -- 2.20.1
[PATCH v1 08/11] target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray
From: Philippe Mathieu-Daudé
Since a010bdbe719 the gdbstub API takes a GByteArray*. Unfortunately
we forgot to update the gdb_get_reg*() calls. Do it now.
Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Reported-by: Peter Xu
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Laurent Vivier
Reviewed-by: Peter Xu
Signed-off-by: Alex Bennée
Message-Id: <20200409172509.4078-1-phi...@redhat.com>
---
target/m68k/helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 014657c6372..cad40838956 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -109,8 +109,8 @@ static int m68k_fpu_gdb_get_reg(CPUM68KState *env,
GByteArray *mem_buf, int n)
{
if (n < 8) {
int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
-len += gdb_get_reg16(mem_buf + len, 0);
-len += gdb_get_reg64(mem_buf + len, env->fregs[n].l.lower);
+len += gdb_get_reg16(mem_buf, 0);
+len += gdb_get_reg64(mem_buf, env->fregs[n].l.lower);
return len;
}
switch (n) {
--
2.20.1
[PATCH v1 04/11] linux-user/ppc: Fix padding in mcontext_t for ppc64
From: Richard Henderson
The padding that was added in 95cda4c44ee was added to a union,
and so it had no effect. This fixes misalignment errors detected
by clang sanitizers for ppc64 and ppc64le.
In addition, only ppc64 allocates space for VSX registers, so do
not save them for ppc32. The kernel only has references to
CONFIG_SPE in signal_32.c, so do not attempt to save them for ppc64.
Fixes: 95cda4c44ee
Signed-off-by: Richard Henderson
Signed-off-by: Alex Bennée
---
linux-user/ppc/signal.c | 69 +
1 file changed, 29 insertions(+), 40 deletions(-)
diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index ecd99736b7e..20a02c197cb 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -35,12 +35,26 @@ struct target_mcontext {
target_ulong mc_gregs[48];
/* Includes fpscr. */
uint64_t mc_fregs[33];
+
#if defined(TARGET_PPC64)
/* Pointer to the vector regs */
target_ulong v_regs;
+/*
+ * On ppc64, this mcontext structure is naturally *unaligned*,
+ * or rather it is aligned on a 8 bytes boundary but not on
+ * a 16 byte boundary. This pad fixes it up. This is why we
+ * cannot use ppc_avr_t, which would force alignment. This is
+ * also why the vector regs are referenced in the ABI by the
+ * v_regs pointer above so any amount of padding can be added here.
+ */
+target_ulong pad;
+/* VSCR and VRSAVE are saved separately. Also reserve space for VSX. */
+struct {
+uint64_t altivec[34 + 16][2];
+} mc_vregs;
#else
target_ulong mc_pad[2];
-#endif
+
/* We need to handle Altivec and SPE at the same time, which no
kernel needs to do. Fortunately, the kernel defines this bit to
be Altivec-register-large all the time, rather than trying to
@@ -48,32 +62,14 @@ struct target_mcontext {
union {
/* SPE vector registers. One extra for SPEFSCR. */
uint32_t spe[33];
-/* Altivec vector registers. The packing of VSCR and VRSAVE
- varies depending on whether we're PPC64 or not: PPC64 splits
- them apart; PPC32 stuffs them together.
- We also need to account for the VSX registers on PPC64
-*/
-#if defined(TARGET_PPC64)
-#define QEMU_NVRREG (34 + 16)
-/* On ppc64, this mcontext structure is naturally *unaligned*,
- * or rather it is aligned on a 8 bytes boundary but not on
- * a 16 bytes one. This pad fixes it up. This is also why the
- * vector regs are referenced by the v_regs pointer above so
- * any amount of padding can be added here
- */
-target_ulong pad;
-#else
-/* On ppc32, we are already aligned to 16 bytes */
-#define QEMU_NVRREG 33
-#endif
-/* We cannot use ppc_avr_t here as we do *not* want the implied
- * 16-bytes alignment that would result from it. This would have
- * the effect of making the whole struct target_mcontext aligned
- * which breaks the layout of struct target_ucontext on ppc64.
+/*
+ * Altivec vector registers. One extra for VRSAVE.
+ * On ppc32, we are already aligned to 16 bytes. We could
+ * use ppc_avr_t, but choose to share the same type as ppc64.
*/
-uint64_t altivec[QEMU_NVRREG][2];
-#undef QEMU_NVRREG
+uint64_t altivec[33][2];
} mc_vregs;
+#endif
};
/* See arch/powerpc/include/asm/sigcontext.h. */
@@ -278,6 +274,7 @@ static void save_user_regs(CPUPPCState *env, struct
target_mcontext *frame)
__put_user((uint32_t)env->spr[SPR_VRSAVE], vrsave);
}
+#if defined(TARGET_PPC64)
/* Save VSX second halves */
if (env->insns_flags2 & PPC2_VSX) {
uint64_t *vsregs = (uint64_t *)&frame->mc_vregs.altivec[34];
@@ -286,6 +283,7 @@ static void save_user_regs(CPUPPCState *env, struct
target_mcontext *frame)
__put_user(*vsrl, &vsregs[i]);
}
}
+#endif
/* Save floating point registers. */
if (env->insns_flags & PPC_FLOAT) {
@@ -296,22 +294,18 @@ static void save_user_regs(CPUPPCState *env, struct
target_mcontext *frame)
__put_user((uint64_t) env->fpscr, &frame->mc_fregs[32]);
}
+#if !defined(TARGET_PPC64)
/* Save SPE registers. The kernel only saves the high half. */
if (env->insns_flags & PPC_SPE) {
-#if defined(TARGET_PPC64)
-for (i = 0; i < ARRAY_SIZE(env->gpr); i++) {
-__put_user(env->gpr[i] >> 32, &frame->mc_vregs.spe[i]);
-}
-#else
for (i = 0; i < ARRAY_SIZE(env->gprh); i++) {
__put_user(env->gprh[i], &frame->mc_vregs.spe[i]);
}
-#endif
/* Set MSR_SPE in the saved MSR value to indicate that
frame->mc_vregs contains valid data. */
msr |= MSR_SPE;
__put_user(env->spe_fscr, &frame->mc_vregs.spe[32]);
}
+#endif
/* Store MSR. */
__put_user(msr, &frame->mc_gregs[TARGET_PT_MSR])
[PATCH v1 01/11] linux-user: completely re-write init_guest_space
First we ensure all guest space initialisation logic comes through
probe_guest_base once we understand the nature of the binary we are
loading. The convoluted init_guest_space routine is removed and
replaced with a number of pgb_* helpers which are called depending on
what requirements we have when loading the binary.
We first try to do what is requested by the host. Failing that we try
and satisfy the guest requested base address. If all those options
fail we fall back to finding a space in the memory map using our
recently written read_self_maps() helper.
There are some additional complications we try and take into account
when looking for holes in the address space. We try not to go directly
after the system brk() space so there is space for a little growth. We
also don't want to have to use negative offsets which would result in
slightly less efficient code on x86 when it's unable to use the
segment offset register.
Less mind-binding gotos and hopefully clearer logic throughout.
Signed-off-by: Alex Bennée
---
v3
- include rth updates that
- split probe_guest_base into multiple functions
- more heuristics on gap finding
---
linux-user/qemu.h | 31 ++-
linux-user/elfload.c | 503 +-
linux-user/flatload.c | 6 +
linux-user/main.c | 23 +-
4 files changed, 277 insertions(+), 286 deletions(-)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 792c74290f8..e1febb88cf5 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -219,18 +219,27 @@ void init_qemu_uname_release(void);
void fork_start(void);
void fork_end(int child);
-/* Creates the initial guest address space in the host memory space using
- * the given host start address hint and size. The guest_start parameter
- * specifies the start address of the guest space. guest_base will be the
- * difference between the host start address computed by this function and
- * guest_start. If fixed is specified, then the mapped address space must
- * start at host_start. The real start address of the mapped memory space is
- * returned or -1 if there was an error.
+/**
+ * probe_guest_base:
+ * @image_name: the executable being loaded
+ * @loaddr: the lowest fixed address in the executable
+ * @hiaddr: the highest fixed address in the executable
+ *
+ * Creates the initial guest address space in the host memory space.
+ *
+ * If @loaddr == 0, then no address in the executable is fixed,
+ * i.e. it is fully relocatable. In that case @hiaddr is the size
+ * of the executable.
+ *
+ * This function will not return if a valid value for guest_base
+ * cannot be chosen. On return, the executable loader can expect
+ *
+ *target_mmap(loaddr, hiaddr - loaddr, ...)
+ *
+ * to succeed.
*/
-unsigned long init_guest_space(unsigned long host_start,
- unsigned long host_size,
- unsigned long guest_start,
- bool fixed);
+void probe_guest_base(const char *image_name,
+ abi_ulong loaddr, abi_ulong hiaddr);
#include "qemu/log.h"
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 619c054cc48..01a9323a637 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -11,6 +11,7 @@
#include "qemu/queue.h"
#include "qemu/guest-random.h"
#include "qemu/units.h"
+#include "qemu/selfmap.h"
#ifdef _ARCH_PPC64
#undef ARCH_DLINFO
@@ -382,68 +383,30 @@ enum {
/* The commpage only exists for 32 bit kernels */
-/* Return 1 if the proposed guest space is suitable for the guest.
- * Return 0 if the proposed guest space isn't suitable, but another
- * address space should be tried.
- * Return -1 if there is no way the proposed guest space can be
- * valid regardless of the base.
- * The guest code may leave a page mapped and populate it if the
- * address is suitable.
- */
-static int init_guest_commpage(unsigned long guest_base,
- unsigned long guest_size)
-{
-unsigned long real_start, test_page_addr;
-
-/* We need to check that we can force a fault on access to the
- * commpage at 0x0fxx
- */
-test_page_addr = guest_base + (0x0f00 & qemu_host_page_mask);
-
-/* If the commpage lies within the already allocated guest space,
- * then there is no way we can allocate it.
- *
- * You may be thinking that that this check is redundant because
- * we already validated the guest size against MAX_RESERVED_VA;
- * but if qemu_host_page_mask is unusually large, then
- * test_page_addr may be lower.
- */
-if (test_page_addr >= guest_base
-&& test_page_addr < (guest_base + guest_size)) {
-return -1;
-}
+#define ARM_COMMPAGE (intptr_t)0x0f00u
-/* Note it needs to be writeable to let us initialise it */
-real_start = (unsigned long)
- mmap((void *)test_page_addr, qemu_host_page_size,
- PROT_READ | PROT_WRITE,
-
Re: [PATCH-for-5.0] .travis.yml: Build OSX 10.14 with Xcode 10.0
Philippe Mathieu-Daudé writes: > Travis recently made a change which generates various warnings > such [*]: > > CC utils.o > In file included from cs.c:11: > In file included from > /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/stdio.h:64: > > /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/_stdio.h:93:16: > warning: pointer is missing a nullability type specifier (_Nonnull, > _Nullable, or _Null_unspecified) [-Wnullability-completeness] > unsigned char *_base; > ^ > > /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/_stdio.h:93:16: > note: insert '_Nullable' if the pointer may be null > unsigned char *_base; > ^ > _Nullable > > We only aim to support MacOS 10.14 and 10.15. 10.14 comes with > Xcode 10.0. These warnings are not emitted with this Xcode version, > so switch back to it. > > [*] https://travis-ci.org/github/qemu/qemu/jobs/673000302#L1387 > > Reported-by: Daniel P. Berrangé > Signed-off-by: Philippe Mathieu-Daudé Queued to for-5.0/more-random-fixes, thanks. > --- > .travis.yml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/.travis.yml b/.travis.yml > index 2fd63eceaa..7c92206ea3 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -272,12 +272,12 @@ jobs: > > # MacOSX builds - cirrus.yml also tests some MacOS builds including > latest Xcode > > -- name: "OSX Xcode 10.3" > +- name: "OSX 10.14 (Xcode 10.0)" >env: > - BASE_CONFIG="--disable-docs --enable-tools" > - > CONFIG="--target-list=i386-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,x86_64-softmmu" >os: osx > - osx_image: xcode10.3 > + osx_image: xcode10 >compiler: clang >addons: > homebrew: -- Alex Bennée
Re: [PATCH 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
Peter Xu writes:
> We should only pass in gdb_get_reg16() with the GByteArray* object
> itself, no need to shift. Without this patch, gdb remote attach will
> crash QEMU.
>
> Cc: Alex Bennée
> Cc: Philippe Mathieu-Daudé
> Signed-off-by: Peter Xu
Queued to for-5.0/more-random-fixes, thanks.
> ---
> target/i386/gdbstub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
> index f3d23b614e..b98a99500a 100644
> --- a/target/i386/gdbstub.c
> +++ b/target/i386/gdbstub.c
> @@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray
> *mem_buf, int n)
> } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
> floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
> int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
> -len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
> +len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
> return len;
> } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
> n -= IDX_XMM_REGS;
--
Alex Bennée
Re: [PATCH-for-5.0] target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray
Philippe Mathieu-Daudé writes:
> Since a010bdbe719 the gdbstub API takes a GByteArray*.
> Unfortunately we forgot to update the gdb_get_reg*()
> calls. Do it now.
>
Queued to for-5.0/more-random-fixes, thanks.
> Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
> Reported-by: Peter Xu
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Based-on: <20200409164954.36902-1-pet...@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> target/m68k/helper.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/m68k/helper.c b/target/m68k/helper.c
> index 014657c637..cad4083895 100644
> --- a/target/m68k/helper.c
> +++ b/target/m68k/helper.c
> @@ -109,8 +109,8 @@ static int m68k_fpu_gdb_get_reg(CPUM68KState *env,
> GByteArray *mem_buf, int n)
> {
> if (n < 8) {
> int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
> -len += gdb_get_reg16(mem_buf + len, 0);
> -len += gdb_get_reg64(mem_buf + len, env->fregs[n].l.lower);
> +len += gdb_get_reg16(mem_buf, 0);
> +len += gdb_get_reg64(mem_buf, env->fregs[n].l.lower);
> return len;
> }
> switch (n) {
--
Alex Bennée
[Bug 1821595] Re: Failed to emulate MMIO access with EmulatorReturnStatus: 2
Sorry, not quite following your comment - but it's likely me :-(. I assume you mean QEMU - but build against what upstream library? I ask because for QEMU to build, only the three header files from Windows are copied over, and then it calls / uses the dll's (libraries) in Windows itself ... right? FYI, I'm running Windows 19041.172, to try to make sure I do have the latest related to whpx. Thanks! -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1821595 Title: Failed to emulate MMIO access with EmulatorReturnStatus: 2 Status in QEMU: New Bug description: I have compiled qemu with enable-whpx parameter for Hyper-V Platform API in Mingw64 . When I tried run with Windows 7 iso file I have faced issue with the following problem: qemu-system-x86_64.exe: WHPX: Failed to emulate MMIO access with EmulatorReturnStatus: 2 qemu-system-x86_64.exe: WHPX: Failed to exec a virtual processor configuration directives: ../configure --target-list=x86_64-softmmu,i386-softmmu --enable-lzo\ --enable-bzip2 --enable-tools --enable-sdl --enable-gtk --enable-hax\ --enable-vdi --enable-qcow1 --enable-whpx --disable-capstone\ --disable-werror --disable-stack-protector --prefix="../../QEMU-bin" Qemu command line: qemu-system-x86_64.exe -m 1024 -cdrom "C:\Users\vmcs\Documents\en_windows_7_home_premium_with_sp1_x86_dvd_u_676701.iso" -display sdl -machine q35 -accel whpx To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1821595/+subscriptions
RE: [RFC PATCH v2 20/67] Hexagon instruction utility functions
> -Original Message-
> From: bcain=codeaurora@mg.codeaurora.org
> On Behalf Of Brian Cain
> Sent: Thursday, April 9, 2020 1:53 PM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: riku.voi...@iki.fi; richard.hender...@linaro.org; laur...@vivier.eu;
> phi...@redhat.com; aleksandar.m.m...@gmail.com
> Subject: RE: [RFC PATCH v2 20/67] Hexagon instruction utility functions
>
> > -Original Message-
> > From: Qemu-devel > bounces+bcain=codeaurora@nongnu.org> On Behalf Of Taylor
> Simpson
> > Sent: Friday, February 28, 2020 10:43 AM
> > To: qemu-devel@nongnu.org
> > Cc: riku.voi...@iki.fi; richard.hender...@linaro.org; laur...@vivier.eu;
> > Taylor Simpson ; phi...@redhat.com;
> > aleksandar.m.m...@gmail.com
> > Subject: [RFC PATCH v2 20/67] Hexagon instruction utility functions
> ...
> > +int arch_sf_invsqrt_common(size4s_t *Rs, size4s_t *Rd, int *adjust)
> > +{
> ...
> > +} else if (r_class == FP_INFINITE) {
> > +/* EJP: or put Inf in num fixup? */
> > +RsV = fSFINFVAL(-1);
> > +RdV = fSFINFVAL(-1);
> > +} else if (r_class == FP_ZERO) {
> > +/* EJP: or put zero in num fixup? */
> > +RsV = RsV;
> > +RdV = fSFONEVAL(0);
> ...
>
> This "RsV = RsV" looks like a logic error? Presumably it's safe to remove --
> unless there's some other field that should get initialized here? PeV maybe?
Probably a copy/paste error. I will remove it.
Thanks,
Taylor
Re: hotplug issue of vhost-user-blk
On Wed, Apr 08, 2020 at 10:25:42AM +0800, Li Feng wrote:
> Hi all,
>
> Hotplug of vhost-user-blk doesn't not work in qemu master branch and
> all previous version.
>
> The action I insert a vhost-user-blk disk is:
> (qemu) chardev-add socket,id=spdk_vhost_blk2,path=/vhost-blk.0,reconnect=1
> (qemu) device_add
> vhost-user-blk-pci,chardev=spdk_vhost_blk2,id=spdk_vhost_blk2,num-queues=4
>
> Until here, it's well.
>
> Then I unplug it from qemu:
> (qemu) device_del spdk_vhost_blk2
> (qemu) chardev-remove spdk_vhost_blk2
> Error: Chardev 'spdk_vhost_blk2' is busy
Pan Nengyuan (cc'd) fixed unplug recently so I expect it
worked for him after the patch ...
> The related code is here:
> qmp_chardev_remove
> -> qemu_chr_is_busy
> -> object_unparent(OBJECT(chr));
>
> 330 static bool qemu_chr_is_busy(Chardev *s)
> 331 {
> 332 if (CHARDEV_IS_MUX(s)) {
> 333 MuxChardev *d = MUX_CHARDEV(s);
> 334 return d->mux_cnt >= 0;
> 335 } else {
> 336 return s->be != NULL;
> 337 }
> 338 }
>
> My question is:
> 1. s->be is set to NULL when qemu_chr_fe_deinit is called.
> However, the qmp_chardev_remove is blocked at qemu_chr_is_busy check,
> then the object_unparent will not be called.
> 2. Is there a path that device_del will trigger the s->be that been set to
> NULL?
>
> How should I fix this issue?
> I have tested that comment the qemu_chr_is_busy works well.
>
> Thanks in advance.
>
> Feng Li
>
> --
> The SmartX email address is only for business purpose. Any sent message
> that is not related to the business is not authorized or permitted by
> SmartX.
> 本邮箱为北京志凌海纳科技有限公司（SmartX）工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.
>
[Bug 1821595] Re: Failed to emulate MMIO access with EmulatorReturnStatus: 2
I don't think QEMU can do anything to fix this - you need to build against a new library. ** Tags added: upstream-bug -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1821595 Title: Failed to emulate MMIO access with EmulatorReturnStatus: 2 Status in QEMU: New Bug description: I have compiled qemu with enable-whpx parameter for Hyper-V Platform API in Mingw64 . When I tried run with Windows 7 iso file I have faced issue with the following problem: qemu-system-x86_64.exe: WHPX: Failed to emulate MMIO access with EmulatorReturnStatus: 2 qemu-system-x86_64.exe: WHPX: Failed to exec a virtual processor configuration directives: ../configure --target-list=x86_64-softmmu,i386-softmmu --enable-lzo\ --enable-bzip2 --enable-tools --enable-sdl --enable-gtk --enable-hax\ --enable-vdi --enable-qcow1 --enable-whpx --disable-capstone\ --disable-werror --disable-stack-protector --prefix="../../QEMU-bin" Qemu command line: qemu-system-x86_64.exe -m 1024 -cdrom "C:\Users\vmcs\Documents\en_windows_7_home_premium_with_sp1_x86_dvd_u_676701.iso" -display sdl -machine q35 -accel whpx To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1821595/+subscriptions
Re: linux-user: keep the name-ending parenthesis in /proc/self/stat
Brice Goglin writes:
> Le 09/04/2020 à 17:27, Alex Bennée a écrit :
>> Brice Goglin writes:
>>
>>> When the program name is very long, qemu-user may truncate it in
>>> /proc/self/stat. However the truncation must keep the ending ") "
>>> to conform to the proc manpage which says:
>>> (2) comm %s
>>>The filename of the executable, in parentheses. This
>>>is visible whether or not the executable is swapped
>>>out.
Huh testing on my box here it seems to truncate a lot earlier than that:
20:54:41 [alex@zen:~/l/q/b/all] sanitiser/fixes-for-5.1|●1✚1…(+1/-1) +
./cat_with9_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890___4567890
/proc/self/stat
23132 (cat_with9_12345) R 15690 23132 15676 34827 23132 4194304 87 0 0 0 0 0 0
0 20 0 1 0 133272440 6172672 188 18446744073709551615 94698916007936
94698916032905 140729243846896 0 0 0 0 0 0 0 0 0 17 2 0 0 0 0 0 94698916052048
94698916053600 94698933542912 140729243849857 140729243850006 140729243850006
140729243852659 0
20:55:21 [alex@zen:~/l/q/b/all] sanitiser/fixes-for-5.1|●1✚1…(+1/-1) 126 +
./x86_64-linux-user/qemu-x86_64
./cat_with9_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890___4567890
/proc/s
elf/stat
23519
(./cat_with9_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890___40
0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
0 0 0 0 0 274903122400 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
>>>
>>> To reproduce:
>>> $ ln -s /bin/cat
>>> $ qemu-x86_64 ./ /proc/self/stat
>>>
>>> Before the patch, you get:
>>> 1134631 (0 0 0 0 0 0 0 0 ...
>>> After the patch:
>>> 1134631 () 0 0 0 0 0 0 0 0 ...
>>>
>>> This fixes an issue with hwloc failing to parse /proc/self/stat
>>> when Ludovic Courtes was testing it in Guix over qemu-aarch64.
>>>
>>> Signed-off-by: Philippe_Mathieu-Daudé
>>> Signed-off-by: Brice Goglin
>>>
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -7305,7 +7305,11 @@ static int open_self_stat(void *cpu_env, int fd)
>>> snprintf(buf, sizeof(buf), "%"PRId64 " ", val);
>>>} else if (i == 1) {
>>> /* app name */
>>> -snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->argv[0]);
>>> +char *ptr = buf;
>>> +
>>> +*ptr++ = '(';
>>> +ptr = stpncpy(ptr, ts->bprm->argv[0], sizeof(buf) - 3);
>>> +strcpy(ptr, ") ");
>> why not just use a format string:
>>
>> snprintf(buf, sizeof(buf), "(%.125s) ", ts->bprm->argv[0]);
>>
>
> Go ahead and apply what you want (maybe 124 instead of 125 because of
> the ending \0).
>
> My commit message above explains how to test things very quickly.
>
> I don't use qemu-user or Guix myself, and I can't spend time
> debugging/testing this further.
>
> Thank you
>
> Brice
--
Alex Bennée
Re: [PATCH for-5.1 8/8] qemu-option: Move is_valid_option_list() to qemu-img.c and rewrite
On 4/9/20 10:30 AM, Markus Armbruster wrote:
is_valid_option_list()'s purpose is ensuring qemu-img.c's can safely
join multiple parameter strings separated by ',' like this:
g_strdup_printf("%s,%s", params1, params2);
How it does that is anything but obvious. A close reading of the code
reveals that it fails exactly when its argument starts with ',' or
ends with an odd number of ','. Makes sense, actually, because when
the argument starts with ',', a separating ',' preceding it would get
escaped, and when it ends with an odd number of ',', a separating ','
following it would get escaped.
Move it to qemu-img.c and rewrite it the obvious way.
Signed-off-by: Markus Armbruster
---
include/qemu/option.h | 1 -
qemu-img.c| 26 ++
util/qemu-option.c| 22 --
3 files changed, 26 insertions(+), 23 deletions(-)
+++ b/qemu-img.c
@@ -223,6 +223,32 @@ static bool qemu_img_object_print_help(const char *type,
QemuOpts *opts)
return true;
}
+/*
+ * Is @optarg safe for accumulate_options()?
+ * It is when multiple of them can be joined together separated by ','.
+ * To make that work, @optarg must not start with ',' (or else a
+ * separating ',' preceding it gets escaped), and it must not end with
+ * an odd number of ',' (or else a separating ',' following it gets
+ * escaped).
+ */
+static bool is_valid_option_list(const char *optarg)
+{
+size_t len = strlen(optarg);
+size_t i;
+
+if (optarg[0] == ',') {
+return false;
+}
+
+for (i = len; i > 0 && optarg[i - 1] == ','; i--) {
+}
+if ((len - i) % 2) {
+return false;
+}
+
+return true;
Okay, that's easy to read. Note that is_valid_option_list("") returns
true. But proving it replaces the obtuse mess is harder...
+++ b/util/qemu-option.c
@@ -165,28 +165,6 @@ void parse_option_size(const char *name, const char *value,
*ret = size;
}
-bool is_valid_option_list(const char *p)
-{
-char *value = NULL;
-bool result = false;
-
-while (*p) {
-p = get_opt_value(p, &value);
Refreshing myself on what get_opt_value() does:
const char *get_opt_value(const char *p, char **value)
{
size_t capacity = 0, length;
const char *offset;
*value = NULL;
start with p pointing to the tail of a string from which to extract a value,
while (1) {
offset = qemu_strchrnul(p, ',');
find the potential end of the value: either the first comma (not yet
sure if it is ',,' or the end of this value), or the end of the string,
length = offset - p;
if (*offset != '\0' && *(offset + 1) == ',') {
length++;
}
if we found a comma and it was doubled, we are going to unescape the comma,
*value = g_renew(char, *value, capacity + length + 1);
strncpy(*value + capacity, p, length);
copy bytes into the tail of value,
(*value)[capacity + length] = '\0';
capacity += length;
if (*offset == '\0' ||
*(offset + 1) != ',') {
break;
}
if we hit the end of the string or the ',' before the next option, we
are done,
p += (offset - p) + 2;
otherwise we unescaped a ',,' and continue appending to the current value.
}
return offset;
}
and the resulting return value is a substring of p: either the NUL byte
ending p, or the last ',' in the first odd run of commas (the next byte
after the return is then NUL if the parser would next see an empty
option name, or non-NUL if the parser would start processing the next
option name).
Some interesting cases:
get_opt_value("", &value) returns "" with value set to ""
get_opt_value(",", &value) returns "," with value set to ""
get_opt_value(",,", &value) returns "" with value set to ","
get_opt_value(",,,", &value) returns "," with value set to ","
get_opt_value(",,a", &value) returns "" with value set to ",a"
get_opt_value("a,,", &value) returns "" with value set to "a,"
get_opt_value("a,b", &value) returns ",b" with value set to "a"
get_opt_value("a,,b", &value) returns "" with value set to "a,b"
With that detour out of the way:
-if ((*p && !*++p) ||
If *p, then we know '*p' == ','; checking !*++p moves past the comma and
determines if we have the empty string as the next potential option name
(if so, we ended in an odd number of commas) or anything else (if so,
repeat the loop, just past the comma). Oddly, this means that
is_valid_option_list("a,,,b") returns true (first pass on "a,,,b"
returned ",b", second pass on "b" returned ""). But I agree that !*++p
only fires on the final iteration through our loop of get_opt_value()
calls, matching your rewrite to check for an odd number of trailing
commas (regardless of even or odd pairing of commas in the middle).
-(!*value || *value == ',')) {
Focusing on "*value == ','", on the first iteration, *value can be
comma; on later iterations, w
[Bug 1871798] Re: Fails to start on Windows host without explicit --disable-pie
I didn't know whether PIE is generally supported on Windows or not. It was possible that Gentoo is just inadvertently disabling support for it. It did stem from a bug report though and reading around, others elsewhere have reported that PIE on Windows doesn't work. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1871798 Title: Fails to start on Windows host without explicit --disable-pie Status in QEMU: Incomplete Bug description: Since commit d2cd29e30736afd4a1e8cac3cf4da360bbc65978, which removed the x86 conditional around PIE, QEMU completely fails to start on a Windows host unless --disable-pie is explicitly given at build time. Even just requesting the help text doesn't work. To make testing easier, this can be replicated with Wine. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1871798/+subscriptions
Re: [PATCH 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
On 4/9/20 8:01 PM, Peter Xu wrote:
Hi, Phil,
On Thu, Apr 09, 2020 at 07:21:04PM +0200, Philippe Mathieu-Daudé wrote:
On 4/9/20 6:49 PM, Peter Xu wrote:
We should only pass in gdb_get_reg16() with the GByteArray* object
itself, no need to shift. Without this patch, gdb remote attach will
crash QEMU.
You are correct.
Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Oh I forgot to paste the fix line. However, is it b7b8756a9c
("target/i386: use gdb_get_reg helpers", 2020-03-17) instead?
b7b8756a9c is correct, at that time the codebase was using the correct
API. the next commit updated the API but missed to update the lines you
are fixed. So I think "fixes a010bdbe719" is correct.
Reviewed-by: Philippe Mathieu-Daudé
Thanks!
Same problem in m68k_fpu_gdb_get_reg().
TODO for 5.1, rename mem_buf -> array.
[PATCH for 5.0] qemu-iotests: allow qcow2 external discarded clusters to contain stale data
Test 244 checks the expected behavior of qcow2 external data files with respect to zero and discarded clusters. Filesystems however are free to ignore discard requests, and this seems to be the case for overlayfs. Relax the tests to skip checks on the external data file for discarded areas, which implies not using qemu-img compare in the data_file_raw=on case. This fixes docker tests on RHEL8. Cc: Kevin Wolf Cc: qemu-bl...@nongnu.org Signed-off-by: Paolo Bonzini --- tests/qemu-iotests/244 | 10 -- tests/qemu-iotests/244.out | 9 ++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/qemu-iotests/244 b/tests/qemu-iotests/244 index 2ec1815e6f..efe3c0428b 100755 --- a/tests/qemu-iotests/244 +++ b/tests/qemu-iotests/244 @@ -143,7 +143,6 @@ $QEMU_IO -c 'read -P 0 0 1M' \ echo $QEMU_IO -c 'read -P 0 0 1M' \ -c 'read -P 0x11 1M 1M' \ - -c 'read -P 0 2M 2M' \ -c 'read -P 0x11 4M 1M' \ -c 'read -P 0 5M 1M' \ -f raw "$TEST_IMG.data" | @@ -180,8 +179,15 @@ $QEMU_IO -c 'read -P 0 0 1M' \ -f $IMGFMT "$TEST_IMG" | _filter_qemu_io +# Discarded clusters are only marked as such in the qcow2 metadata, but +# they can contain stale data in the external data file. Instead, zero +# clusters must be zeroed in the external data file too. echo -$QEMU_IMG compare "$TEST_IMG" "$TEST_IMG.data" +$QEMU_IO -c 'read -P 0 0 1M' \ + -c 'read -P 0x11 1M 1M' \ + -c 'read -P 0 3M 3M' \ + -f raw "$TEST_IMG".data | + _filter_qemu_io echo -n "qcow2 file size after I/O: " du -b $TEST_IMG | cut -f1 diff --git a/tests/qemu-iotests/244.out b/tests/qemu-iotests/244.out index e6f4dc7993..4afa32026d 100644 --- a/tests/qemu-iotests/244.out +++ b/tests/qemu-iotests/244.out @@ -74,8 +74,6 @@ read 1048576/1048576 bytes at offset 0 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 1048576/1048576 bytes at offset 1048576 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -read 2097152/2097152 bytes at offset 2097152 -2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 1048576/1048576 bytes at offset 4194304 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) read 1048576/1048576 bytes at offset 5242880 @@ -108,7 +106,12 @@ read 1048576/1048576 bytes at offset 1048576 read 4194304/4194304 bytes at offset 2097152 4 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -Images are identical. +read 1048576/1048576 bytes at offset 0 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 1048576/1048576 bytes at offset 1048576 +1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 3145728/3145728 bytes at offset 3145728 +3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) qcow2 file size after I/O: 327680 === bdrv_co_block_status test for file and offset=0 === -- 2.18.2
[PATCH-for-5.0] .travis.yml: Build OSX 10.14 with Xcode 10.0
Travis recently made a change which generates various warnings such [*]: CC utils.o In file included from cs.c:11: In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/stdio.h:64: /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/_stdio.h:93:16: warning: pointer is missing a nullability type specifier (_Nonnull, _Nullable, or _Null_unspecified) [-Wnullability-completeness] unsigned char *_base; ^ /Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/_stdio.h:93:16: note: insert '_Nullable' if the pointer may be null unsigned char *_base; ^ _Nullable We only aim to support MacOS 10.14 and 10.15. 10.14 comes with Xcode 10.0. These warnings are not emitted with this Xcode version, so switch back to it. [*] https://travis-ci.org/github/qemu/qemu/jobs/673000302#L1387 Reported-by: Daniel P. Berrangé Signed-off-by: Philippe Mathieu-Daudé --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2fd63eceaa..7c92206ea3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -272,12 +272,12 @@ jobs: # MacOSX builds - cirrus.yml also tests some MacOS builds including latest Xcode -- name: "OSX Xcode 10.3" +- name: "OSX 10.14 (Xcode 10.0)" env: - BASE_CONFIG="--disable-docs --enable-tools" - CONFIG="--target-list=i386-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,x86_64-softmmu" os: osx - osx_image: xcode10.3 + osx_image: xcode10 compiler: clang addons: homebrew: -- 2.21.1
Re: [PATCH v6 21/36] multi-process: PCI BAR read/write handling for proxy & remote endpoints
* elena.ufimts...@oracle.com (elena.ufimts...@oracle.com) wrote:
> From: Jagannathan Raman
>
> Proxy device object implements handler for PCI BAR writes and reads.
> The handler uses BAR_WRITE/BAR_READ message to communicate to the
> remote process with the BAR address and value to be written/read.
> The remote process implements handler for BAR_WRITE/BAR_READ
> message.
>
> Signed-off-by: Jagannathan Raman
> Signed-off-by: Elena Ufimtseva
> Signed-off-by: John G Johnson
Again please see my comments on v5
> ---
> hw/proxy/qemu-proxy.c | 64 ++
> include/hw/proxy/qemu-proxy.h | 20 +-
> include/io/mpqemu-link.h | 12 ++
> io/mpqemu-link.c | 6 +++
> remote/remote-main.c | 73 +++
> 5 files changed, 173 insertions(+), 2 deletions(-)
>
> diff --git a/hw/proxy/qemu-proxy.c b/hw/proxy/qemu-proxy.c
> index 87cf39c672..7fd0a312a5 100644
> --- a/hw/proxy/qemu-proxy.c
> +++ b/hw/proxy/qemu-proxy.c
> @@ -169,3 +169,67 @@ static void pci_proxy_dev_register_types(void)
>
> type_init(pci_proxy_dev_register_types)
>
> +static void send_bar_access_msg(PCIProxyDev *dev, MemoryRegion *mr,
> +bool write, hwaddr addr, uint64_t *val,
> +unsigned size, bool memory)
> +{
> +MPQemuLinkState *mpqemu_link = dev->mpqemu_link;
> +MPQemuMsg msg;
> +int wait;
> +
> +memset(&msg, 0, sizeof(MPQemuMsg));
> +
> +msg.bytestream = 0;
> +msg.size = sizeof(msg.data1);
> +msg.data1.bar_access.addr = mr->addr + addr;
> +msg.data1.bar_access.size = size;
> +msg.data1.bar_access.memory = memory;
> +
> +if (write) {
> +msg.cmd = BAR_WRITE;
> +msg.data1.bar_access.val = *val;
> +} else {
> +wait = GET_REMOTE_WAIT;
> +
> +msg.cmd = BAR_READ;
> +msg.num_fds = 1;
> +msg.fds[0] = wait;
> +}
> +
> +mpqemu_msg_send(&msg, mpqemu_link->dev);
> +
> +if (!write) {
> +*val = wait_for_remote(wait);
> +PUT_REMOTE_WAIT(wait);
> +}
> +}
> +
> +void proxy_default_bar_write(void *opaque, hwaddr addr, uint64_t val,
> + unsigned size)
> +{
> +ProxyMemoryRegion *pmr = opaque;
> +
> +send_bar_access_msg(pmr->dev, &pmr->mr, true, addr, &val, size,
> +pmr->memory);
> +}
> +
> +uint64_t proxy_default_bar_read(void *opaque, hwaddr addr, unsigned size)
> +{
> +ProxyMemoryRegion *pmr = opaque;
> +uint64_t val;
> +
> +send_bar_access_msg(pmr->dev, &pmr->mr, false, addr, &val, size,
> +pmr->memory);
> +
> + return val;
> +}
> +
> +const MemoryRegionOps proxy_default_ops = {
> +.read = proxy_default_bar_read,
> +.write = proxy_default_bar_write,
> +.endianness = DEVICE_NATIVE_ENDIAN,
> +.impl = {
> +.min_access_size = 1,
> +.max_access_size = 1,
> +},
> +};
> diff --git a/include/hw/proxy/qemu-proxy.h b/include/hw/proxy/qemu-proxy.h
> index d7eaf26f29..9e4127eccb 100644
> --- a/include/hw/proxy/qemu-proxy.h
> +++ b/include/hw/proxy/qemu-proxy.h
> @@ -26,14 +26,25 @@
> #define PCI_PROXY_DEV_GET_CLASS(obj) \
> OBJECT_GET_CLASS(PCIProxyDevClass, (obj), TYPE_PCI_PROXY_DEV)
>
> -typedef struct PCIProxyDev {
> +typedef struct PCIProxyDev PCIProxyDev;
> +
> +typedef struct ProxyMemoryRegion {
> +PCIProxyDev *dev;
> +MemoryRegion mr;
> +bool memory;
> +bool present;
> +uint8_t type;
> +} ProxyMemoryRegion;
> +
> +struct PCIProxyDev {
> PCIDevice parent_dev;
>
> MPQemuLinkState *mpqemu_link;
>
> int socket;
>
> -} PCIProxyDev;
> +ProxyMemoryRegion region[PCI_NUM_REGIONS];
> +};
>
> typedef struct PCIProxyDevClass {
> PCIDeviceClass parent_class;
> @@ -43,4 +54,9 @@ typedef struct PCIProxyDevClass {
> char *command;
> } PCIProxyDevClass;
>
> +void proxy_default_bar_write(void *opaque, hwaddr addr, uint64_t val,
> + unsigned size);
> +
> +uint64_t proxy_default_bar_read(void *opaque, hwaddr addr, unsigned size);
> +
> #endif /* QEMU_PROXY_H */
> diff --git a/include/io/mpqemu-link.h b/include/io/mpqemu-link.h
> index 7228a1915e..41cf092f9e 100644
> --- a/include/io/mpqemu-link.h
> +++ b/include/io/mpqemu-link.h
> @@ -31,6 +31,8 @@
> /**
> * mpqemu_cmd_t:
> * SYNC_SYSMEM Shares QEMU's RAM with remote device's RAM
> + * BAR_WRITEWrites to PCI BAR region
> + * BAR_READ Reads from PCI BAR region
> *
> * proc_cmd_t enum type to specify the command to be executed on the remote
> * device.
> @@ -41,6 +43,8 @@ typedef enum {
> CONNECT_DEV,
> PCI_CONFIG_WRITE,
> PCI_CONFIG_READ,
> +BAR_WRITE,
> +BAR_READ,
> MAX,
> } mpqemu_cmd_t;
>
> @@ -56,6 +60,13 @@ typedef struct {
> ram_addr_t offsets[REMOTE_MAX_FDS];
> } sync_sysmem_msg_t;
>
> +typedef struct {
> +hwaddr addr;
> +uint64_t
Re: [PATCH v2 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
On Thu, Apr 09, 2020 at 02:25:38PM -0400, Peter Xu wrote:
> We should only pass in gdb_get_reg16() with the GByteArray* object
> itself, no need to shift. Without this patch, gdb remote attach will
> crash QEMU.
>
> Cc: Alex Bennée
> Cc: Philippe Mathieu-Daudé
> Fixes: b7b8756a9c ("target/i386: use gdb_get_reg helpers", 2020-03-17)
So I think this should still be:
Fixes: a010bdbe71 ("gdbstub: extend GByteArray to read register helpers")
As Phil & Laurent pointed out.
> Reviewed-by: Philippe Mathieu-Daudé
> Signed-off-by: Peter Xu
> ---
> target/i386/gdbstub.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
> index f3d23b614e..b98a99500a 100644
> --- a/target/i386/gdbstub.c
> +++ b/target/i386/gdbstub.c
> @@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray
> *mem_buf, int n)
> } else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
> floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
> int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
> -len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
> +len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
> return len;
> } else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
> n -= IDX_XMM_REGS;
> --
> 2.24.1
>
--
Peter Xu
Re: [PATCH-for-5.0] target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray
On Thu, Apr 09, 2020 at 08:36:22PM +0200, Laurent Vivier wrote:
> Le 09/04/2020 à 20:22, Peter Xu a écrit :
> > On Thu, Apr 09, 2020 at 07:25:08PM +0200, Philippe Mathieu-Daudé wrote:
> >> Since a010bdbe719 the gdbstub API takes a GByteArray*.
> >> Unfortunately we forgot to update the gdb_get_reg*()
> >> calls. Do it now.
> >>
> >> Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
> >
> > Should this be instead 462474d760 ("target/m68k: use gdb_get_reg
> > helpers", 2020-03-17)?
>
> No, this one is correct because it uses an "uint8_t *", then a010bdbe719
> changed this to a GByteArray and didn't remove the "+ len".
Ah right...
--
Peter Xu
RE: [RFC PATCH v2 20/67] Hexagon instruction utility functions
> -Original Message-
> From: Qemu-devel bounces+bcain=codeaurora@nongnu.org> On Behalf Of Taylor Simpson
> Sent: Friday, February 28, 2020 10:43 AM
> To: qemu-devel@nongnu.org
> Cc: riku.voi...@iki.fi; richard.hender...@linaro.org; laur...@vivier.eu;
> Taylor Simpson ; phi...@redhat.com;
> aleksandar.m.m...@gmail.com
> Subject: [RFC PATCH v2 20/67] Hexagon instruction utility functions
...
> +int arch_sf_invsqrt_common(size4s_t *Rs, size4s_t *Rd, int *adjust)
> +{
...
> +} else if (r_class == FP_INFINITE) {
> +/* EJP: or put Inf in num fixup? */
> +RsV = fSFINFVAL(-1);
> +RdV = fSFINFVAL(-1);
> +} else if (r_class == FP_ZERO) {
> +/* EJP: or put zero in num fixup? */
> +RsV = RsV;
> +RdV = fSFONEVAL(0);
...
This "RsV = RsV" looks like a logic error? Presumably it's safe to remove --
unless there's some other field that should get initialized here? PeV maybe?
Re: [PATCH for-5.0? 0/9] block/io: safer inc/dec in_flight sections
09.04.2020 20:00, Stefan Hajnoczi wrote: On Wed, Apr 08, 2020 at 12:30:42PM +0300, Vladimir Sementsov-Ogievskiy wrote: Hi all! This is inspired by Kevin's "block: Fix blk->in_flight during blk_wait_while_drained()" series. So, like it's now done for block-backends, let's expand in_flight-protected sections for bdrv_ interfaces, including coroutine_enter and BDRV_POLL_WHILE loop into these sections. This looks like a code improvement but let's leave it for the next release since QEMU 5.0 is in freeze and this patch series does not fix a specific user-visible bug. I will review this in depth next week. Thanks! Hmm, it possibly fixes some bugs, but at least I didn't see them :) Anyway, it shouldn't be a degradation. -- Best regards, Vladimir
[Bug 1871798] Re: Fails to start on Windows host without explicit --disable-pie
I'm using GCC 9.3.0 with mingw-w64 7.0.0, all built with Gentoo Linux's crossdev. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1871798 Title: Fails to start on Windows host without explicit --disable-pie Status in QEMU: Incomplete Bug description: Since commit d2cd29e30736afd4a1e8cac3cf4da360bbc65978, which removed the x86 conditional around PIE, QEMU completely fails to start on a Windows host unless --disable-pie is explicitly given at build time. Even just requesting the help text doesn't work. To make testing easier, this can be replicated with Wine. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1871798/+subscriptions
Re: [PATCH v4 03/30] qcow2: Add calculate_l2_meta()
09.04.2020 18:12, Alberto Garcia wrote:
On Thu 09 Apr 2020 10:30:13 AM CEST, Vladimir Sementsov-Ogievskiy wrote:
+static void calculate_l2_meta(BlockDriverState *bs,
+ uint64_t host_cluster_offset,
+ uint64_t guest_offset, unsigned bytes,
+ QCowL2Meta **m, bool keep_old)
+{
+BDRVQcow2State *s = bs->opaque;
+unsigned cow_start_from = 0;
+unsigned cow_start_to = offset_into_cluster(s, guest_offset);
+unsigned cow_end_from = cow_start_to + bytes;
+unsigned cow_end_to = ROUND_UP(cow_end_from, s->cluster_size);
+unsigned nb_clusters = size_to_clusters(s, cow_end_from);
+QCowL2Meta *old_m = *m;
+
+*m = g_malloc0(sizeof(**m));
+**m = (QCowL2Meta) {
+.next = old_m,
+
+.alloc_offset = host_cluster_offset,
+.offset = start_of_cluster(s, guest_offset),
+.nb_clusters= nb_clusters,
+
+.keep_old_clusters = keep_old,
+
+.cow_start = {
+.offset = cow_start_from,
+.nb_bytes = cow_start_to - cow_start_from,
+},
+.cow_end = {
+.offset = cow_end_from,
Hmm. So, you make it equal to requested_bytes from handle_alloc().
No, requested_bytes from handle_alloc is:
requested_bytes = *bytes + offset_into_cluster(s, guest_offset);
But *bytes is later modified before calling calculate_l2_meta():
*bytes = MIN(*bytes, nb_bytes - offset_into_cluster(s, guest_offset));
More details here:
https://lists.gnu.org/archive/html/qemu-block/2019-10/msg01808.html
Ahah, me again, sorry :)
--
Best regards,
Vladimir
[Bug 1821595] Re: Failed to emulate MMIO access with EmulatorReturnStatus: 2
Folks seem to think this is the fix, https://stackoverflow.com/questions/55197032/android-emulator-whpx-failed-to-emulate-mmio-access-exit-code-3 But it's not working for me ... does it help you? Thanks! -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1821595 Title: Failed to emulate MMIO access with EmulatorReturnStatus: 2 Status in QEMU: New Bug description: I have compiled qemu with enable-whpx parameter for Hyper-V Platform API in Mingw64 . When I tried run with Windows 7 iso file I have faced issue with the following problem: qemu-system-x86_64.exe: WHPX: Failed to emulate MMIO access with EmulatorReturnStatus: 2 qemu-system-x86_64.exe: WHPX: Failed to exec a virtual processor configuration directives: ../configure --target-list=x86_64-softmmu,i386-softmmu --enable-lzo\ --enable-bzip2 --enable-tools --enable-sdl --enable-gtk --enable-hax\ --enable-vdi --enable-qcow1 --enable-whpx --disable-capstone\ --disable-werror --disable-stack-protector --prefix="../../QEMU-bin" Qemu command line: qemu-system-x86_64.exe -m 1024 -cdrom "C:\Users\vmcs\Documents\en_windows_7_home_premium_with_sp1_x86_dvd_u_676701.iso" -display sdl -machine q35 -accel whpx To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1821595/+subscriptions
[Bug 1821595] Re: Failed to emulate MMIO access with EmulatorReturnStatus: 2
And here, https://www.reddit.com/r/androiddev/comments/c7u6h2/android_virtual_device_on_ryzen/ -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1821595 Title: Failed to emulate MMIO access with EmulatorReturnStatus: 2 Status in QEMU: New Bug description: I have compiled qemu with enable-whpx parameter for Hyper-V Platform API in Mingw64 . When I tried run with Windows 7 iso file I have faced issue with the following problem: qemu-system-x86_64.exe: WHPX: Failed to emulate MMIO access with EmulatorReturnStatus: 2 qemu-system-x86_64.exe: WHPX: Failed to exec a virtual processor configuration directives: ../configure --target-list=x86_64-softmmu,i386-softmmu --enable-lzo\ --enable-bzip2 --enable-tools --enable-sdl --enable-gtk --enable-hax\ --enable-vdi --enable-qcow1 --enable-whpx --disable-capstone\ --disable-werror --disable-stack-protector --prefix="../../QEMU-bin" Qemu command line: qemu-system-x86_64.exe -m 1024 -cdrom "C:\Users\vmcs\Documents\en_windows_7_home_premium_with_sp1_x86_dvd_u_676701.iso" -display sdl -machine q35 -accel whpx To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1821595/+subscriptions
Re: [PATCH v12 2/3] qcow2: Allow writing compressed data of multiple clusters
09.04.2020 19:50, Alberto Garcia wrote:
On Mon 02 Dec 2019 01:15:05 PM CET, Andrey Shinkevich wrote:
+static coroutine_fn int
+qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ QEMUIOVector *qiov, size_t qiov_offset)
+{
+BDRVQcow2State *s = bs->opaque;
+AioTaskPool *aio = NULL;
+int ret = 0;
+
+if (has_data_file(bs)) {
+return -ENOTSUP;
+}
+
+if (bytes == 0) {
+/*
+ * align end of file to a sector boundary to ease reading with
+ * sector based I/Os
+ */
+int64_t len = bdrv_getlength(bs->file->bs);
+if (len < 0) {
+return len;
+}
+return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, NULL);
+}
+
+if (offset_into_cluster(s, offset)) {
+return -EINVAL;
+}
+
+while (bytes && aio_task_pool_status(aio) == 0) {
+uint64_t chunk_size = MIN(bytes, s->cluster_size);
+
+if (!aio && chunk_size != bytes) {
+aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
+}
+
+ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_compressed_task_entry,
+ 0, 0, offset, chunk_size, qiov, qiov_offset,
NULL);
+if (ret < 0) {
+break;
+}
+qiov_offset += chunk_size;
+offset += chunk_size;
+bytes -= chunk_size;
+}
This patch allows the user to write more than one cluster of compressed
data at a time, and it does so by splitting the request into many
cluster-sized requests and using qcow2_add_task() for each one of them.
What happens however is that there's no guarantee that the requests are
processed in the same order that they were added.
One consequence is that running on an empty qcow2 file a command as
simple as this one:
qemu-io -c 'write -c 0 256k' image.qcow2
does not always produce the same results.
This does not have any user-visible consequences for the guest. In all
cases the data is correctly written, it's just that the ordering of the
compressed clusters (and therefore the contents of the L2 entries) will
be different each time.
Because of this a test cannot expect that running the same commands on
an empty image produces always the same results.
Is this something that we should be concerned about?
Parallel writing compressed clusters is significant improvement, as it allow
compressing in really parallel threads.
Generally, async parallel issuing of several requests gives more performance
than handling peaces one-by-one, mirror works on this basis and it is fast.
I've already moved qcow2 to this idea (aio tasks in qcow2 code), and in
progress of moving backup job. So, I think that asynchrony and ambiguity would
be native for block-layer anyway.
Hmm. Still, what about cluster sequence? For normal clusters there may be
simple thing to do: preallocation (at least of metadata). So, we can pre-create
cluster sequence.. But what to do with compressed clusters if we want specific
order for them, I don't know. On the other hand, ordering of normal cluster may
make sence: it should increase performnace of following IO. But for compressed
clusters it's not the case.
So, I don't think we should make specific workaround for testing... What
exactly is the case?
--
Best regards,
Vladimir
Re: [Bug 1871842] [NEW] AMD CPUID leaf 0x8000'0008 reported number of cores inconsistent with ACPI.MADT
On 4/9/20 9:00 AM, Igor Mammedov wrote: > On Thu, 09 Apr 2020 12:58:11 - > Philipp Eppelt <1871...@bugs.launchpad.net> wrote: > >> Public bug reported: >> >> Setup: >> CPU: AMD EPYC-v2 or host's EPYC cpu >> Linux 64-bit fedora host; Kernel version 5.5.15-200.fc31 >> qemu version: self build >> git-head: f3bac27cc1e303e1860cc55b9b6889ba39dee587 >> config: Configured with: '../configure' >> '--target-list=x86_64-softmmu,mips64el-softmmu,mips64-softmmu,mipsel-softmmu,mips-softmmu,i386-softmmu,aarch64-softmmu,arm-softmmu' >> '--prefix=/opt/qemu-master' >> >> Cmdline: >> qemu-system-x86_64 -kernel >> /home/peppelt/code/l4/internal/.build-x86_64/bin/amd64_gen/bootstrap -append >> "" -initrd "./fiasco/.build-x86_64/fiasco , ... " -serial stdio -nographic >> -monitor none -nographic -monitor none -cpu EPYC-v2 -m 4G -smp 4 >> >> Issue: >> We are developing an microkernel operating system called L4Re. We recently >> got an AMD EPYC server for testing and we couldn't execute SMP tests of our >> system when running Linux + qemu + VM w/ L4Re. >> In fact, the kernel did not recognize any APs at all. On AMD CPUs the kernel >> checks for the number of cores reported in CPUID leaf 0x8000_0008.ECX[NC] or >> [ApicIdSize]. [0][1] >> >> The physical machine reports for leaf 0x8000_0008: EAX: 0x3030 EBX: >> 0x18cf757 ECX: 0x703f EDX: 0x1000 >> The lower four bits of ECX are the [NC] field and all set. >> >> When querying inside qemu with -enable-kvm -cpu host -smp 4 (basically as >> replacement and addition to the above cmdline) the CPUID leaf shows: EAX: >> 0x3024, EBX: 0x1001000, ECX: 0x0, EDX: 0x0 >> Note, ECX is zero. Indicating that this is no SMP capabale CPU. >> >> I'm debugging it using my local machine and the QEMU provided EPYC-v2 >> CPU model and it is reproducible there as well and reports: EAX: >> 0x3028, EBX: 0x0, ECX: 0x0, EDX: 0x0 >> >> I checked other AMD based CPU models (phenom, opteron_g3/g5) and they behave >> the same. [2] shows the CPUID 0x8000'0008 handling in the QEMU source. >> I believe that behavior here is wrong as ECX[NC] should report the number of >> cores per processor, as stated in the AMD manual [2] p.584. In my >> understanding -smp 4 should then lead to ECX[NC] = 0x3. >> >> The following table shows my findings with the -smp option: >> Option | Qemu guest observed ECX value >> -smp 4 | 0x0 >> -smp 4,cores=4 | 0x3 >> -smp 4,cores=2,thread=2 | 0x3 >> -smp 4,cores=4,threads=2 | QEMU boot error: topology false. >> >> Now, I'm asking myself how the terminology of the AMD manual maps to QEMU's >> -smp option. >> Obviously, nr_cores and nr_threads correspond to the cores and threads >> options on the cmdline and cores * threads <= 4 (in this example), but what >> corresponds the X in -smp X to? > I'd say X corresponds to number of logical CPUs. > Depending on presence of other options these are distributed among them in > magical manner > (see pc_smp_parse() for starters) > >> Querying 0x8000'0008 on the physical processor results in different >> reports than quering QEMU's model as does it with -enable-kvm -cpu host. >> >> Furthermore, the ACPI.MADT shows 4 local APICs to be present while the >> CPU leave reports a single core processor. > it matches -smp X as it should be. > >> >> This leads me to the conclusion that CPUID 0x8000'0008.ECX reports the >> wrong number. > CCed author of recent epyc patches who might know how AMD should work better > than me. Hmm.. Interesting.. Not sure why this did not come up during my testing. Probably this cpuid information is not widely used. Yes. I am looking at it right now. I see that EPYC model is reporting wrong. Not sure why -cpu host is reporting wrong. I thought -cpu host gets the information directly from the host. Will investigate.
Re: [PATCH-for-5.0] target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray
Le 09/04/2020 à 20:22, Peter Xu a écrit :
> On Thu, Apr 09, 2020 at 07:25:08PM +0200, Philippe Mathieu-Daudé wrote:
>> Since a010bdbe719 the gdbstub API takes a GByteArray*.
>> Unfortunately we forgot to update the gdb_get_reg*()
>> calls. Do it now.
>>
>> Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
>
> Should this be instead 462474d760 ("target/m68k: use gdb_get_reg
> helpers", 2020-03-17)?
No, this one is correct because it uses an "uint8_t *", then a010bdbe719
changed this to a GByteArray and didn't remove the "+ len".
Thanks,
Laurent
>> Reported-by: Peter Xu
>> Signed-off-by: Philippe Mathieu-Daudé
>
> Reviewed-by: Peter Xu
>
>> ---
>> Based-on: <20200409164954.36902-1-pet...@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé
>
> (One benigh extra line; same thing could happen to me when amending
> like this with "---")
>
>> ---
>> target/m68k/helper.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/m68k/helper.c b/target/m68k/helper.c
>> index 014657c637..cad4083895 100644
>> --- a/target/m68k/helper.c
>> +++ b/target/m68k/helper.c
>> @@ -109,8 +109,8 @@ static int m68k_fpu_gdb_get_reg(CPUM68KState *env,
>> GByteArray *mem_buf, int n)
>> {
>> if (n < 8) {
>> int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
>> -len += gdb_get_reg16(mem_buf + len, 0);
>> -len += gdb_get_reg64(mem_buf + len, env->fregs[n].l.lower);
>> +len += gdb_get_reg16(mem_buf, 0);
>> +len += gdb_get_reg64(mem_buf, env->fregs[n].l.lower);
>> return len;
>> }
>> switch (n) {
>> --
>> 2.21.1
>>
>
Re: [PATCH-for-5.0] target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray
Le 09/04/2020 à 20:22, Peter Xu a écrit :
> On Thu, Apr 09, 2020 at 07:25:08PM +0200, Philippe Mathieu-Daudé wrote:
>> Since a010bdbe719 the gdbstub API takes a GByteArray*.
>> Unfortunately we forgot to update the gdb_get_reg*()
>> calls. Do it now.
>>
>> Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
>
> Should this be instead 462474d760 ("target/m68k: use gdb_get_reg
> helpers", 2020-03-17)?
No, this one is correct because it uses an "uint8_t *", then a010bdbe719
changed this to a GByteArray and didn't remove the "+ len".
Thanks,
Laurent
>
>> Reported-by: Peter Xu
>> Signed-off-by: Philippe Mathieu-Daudé
>
> Reviewed-by: Peter Xu
>
>> ---
>> Based-on: <20200409164954.36902-1-pet...@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé
>
> (One benigh extra line; same thing could happen to me when amending
> like this with "---")
>
>> ---
>> target/m68k/helper.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/m68k/helper.c b/target/m68k/helper.c
>> index 014657c637..cad4083895 100644
>> --- a/target/m68k/helper.c
>> +++ b/target/m68k/helper.c
>> @@ -109,8 +109,8 @@ static int m68k_fpu_gdb_get_reg(CPUM68KState *env,
>> GByteArray *mem_buf, int n)
>> {
>> if (n < 8) {
>> int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
>> -len += gdb_get_reg16(mem_buf + len, 0);
>> -len += gdb_get_reg64(mem_buf + len, env->fregs[n].l.lower);
>> +len += gdb_get_reg16(mem_buf, 0);
>> +len += gdb_get_reg64(mem_buf, env->fregs[n].l.lower);
>> return len;
>> }
>> switch (n) {
>> --
>> 2.21.1
>>
>
[PATCH v2 1/2] gdbstub: Assert len read from each register
This can expose the issue earlier on which register returned the wrong
result.
Signed-off-by: Peter Xu
---
gdbstub.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 171e150950..69656e1052 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -911,17 +911,23 @@ static int gdb_read_register(CPUState *cpu, GByteArray
*buf, int reg)
CPUClass *cc = CPU_GET_CLASS(cpu);
CPUArchState *env = cpu->env_ptr;
GDBRegisterState *r;
+int len = 0, orig_len = buf->len;
if (reg < cc->gdb_num_core_regs) {
-return cc->gdb_read_register(cpu, buf, reg);
+len = cc->gdb_read_register(cpu, buf, reg);
+goto out;
}
for (r = cpu->gdb_regs; r; r = r->next) {
if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) {
-return r->get_reg(env, buf, reg - r->base_reg);
+len = r->get_reg(env, buf, reg - r->base_reg);
+break;
}
}
-return 0;
+
+out:
+assert(len == buf->len - orig_len);
+return len;
}
static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
--
2.24.1
[PATCH v2 0/2] gdbstub: Unbreak i386 with gdb remote
v2: - use "goto" in patch 1 [Phil] - pick some tags Thanks, Peter Xu (2): gdbstub: Assert len read from each register gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb gdbstub.c | 12 +--- target/i386/gdbstub.c | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) -- 2.24.1
[PATCH v2 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
We should only pass in gdb_get_reg16() with the GByteArray* object
itself, no need to shift. Without this patch, gdb remote attach will
crash QEMU.
Cc: Alex Bennée
Cc: Philippe Mathieu-Daudé
Fixes: b7b8756a9c ("target/i386: use gdb_get_reg helpers", 2020-03-17)
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Peter Xu
---
target/i386/gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index f3d23b614e..b98a99500a 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray
*mem_buf, int n)
} else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
-len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
+len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
return len;
} else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
n -= IDX_XMM_REGS;
--
2.24.1
Re: [PATCH-for-5.0] target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray
On Thu, Apr 09, 2020 at 07:25:08PM +0200, Philippe Mathieu-Daudé wrote:
> Since a010bdbe719 the gdbstub API takes a GByteArray*.
> Unfortunately we forgot to update the gdb_get_reg*()
> calls. Do it now.
>
> Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Should this be instead 462474d760 ("target/m68k: use gdb_get_reg
helpers", 2020-03-17)?
> Reported-by: Peter Xu
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Peter Xu
> ---
> Based-on: <20200409164954.36902-1-pet...@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé
(One benigh extra line; same thing could happen to me when amending
like this with "---")
> ---
> target/m68k/helper.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/m68k/helper.c b/target/m68k/helper.c
> index 014657c637..cad4083895 100644
> --- a/target/m68k/helper.c
> +++ b/target/m68k/helper.c
> @@ -109,8 +109,8 @@ static int m68k_fpu_gdb_get_reg(CPUM68KState *env,
> GByteArray *mem_buf, int n)
> {
> if (n < 8) {
> int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
> -len += gdb_get_reg16(mem_buf + len, 0);
> -len += gdb_get_reg64(mem_buf + len, env->fregs[n].l.lower);
> +len += gdb_get_reg16(mem_buf, 0);
> +len += gdb_get_reg64(mem_buf, env->fregs[n].l.lower);
> return len;
> }
> switch (n) {
> --
> 2.21.1
>
--
Peter Xu
Re: [PATCH for-5.1 7/8] qemu-img: Factor out accumulate_options() helper
On 4/9/20 10:30 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- qemu-img.c | 59 +- 1 file changed, 23 insertions(+), 36 deletions(-) Nice. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH for-5.1 6/8] test-qemu-opts: Simplify test_has_help_option() after bug fix
On 4/9/20 10:30 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- tests/test-qemu-opts.c | 24 +++- 1 file changed, 11 insertions(+), 13 deletions(-) Assuming that the interpretation of a,b,,help is intended to match the interpretation you chose in 5/8 (and not the one I questioned in 1/8), this is fine. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 1/2] gdbstub: Assert len read from each register
On Thu, Apr 09, 2020 at 07:51:49PM +0200, Philippe Mathieu-Daudé wrote:
> On 4/9/20 6:49 PM, Peter Xu wrote:
> > This can expose the issue earlier on which register returned the wrong
> > result.
> >
> > Signed-off-by: Peter Xu
> > ---
> > gdbstub.c | 11 ---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/gdbstub.c b/gdbstub.c
> > index 171e150950..f763545e81 100644
> > --- a/gdbstub.c
> > +++ b/gdbstub.c
> > @@ -911,17 +911,22 @@ static int gdb_read_register(CPUState *cpu,
> > GByteArray *buf, int reg)
> > CPUClass *cc = CPU_GET_CLASS(cpu);
> > CPUArchState *env = cpu->env_ptr;
> > GDBRegisterState *r;
> > +int len = 0, orig_len = buf->len;
> > if (reg < cc->gdb_num_core_regs) {
> > -return cc->gdb_read_register(cpu, buf, reg);
> > +len = cc->gdb_read_register(cpu, buf, reg);
>
> This change the code flow. We could add ...:
I didn't expect the "if" and "for" would collapse each other, but yeah
that could still be better.
Thanks,
>
>goto out;
>
> > }
>
> ... or use else?
> > for (r = cpu->gdb_regs; r; r = r->next) {
> > if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) {
> > -return r->get_reg(env, buf, reg - r->base_reg);
> > +len = r->get_reg(env, buf, reg - r->base_reg);
> > +break;
> > }
> > }
> > -return 0;
> > +
>
> out:
>
> > +assert(len == buf->len - orig_len);
> > +
> > +return len;
> > }
> > static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
> >
>
--
Peter Xu
Re: [PATCH for-5.1 5/8] qemu-option: Fix has_help_option()'s sloppy parsing
On 4/9/20 10:30 AM, Markus Armbruster wrote:
has_help_option() uses its own parser. It's inconsistent with
qemu_opts_parse(), as demonstrated by test-qemu-opts case
/qemu-opts/has_help_option. Fix by reusing the common parser.
Signed-off-by: Markus Armbruster
---
tests/test-qemu-opts.c | 2 +-
util/qemu-option.c | 39 +++
2 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
index 27c24bb1a2..58a4ea2408 100644
--- a/tests/test-qemu-opts.c
+++ b/tests/test-qemu-opts.c
@@ -744,7 +744,7 @@ static void test_has_help_option(void)
{ "a,help", true, true, true },
{ "a=0,help,b", true, true, true },
{ "help,b=1", true, true, false },
-{ "a,b,,help", false /* BUG */, true, true },
+{ "a,b,,help", true, true, true },
Okay, this revisits my question from 1/8.
I guess the argument is that since 'b,help' is NOT a valid option name
(only as an option value), that we are instead parsing three separate
options 'b', '', and 'help', and whether or not the empty option is
valid, the face that 'help' is valid is what makes this return true?
+++ b/util/qemu-option.c
@@ -165,26 +165,6 @@ void parse_option_size(const char *name, const char *value,
*ret = size;
}
-bool has_help_option(const char *param)
-{
-const char *p = param;
-bool result = false;
-
-while (*p && !result) {
-char *value;
-
-p = get_opt_value(p, &value);
-if (*p) {
-p++;
-}
-
-result = is_help_option(value);
Old code: both 'help' and '?' are accepted.
+bool has_help_option(const char *params)
+{
+const char *p;
+char *name, *value;
+bool ret;
+
+for (p = params; *p;) {
+p = get_opt_name_value(p, NULL, &name, &value);
+ret = !strcmp(name, "help");
New code: only 'help' is accepted. Is the loss of '?' intentional?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH for-5.1 4/8] qemu-option: Avoid has_help_option() in qemu_opts_parse_noisily()
On 4/9/20 10:30 AM, Markus Armbruster wrote:
When opts_parse() sets @invalidp to true, qemu_opts_parse_noisily()
uses has_help_option() to decide whether to print help. This parses
the input string a second time, in a slightly different way.
Easy to avoid: replace @invalidp by @help_wanted.
Signed-off-by: Markus Armbruster
---
util/qemu-option.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
-opts = opts_parse(list, params, permit_abbrev, false, &invalidp, &err);
+opts = opts_parse(list, params, permit_abbrev, false, &help_wanted, &err);
if (err) {
-if (invalidp && has_help_option(params)) {
+if (help_wanted) {
Yep, that is cleaner. Should there be testsuite coverage changing as a
result of this?
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH for-5.1 3/8] qemu-option: Fix sloppy recognition of "id=..." after ", , "
On 4/9/20 10:30 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- tests/test-qemu-opts.c | 4 ++-- util/qemu-option.c | 27 +++ 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c index 0efe93b45e..27c24bb1a2 100644 --- a/tests/test-qemu-opts.c +++ b/tests/test-qemu-opts.c @@ -500,10 +500,10 @@ static void test_opts_parse(void) g_assert(!opts); /* TODO Cover .merge_lists = true */ -/* Buggy ID recognition */ +/* Buggy ID recognition (fixed) */ opts = qemu_opts_parse(&opts_list_03, "x=,,id=bar", false, &error_abort); g_assert_cmpuint(opts_count(opts), ==, 1); -g_assert_cmpstr(qemu_opts_id(opts), ==, "bar"); /* BUG */ +g_assert(!qemu_opts_id(opts)); g_assert_cmpstr(qemu_opt_get(opts, "x"), ==, ",id=bar"); Yay - that matches my intuition better about ,, handling. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH-for-5.0] target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray
Le 09/04/2020 à 19:25, Philippe Mathieu-Daudé a écrit :
> Since a010bdbe719 the gdbstub API takes a GByteArray*.
> Unfortunately we forgot to update the gdb_get_reg*()
> calls. Do it now.
>
> Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
> Reported-by: Peter Xu
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Based-on: <20200409164954.36902-1-pet...@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> target/m68k/helper.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/m68k/helper.c b/target/m68k/helper.c
> index 014657c637..cad4083895 100644
> --- a/target/m68k/helper.c
> +++ b/target/m68k/helper.c
> @@ -109,8 +109,8 @@ static int m68k_fpu_gdb_get_reg(CPUM68KState *env,
> GByteArray *mem_buf, int n)
> {
> if (n < 8) {
> int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
> -len += gdb_get_reg16(mem_buf + len, 0);
> -len += gdb_get_reg64(mem_buf + len, env->fregs[n].l.lower);
> +len += gdb_get_reg16(mem_buf, 0);
> +len += gdb_get_reg64(mem_buf, env->fregs[n].l.lower);
> return len;
> }
> switch (n) {
>
Reviewed-by: Laurent Vivier
Re: [PATCH for-5.1 2/8] qemu-options: Factor out get_opt_name_value() helper
On 4/9/20 10:30 AM, Markus Armbruster wrote:
The next commits will put it to use.
Signed-off-by: Markus Armbruster
---
util/qemu-option.c | 102 +
1 file changed, 56 insertions(+), 46 deletions(-)
+static const char *get_opt_name_value(const char *params,
+ const char *firstname,
+ char **name, char **value)
+{
+const char *p, *pe, *pc;
+
+pe = strchr(params, '=');
+pc = strchr(params, ',');
+
+if (!pe || (pc && pc < pe)) {
+/* found "foo,more" */
+if (firstname) {
+/* implicitly named first option */
+*name = g_strdup(firstname);
+p = get_opt_value(params, value);
Is this correct even when params is "foo,,more"? But...
static void opts_do_parse(QemuOpts *opts, const char *params,
const char *firstname, bool prepend,
bool *invalidp, Error **errp)
{
-char *option = NULL;
-char *value = NULL;
-const char *p,*pe,*pc;
Error *local_err = NULL;
+char *option, *value;
+const char *p;
-for (p = params; *p != '\0'; p++) {
-pe = strchr(p, '=');
-pc = strchr(p, ',');
-if (!pe || (pc && pc < pe)) {
-/* found "foo,more" */
-if (p == params && firstname) {
-/* implicitly named first option */
-option = g_strdup(firstname);
-p = get_opt_value(p, &value);
...in this patch, it is just code motion, so if it is a bug, it's
pre-existing and worth a separate fix.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
Hi, Phil,
On Thu, Apr 09, 2020 at 07:21:04PM +0200, Philippe Mathieu-Daudé wrote:
> On 4/9/20 6:49 PM, Peter Xu wrote:
> > We should only pass in gdb_get_reg16() with the GByteArray* object
> > itself, no need to shift. Without this patch, gdb remote attach will
> > crash QEMU.
>
> You are correct.
>
> Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Oh I forgot to paste the fix line. However, is it b7b8756a9c
("target/i386: use gdb_get_reg helpers", 2020-03-17) instead?
>
> Reviewed-by: Philippe Mathieu-Daudé
Thanks!
>
> Same problem in m68k_fpu_gdb_get_reg().
>
> TODO for 5.1, rename mem_buf -> array.
--
Peter Xu
[Bug 1821595] Re: Failed to emulate MMIO access with EmulatorReturnStatus: 2
I think WHPX can't some MMIO requests for EFI. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1821595 Title: Failed to emulate MMIO access with EmulatorReturnStatus: 2 Status in QEMU: New Bug description: I have compiled qemu with enable-whpx parameter for Hyper-V Platform API in Mingw64 . When I tried run with Windows 7 iso file I have faced issue with the following problem: qemu-system-x86_64.exe: WHPX: Failed to emulate MMIO access with EmulatorReturnStatus: 2 qemu-system-x86_64.exe: WHPX: Failed to exec a virtual processor configuration directives: ../configure --target-list=x86_64-softmmu,i386-softmmu --enable-lzo\ --enable-bzip2 --enable-tools --enable-sdl --enable-gtk --enable-hax\ --enable-vdi --enable-qcow1 --enable-whpx --disable-capstone\ --disable-werror --disable-stack-protector --prefix="../../QEMU-bin" Qemu command line: qemu-system-x86_64.exe -m 1024 -cdrom "C:\Users\vmcs\Documents\en_windows_7_home_premium_with_sp1_x86_dvd_u_676701.iso" -display sdl -machine q35 -accel whpx To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1821595/+subscriptions
Re: [PATCH v6 16/36] multi-process: remote process initialization
* elena.ufimts...@oracle.com (elena.ufimts...@oracle.com) wrote:
> From: Jagannathan Raman
>
> Adds the handler to process message from QEMU,
> Initialize remote process main loop, handles SYNC_SYSMEM
> message by updating its "system_memory" container using
> shared file descriptors received from QEMU.
>
> Signed-off-by: John G Johnson
> Signed-off-by: Elena Ufimtseva
> Signed-off-by: Jagannathan Raman
> ---
> remote/remote-main.c | 87
> 1 file changed, 87 insertions(+)
>
> diff --git a/remote/remote-main.c b/remote/remote-main.c
> index ecf30e0cba..51595f3141 100644
> --- a/remote/remote-main.c
> +++ b/remote/remote-main.c
> @@ -12,6 +12,7 @@
> #include "qemu-common.h"
>
> #include
> +#include
>
> #include "qemu/module.h"
> #include "remote/pcihost.h"
> @@ -19,12 +20,98 @@
> #include "hw/boards.h"
> #include "hw/qdev-core.h"
> #include "qemu/main-loop.h"
> +#include "remote/memory.h"
> +#include "io/mpqemu-link.h"
> +#include "qapi/error.h"
> +#include "qemu/main-loop.h"
> +#include "sysemu/cpus.h"
> +#include "qemu-common.h"
> +#include "hw/pci/pci.h"
> +#include "qemu/thread.h"
> +#include "qemu/main-loop.h"
> +#include "qemu/config-file.h"
> +#include "sysemu/sysemu.h"
> +#include "block/block.h"
> +#include "exec/ramlist.h"
> +
> +static MPQemuLinkState *mpqemu_link;
> +
> +static void process_msg(GIOCondition cond, MPQemuLinkState *link,
> +MPQemuChannel *chan)
> +{
> +MPQemuMsg *msg = NULL;
> +Error *err = NULL;
> +
> +if ((cond & G_IO_HUP) || (cond & G_IO_ERR)) {
> +goto finalize_loop;
> +}
> +
> +msg = g_malloc0(sizeof(MPQemuMsg));
> +
> +if (mpqemu_msg_recv(msg, chan) < 0) {
> +error_setg(&err, "Failed to receive message");
> +goto finalize_loop;
> +}
> +
> +switch (msg->cmd) {
> +case INIT:
> +break;
> +default:
> +error_setg(&err, "Unknown command");
Again this doesn't seem to have changed since my 4th March review where
I asked for better error messages.
Dave
> +goto finalize_loop;
> +}
> +
> +g_free(msg->data2);
> +g_free(msg);
> +
> +return;
> +
> +finalize_loop:
> +if (err) {
> +error_report_err(err);
> +}
> +g_free(msg);
> +mpqemu_link_finalize(mpqemu_link);
> +mpqemu_link = NULL;
> +}
>
> int main(int argc, char *argv[])
> {
> +Error *err = NULL;
> +
> module_call_init(MODULE_INIT_QOM);
>
> +bdrv_init_with_whitelist();
> +
> +if (qemu_init_main_loop(&err)) {
> +error_report_err(err);
> +return -EBUSY;
> +}
> +
> +qemu_init_cpu_loop();
> +
> +page_size_init();
> +
> +qemu_mutex_init(&ram_list.mutex);
> +
> current_machine =
> MACHINE(REMOTE_MACHINE(object_new(TYPE_REMOTE_MACHINE)));
>
> +mpqemu_link = mpqemu_link_create();
> +if (!mpqemu_link) {
> +printf("Could not create MPQemu link\n");
> +return -1;
> +}
> +
> +mpqemu_init_channel(mpqemu_link, &mpqemu_link->com, STDIN_FILENO);
> +
> +mpqemu_link_set_callback(mpqemu_link, process_msg);
> +
> +qdev_machine_creation_done();
> +qemu_mutex_lock_iothread();
> +qemu_run_machine_init_done_notifiers();
> +qemu_mutex_unlock_iothread();
> +
> +mpqemu_start_coms(mpqemu_link, mpqemu_link->com);
> +
> return 0;
> }
> --
> 2.25.GIT
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v6 12/36] multi-process: add functions to synchronize proxy and remote endpoints
* elena.ufimts...@oracle.com (elena.ufimts...@oracle.com) wrote:
> From: Jagannathan Raman
>
> In some cases, for example MMIO read, QEMU has to wait for the remote to
> complete a command before proceeding. An eventfd based mechanism is
> added to synchronize QEMU & remote process.
>
> Signed-off-by: John G Johnson
> Signed-off-by: Jagannathan Raman
> Signed-off-by: Elena Ufimtseva
> ---
> include/io/mpqemu-link.h | 7 +++
> io/mpqemu-link.c | 41
> 2 files changed, 48 insertions(+)
>
> diff --git a/include/io/mpqemu-link.h b/include/io/mpqemu-link.h
> index af401e640c..ef95599bca 100644
> --- a/include/io/mpqemu-link.h
> +++ b/include/io/mpqemu-link.h
> @@ -124,4 +124,11 @@ void mpqemu_link_set_callback(MPQemuLinkState *s,
> void mpqemu_start_coms(MPQemuLinkState *s, MPQemuChannel* chan);
> bool mpqemu_msg_valid(MPQemuMsg *msg);
>
> +#define GET_REMOTE_WAIT eventfd(0, EFD_CLOEXEC)
> +#define PUT_REMOTE_WAIT(wait) close(wait)
> +#define PROXY_LINK_WAIT_DONE 1
> +
> +uint64_t wait_for_remote(int efd);
> +void notify_proxy(int fd, uint64_t val);
> +
> #endif
> diff --git a/io/mpqemu-link.c b/io/mpqemu-link.c
> index b7d3e53ae8..fa9b3a66b1 100644
> --- a/io/mpqemu-link.c
> +++ b/io/mpqemu-link.c
> @@ -10,6 +10,7 @@
>
> #include "qemu/osdep.h"
> #include "qemu-common.h"
> +#include
>
> #include "qemu/module.h"
> #include "io/mpqemu-link.h"
> @@ -204,6 +205,46 @@ int mpqemu_msg_recv(MPQemuMsg *msg, MPQemuChannel *chan)
> return rc;
> }
>
> +uint64_t wait_for_remote(int efd)
> +{
> +struct pollfd pfd = { .fd = efd, .events = POLLIN };
> +uint64_t val;
> +int ret;
> +
> +ret = poll(&pfd, 1, 1000);
> +
> +switch (ret) {
> +case 0:
> +qemu_log_mask(LOG_REMOTE_DEBUG, "Error wait_for_remote: Timed
> out\n");
> +/* TODO: Kick-off error recovery */
> +return ULLONG_MAX;
> +case -1:
> +qemu_log_mask(LOG_REMOTE_DEBUG, "Poll error wait_for_remote: %s\n",
> + strerror(errno));
> +return ULLONG_MAX;
> +default:
> +if (read(efd, &val, sizeof(val)) == -1) {
> +qemu_log_mask(LOG_REMOTE_DEBUG, "Error wait_for_remote: %s\n",
> + strerror(errno));
> +return ULLONG_MAX;
> +}
> +}
> +
> +val = (val == ULLONG_MAX) ? val : (val - 1);
> +
> +return val;
These don't seem to have changed since my review of v5 on the 4th March
in which Jag said they would change to UINT64_MAX etc and there would be
a big comment etc on the next function.
Dave
> +}
> +
> +void notify_proxy(int efd, uint64_t val)
> +{
> +val = (val == ULLONG_MAX) ? val : (val + 1);
> +
> +if (write(efd, &val, sizeof(val)) == -1) {
> +qemu_log_mask(LOG_REMOTE_DEBUG, "Error notify_proxy: %s\n",
> + strerror(errno));
> +}
> +}
> +
> static gboolean mpqemu_link_handler_prepare(GSource *gsrc, gint *timeout)
> {
> g_assert(timeout);
> --
> 2.25.GIT
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH RESEND 3/3] .travis.yml: Test building with Xcode 11.3
On Thu, Apr 09, 2020 at 07:44:46PM +0200, Philippe Mathieu-Daudé wrote:
> On 2/25/20 1:29 PM, Alex Bennée wrote:
> >
> > Philippe Mathieu-Daudé writes:
> >
> > > We currently run a CI job on macOS Mojave with Xcode 10.
> > >
> > > QEMU policy is to support the two last major OS releases.
> > > Add a job building on macOS Catalina, which comes with Xcode 11.
> > >
> > > Split the target list in two, as we don't need to cover twice the
> > > same targets.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé
> > > ---
> > > .travis.yml | 36 +++-
> > > 1 file changed, 35 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/.travis.yml b/.travis.yml
> > > index a2a7fd0dd1..d02a477623 100644
> > > --- a/.travis.yml
> > > +++ b/.travis.yml
> > > @@ -269,9 +269,10 @@ matrix:
> > > # MacOSX builds - cirrus.yml also tests some MacOS builds including
> > > latest Xcode
> > > +# On macOS Mojave, the SDK comes bundled with Xcode 10.
> > > - name: "OSX Xcode 10.3"
> > > env:
> > > --
> > > CONFIG="--target-list=i386-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,x86_64-softmmu
> > > --extra-cflags=-I/usr/local/opt/ncurses/include
> > > --extra-ldflags=-L/usr/local/opt/ncurses/lib"
> > > +-
> > > CONFIG="--target-list=i386-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu
> > > --extra-cflags=-I/usr/local/opt/ncurses/include
> > > --extra-ldflags=-L/usr/local/opt/ncurses/lib"
> > > os: osx
> > > osx_image: xcode10.3
> > > compiler: clang
> > > @@ -301,6 +302,39 @@ matrix:
> > > - ${SRC_DIR}/configure ${BASE_CONFIG} ${CONFIG} || { cat
> > > config.log && exit 1; }
> > > +# On macOS Catalina, the SDK comes bundled with Xcode 11.
> > > +- name: "OSX Xcode 11.3"
> > > + env:
> > > +- CONFIG="--target-list=arm-softmmu,ppc64-softmmu,x86_64-softmmu
> > > --extra-cflags=-I/usr/local/opt/ncurses/include
> > > --extra-ldflags=-L/usr/local/opt/ncurses/lib"
> > > + os: osx
> > > + osx_image: xcode11.3
> >
> > Are we duplicating what the latest Xcode on Cirrus is here?
>
> Maybe, I'm not sure. It seems only few people care about Cirrus/Shippable
> but they are not taken seriously by the community, as they are often broken
> and nobody is notified. Currently Travis has a broader audience.
>
> Also I sent a series to fix various things that break on Cirrus from time to
> time but I felt there is not many interest so I stopped spending energy on
> it:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg675074.html
>
> We could change that by refusing to merge pullreq that break such CI.
IMHO less is more. IOW, we should use/support the fewest possible CI
systems required to get the coverage we want. If we can get all macOS
coverage on Travis, I'd remove it from Cirrus, or vica-verca. The fewer
places we have to look at the more likely we'll pay attention to it
when it breaks.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 1/2] gdbstub: Assert len read from each register
On 4/9/20 6:49 PM, Peter Xu wrote:
This can expose the issue earlier on which register returned the wrong
result.
Signed-off-by: Peter Xu
---
gdbstub.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 171e150950..f763545e81 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -911,17 +911,22 @@ static int gdb_read_register(CPUState *cpu, GByteArray
*buf, int reg)
CPUClass *cc = CPU_GET_CLASS(cpu);
CPUArchState *env = cpu->env_ptr;
GDBRegisterState *r;
+int len = 0, orig_len = buf->len;
if (reg < cc->gdb_num_core_regs) {
-return cc->gdb_read_register(cpu, buf, reg);
+len = cc->gdb_read_register(cpu, buf, reg);
This change the code flow. We could add ...:
goto out;
}
... or use else?
for (r = cpu->gdb_regs; r; r = r->next) {
if (r->base_reg <= reg && reg < r->base_reg + r->num_regs) {
-return r->get_reg(env, buf, reg - r->base_reg);
+len = r->get_reg(env, buf, reg - r->base_reg);
+break;
}
}
-return 0;
+
out:
+assert(len == buf->len - orig_len);
+
+return len;
}
static int gdb_write_register(CPUState *cpu, uint8_t *mem_buf, int reg)
Re: [PATCH v6 06/36] monitor: destaticize HMP commands
* elena.ufimts...@oracle.com (elena.ufimts...@oracle.com) wrote:
> From: Jagannathan Raman
>
> Signed-off-by: Elena Ufimtseva
> Signed-off-by: John G Johnson
> Signed-off-by: Jagannathan Raman
Hmm OK, so
Reviewed-by: Dr. David Alan Gilbert
but it might be better where you can, to restrict these to the ones you
want to share with your processes; for example I doubt hmp_wavcapture
would be shared.
Dave
> ---
> hmp-commands.hx| 4 +-
> monitor/misc.c | 76 +++---
> monitor/monitor-internal.h | 38 +++
> 3 files changed, 78 insertions(+), 40 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 7f0f3974ad..02cae25c24 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -11,7 +11,7 @@ HXCOMM HXCOMM can be used for comments, discarded from both
> rST and C.
> .args_type = "name:S?",
> .params = "[cmd]",
> .help = "show the help",
> -.cmd= do_help_cmd,
> +.cmd= hmp_do_help_cmd,
> .flags = "p",
> },
>
> @@ -555,7 +555,7 @@ ERST
> .args_type = "fmt:/,val:l",
> .params = "/fmt expr",
> .help = "print expression value (use $reg for CPU register
> access)",
> -.cmd= do_print,
> +.cmd= hmp_do_print,
> },
>
> SRST
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 6c45fa490f..c0eee6f4ab 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -178,12 +178,12 @@ int hmp_compare_cmd(const char *name, const char *list)
> return 0;
> }
>
> -static void do_help_cmd(Monitor *mon, const QDict *qdict)
> +void hmp_do_help_cmd(Monitor *mon, const QDict *qdict)
> {
> help_cmd(mon, qdict_get_try_str(qdict, "name"));
> }
>
> -static void hmp_trace_event(Monitor *mon, const QDict *qdict)
> +void hmp_trace_event(Monitor *mon, const QDict *qdict)
> {
> const char *tp_name = qdict_get_str(qdict, "name");
> bool new_state = qdict_get_bool(qdict, "option");
> @@ -227,7 +227,7 @@ static void hmp_trace_file(Monitor *mon, const QDict
> *qdict)
> }
> #endif
>
> -static void hmp_info_help(Monitor *mon, const QDict *qdict)
> +void hmp_info_help(Monitor *mon, const QDict *qdict)
> {
> help_cmd(mon, "info");
> }
> @@ -315,7 +315,7 @@ int monitor_get_cpu_index(void)
> return cs ? cs->cpu_index : UNASSIGNED_CPU_INDEX;
> }
>
> -static void hmp_info_registers(Monitor *mon, const QDict *qdict)
> +void hmp_info_registers(Monitor *mon, const QDict *qdict)
> {
> bool all_cpus = qdict_get_try_bool(qdict, "cpustate_all", false);
> CPUState *cs;
> @@ -338,7 +338,7 @@ static void hmp_info_registers(Monitor *mon, const QDict
> *qdict)
> }
>
> #ifdef CONFIG_TCG
> -static void hmp_info_jit(Monitor *mon, const QDict *qdict)
> +void hmp_info_jit(Monitor *mon, const QDict *qdict)
> {
> if (!tcg_enabled()) {
> error_report("JIT information is only available with accel=tcg");
> @@ -349,13 +349,13 @@ static void hmp_info_jit(Monitor *mon, const QDict
> *qdict)
> dump_drift_info();
> }
>
> -static void hmp_info_opcount(Monitor *mon, const QDict *qdict)
> +void hmp_info_opcount(Monitor *mon, const QDict *qdict)
> {
> dump_opcount_info();
> }
> #endif
>
> -static void hmp_info_sync_profile(Monitor *mon, const QDict *qdict)
> +void hmp_info_sync_profile(Monitor *mon, const QDict *qdict)
> {
> int64_t max = qdict_get_try_int(qdict, "max", 10);
> bool mean = qdict_get_try_bool(qdict, "mean", false);
> @@ -366,7 +366,7 @@ static void hmp_info_sync_profile(Monitor *mon, const
> QDict *qdict)
> qsp_report(max, sort_by, coalesce);
> }
>
> -static void hmp_info_history(Monitor *mon, const QDict *qdict)
> +void hmp_info_history(Monitor *mon, const QDict *qdict)
> {
> MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
> int i;
> @@ -386,7 +386,7 @@ static void hmp_info_history(Monitor *mon, const QDict
> *qdict)
> }
> }
>
> -static void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
> +void hmp_info_cpustats(Monitor *mon, const QDict *qdict)
> {
> CPUState *cs = mon_get_cpu();
>
> @@ -397,7 +397,7 @@ static void hmp_info_cpustats(Monitor *mon, const QDict
> *qdict)
> cpu_dump_statistics(cs, 0);
> }
>
> -static void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
> +void hmp_info_trace_events(Monitor *mon, const QDict *qdict)
> {
> const char *name = qdict_get_try_str(qdict, "name");
> bool has_vcpu = qdict_haskey(qdict, "vcpu");
> @@ -457,7 +457,7 @@ void qmp_client_migrate_info(const char *protocol, const
> char *hostname,
> error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "protocol", "spice");
> }
>
> -static void hmp_logfile(Monitor *mon, const QDict *qdict)
> +void hmp_logfile(Monitor *mon, const QDict *qdict)
> {
> Error *err = NULL;
>
> @@ -467,7 +467,7 @@ static void hmp_logfile(Monitor *m
[Bug 1821595] Re: Failed to emulate MMIO access with EmulatorReturnStatus: 2
Hi, I tried backing up ... nothing but startup and UEFI (i.e. no OS, just get to the EFI Shell). Can't even get there ... does that make sense? Thanks! -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1821595 Title: Failed to emulate MMIO access with EmulatorReturnStatus: 2 Status in QEMU: New Bug description: I have compiled qemu with enable-whpx parameter for Hyper-V Platform API in Mingw64 . When I tried run with Windows 7 iso file I have faced issue with the following problem: qemu-system-x86_64.exe: WHPX: Failed to emulate MMIO access with EmulatorReturnStatus: 2 qemu-system-x86_64.exe: WHPX: Failed to exec a virtual processor configuration directives: ../configure --target-list=x86_64-softmmu,i386-softmmu --enable-lzo\ --enable-bzip2 --enable-tools --enable-sdl --enable-gtk --enable-hax\ --enable-vdi --enable-qcow1 --enable-whpx --disable-capstone\ --disable-werror --disable-stack-protector --prefix="../../QEMU-bin" Qemu command line: qemu-system-x86_64.exe -m 1024 -cdrom "C:\Users\vmcs\Documents\en_windows_7_home_premium_with_sp1_x86_dvd_u_676701.iso" -display sdl -machine q35 -accel whpx To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1821595/+subscriptions
Re: [PATCH for-5.1 1/8] tests-qemu-opts: Cover has_help_option(), qemu_opt_has_help_opt()
On 4/9/20 10:30 AM, Markus Armbruster wrote:
The two turn out to be inconsistent for "a,b,,help". Test case
marked /* BUG */.
Signed-off-by: Markus Armbruster
---
tests/test-qemu-opts.c | 38 ++
1 file changed, 38 insertions(+)
+static void test_has_help_option(void)
+{
+static const struct {
+const char *params;
+/* expected value of has_help_option() */
+bool expect_has_help_option;
+/* expected value of qemu_opt_has_help_opt() with implied=false */
+bool expect_opt_has_help_opt;
+/* expected value of qemu_opt_has_help_opt() with implied=true */
+bool expect_opt_has_help_opt_implied;
+} test[] = {
+{ "help", true, true, false },
+{ "helpme", false, false, false },
+{ "a,help", true, true, true },
+{ "a=0,help,b", true, true, true },
+{ "help,b=1", true, true, false },
+{ "a,b,,help", false /* BUG */, true, true },
So which way are you calling the bug? Without looking at the code but
going off my intuition, I parse this as option 'a' and option 'b,help'.
The latter is not a normal option name because it contains a ',', but is
a valid option value.
I agree that we have a bug, but I'm not yet sure in which direction the
bug lies (should has_help_option be fixed to report true, in which case
the substring ",help" has precedence over ',,' escaping; or should
qemu_opt_has_help_opt be fixed to report false, due to treating 'b,help'
after ',,' escape removal as an invalid option name). So the placement
of the /* BUG */ comment matters - where you placed it, I'm presuming
that later in the series you change has_help_option to return true, even
though that goes against my intuitive parse.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH RESEND 0/3] travis-ci: Improve OSX coverage
On 2/25/20 10:18 AM, Philippe Mathieu-Daudé wrote: On 2/18/20 3:20 PM, Philippe Mathieu-Daudé wrote: Add more packages on the Mojave OSX job (Xcode 10), and duplicate the job to build on Catalina (Xcode 11). ping? Ping for patches 1 + 2 (extend OSX coverage)? Each job takes ~34min: https://travis-ci.org/philmd/qemu/builds/651473221 Philippe Mathieu-Daudé (3): .travis.yml: Expand OSX code coverage .travis.yml: Build with ncurses on OSX .travis.yml: Test building with Xcode 11.3 .travis.yml | 48 ++-- 1 file changed, 46 insertions(+), 2 deletions(-)
Re: [PATCH RESEND 3/3] .travis.yml: Test building with Xcode 11.3
On 2/25/20 1:29 PM, Alex Bennée wrote:
Philippe Mathieu-Daudé writes:
We currently run a CI job on macOS Mojave with Xcode 10.
QEMU policy is to support the two last major OS releases.
Add a job building on macOS Catalina, which comes with Xcode 11.
Split the target list in two, as we don't need to cover twice the
same targets.
Signed-off-by: Philippe Mathieu-Daudé
---
.travis.yml | 36 +++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/.travis.yml b/.travis.yml
index a2a7fd0dd1..d02a477623 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -269,9 +269,10 @@ matrix:
# MacOSX builds - cirrus.yml also tests some MacOS builds including latest Xcode
+# On macOS Mojave, the SDK comes bundled with Xcode 10.
- name: "OSX Xcode 10.3"
env:
--
CONFIG="--target-list=i386-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu,x86_64-softmmu
--extra-cflags=-I/usr/local/opt/ncurses/include
--extra-ldflags=-L/usr/local/opt/ncurses/lib"
+- CONFIG="--target-list=i386-softmmu,ppc-softmmu,ppc64-softmmu,m68k-softmmu
--extra-cflags=-I/usr/local/opt/ncurses/include
--extra-ldflags=-L/usr/local/opt/ncurses/lib"
os: osx
osx_image: xcode10.3
compiler: clang
@@ -301,6 +302,39 @@ matrix:
- ${SRC_DIR}/configure ${BASE_CONFIG} ${CONFIG} || { cat config.log
&& exit 1; }
+# On macOS Catalina, the SDK comes bundled with Xcode 11.
+- name: "OSX Xcode 11.3"
+ env:
+- CONFIG="--target-list=arm-softmmu,ppc64-softmmu,x86_64-softmmu
--extra-cflags=-I/usr/local/opt/ncurses/include
--extra-ldflags=-L/usr/local/opt/ncurses/lib"
+ os: osx
+ osx_image: xcode11.3
Are we duplicating what the latest Xcode on Cirrus is here?
Maybe, I'm not sure. It seems only few people care about
Cirrus/Shippable but they are not taken seriously by the community, as
they are often broken and nobody is notified. Currently Travis has a
broader audience.
Also I sent a series to fix various things that break on Cirrus from
time to time but I felt there is not many interest so I stopped spending
energy on it:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg675074.html
We could change that by refusing to merge pullreq that break such CI.
[PULL for-5.0 3/3] async: use explicit memory barriers
From: Paolo Bonzini
When using C11 atomics, non-seqcst reads and writes do not participate
in the total order of seqcst operations. In util/async.c and util/aio-posix.c,
in particular, the pattern that we use
write ctx->notify_me write bh->scheduled
read bh->scheduled read ctx->notify_me
if !bh->scheduled, sleep if ctx->notify_me, notify
needs to use seqcst operations for both the write and the read. In
general this is something that we do not want, because there can be
many sources that are polled in addition to bottom halves. The
alternative is to place a seqcst memory barrier between the write
and the read. This also comes with a disadvantage, in that the
memory barrier is implicit on strongly-ordered architectures and
it wastes a few dozen clock cycles.
Fortunately, ctx->notify_me is never written concurrently by two
threads, so we can assert that and relax the writes to ctx->notify_me.
The resulting solution works and performs well on both aarch64 and x86.
Note that the atomic_set/atomic_read combination is not an atomic
read-modify-write, and therefore it is even weaker than C11 ATOMIC_RELAXED;
on x86, ATOMIC_RELAXED compiles to a locked operation.
Analyzed-by: Ying Fang
Signed-off-by: Paolo Bonzini
Tested-by: Ying Fang
Message-Id: <20200407140746.8041-6-pbonz...@redhat.com>
Signed-off-by: Stefan Hajnoczi
---
util/aio-posix.c | 16 ++--
util/aio-win32.c | 17 ++---
util/async.c | 16
3 files changed, 40 insertions(+), 9 deletions(-)
diff --git a/util/aio-posix.c b/util/aio-posix.c
index cd6cf0a4a9..c3613d299e 100644
--- a/util/aio-posix.c
+++ b/util/aio-posix.c
@@ -559,6 +559,11 @@ bool aio_poll(AioContext *ctx, bool blocking)
int64_t timeout;
int64_t start = 0;
+/*
+ * There cannot be two concurrent aio_poll calls for the same AioContext
(or
+ * an aio_poll concurrent with a GSource prepare/check/dispatch callback).
+ * We rely on this below to avoid slow locked accesses to ctx->notify_me.
+ */
assert(in_aio_context_home_thread(ctx));
/* aio_notify can avoid the expensive event_notifier_set if
@@ -569,7 +574,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
* so disable the optimization now.
*/
if (blocking) {
-atomic_add(&ctx->notify_me, 2);
+atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) + 2);
+/*
+ * Write ctx->notify_me before computing the timeout
+ * (reading bottom half flags, etc.). Pairs with
+ * smp_mb in aio_notify().
+ */
+smp_mb();
}
qemu_lockcnt_inc(&ctx->list_lock);
@@ -590,7 +601,8 @@ bool aio_poll(AioContext *ctx, bool blocking)
}
if (blocking) {
-atomic_sub(&ctx->notify_me, 2);
+/* Finish the poll before clearing the flag. */
+atomic_store_release(&ctx->notify_me, atomic_read(&ctx->notify_me) -
2);
aio_notify_accept(ctx);
}
diff --git a/util/aio-win32.c b/util/aio-win32.c
index a23b9c364d..729d533faf 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -321,6 +321,12 @@ bool aio_poll(AioContext *ctx, bool blocking)
int count;
int timeout;
+/*
+ * There cannot be two concurrent aio_poll calls for the same AioContext
(or
+ * an aio_poll concurrent with a GSource prepare/check/dispatch callback).
+ * We rely on this below to avoid slow locked accesses to ctx->notify_me.
+ */
+assert(in_aio_context_home_thread(ctx));
progress = false;
/* aio_notify can avoid the expensive event_notifier_set if
@@ -331,7 +337,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
* so disable the optimization now.
*/
if (blocking) {
-atomic_add(&ctx->notify_me, 2);
+atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) + 2);
+/*
+ * Write ctx->notify_me before computing the timeout
+ * (reading bottom half flags, etc.). Pairs with
+ * smp_mb in aio_notify().
+ */
+smp_mb();
}
qemu_lockcnt_inc(&ctx->list_lock);
@@ -364,8 +376,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
ret = WaitForMultipleObjects(count, events, FALSE, timeout);
if (blocking) {
assert(first);
-assert(in_aio_context_home_thread(ctx));
-atomic_sub(&ctx->notify_me, 2);
+atomic_store_release(&ctx->notify_me, atomic_read(&ctx->notify_me)
- 2);
aio_notify_accept(ctx);
}
diff --git a/util/async.c b/util/async.c
index b94518b948..3165a28f2f 100644
--- a/util/async.c
+++ b/util/async.c
@@ -249,7 +249,14 @@ aio_ctx_prepare(GSource *source, gint*timeout)
{
AioContext *ctx = (AioContext *) source;
-atomic_or(&ctx->notify_me, 1);
+atomic_set(&ctx->notify_me, atomic_read(&ctx->notify_me) | 1);
+
+/*
+ * Write ctx->notify_me before computing the ti
Re: [PATCH for-5.1 0/8] qemu-option: Fix corner cases and clean up
On 4/9/20 12:09 PM, no-re...@patchew.org wrote:
Patchew URL: https://patchew.org/QEMU/20200409153041.17576-1-arm...@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
=== OUTPUT BEGIN ===
1/8 Checking commit 2e003109273b (tests-qemu-opts: Cover has_help_option(),
qemu_opt_has_help_opt())
WARNING: Block comments use a leading /* on a separate line
#37: FILE: tests/test-qemu-opts.c:747:
+{ "a,b,,help", false /* BUG */, true, true },
Annoying, but I don't mind ignoring it (especially since we fix the bug
later in the series).
total: 0 errors, 1 warnings, 50 lines checked
Patch 1/8 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/8 Checking commit 8bb805dd3730 (qemu-options: Factor out get_opt_name_value()
helper)
3/8 Checking commit b3630a346906 (qemu-option: Fix sloppy recognition of "id=..." after
", , ")
4/8 Checking commit 5c1b2db0b7ad (qemu-option: Avoid has_help_option() in
qemu_opts_parse_noisily())
5/8 Checking commit 6845c29bee11 (qemu-option: Fix has_help_option()'s sloppy
parsing)
6/8 Checking commit b7fcaaeeeb6f (test-qemu-opts: Simplify
test_has_help_option() after bug fix)
7/8 Checking commit b164930d4c8d (qemu-img: Factor out accumulate_options()
helper)
8/8 Checking commit 505f5f389855 (qemu-option: Move is_valid_option_list() to
qemu-img.c and rewrite)
ERROR: suspect code indent for conditional statements (4, 4)
#61: FILE: qemu-img.c:243:
+for (i = len; i > 0 && optarg[i - 1] == ','; i--) {
+}
False positive. Maybe you can shut up the checker by:
for (...) {
/* nothing further to do */
}
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PULL for-5.0 1/3] aio-posix: signal-proof fdmon-io_uring
The io_uring_enter(2) syscall returns with errno=EINTR when interrupted
by a signal. Retry the syscall in this case.
It's essential to do this in the io_uring_submit_and_wait() case. My
interpretation of the Linux v5.5 io_uring_enter(2) code is that it
shouldn't affect the io_uring_submit() case, but there is no guarantee
this will always be the case. Let's check for -EINTR around both APIs.
Note that the liburing APIs have -errno return values.
Signed-off-by: Stefan Hajnoczi
Reviewed-by: Stefano Garzarella
Reviewed-by: Philippe Mathieu-Daudé
Message-id: 20200408091139.273851-1-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi
---
util/fdmon-io_uring.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/util/fdmon-io_uring.c b/util/fdmon-io_uring.c
index b4d6109f20..d5a80ed6fb 100644
--- a/util/fdmon-io_uring.c
+++ b/util/fdmon-io_uring.c
@@ -88,7 +88,10 @@ static struct io_uring_sqe *get_sqe(AioContext *ctx)
}
/* No free sqes left, submit pending sqes first */
-ret = io_uring_submit(ring);
+do {
+ret = io_uring_submit(ring);
+} while (ret == -EINTR);
+
assert(ret > 1);
sqe = io_uring_get_sqe(ring);
assert(sqe);
@@ -282,7 +285,10 @@ static int fdmon_io_uring_wait(AioContext *ctx,
AioHandlerList *ready_list,
fill_sq_ring(ctx);
-ret = io_uring_submit_and_wait(&ctx->fdmon_io_uring, wait_nr);
+do {
+ret = io_uring_submit_and_wait(&ctx->fdmon_io_uring, wait_nr);
+} while (ret == -EINTR);
+
assert(ret >= 0);
return process_cq_ring(ctx, ready_list);
--
2.25.1
[PULL for-5.0 0/3] Block patches
The following changes since commit 8bac3ba57eecc466b7e73dabf7d19328a59f684e: Merge remote-tracking branch 'remotes/rth/tags/pull-rx-20200408' into staging (2020-04-09 13:23:30 +0100) are available in the Git repository at: https://github.com/stefanha/qemu.git tags/block-pull-request for you to fetch changes up to 5710a3e09f9b85801e5ce70797a4a511e5fc9e2c: async: use explicit memory barriers (2020-04-09 16:17:14 +0100) Pull request Fixes for QEMU on aarch64 ARM hosts and fdmon-io_uring. Paolo Bonzini (2): aio-wait: delegate polling of main AioContext if BQL not held async: use explicit memory barriers Stefan Hajnoczi (1): aio-posix: signal-proof fdmon-io_uring include/block/aio-wait.h | 22 ++ include/block/aio.h | 29 ++--- util/aio-posix.c | 16 ++-- util/aio-win32.c | 17 ++--- util/async.c | 16 util/fdmon-io_uring.c| 10 -- 6 files changed, 80 insertions(+), 30 deletions(-) -- 2.25.1
[PULL for-5.0 2/3] aio-wait: delegate polling of main AioContext if BQL not held
From: Paolo Bonzini
Any thread that is not a iothread returns NULL for
qemu_get_current_aio_context().
As a result, it would also return true for
in_aio_context_home_thread(qemu_get_aio_context()), causing
AIO_WAIT_WHILE to invoke aio_poll() directly. This is incorrect
if the BQL is not held, because aio_poll() does not expect to
run concurrently from multiple threads, and it can actually
happen when savevm writes to the vmstate file from the
migration thread.
Therefore, restrict in_aio_context_home_thread to return true
for the main AioContext only if the BQL is held.
The function is moved to aio-wait.h because it is mostly used
there and to avoid a circular reference between main-loop.h
and block/aio.h.
Signed-off-by: Paolo Bonzini
Message-Id: <20200407140746.8041-5-pbonz...@redhat.com>
Signed-off-by: Stefan Hajnoczi
---
include/block/aio-wait.h | 22 ++
include/block/aio.h | 29 ++---
2 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h
index afeeb18f95..716d2639df 100644
--- a/include/block/aio-wait.h
+++ b/include/block/aio-wait.h
@@ -26,6 +26,7 @@
#define QEMU_AIO_WAIT_H
#include "block/aio.h"
+#include "qemu/main-loop.h"
/**
* AioWait:
@@ -124,4 +125,25 @@ void aio_wait_kick(void);
*/
void aio_wait_bh_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
+/**
+ * in_aio_context_home_thread:
+ * @ctx: the aio context
+ *
+ * Return whether we are running in the thread that normally runs @ctx. Note
+ * that acquiring/releasing ctx does not affect the outcome, each AioContext
+ * still only has one home thread that is responsible for running it.
+ */
+static inline bool in_aio_context_home_thread(AioContext *ctx)
+{
+if (ctx == qemu_get_current_aio_context()) {
+return true;
+}
+
+if (ctx == qemu_get_aio_context()) {
+return qemu_mutex_iothread_locked();
+} else {
+return false;
+}
+}
+
#endif /* QEMU_AIO_WAIT_H */
diff --git a/include/block/aio.h b/include/block/aio.h
index cb1989105a..62ed954344 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -133,12 +133,16 @@ struct AioContext {
AioHandlerList deleted_aio_handlers;
/* Used to avoid unnecessary event_notifier_set calls in aio_notify;
- * accessed with atomic primitives. If this field is 0, everything
- * (file descriptors, bottom halves, timers) will be re-evaluated
- * before the next blocking poll(), thus the event_notifier_set call
- * can be skipped. If it is non-zero, you may need to wake up a
- * concurrent aio_poll or the glib main event loop, making
- * event_notifier_set necessary.
+ * only written from the AioContext home thread, or under the BQL in
+ * the case of the main AioContext. However, it is read from any
+ * thread so it is still accessed with atomic primitives.
+ *
+ * If this field is 0, everything (file descriptors, bottom halves,
+ * timers) will be re-evaluated before the next blocking poll() or
+ * io_uring wait; therefore, the event_notifier_set call can be
+ * skipped. If it is non-zero, you may need to wake up a concurrent
+ * aio_poll or the glib main event loop, making event_notifier_set
+ * necessary.
*
* Bit 0 is reserved for GSource usage of the AioContext, and is 1
* between a call to aio_ctx_prepare and the next call to aio_ctx_check.
@@ -681,19 +685,6 @@ void aio_co_enter(AioContext *ctx, struct Coroutine *co);
*/
AioContext *qemu_get_current_aio_context(void);
-/**
- * in_aio_context_home_thread:
- * @ctx: the aio context
- *
- * Return whether we are running in the thread that normally runs @ctx. Note
- * that acquiring/releasing ctx does not affect the outcome, each AioContext
- * still only has one home thread that is responsible for running it.
- */
-static inline bool in_aio_context_home_thread(AioContext *ctx)
-{
-return ctx == qemu_get_current_aio_context();
-}
-
/**
* aio_context_setup:
* @ctx: the aio context
--
2.25.1
[Bug 1871250] Re: Failed to create HAX VM
NP, will do. Thanks again for the help! ** Changed in: qemu Status: New => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1871250 Title: Failed to create HAX VM Status in QEMU: Fix Committed Bug description: Hi, I'm running the latest (master) of QEMU, though the version doesn't seem to matter - I also checked back to v4.2.0, exactly the same issue. And this isn't about the VM (guest), if I even just try to run, > "c:\Program Files\qemu\qemu-system-x86_64.exe" -accel hax Basically, just get a window to open, with acceleration enabled ... I get, Open the vm device error:/dev/hax_vm/vm00, ec:3 Failed to open vm 0 Failed to create HAX VM No accelerator found. But I checked - I have installed Intel HAXM, and verified it's running, > sc query intelhaxm SERVICE_NAME: intelhaxm TYPE : 1 KERNEL_DRIVER STATE : 4 RUNNING (STOPPABLE, NOT_PAUSABLE, IGNORES_SHUTDOWN) WIN32_EXIT_CODE: 0 (0x0) SERVICE_EXIT_CODE : 0 (0x0) CHECKPOINT : 0x0 WAIT_HINT : 0x0 Just remove the accelerator (-accel hax), and I get a window - so this is related to QEMU being able to contact / use the accelerator. Help!?!? Thanks! To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1871250/+subscriptions
Re: [PATCH 2/2] target/arm: Fail on invalid size for VMUL (float)
On 08.04.2020 18:27, Peter Maydell wrote: > On Wed, 8 Apr 2020 at 16:29, Fredrik Strupe wrote: >> >> Bit 1 of VMUL (float)'s size field encodes the opcode and must be 0, >> with 1 making it undefined. Thus, make VMUL (float) instructions >> with size=0b10 or size=0b11 (size >= 2) undefined. >> >> (U is 1 for VMUL, while it is 0 for VMLA/VMLS.) >> >> Signed-off-by: Fredrik Strupe >> --- >> target/arm/translate.c | 5 + >> 1 file changed, 5 insertions(+) > > Thanks for this patch. I'm actually in the middle of a > refactoring of this code to use decodetree, but I'll make > sure I check that the refactoring fixes this decode bug. > > Also undef-checks like this in the old neon decode structure > should be in the switch (op) outside the loop-for-each-element: > compare NEON_3R_FLOAT_CMP; but it's a bit moot with the > refactoring as all that code will be deleted. > > thanks > -- PMM Alright, glad to see it being fixed. Fredrik
[Bug 1871798] Re: Fails to start on Windows host without explicit --disable-pie
What compiler and toolchain are you using? ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1871798 Title: Fails to start on Windows host without explicit --disable-pie Status in QEMU: Incomplete Bug description: Since commit d2cd29e30736afd4a1e8cac3cf4da360bbc65978, which removed the x86 conditional around PIE, QEMU completely fails to start on a Windows host unless --disable-pie is explicitly given at build time. Even just requesting the help text doesn't work. To make testing easier, this can be replicated with Wine. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1871798/+subscriptions
Re: [PATCH v18 QEMU 3/3] virtio-balloon: Provide a interface for free page reporting
On 09.04.20 19:34, Alexander Duyck wrote:
> On Thu, Apr 9, 2020 at 12:44 AM David Hildenbrand wrote:
>>
>> On 09.04.20 00:55, Alexander Duyck wrote:
>>> From: Alexander Duyck
>>>
>>> Add support for what I am referring to as "free page reporting".
>>
>> "Add support for "free page reporting".
>>
>>> Basically the idea is to function very similar to how the balloon works
>>> in that we basically end up madvising the page as not being used. However
>>
>> I'd get rid of one "basically".
>>
>>> we don't really need to bother with any deflate type logic since the page
>>> will be faulted back into the guest when it is read or written to.
>>>
>>> This is meant to be a simplification of the existing balloon interface
>>> to use for providing hints to what memory needs to be freed. I am assuming
>>
>> It's not really a simplification, it's something new. It's a new way of
>> letting the guest automatically report free pages to the hypervisor, so
>> the hypervisor can reuse them. In contrast to inflate/deflate, that's
>> triggered via the hypervisor explicitly.
>>
>>> this is safe to do as the deflate logic does not actually appear to do very
>>> much other than tracking what subpages have been released and which ones
>>> haven't.
>>
>> "I assume this is safe" does not sound very confident. Can we just drop
>> the last sentence?
>
> Agreed. I will make the requested changes to the patch description.
>
>>
>>>
>>> Signed-off-by: Alexander Duyck
>>> ---
>>> hw/virtio/virtio-balloon.c | 48
>>> +++-
>>> include/hw/virtio/virtio-balloon.h |2 +-
>>> 2 files changed, 47 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>>> index 1c6d36a29a04..297b267198ac 100644
>>> --- a/hw/virtio/virtio-balloon.c
>>> +++ b/hw/virtio/virtio-balloon.c
>>> @@ -321,6 +321,42 @@ static void balloon_stats_set_poll_interval(Object
>>> *obj, Visitor *v,
>>> balloon_stats_change_timer(s, 0);
>>> }
>>>
>>> +static void virtio_balloon_handle_report(VirtIODevice *vdev, VirtQueue *vq)
>>> +{
>>> +VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
>>> +VirtQueueElement *elem;
>>> +
>>> +while ((elem = virtqueue_pop(vq, sizeof(VirtQueueElement {
>>> +unsigned int i;
>>> +
>>> +for (i = 0; i < elem->in_num; i++) {
>>> +void *addr = elem->in_sg[i].iov_base;
>>> +size_t size = elem->in_sg[i].iov_len;
>>> +ram_addr_t ram_offset;
>>> +size_t rb_page_size;
>>> +RAMBlock *rb;
>>> +
>>> +if (qemu_balloon_is_inhibited() || dev->poison_val) {
>>> +continue;
>>> +}
>>
>> These checks are not sufficient. See virtio_balloon_handle_output(),
>> where we e.g., check that somebody doesn't try to discard the bios.
>>
>> Maybe we can factor our/unify the handling in both code paths.
>
> I am going to have to look at this further. If I understand correctly
> you are asking me to add all of the memory section checks that are in
> virtio_balloon_handle_output correct?
>
> I'm not sure it makes sense with the scatterlist approach I have taken
> here. All of the mappings were created as a scatterlist of writable
> DMA mappings unlike the regular balloon which is just stuffing PFN
> numbers into an array and then sending the array across. As such I
> would think there are already such protections in place. For instance,
> you wouldn't want to let virtio-net map the BIOS and request data be
> written into that either, correct? So I am assuming there is something
> there to prevent that already isn't there?
Good point, let's find out :)
--
Thanks,
David / dhildenb
Re: [PATCH v18 QEMU 3/3] virtio-balloon: Provide a interface for free page reporting
On Thu, Apr 9, 2020 at 12:44 AM David Hildenbrand wrote:
>
> On 09.04.20 00:55, Alexander Duyck wrote:
> > From: Alexander Duyck
> >
> > Add support for what I am referring to as "free page reporting".
>
> "Add support for "free page reporting".
>
> > Basically the idea is to function very similar to how the balloon works
> > in that we basically end up madvising the page as not being used. However
>
> I'd get rid of one "basically".
>
> > we don't really need to bother with any deflate type logic since the page
> > will be faulted back into the guest when it is read or written to.
> >
> > This is meant to be a simplification of the existing balloon interface
> > to use for providing hints to what memory needs to be freed. I am assuming
>
> It's not really a simplification, it's something new. It's a new way of
> letting the guest automatically report free pages to the hypervisor, so
> the hypervisor can reuse them. In contrast to inflate/deflate, that's
> triggered via the hypervisor explicitly.
>
> > this is safe to do as the deflate logic does not actually appear to do very
> > much other than tracking what subpages have been released and which ones
> > haven't.
>
> "I assume this is safe" does not sound very confident. Can we just drop
> the last sentence?
Agreed. I will make the requested changes to the patch description.
>
> >
> > Signed-off-by: Alexander Duyck
> > ---
> > hw/virtio/virtio-balloon.c | 48
> > +++-
> > include/hw/virtio/virtio-balloon.h |2 +-
> > 2 files changed, 47 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> > index 1c6d36a29a04..297b267198ac 100644
> > --- a/hw/virtio/virtio-balloon.c
> > +++ b/hw/virtio/virtio-balloon.c
> > @@ -321,6 +321,42 @@ static void balloon_stats_set_poll_interval(Object
> > *obj, Visitor *v,
> > balloon_stats_change_timer(s, 0);
> > }
> >
> > +static void virtio_balloon_handle_report(VirtIODevice *vdev, VirtQueue *vq)
> > +{
> > +VirtIOBalloon *dev = VIRTIO_BALLOON(vdev);
> > +VirtQueueElement *elem;
> > +
> > +while ((elem = virtqueue_pop(vq, sizeof(VirtQueueElement {
> > +unsigned int i;
> > +
> > +for (i = 0; i < elem->in_num; i++) {
> > +void *addr = elem->in_sg[i].iov_base;
> > +size_t size = elem->in_sg[i].iov_len;
> > +ram_addr_t ram_offset;
> > +size_t rb_page_size;
> > +RAMBlock *rb;
> > +
> > +if (qemu_balloon_is_inhibited() || dev->poison_val) {
> > +continue;
> > +}
>
> These checks are not sufficient. See virtio_balloon_handle_output(),
> where we e.g., check that somebody doesn't try to discard the bios.
>
> Maybe we can factor our/unify the handling in both code paths.
I am going to have to look at this further. If I understand correctly
you are asking me to add all of the memory section checks that are in
virtio_balloon_handle_output correct?
I'm not sure it makes sense with the scatterlist approach I have taken
here. All of the mappings were created as a scatterlist of writable
DMA mappings unlike the regular balloon which is just stuffing PFN
numbers into an array and then sending the array across. As such I
would think there are already such protections in place. For instance,
you wouldn't want to let virtio-net map the BIOS and request data be
written into that either, correct? So I am assuming there is something
there to prevent that already isn't there?
> > +
> > +rb = qemu_ram_block_from_host(addr, false, &ram_offset);
> > +rb_page_size = qemu_ram_pagesize(rb);
> > +
> > +/* For now we will simply ignore unaligned memory regions */
> > +if ((ram_offset | size) & (rb_page_size - 1)) {
>
> "!QEMU_IS_ALIGNED()" please to make this easier to read.
done.
> > +continue;
> > +}
> > +
> > +ram_block_discard_range(rb, ram_offset, size);
> > +}
> > +
> > +virtqueue_push(vq, elem, 0);
> > +virtio_notify(vdev, vq);
> > +g_free(elem);
> > +}
> > +}
> > +
>
> [...]
>
> > if (virtio_has_feature(s->host_features,
> > VIRTIO_BALLOON_F_FREE_PAGE_HINT)) {
> > s->free_page_vq = virtio_add_queue(vdev, VIRTQUEUE_MAX_SIZE,
> > @@ -940,6 +982,8 @@ static Property virtio_balloon_properties[] = {
> > */
> > DEFINE_PROP_BOOL("qemu-4-0-config-size", VirtIOBalloon,
> > qemu_4_0_config_size, false),
> > +DEFINE_PROP_BIT("unused-page-reporting", VirtIOBalloon, host_features,
>
> "free-page-reporting"
Thanks for catching that. It has been a while since that was renamed
and I must have let it slip through the cracks.
- Alex
[PATCH-for-5.0] target/m68k/helper: Fix m68k_fpu_gdb_get_reg() use of GByteArray
Since a010bdbe719 the gdbstub API takes a GByteArray*.
Unfortunately we forgot to update the gdb_get_reg*()
calls. Do it now.
Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Reported-by: Peter Xu
Signed-off-by: Philippe Mathieu-Daudé
---
Based-on: <20200409164954.36902-1-pet...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé
---
target/m68k/helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/m68k/helper.c b/target/m68k/helper.c
index 014657c637..cad4083895 100644
--- a/target/m68k/helper.c
+++ b/target/m68k/helper.c
@@ -109,8 +109,8 @@ static int m68k_fpu_gdb_get_reg(CPUM68KState *env,
GByteArray *mem_buf, int n)
{
if (n < 8) {
int len = gdb_get_reg16(mem_buf, env->fregs[n].l.upper);
-len += gdb_get_reg16(mem_buf + len, 0);
-len += gdb_get_reg64(mem_buf + len, env->fregs[n].l.lower);
+len += gdb_get_reg16(mem_buf, 0);
+len += gdb_get_reg64(mem_buf, env->fregs[n].l.lower);
return len;
}
switch (n) {
--
2.21.1
Re: [PATCH 2/2] gdbstub: i386: Fix gdb_get_reg16() parameter to unbreak gdb
On 4/9/20 6:49 PM, Peter Xu wrote:
We should only pass in gdb_get_reg16() with the GByteArray* object
itself, no need to shift. Without this patch, gdb remote attach will
crash QEMU.
You are correct.
Fixes: a010bdbe719 ("extend GByteArray to read register helpers")
Reviewed-by: Philippe Mathieu-Daudé
Same problem in m68k_fpu_gdb_get_reg().
TODO for 5.1, rename mem_buf -> array.
Cc: Alex Bennée
Cc: Philippe Mathieu-Daudé
Signed-off-by: Peter Xu
---
target/i386/gdbstub.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/gdbstub.c b/target/i386/gdbstub.c
index f3d23b614e..b98a99500a 100644
--- a/target/i386/gdbstub.c
+++ b/target/i386/gdbstub.c
@@ -106,7 +106,7 @@ int x86_cpu_gdb_read_register(CPUState *cs, GByteArray
*mem_buf, int n)
} else if (n >= IDX_FP_REGS && n < IDX_FP_REGS + 8) {
floatx80 *fp = (floatx80 *) &env->fpregs[n - IDX_FP_REGS];
int len = gdb_get_reg64(mem_buf, cpu_to_le64(fp->low));
-len += gdb_get_reg16(mem_buf + len, cpu_to_le16(fp->high));
+len += gdb_get_reg16(mem_buf, cpu_to_le16(fp->high));
return len;
} else if (n >= IDX_XMM_REGS && n < IDX_XMM_REGS + CPU_NB_REGS) {
n -= IDX_XMM_REGS;
Re: [PATCH for-5.1 0/8] qemu-option: Fix corner cases and clean up
Patchew URL: https://patchew.org/QEMU/20200409153041.17576-1-arm...@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH for-5.1 0/8] qemu-option: Fix corner cases and clean up
Message-id: 20200409153041.17576-1-arm...@redhat.com
Type: series
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20200409164954.36902-1-pet...@redhat.com ->
patchew/20200409164954.36902-1-pet...@redhat.com
Switched to a new branch 'test'
505f5f3 qemu-option: Move is_valid_option_list() to qemu-img.c and rewrite
b164930 qemu-img: Factor out accumulate_options() helper
b7fcaae test-qemu-opts: Simplify test_has_help_option() after bug fix
6845c29 qemu-option: Fix has_help_option()'s sloppy parsing
5c1b2db qemu-option: Avoid has_help_option() in qemu_opts_parse_noisily()
b3630a3 qemu-option: Fix sloppy recognition of "id=..." after ", , "
8bb805d qemu-options: Factor out get_opt_name_value() helper
2e00310 tests-qemu-opts: Cover has_help_option(), qemu_opt_has_help_opt()
=== OUTPUT BEGIN ===
1/8 Checking commit 2e003109273b (tests-qemu-opts: Cover has_help_option(),
qemu_opt_has_help_opt())
WARNING: Block comments use a leading /* on a separate line
#37: FILE: tests/test-qemu-opts.c:747:
+{ "a,b,,help", false /* BUG */, true, true },
total: 0 errors, 1 warnings, 50 lines checked
Patch 1/8 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/8 Checking commit 8bb805dd3730 (qemu-options: Factor out get_opt_name_value()
helper)
3/8 Checking commit b3630a346906 (qemu-option: Fix sloppy recognition of
"id=..." after ", , ")
4/8 Checking commit 5c1b2db0b7ad (qemu-option: Avoid has_help_option() in
qemu_opts_parse_noisily())
5/8 Checking commit 6845c29bee11 (qemu-option: Fix has_help_option()'s sloppy
parsing)
6/8 Checking commit b7fcaaeeeb6f (test-qemu-opts: Simplify
test_has_help_option() after bug fix)
7/8 Checking commit b164930d4c8d (qemu-img: Factor out accumulate_options()
helper)
8/8 Checking commit 505f5f389855 (qemu-option: Move is_valid_option_list() to
qemu-img.c and rewrite)
ERROR: suspect code indent for conditional statements (4, 4)
#61: FILE: qemu-img.c:243:
+for (i = len; i > 0 && optarg[i - 1] == ','; i--) {
+}
total: 1 errors, 0 warnings, 67 lines checked
Patch 8/8 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20200409153041.17576-1-arm...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] exec: fetch the alignment of Linux devdax pmem character device nodes
On 09/04/20 18:46, Dan Williams wrote: > Alternatively maybe you can use libdaxctl that automatically handles > the ABI differences between compat-dax-class and dax-bus layouts? I > didn't recommend it before because I was not sure about qemu's policy > about taking on new library dependencies In this case I think the new dependency is more than justified, thanks! Paolo
