* [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18
@ 2017-10-18 16:11 Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 01/29] checkpatch: refine mode selection Paolo Bonzini
` (30 more replies)
0 siblings, 31 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:11 UTC (permalink / raw)
To: qemu-devel
The following changes since commit a0b261db8c030813e30a39eae47359ac2a37f7e2:
Merge remote-tracking branch 'remotes/ehabkost/tags/python-next-pull-request' into staging (2017-10-12 10:02:09 +0100)
are available in the git repository at:
git://github.com/bonzini/qemu.git tags/for-upstream
for you to fetch changes up to 3da023b5827543ee4c022986ea2ad9d1274410b2:
scsi: reject configurations with logical block size > physical block size (2017-10-18 11:56:14 +0200)
----------------------------------------------------------------
* TCG 8-byte atomic accesses bugfix (Andrew)
* Report disk rotation rate (Daniel)
* Report invalid scsi-disk block size configuration (Mark)
* KVM and memory API MemoryListener fixes (David, Maxime, Peter Xu)
* x86 CPU hotplug crash fix (Igor)
* Load/store API documentation (Peter Maydell)
* Small fixes by myself and Thomas
* qdev DEVICE_DELETED deferral (Michael)
----------------------------------------------------------------
Andrew Baumann (1):
notdirty_mem_write: implement 8-byte accesses
Daniel P. Berrange (3):
scsi-disk: support reporting of rotation rate
ide: support reporting of rotation rate
char: don't skip client cleanup if 'connected' flag is unset
David Hildenbrand (7):
memory: call log_start after region_add
kvm: fix alignment of ram address
kvm: tolerate non-existing slot for log_start/log_stop/log_sync
kvm: fix error message when failing to unregister slot
kvm: region_add and region_del is not called on updates
kvm: simplify kvm_align_section()
memory: reuse section_from_flat_range()
Igor Mammedov (1):
pc: make sure that plugged CPUs are of the same type
Mark Kanda (1):
scsi: reject configurations with logical block size > physical block size
Maxime Coquelin (1):
memory: fix off-by-one error in memory_region_notify_one()
Michael Roth (3):
qdev: store DeviceState's canonical path to use when unparenting
Revert "qdev: Free QemuOpts when the QOM path goes away"
qdev: defer DEVICE_DEL event until instance_finalize()
Paolo Bonzini (8):
checkpatch: refine mode selection
build: remove CONFIG_LIBDECNUMBER
nios2: define tcg_env
tco: add trace events
target/i386: introduce x86_ld*_code
target/i386: trap on instructions longer than >15 bytes
watch_mem_write: implement 8-byte accesses
qemu-pr-helper: use new libmultipath API
Peter Maydell (1):
docs/devel/loads-stores.rst: Document our various load and store APIs
Peter Xu (2):
exec: add page_mask for flatview_do_translate
exec: simplify address_space_get_iotlb_entry
Thomas Huth (1):
disas: Always initialize read_memory_inner_func properly
Makefile.target | 6 -
accel/kvm/kvm-all.c | 39 ++-
chardev/char-socket.c | 19 +-
configure | 12 +-
default-configs/ppc-linux-user.mak | 1 -
default-configs/ppc-softmmu.mak | 1 -
default-configs/ppc64-linux-user.mak | 1 -
default-configs/ppc64-softmmu.mak | 1 -
default-configs/ppc64abi32-linux-user.mak | 1 -
default-configs/ppc64le-linux-user.mak | 1 -
default-configs/ppcemb-softmmu.mak | 1 -
disas.c | 1 -
docs/devel/loads-stores.rst | 396 ++++++++++++++++++++++++++++++
exec.c | 109 ++++++--
hw/acpi/tco.c | 11 +-
hw/acpi/trace-events | 4 +
hw/core/qdev.c | 32 ++-
hw/i386/pc.c | 7 +
hw/ide/core.c | 1 +
hw/ide/qdev.c | 1 +
hw/scsi/scsi-disk.c | 28 +++
include/disas/bfd.h | 1 +
include/hw/ide/internal.h | 8 +
include/hw/qdev-core.h | 1 +
libdecnumber/Makefile.objs | 5 +
memory.c | 18 +-
scripts/checkpatch.pl | 19 +-
scsi/qemu-pr-helper.c | 17 +-
target/i386/translate.c | 257 ++++++++++---------
target/nios2/translate.c | 1 +
target/ppc/Makefile.objs | 1 +
31 files changed, 777 insertions(+), 224 deletions(-)
create mode 100644 docs/devel/loads-stores.rst
create mode 100644 libdecnumber/Makefile.objs
--
1.8.3.1
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 01/29] checkpatch: refine mode selection
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
@ 2017-10-18 16:11 ` Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 02/29] scsi-disk: support reporting of rotation rate Paolo Bonzini
` (29 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:11 UTC (permalink / raw)
To: qemu-devel
stgit produces patch files that lack the ".patch" extensions. Others
might be using ".diff" too. But since we are already limiting source files
to only a handful of extensions, we can reuse that in the mode selection
code.
While at it, do not match "../foo" as a branch name.
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
scripts/checkpatch.pl | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 0c41f12..ce9f08e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -11,6 +11,8 @@ use warnings;
my $P = $0;
$P =~ s@.*/@@g;
+our $SrcFile = qr{\.(?:h|c|cpp|s|S|pl|py|sh)$};
+
my $V = '0.31';
use Getopt::Long qw(:config no_auto_abbrev);
@@ -101,30 +103,29 @@ if ($#ARGV < 0) {
}
if (!defined $chk_branch && !defined $chk_patch && !defined $file) {
- $chk_branch = $ARGV[0] =~ /\.\./ ? 1 : 0;
- $chk_patch = $chk_branch ? 0 :
- $ARGV[0] =~ /\.patch$/ || $ARGV[0] eq "-" ? 1 : 0;
- $file = $chk_branch || $chk_patch ? 0 : 1;
+ $chk_branch = $ARGV[0] =~ /.\.\./ ? 1 : 0;
+ $file = $ARGV[0] =~ /$SrcFile/ ? 1 : 0;
+ $chk_patch = $chk_branch || $file ? 0 : 1;
} elsif (!defined $chk_branch && !defined $chk_patch) {
if ($file) {
$chk_branch = $chk_patch = 0;
} else {
- $chk_branch = $ARGV[0] =~ /\.\./ ? 1 : 0;
+ $chk_branch = $ARGV[0] =~ /.\.\./ ? 1 : 0;
$chk_patch = $chk_branch ? 0 : 1;
}
} elsif (!defined $chk_branch && !defined $file) {
if ($chk_patch) {
$chk_branch = $file = 0;
} else {
- $chk_branch = $ARGV[0] =~ /\.\./ ? 1 : 0;
+ $chk_branch = $ARGV[0] =~ /.\.\./ ? 1 : 0;
$file = $chk_branch ? 0 : 1;
}
} elsif (!defined $chk_patch && !defined $file) {
if ($chk_branch) {
$chk_patch = $file = 0;
} else {
- $chk_patch = $ARGV[0] =~ /\.patch$/ || $ARGV[0] eq "-" ? 1 : 0;
- $file = $chk_patch ? 0 : 1;
+ $file = $ARGV[0] =~ /$SrcFile/ ? 1 : 0;
+ $chk_patch = $file ? 0 : 1;
}
} elsif (!defined $chk_branch) {
$chk_branch = $chk_patch || $file ? 0 : 1;
@@ -1443,7 +1444,7 @@ sub process {
}
# check we are in a valid source file if not then ignore this hunk
- next if ($realfile !~ /\.(h|c|cpp|s|S|pl|py|sh)$/);
+ next if ($realfile !~ /$SrcFile/);
#90 column limit
if ($line =~ /^\+/ &&
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 02/29] scsi-disk: support reporting of rotation rate
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 01/29] checkpatch: refine mode selection Paolo Bonzini
@ 2017-10-18 16:11 ` Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 03/29] ide: " Paolo Bonzini
` (28 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:11 UTC (permalink / raw)
To: qemu-devel
From: "Daniel P. Berrange" <berrange@redhat.com>
The Linux kernel will query the SCSI "Block device characteristics"
VPD to determine the rotations per minute of the disk. If this has
the value 1, it is taken to be an SSD and so Linux sets the
'rotational' flag to 0 for the I/O queue and will stop using that
disk as a source of random entropy. Other operating systems may
also take into account rotation rate when setting up default
behaviour.
Mgmt apps should be able to set the rotation rate for virtualized
block devices, based on characteristics of the host storage in use,
so that the guest OS gets sensible behaviour out of the box. This
patch thus adds a 'rotation-rate' parameter for 'scsi-hd' and
'scsi-block' device types. For the latter, this parameter will be
ignored unless the host device has TYPE_DISK.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <20171004114008.14849-2-berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-disk.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 6e841fb..a518080 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -104,6 +104,14 @@ typedef struct SCSIDiskState
char *product;
bool tray_open;
bool tray_locked;
+ /*
+ * 0x0000 - rotation rate not reported
+ * 0x0001 - non-rotating medium (SSD)
+ * 0x0002-0x0400 - reserved
+ * 0x0401-0xffe - rotations per minute
+ * 0xffff - reserved
+ */
+ uint16_t rotation_rate;
} SCSIDiskState;
static bool scsi_handle_rw_error(SCSIDiskReq *r, int error, bool acct_failed);
@@ -605,6 +613,7 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
outbuf[buflen++] = 0x83; // device identification
if (s->qdev.type == TYPE_DISK) {
outbuf[buflen++] = 0xb0; // block limits
+ outbuf[buflen++] = 0xb1; /* block device characteristics */
outbuf[buflen++] = 0xb2; // thin provisioning
}
break;
@@ -747,6 +756,15 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf)
outbuf[43] = max_io_sectors & 0xff;
break;
}
+ case 0xb1: /* block device characteristics */
+ {
+ buflen = 8;
+ outbuf[4] = (s->rotation_rate >> 8) & 0xff;
+ outbuf[5] = s->rotation_rate & 0xff;
+ outbuf[6] = 0;
+ outbuf[7] = 0;
+ break;
+ }
case 0xb2: /* thin provisioning */
{
buflen = 8;
@@ -2911,6 +2929,7 @@ static Property scsi_hd_properties[] = {
DEFAULT_MAX_UNMAP_SIZE),
DEFINE_PROP_UINT64("max_io_size", SCSIDiskState, max_io_size,
DEFAULT_MAX_IO_SIZE),
+ DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0),
DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf),
DEFINE_PROP_END_OF_LIST(),
};
@@ -2982,6 +3001,7 @@ static const TypeInfo scsi_cd_info = {
static Property scsi_block_properties[] = {
DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \
DEFINE_PROP_DRIVE("drive", SCSIDiskState, qdev.conf.blk),
+ DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0),
DEFINE_PROP_END_OF_LIST(),
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 03/29] ide: support reporting of rotation rate
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 01/29] checkpatch: refine mode selection Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 02/29] scsi-disk: support reporting of rotation rate Paolo Bonzini
@ 2017-10-18 16:11 ` Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 04/29] char: don't skip client cleanup if 'connected' flag is unset Paolo Bonzini
` (27 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:11 UTC (permalink / raw)
To: qemu-devel
From: "Daniel P. Berrange" <berrange@redhat.com>
The Linux kernel will query the ATA IDENTITY DEVICE data, word 217
to determine the rotations per minute of the disk. If this has
the value 1, it is taken to be an SSD and so Linux sets the
'rotational' flag to 0 for the I/O queue and will stop using that
disk as a source of random entropy. Other operating systems may
also take into account rotation rate when setting up default
behaviour.
Mgmt apps should be able to set the rotation rate for virtualized
block devices, based on characteristics of the host storage in use,
so that the guest OS gets sensible behaviour out of the box. This
patch thus adds a 'rotation-rate' parameter for 'ide-hd' device
types.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <20171004114008.14849-3-berrange@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/ide/core.c | 1 +
hw/ide/qdev.c | 1 +
include/hw/ide/internal.h | 8 ++++++++
3 files changed, 10 insertions(+)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 5f1cd3b..a04766a 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -208,6 +208,7 @@ static void ide_identify(IDEState *s)
if (dev && dev->conf.discard_granularity) {
put_le16(p + 169, 1); /* TRIM support */
}
+ put_le16(p + 217, dev->rotation_rate); /* Nominal media rotation rate */
ide_identify_size(s);
s->identify_set = 1;
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index d60ac25..a5181b4 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -299,6 +299,7 @@ static Property ide_hd_properties[] = {
DEFINE_BLOCK_CHS_PROPERTIES(IDEDrive, dev.conf),
DEFINE_PROP_BIOS_CHS_TRANS("bios-chs-trans",
IDEDrive, dev.chs_trans, BIOS_ATA_TRANSLATION_AUTO),
+ DEFINE_PROP_UINT16("rotation_rate", IDEDrive, dev.rotation_rate, 0),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index e641012..31851b4 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -508,6 +508,14 @@ struct IDEDevice {
char *serial;
char *model;
uint64_t wwn;
+ /*
+ * 0x0000 - rotation rate not reported
+ * 0x0001 - non-rotating medium (SSD)
+ * 0x0002-0x0400 - reserved
+ * 0x0401-0xffe - rotations per minute
+ * 0xffff - reserved
+ */
+ uint16_t rotation_rate;
};
/* These are used for the error_status field of IDEBus */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 04/29] char: don't skip client cleanup if 'connected' flag is unset
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (2 preceding siblings ...)
2017-10-18 16:11 ` [Qemu-devel] [PULL 03/29] ide: " Paolo Bonzini
@ 2017-10-18 16:11 ` Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 05/29] exec: add page_mask for flatview_do_translate Paolo Bonzini
` (26 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:11 UTC (permalink / raw)
To: qemu-devel
From: "Daniel P. Berrange" <berrange@redhat.com>
The tcp_chr_free_connection & tcp_chr_disconnect methods both
skip all of their cleanup work unless the 's->connected' flag
is set. This flag is set when the incoming client connection
is ready to use. Crucially this is *after* the TLS handshake
has been completed. So if the TLS handshake fails and we try
to cleanup the failed client, all the cleanup is skipped as
's->connected' is still false.
The only important thing that should be skipped in this case
is sending of the CHR_EVENT_CLOSED, because we never got as
far as sending the corresponding CHR_EVENT_OPENED. Every other
bit of cleanup can be robust against being called even when
s->connected is false.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <20171005155057.7664-1-berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
chardev/char-socket.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index e65148f..53eda8e 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -332,10 +332,6 @@ static void tcp_chr_free_connection(Chardev *chr)
SocketChardev *s = SOCKET_CHARDEV(chr);
int i;
- if (!s->connected) {
- return;
- }
-
if (s->read_msgfds_num) {
for (i = 0; i < s->read_msgfds_num; i++) {
close(s->read_msgfds[i]);
@@ -394,22 +390,25 @@ static void update_disconnected_filename(SocketChardev *s)
s->is_listen, s->is_telnet);
}
+/* NB may be called even if tcp_chr_connect has not been
+ * reached, due to TLS or telnet initialization failure,
+ * so can *not* assume s->connected == true
+ */
static void tcp_chr_disconnect(Chardev *chr)
{
SocketChardev *s = SOCKET_CHARDEV(chr);
-
- if (!s->connected) {
- return;
- }
+ bool emit_close = s->connected;
tcp_chr_free_connection(chr);
- if (s->listen_ioc) {
+ if (s->listen_ioc && s->listen_tag == 0) {
s->listen_tag = qio_channel_add_watch(
QIO_CHANNEL(s->listen_ioc), G_IO_IN, tcp_chr_accept, chr, NULL);
}
update_disconnected_filename(s);
- qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
+ if (emit_close) {
+ qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
+ }
if (s->reconnect_time) {
qemu_chr_socket_restart_timer(chr);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 05/29] exec: add page_mask for flatview_do_translate
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (3 preceding siblings ...)
2017-10-18 16:11 ` [Qemu-devel] [PULL 04/29] char: don't skip client cleanup if 'connected' flag is unset Paolo Bonzini
@ 2017-10-18 16:11 ` Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 06/29] exec: simplify address_space_get_iotlb_entry Paolo Bonzini
` (25 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Maxime Coquelin
From: Peter Xu <peterx@redhat.com>
The function is originally used for flatview_space_translate() and what
we care about most is (xlat, plen) range. However for iotlb requests, we
don't really care about "plen", but the size of the page that "xlat" is
located on. While, plen cannot really contain this information.
A simple example to show why "plen" is not good for IOTLB translations:
E.g., for huge pages, it is possible that guest mapped 1G huge page on
device side that used this GPA range:
0x100000000 - 0x13fffffff
Then let's say we want to translate one IOVA that finally mapped to GPA
0x13ffffe00 (which is located on this 1G huge page). Then here we'll
get:
(xlat, plen) = (0x13fffe00, 0x200)
So the IOTLB would be only covering a very small range since from
"plen" (which is 0x200 bytes) we cannot tell the size of the page.
Actually we can really know that this is a huge page - we just throw the
information away in flatview_do_translate().
This patch introduced "page_mask" optional parameter to capture that
page mask info. Also, I made "plen" an optional parameter as well, with
some comments for the whole function.
No functional change yet.
Signed-off-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Message-Id: <20171010094247.10173-2-maxime.coquelin@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 51 +++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 45 insertions(+), 6 deletions(-)
diff --git a/exec.c b/exec.c
index 6378714..ca520ac 100644
--- a/exec.c
+++ b/exec.c
@@ -465,11 +465,29 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
return section;
}
-/* Called from RCU critical section */
+/**
+ * flatview_do_translate - translate an address in FlatView
+ *
+ * @fv: the flat view that we want to translate on
+ * @addr: the address to be translated in above address space
+ * @xlat: the translated address offset within memory region. It
+ * cannot be @NULL.
+ * @plen_out: valid read/write length of the translated address. It
+ * can be @NULL when we don't care about it.
+ * @page_mask_out: page mask for the translated address. This
+ * should only be meaningful for IOMMU translated
+ * addresses, since there may be huge pages that this bit
+ * would tell. It can be @NULL if we don't care about it.
+ * @is_write: whether the translation operation is for write
+ * @is_mmio: whether this can be MMIO, set true if it can
+ *
+ * This function is called from RCU critical section
+ */
static MemoryRegionSection flatview_do_translate(FlatView *fv,
hwaddr addr,
hwaddr *xlat,
- hwaddr *plen,
+ hwaddr *plen_out,
+ hwaddr *page_mask_out,
bool is_write,
bool is_mmio,
AddressSpace **target_as)
@@ -478,11 +496,17 @@ static MemoryRegionSection flatview_do_translate(FlatView *fv,
MemoryRegionSection *section;
IOMMUMemoryRegion *iommu_mr;
IOMMUMemoryRegionClass *imrc;
+ hwaddr page_mask = (hwaddr)(-1);
+ hwaddr plen = (hwaddr)(-1);
+
+ if (plen_out) {
+ plen = *plen_out;
+ }
for (;;) {
section = address_space_translate_internal(
flatview_to_dispatch(fv), addr, &addr,
- plen, is_mmio);
+ &plen, is_mmio);
iommu_mr = memory_region_get_iommu(section->mr);
if (!iommu_mr) {
@@ -494,7 +518,8 @@ static MemoryRegionSection flatview_do_translate(FlatView *fv,
IOMMU_WO : IOMMU_RO);
addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
| (addr & iotlb.addr_mask));
- *plen = MIN(*plen, (addr | iotlb.addr_mask) - addr + 1);
+ page_mask &= iotlb.addr_mask;
+ plen = MIN(plen, (addr | iotlb.addr_mask) - addr + 1);
if (!(iotlb.perm & (1 << is_write))) {
goto translate_fail;
}
@@ -505,6 +530,19 @@ static MemoryRegionSection flatview_do_translate(FlatView *fv,
*xlat = addr;
+ if (page_mask == (hwaddr)(-1)) {
+ /* Not behind an IOMMU, use default page size. */
+ page_mask = ~TARGET_PAGE_MASK;
+ }
+
+ if (page_mask_out) {
+ *page_mask_out = page_mask;
+ }
+
+ if (plen_out) {
+ *plen_out = plen;
+ }
+
return *section;
translate_fail:
@@ -523,7 +561,7 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
/* This can never be MMIO. */
section = flatview_do_translate(address_space_to_flatview(as), addr,
- &xlat, &plen, is_write, false, &as);
+ &xlat, &plen, NULL, is_write, false, &as);
/* Illegal translation */
if (section.mr == &io_mem_unassigned) {
@@ -567,7 +605,8 @@ MemoryRegion *flatview_translate(FlatView *fv, hwaddr addr, hwaddr *xlat,
AddressSpace *as = NULL;
/* This can be MMIO, so setup MMIO bit. */
- section = flatview_do_translate(fv, addr, xlat, plen, is_write, true, &as);
+ section = flatview_do_translate(fv, addr, xlat, plen, NULL,
+ is_write, true, &as);
mr = section.mr;
if (xen_enabled() && memory_access_is_direct(mr, is_write)) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 06/29] exec: simplify address_space_get_iotlb_entry
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (4 preceding siblings ...)
2017-10-18 16:11 ` [Qemu-devel] [PULL 05/29] exec: add page_mask for flatview_do_translate Paolo Bonzini
@ 2017-10-18 16:11 ` Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 07/29] memory: fix off-by-one error in memory_region_notify_one() Paolo Bonzini
` (24 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Maxime Coquelin
From: Peter Xu <peterx@redhat.com>
This patch let address_space_get_iotlb_entry() to use the newly
introduced page_mask parameter in flatview_do_translate(). Then we
will be sure the IOTLB can be aligned to page mask, also we should
nicely support huge pages now when introducing a764040.
Fixes: a764040 ("exec: abstract address_space_do_translate()")
Signed-off-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20171010094247.10173-3-maxime.coquelin@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 31 ++++++++++---------------------
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/exec.c b/exec.c
index ca520ac..6579e7a 100644
--- a/exec.c
+++ b/exec.c
@@ -554,14 +554,14 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
bool is_write)
{
MemoryRegionSection section;
- hwaddr xlat, plen;
+ hwaddr xlat, page_mask;
- /* Try to get maximum page mask during translation. */
- plen = (hwaddr)-1;
-
- /* This can never be MMIO. */
- section = flatview_do_translate(address_space_to_flatview(as), addr,
- &xlat, &plen, NULL, is_write, false, &as);
+ /*
+ * This can never be MMIO, and we don't really care about plen,
+ * but page mask.
+ */
+ section = flatview_do_translate(address_space_to_flatview(as), addr, &xlat,
+ NULL, &page_mask, is_write, false, &as);
/* Illegal translation */
if (section.mr == &io_mem_unassigned) {
@@ -572,22 +572,11 @@ IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
xlat += section.offset_within_address_space -
section.offset_within_region;
- if (plen == (hwaddr)-1) {
- /*
- * We use default page size here. Logically it only happens
- * for identity mappings.
- */
- plen = TARGET_PAGE_SIZE;
- }
-
- /* Convert to address mask */
- plen -= 1;
-
return (IOMMUTLBEntry) {
.target_as = as,
- .iova = addr & ~plen,
- .translated_addr = xlat & ~plen,
- .addr_mask = plen,
+ .iova = addr & ~page_mask,
+ .translated_addr = xlat & ~page_mask,
+ .addr_mask = page_mask,
/* IOTLBs are for DMAs, and DMA only allows on RAMs. */
.perm = IOMMU_RW,
};
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 07/29] memory: fix off-by-one error in memory_region_notify_one()
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (5 preceding siblings ...)
2017-10-18 16:11 ` [Qemu-devel] [PULL 06/29] exec: simplify address_space_get_iotlb_entry Paolo Bonzini
@ 2017-10-18 16:11 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 08/29] pc: make sure that plugged CPUs are of the same type Paolo Bonzini
` (23 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Maxime Coquelin, qemu-stable, Peter Xu
From: Maxime Coquelin <maxime.coquelin@redhat.com>
This patch fixes an off-by-one error that could lead to the
notifyee to receive notifications for ranges it is not
registered to.
The bug has been spotted by code review.
Fixes: bd2bfa4c52e5 ("memory: introduce memory_region_notify_one()")
Cc: qemu-stable@nongnu.org
Cc: Peter Xu <peterx@redhat.com>
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Message-Id: <20171010094247.10173-4-maxime.coquelin@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
memory.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/memory.c b/memory.c
index 5e6351a..b637c12 100644
--- a/memory.c
+++ b/memory.c
@@ -1892,7 +1892,7 @@ void memory_region_notify_one(IOMMUNotifier *notifier,
* Skip the notification if the notification does not overlap
* with registered range.
*/
- if (notifier->start > entry->iova + entry->addr_mask + 1 ||
+ if (notifier->start > entry->iova + entry->addr_mask ||
notifier->end < entry->iova) {
return;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 08/29] pc: make sure that plugged CPUs are of the same type
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (6 preceding siblings ...)
2017-10-18 16:11 ` [Qemu-devel] [PULL 07/29] memory: fix off-by-one error in memory_region_notify_one() Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 09/29] disas: Always initialize read_memory_inner_func properly Paolo Bonzini
` (22 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mammedov
From: Igor Mammedov <imammedo@redhat.com>
heterogeneous cpus are not supported and hotplugging different
cpu model crashes QEMU:
qemu-system-x86_64 -cpu qemu64 -smp 1,maxcpus=2
(qemu) device_add host-x86_64-cpu,socket-id=1,core-id=0,thread-id=0,id=foo
(qemu) info cpus
error: failed to get MSR 0x38d
qemu-system-x86_64: target/i386/kvm.c:2121: kvm_get_msrs: Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.
Aborted (core dumped)
Gracefully fail hotplug process in case of user mistake.
Reported-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1507638879-200718-1-git-send-email-imammedo@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/i386/pc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 05985d4..8e307f7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1876,8 +1876,15 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
CPUArchId *cpu_slot;
X86CPUTopoInfo topo;
X86CPU *cpu = X86_CPU(dev);
+ MachineState *ms = MACHINE(hotplug_dev);
PCMachineState *pcms = PC_MACHINE(hotplug_dev);
+ if(!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
+ error_setg(errp, "Invalid CPU type, expected cpu type: '%s'",
+ ms->cpu_type);
+ return;
+ }
+
/* if APIC ID is not set, set it based on socket/core/thread properties */
if (cpu->apic_id == UNASSIGNED_APIC_ID) {
int max_socket = (max_cpus - 1) / smp_threads / smp_cores;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 09/29] disas: Always initialize read_memory_inner_func properly
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (7 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 08/29] pc: make sure that plugged CPUs are of the same type Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 10/29] build: remove CONFIG_LIBDECNUMBER Paolo Bonzini
` (21 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Thomas Huth
From: Thomas Huth <thuth@redhat.com>
I've recently seen this with valgrind while running the HMP tester:
==22373== Conditional jump or move depends on uninitialised value(s)
==22373== at 0x4A41FD: arm_disas_set_info (cpu.c:504)
==22373== by 0x3867A7: monitor_disas (disas.c:390)
==22373== by 0x38E80E: memory_dump (monitor.c:1339)
==22373== by 0x38FA43: handle_hmp_command (monitor.c:3123)
==22373== by 0x38FB9E: qmp_human_monitor_command (monitor.c:613)
==22373== by 0x4E3124: qmp_marshal_human_monitor_command (qmp-marshal.c:1736)
==22373== by 0x769678: do_qmp_dispatch (qmp-dispatch.c:104)
==22373== by 0x769678: qmp_dispatch (qmp-dispatch.c:131)
==22373== by 0x38B734: handle_qmp_command (monitor.c:3853)
==22373== by 0x76ED07: json_message_process_token (json-streamer.c:105)
==22373== by 0x78D40A: json_lexer_feed_char (json-lexer.c:323)
==22373== by 0x78D4CD: json_lexer_feed (json-lexer.c:373)
==22373== by 0x38A08D: monitor_qmp_read (monitor.c:3895)
And indeed, in monitor_disas, the read_memory_inner_func variable was
not initialized, but arm_disas_set_info() expects this to be NULL
or a valid pointer. Let's properly set this to NULL in the
INIT_DISASSEMBLE_INFO to fix it in all functions that use the
disassemble_info struct.
Fixes: f7478a92dd9ee2276bfaa5b7317140d3f9d6a53b ("Fix Thumb-1 BE32 execution")
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1506524313-20037-1-git-send-email-thuth@redhat.com>
---
disas.c | 1 -
include/disas/bfd.h | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/disas.c b/disas.c
index d6a1eb9..54eea3f 100644
--- a/disas.c
+++ b/disas.c
@@ -190,7 +190,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code,
s.cpu = cpu;
s.info.read_memory_func = target_read_memory;
- s.info.read_memory_inner_func = NULL;
s.info.buffer_vma = code;
s.info.buffer_length = size;
s.info.print_address_func = generic_print_address;
diff --git a/include/disas/bfd.h b/include/disas/bfd.h
index b01e002..d99da68 100644
--- a/include/disas/bfd.h
+++ b/include/disas/bfd.h
@@ -479,6 +479,7 @@ int generic_symbol_at_address(bfd_vma, struct disassemble_info *);
(INFO).buffer_vma = 0, \
(INFO).buffer_length = 0, \
(INFO).read_memory_func = buffer_read_memory, \
+ (INFO).read_memory_inner_func = NULL, \
(INFO).memory_error_func = perror_memory, \
(INFO).print_address_func = generic_print_address, \
(INFO).print_insn = NULL, \
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 10/29] build: remove CONFIG_LIBDECNUMBER
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (8 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 09/29] disas: Always initialize read_memory_inner_func properly Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 11/29] nios2: define tcg_env Paolo Bonzini
` (20 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel
It is used by all PPC targets; we can give the directory its own
Makefile.objs file, and include it directly from target/ppc.
target/s390 can do the same when it starts using it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
Makefile.target | 6 ------
default-configs/ppc-linux-user.mak | 1 -
default-configs/ppc-softmmu.mak | 1 -
default-configs/ppc64-linux-user.mak | 1 -
default-configs/ppc64-softmmu.mak | 1 -
default-configs/ppc64abi32-linux-user.mak | 1 -
default-configs/ppc64le-linux-user.mak | 1 -
default-configs/ppcemb-softmmu.mak | 1 -
libdecnumber/Makefile.objs | 5 +++++
target/ppc/Makefile.objs | 1 +
10 files changed, 6 insertions(+), 13 deletions(-)
create mode 100644 libdecnumber/Makefile.objs
diff --git a/Makefile.target b/Makefile.target
index a4b292d..e4244c1 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -102,12 +102,6 @@ obj-y += target/$(TARGET_BASE_ARCH)/
obj-y += disas.o
obj-$(call notempty,$(TARGET_XML_FILES)) += gdbstub-xml.o
-obj-$(CONFIG_LIBDECNUMBER) += libdecnumber/decContext.o
-obj-$(CONFIG_LIBDECNUMBER) += libdecnumber/decNumber.o
-obj-$(CONFIG_LIBDECNUMBER) += libdecnumber/dpd/decimal32.o
-obj-$(CONFIG_LIBDECNUMBER) += libdecnumber/dpd/decimal64.o
-obj-$(CONFIG_LIBDECNUMBER) += libdecnumber/dpd/decimal128.o
-
#########################################################
# Linux user emulator target
diff --git a/default-configs/ppc-linux-user.mak b/default-configs/ppc-linux-user.mak
index 260ba41..6273df2 100644
--- a/default-configs/ppc-linux-user.mak
+++ b/default-configs/ppc-linux-user.mak
@@ -1,2 +1 @@
# Default configuration for ppc-linux-user
-CONFIG_LIBDECNUMBER=y
diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index d7a3755..bb225c6 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -46,7 +46,6 @@ CONFIG_E500=y
CONFIG_OPENPIC_KVM=$(call land,$(CONFIG_E500),$(CONFIG_KVM))
CONFIG_PLATFORM_BUS=y
CONFIG_ETSEC=y
-CONFIG_LIBDECNUMBER=y
CONFIG_SM501=y
# For PReP
CONFIG_SERIAL_ISA=y
diff --git a/default-configs/ppc64-linux-user.mak b/default-configs/ppc64-linux-user.mak
index e731ce0..422d3fb 100644
--- a/default-configs/ppc64-linux-user.mak
+++ b/default-configs/ppc64-linux-user.mak
@@ -1,2 +1 @@
# Default configuration for ppc64-linux-user
-CONFIG_LIBDECNUMBER=y
diff --git a/default-configs/ppc64-softmmu.mak b/default-configs/ppc64-softmmu.mak
index 9086475..d1b3a6d 100644
--- a/default-configs/ppc64-softmmu.mak
+++ b/default-configs/ppc64-softmmu.mak
@@ -51,7 +51,6 @@ CONFIG_E500=y
CONFIG_OPENPIC_KVM=$(call land,$(CONFIG_E500),$(CONFIG_KVM))
CONFIG_PLATFORM_BUS=y
CONFIG_ETSEC=y
-CONFIG_LIBDECNUMBER=y
CONFIG_SM501=y
# For pSeries
CONFIG_XICS=$(CONFIG_PSERIES)
diff --git a/default-configs/ppc64abi32-linux-user.mak b/default-configs/ppc64abi32-linux-user.mak
index c244d07..1c657ec 100644
--- a/default-configs/ppc64abi32-linux-user.mak
+++ b/default-configs/ppc64abi32-linux-user.mak
@@ -1,2 +1 @@
# Default configuration for ppc64abi32-linux-user
-CONFIG_LIBDECNUMBER=y
diff --git a/default-configs/ppc64le-linux-user.mak b/default-configs/ppc64le-linux-user.mak
index 4ba4eae..63f4269 100644
--- a/default-configs/ppc64le-linux-user.mak
+++ b/default-configs/ppc64le-linux-user.mak
@@ -1,2 +1 @@
# Default configuration for ppc64le-linux-user
-CONFIG_LIBDECNUMBER=y
diff --git a/default-configs/ppcemb-softmmu.mak b/default-configs/ppcemb-softmmu.mak
index 635923a..13917fb 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -15,5 +15,4 @@ CONFIG_PTIMER=y
CONFIG_I8259=y
CONFIG_XILINX=y
CONFIG_XILINX_ETHLITE=y
-CONFIG_LIBDECNUMBER=y
CONFIG_SM501=y
diff --git a/libdecnumber/Makefile.objs b/libdecnumber/Makefile.objs
new file mode 100644
index 0000000..d81db04
--- /dev/null
+++ b/libdecnumber/Makefile.objs
@@ -0,0 +1,5 @@
+obj-y += decContext.o
+obj-y += decNumber.o
+obj-y += dpd/decimal32.o
+obj-y += dpd/decimal64.o
+obj-y += dpd/decimal128.o
diff --git a/target/ppc/Makefile.objs b/target/ppc/Makefile.objs
index f92ba67..e8fa18c 100644
--- a/target/ppc/Makefile.objs
+++ b/target/ppc/Makefile.objs
@@ -15,5 +15,6 @@ obj-y += int_helper.o
obj-y += timebase_helper.o
obj-y += misc_helper.o
obj-y += mem_helper.o
+obj-y += ../../libdecnumber/
obj-$(CONFIG_USER_ONLY) += user_only_helper.o
obj-y += gdbstub.o
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 11/29] nios2: define tcg_env
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (9 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 10/29] build: remove CONFIG_LIBDECNUMBER Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 12/29] docs/devel/loads-stores.rst: Document our various load and store APIs Paolo Bonzini
` (19 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-stable, Marek Vasut
This should be done by all target and, since commit 53f6672bcf
("gen-icount: use tcg_ctx.tcg_env instead of cpu_env", 2017-06-30),
is causing the NIOS2 target to hang.
This is because the test for "should I exit to the main loop"
was being done with the correct offset to the icount decrementer,
but using TCG temporary 0 (the frame pointer) rather than the
env pointer.
Cc: qemu-stable@nongnu.org
Cc: Marek Vasut <marex@denx.de>
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/nios2/translate.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/nios2/translate.c b/target/nios2/translate.c
index 6b09618..54fbe89 100644
--- a/target/nios2/translate.c
+++ b/target/nios2/translate.c
@@ -948,6 +948,7 @@ void nios2_tcg_init(void)
int i;
cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
+ tcg_ctx.tcg_env = cpu_env;
for (i = 0; i < NUM_CORE_REGS; i++) {
cpu_R[i] = tcg_global_mem_new(cpu_env,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 12/29] docs/devel/loads-stores.rst: Document our various load and store APIs
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (10 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 11/29] nios2: define tcg_env Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 13/29] tco: add trace events Paolo Bonzini
` (18 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell
From: Peter Maydell <peter.maydell@linaro.org>
QEMU has a wide selection of different functions for doing
loads and stores; provide some overview documentation of
what they do and how to pick which one to use.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1507813181-11860-1-git-send-email-peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/devel/loads-stores.rst | 396 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 396 insertions(+)
create mode 100644 docs/devel/loads-stores.rst
diff --git a/docs/devel/loads-stores.rst b/docs/devel/loads-stores.rst
new file mode 100644
index 0000000..6a990cc
--- /dev/null
+++ b/docs/devel/loads-stores.rst
@@ -0,0 +1,396 @@
+..
+ Copyright (c) 2017 Linaro Limited
+ Written by Peter Maydell
+
+===================
+Load and Store APIs
+===================
+
+QEMU internally has multiple families of functions for performing
+loads and stores. This document attempts to enumerate them all
+and indicate when to use them. It does not provide detailed
+documentation of each API -- for that you should look at the
+documentation comments in the relevant header files.
+
+
+``ld*_p and st*_p``
+~~~~~~~~~~~~~~~~~~~
+
+These functions operate on a host pointer, and should be used
+when you already have a pointer into host memory (corresponding
+to guest ram or a local buffer). They deal with doing accesses
+with the desired endianness and with correctly handling
+potentially unaligned pointer values.
+
+Function names follow the pattern:
+
+load: ``ld{type}{sign}{size}_{endian}_p(ptr)``
+
+store: ``st{type}{size}_{endian}_p(ptr, val)``
+
+``type``
+ - (empty) : integer access
+ - ``f`` : float access
+
+``sign``
+ - (empty) : for 32 or 64 bit sizes (including floats and doubles)
+ - ``u`` : unsigned
+ - ``s`` : signed
+
+``size``
+ - ``b`` : 8 bits
+ - ``w`` : 16 bits
+ - ``l`` : 32 bits
+ - ``q`` : 64 bits
+
+``endian``
+ - ``he`` : host endian
+ - ``be`` : big endian
+ - ``le`` : little endian
+
+The ``_{endian}`` infix is omitted for target-endian accesses.
+
+The target endian accessors are only available to source
+files which are built per-target.
+
+Regexes for git grep
+ - ``\<ldf\?[us]\?[bwlq]\(_[hbl]e\)\?_p\>``
+ - ``\<stf\?[bwlq]\(_[hbl]e\)\?_p\>``
+
+``cpu_{ld,st}_*``
+~~~~~~~~~~~~~~~~~
+
+These functions operate on a guest virtual address. Be aware
+that these functions may cause a guest CPU exception to be
+taken (e.g. for an alignment fault or MMU fault) which will
+result in guest CPU state being updated and control longjumping
+out of the function call. They should therefore only be used
+in code that is implementing emulation of the target CPU.
+
+These functions may throw an exception (longjmp() back out
+to the top level TCG loop). This means they must only be used
+from helper functions where the translator has saved all
+necessary CPU state before generating the helper function call.
+It's usually better to use the ``_ra`` variants described below
+from helper functions, but these functions are the right choice
+for calls made from hooks like the CPU do_interrupt hook or
+when you know for certain that the translator had to save all
+the CPU state that ``cpu_restore_state()`` would restore anyway.
+
+Function names follow the pattern:
+
+load: ``cpu_ld{sign}{size}_{mmusuffix}(env, ptr)``
+
+store: ``cpu_st{size}_{mmusuffix}(env, ptr, val)``
+
+``sign``
+ - (empty) : for 32 or 64 bit sizes
+ - ``u`` : unsigned
+ - ``s`` : signed
+
+``size``
+ - ``b`` : 8 bits
+ - ``w`` : 16 bits
+ - ``l`` : 32 bits
+ - ``q`` : 64 bits
+
+``mmusuffix`` is one of the generic suffixes ``data`` or ``code``, or
+(for softmmu configs) a target-specific MMU mode suffix as defined
+in the target's ``cpu.h``.
+
+Regexes for git grep
+ - ``\<cpu_ld[us]\?[bwlq]_[a-zA-Z0-9]\+\>``
+ - ``\<cpu_st[bwlq]_[a-zA-Z0-9]\+\>``
+
+``cpu_{ld,st}_*_ra``
+~~~~~~~~~~~~~~~~~~~~
+
+These functions work like the ``cpu_{ld,st}_*`` functions except
+that they also take a ``retaddr`` argument. This extra argument
+allows for correct unwinding of any exception that is taken,
+and should generally be the result of GETPC() called directly
+from the top level HELPER(foo) function (i.e. the return address
+in the generated code).
+
+These are generally the preferred way to do accesses by guest
+virtual address from helper functions; see the documentation
+of the non-``_ra`` variants for when those would be better.
+
+Calling these functions with a ``retaddr`` argument of 0 is
+equivalent to calling the non-``_ra`` version of the function.
+
+Function names follow the pattern:
+
+load: ``cpu_ld{sign}{size}_{mmusuffix}_ra(env, ptr, retaddr)``
+
+store: ``cpu_st{sign}{size}_{mmusuffix}_ra(env, ptr, val, retaddr)``
+
+Regexes for git grep
+ - ``\<cpu_ld[us]\?[bwlq]_[a-zA-Z0-9]\+_ra\>``
+ - ``\<cpu_st[bwlq]_[a-zA-Z0-9]\+_ra\>``
+
+``helper_*_{ld,st}*mmu``
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+These functions are intended primarily to be called by the code
+generated by the TCG backend. They may also be called by target
+CPU helper function code. Like the ``cpu_{ld,st}_*_ra`` functions
+they perform accesses by guest virtual address; the difference is
+that these functions allow you to specify an ``opindex`` parameter
+which encodes (among other things) the mmu index to use for the
+access. This is necessary if your helper needs to make an access
+via a specific mmu index (for instance, an "always as non-privileged"
+access) rather than using the default mmu index for the current state
+of the guest CPU.
+
+The ``opindex`` parameter should be created by calling ``make_memop_idx()``.
+
+The ``retaddr`` parameter should be the result of GETPC() called directly
+from the top level HELPER(foo) function (or 0 if no guest CPU state
+unwinding is required).
+
+**TODO** The names of these functions are a bit odd for historical
+reasons because they were originally expected to be called only from
+within generated code. We should rename them to bring them
+more in line with the other memory access functions.
+
+load: ``helper_{endian}_ld{sign}{size}_mmu(env, addr, opindex, retaddr)``
+
+load (code): ``helper_{endian}_ld{sign}{size}_cmmu(env, addr, opindex, retaddr)``
+
+store: ``helper_{endian}_st{size}_mmu(env, addr, val, opindex, retaddr)``
+
+``sign``
+ - (empty) : for 32 or 64 bit sizes
+ - ``u`` : unsigned
+ - ``s`` : signed
+
+``size``
+ - ``b`` : 8 bits
+ - ``w`` : 16 bits
+ - ``l`` : 32 bits
+ - ``q`` : 64 bits
+
+``endian``
+ - ``le`` : little endian
+ - ``be`` : big endian
+ - ``ret`` : target endianness
+
+Regexes for git grep
+ - ``\<helper_\(le\|be\|ret\)_ld[us]\?[bwlq]_c\?mmu\>``
+ - ``\<helper_\(le\|be\|ret\)_st[bwlq]_mmu\>``
+
+``address_space_*``
+~~~~~~~~~~~~~~~~~~~
+
+These functions are the primary ones to use when emulating CPU
+or device memory accesses. They take an AddressSpace, which is the
+way QEMU defines the view of memory that a device or CPU has.
+(They generally correspond to being the "master" end of a hardware bus
+or bus fabric.)
+
+Each CPU has an AddressSpace. Some kinds of CPU have more than
+one AddressSpace (for instance ARM guest CPUs have an AddressSpace
+for the Secure world and one for NonSecure if they implement TrustZone).
+Devices which can do DMA-type operations should generally have an
+AddressSpace. There is also a "system address space" which typically
+has all the devices and memory that all CPUs can see. (Some older
+device models use the "system address space" rather than properly
+modelling that they have an AddressSpace of their own.)
+
+Functions are provided for doing byte-buffer reads and writes,
+and also for doing one-data-item loads and stores.
+
+In all cases the caller provides a MemTxAttrs to specify bus
+transaction attributes, and can check whether the memory transaction
+succeeded using a MemTxResult return code.
+
+``address_space_read(address_space, addr, attrs, buf, len)``
+
+``address_space_write(address_space, addr, attrs, buf, len)``
+
+``address_space_rw(address_space, addr, attrs, buf, len, is_write)``
+
+``address_space_ld{sign}{size}_{endian}(address_space, addr, attrs, txresult)``
+
+``address_space_st{size}_{endian}(address_space, addr, val, attrs, txresult)``
+
+``sign``
+ - (empty) : for 32 or 64 bit sizes
+ - ``u`` : unsigned
+
+(No signed load operations are provided.)
+
+``size``
+ - ``b`` : 8 bits
+ - ``w`` : 16 bits
+ - ``l`` : 32 bits
+ - ``q`` : 64 bits
+
+``endian``
+ - ``le`` : little endian
+ - ``be`` : big endian
+
+The ``_{endian}`` suffix is omitted for byte accesses.
+
+Regexes for git grep
+ - ``\<address_space_\(read\|write\|rw\)\>``
+ - ``\<address_space_ldu\?[bwql]\(_[lb]e\)\?\>``
+ - ``\<address_space_st[bwql]\(_[lb]e\)\?\>``
+
+``{ld,st}*_phys``
+~~~~~~~~~~~~~~~~~
+
+These are functions which are identical to
+``address_space_{ld,st}*``, except that they always pass
+``MEMTXATTRS_UNSPECIFIED`` for the transaction attributes, and ignore
+whether the transaction succeeded or failed.
+
+The fact that they ignore whether the transaction succeeded means
+they should not be used in new code, unless you know for certain
+that your code will only be used in a context where the CPU or
+device doing the access has no way to report such an error.
+
+``load: ld{sign}{size}_{endian}_phys``
+
+``store: st{size}_{endian}_phys``
+
+``sign``
+ - (empty) : for 32 or 64 bit sizes
+ - ``u`` : unsigned
+
+(No signed load operations are provided.)
+
+``size``
+ - ``b`` : 8 bits
+ - ``w`` : 16 bits
+ - ``l`` : 32 bits
+ - ``q`` : 64 bits
+
+``endian``
+ - ``le`` : little endian
+ - ``be`` : big endian
+
+The ``_{endian}_`` infix is omitted for byte accesses.
+
+Regexes for git grep
+ - ``\<ldu\?[bwlq]\(_[bl]e\)\?_phys\>``
+ - ``\<st[bwlq]\(_[bl]e\)\?_phys\>``
+
+``cpu_physical_memory_*``
+~~~~~~~~~~~~~~~~~~~~~~~~~
+
+These are convenience functions which are identical to
+``address_space_*`` but operate specifically on the system address space,
+always pass a ``MEMTXATTRS_UNSPECIFIED`` set of memory attributes and
+ignore whether the memory transaction succeeded or failed.
+For new code they are better avoided:
+
+* there is likely to be behaviour you need to model correctly for a
+ failed read or write operation
+* a device should usually perform operations on its own AddressSpace
+ rather than using the system address space
+
+``cpu_physical_memory_read``
+
+``cpu_physical_memory_write``
+
+``cpu_physical_memory_rw``
+
+Regexes for git grep
+ - ``\<cpu_physical_memory_\(read\|write\|rw\)\>``
+
+``cpu_physical_memory_write_rom``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+This function performs a write by physical address like
+``address_space_write``, except that if the write is to a ROM then
+the ROM contents will be modified, even though a write by the guest
+CPU to the ROM would be ignored.
+
+Note that unlike ``cpu_physical_memory_write()`` this function takes
+an AddressSpace argument, but unlike ``address_space_write()`` this
+function does not take a ``MemTxAttrs`` or return a ``MemTxResult``.
+
+**TODO**: we should probably clean up this inconsistency and
+turn the function into ``address_space_write_rom`` with an API
+matching ``address_space_write``.
+
+``cpu_physical_memory_write_rom``
+
+
+``cpu_memory_rw_debug``
+~~~~~~~~~~~~~~~~~~~~~~~
+
+Access CPU memory by virtual address for debug purposes.
+
+This function is intended for use by the GDB stub and similar code.
+It takes a virtual address, converts it to a physical address via
+an MMU lookup using the current settings of the specified CPU,
+and then performs the access (using ``address_space_rw`` for
+reads or ``cpu_physical_memory_write_rom`` for writes).
+This means that if the access is a write to a ROM then this
+function will modify the contents (whereas a normal guest CPU access
+would ignore the write attempt).
+
+``cpu_memory_rw_debug``
+
+``dma_memory_*``
+~~~~~~~~~~~~~~~~
+
+These behave like ``address_space_*``, except that they perform a DMA
+barrier operation first.
+
+**TODO**: We should provide guidance on when you need the DMA
+barrier operation and when it's OK to use ``address_space_*``, and
+make sure our existing code is doing things correctly.
+
+``dma_memory_read``
+
+``dma_memory_write``
+
+``dma_memory_rw``
+
+Regexes for git grep
+ - ``\<dma_memory_\(read\|write\|rw\)\>``
+
+``pci_dma_*`` and ``{ld,st}*_pci_dma``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+These functions are specifically for PCI device models which need to
+perform accesses where the PCI device is a bus master. You pass them a
+``PCIDevice *`` and they will do ``dma_memory_*`` operations on the
+correct address space for that device.
+
+``pci_dma_read``
+
+``pci_dma_write``
+
+``pci_dma_rw``
+
+``load: ld{sign}{size}_{endian}_pci_dma``
+
+``store: st{size}_{endian}_pci_dma``
+
+``sign``
+ - (empty) : for 32 or 64 bit sizes
+ - ``u`` : unsigned
+
+(No signed load operations are provided.)
+
+``size``
+ - ``b`` : 8 bits
+ - ``w`` : 16 bits
+ - ``l`` : 32 bits
+ - ``q`` : 64 bits
+
+``endian``
+ - ``le`` : little endian
+ - ``be`` : big endian
+
+The ``_{endian}_`` infix is omitted for byte accesses.
+
+Regexes for git grep
+ - ``\<pci_dma_\(read\|write\|rw\)\>``
+ - ``\<ldu\?[bwlq]\(_[bl]e\)\?_pci_dma\>``
+ - ``\<st[bwlq]\(_[bl]e\)\?_pci_dma\>``
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 13/29] tco: add trace events
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (11 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 12/29] docs/devel/loads-stores.rst: Document our various load and store APIs Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 14/29] target/i386: introduce x86_ld*_code Paolo Bonzini
` (17 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel
Add trace events to the PCH watchdog timer, it can be useful to see how
the guest is using it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <1507816448-86665-1-git-send-email-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/acpi/tco.c | 11 +++++++++--
hw/acpi/trace-events | 4 ++++
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/hw/acpi/tco.c b/hw/acpi/tco.c
index 05b9d7b..a914396 100644
--- a/hw/acpi/tco.c
+++ b/hw/acpi/tco.c
@@ -12,6 +12,7 @@
#include "hw/i386/ich9.h"
#include "hw/acpi/tco.h"
+#include "trace.h"
//#define DEBUG
@@ -41,8 +42,11 @@ enum {
static inline void tco_timer_reload(TCOIORegs *tr)
{
- tr->expire_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
- ((int64_t)(tr->tco.tmr & TCO_TMR_MASK) * TCO_TICK_NSEC);
+ int ticks = tr->tco.tmr & TCO_TMR_MASK;
+ int64_t nsec = (int64_t)ticks * TCO_TICK_NSEC;
+
+ trace_tco_timer_reload(ticks, nsec / 1000000);
+ tr->expire_time = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + nsec;
timer_mod(tr->tco_timer, tr->expire_time);
}
@@ -59,6 +63,9 @@ static void tco_timer_expired(void *opaque)
ICH9LPCState *lpc = container_of(pm, ICH9LPCState, pm);
uint32_t gcs = pci_get_long(lpc->chip_config + ICH9_CC_GCS);
+ trace_tco_timer_expired(tr->timeouts_no,
+ lpc->pin_strap.spkr_hi,
+ !!(gcs & ICH9_CC_GCS_NO_REBOOT));
tr->tco.rld = 0;
tr->tco.sts1 |= TCO_TIMEOUT;
if (++tr->timeouts_no == 2) {
diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
index e3b41e9..df0024f 100644
--- a/hw/acpi/trace-events
+++ b/hw/acpi/trace-events
@@ -30,3 +30,7 @@ cpuhp_acpi_ejecting_invalid_cpu(uint32_t idx) "0x%"PRIx32
cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
+
+# hw/acpi/tco.c
+tco_timer_reload(int ticks, int msec) "ticks=%d (%d ms)"
+tco_timer_expired(int timeouts_no, bool strap, bool no_reboot) "timeouts_no=%d no_reboot=%d/%d"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 14/29] target/i386: introduce x86_ld*_code
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (12 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 13/29] tco: add trace events Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 15/29] target/i386: trap on instructions longer than >15 bytes Paolo Bonzini
` (16 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel
These take care of advancing s->pc, and will provide a unified point
where to check for the 15-byte instruction length limit.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/translate.c | 228 ++++++++++++++++++++++++++----------------------
1 file changed, 125 insertions(+), 103 deletions(-)
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 5d61fa9..4a938c2 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -1863,6 +1863,41 @@ static void gen_shifti(DisasContext *s1, int op, TCGMemOp ot, int d, int c)
}
}
+static uint64_t advance_pc(CPUX86State *env, DisasContext *s, int num_bytes)
+{
+ uint64_t pc = s->pc;
+
+ s->pc += num_bytes;
+ return pc;
+}
+
+static inline uint8_t x86_ldub_code(CPUX86State *env, DisasContext *s)
+{
+ return cpu_ldub_code(env, advance_pc(env, s, 1));
+}
+
+static inline int16_t x86_ldsw_code(CPUX86State *env, DisasContext *s)
+{
+ return cpu_ldsw_code(env, advance_pc(env, s, 2));
+}
+
+static inline uint16_t x86_lduw_code(CPUX86State *env, DisasContext *s)
+{
+ return cpu_lduw_code(env, advance_pc(env, s, 2));
+}
+
+static inline uint32_t x86_ldl_code(CPUX86State *env, DisasContext *s)
+{
+ return cpu_ldl_code(env, advance_pc(env, s, 4));
+}
+
+#ifdef TARGET_X86_64
+static inline uint64_t x86_ldq_code(CPUX86State *env, DisasContext *s)
+{
+ return cpu_ldq_code(env, advance_pc(env, s, 8));
+}
+#endif
+
/* Decompose an address. */
typedef struct AddressParts {
@@ -1900,7 +1935,7 @@ static AddressParts gen_lea_modrm_0(CPUX86State *env, DisasContext *s,
case MO_32:
havesib = 0;
if (rm == 4) {
- int code = cpu_ldub_code(env, s->pc++);
+ int code = x86_ldub_code(env, s);
scale = (code >> 6) & 3;
index = ((code >> 3) & 7) | REX_X(s);
if (index == 4) {
@@ -1914,8 +1949,7 @@ static AddressParts gen_lea_modrm_0(CPUX86State *env, DisasContext *s,
case 0:
if ((base & 7) == 5) {
base = -1;
- disp = (int32_t)cpu_ldl_code(env, s->pc);
- s->pc += 4;
+ disp = (int32_t)x86_ldl_code(env, s);
if (CODE64(s) && !havesib) {
base = -2;
disp += s->pc + s->rip_offset;
@@ -1923,12 +1957,11 @@ static AddressParts gen_lea_modrm_0(CPUX86State *env, DisasContext *s,
}
break;
case 1:
- disp = (int8_t)cpu_ldub_code(env, s->pc++);
+ disp = (int8_t)x86_ldub_code(env, s);
break;
default:
case 2:
- disp = (int32_t)cpu_ldl_code(env, s->pc);
- s->pc += 4;
+ disp = (int32_t)x86_ldl_code(env, s);
break;
}
@@ -1945,15 +1978,13 @@ static AddressParts gen_lea_modrm_0(CPUX86State *env, DisasContext *s,
if (mod == 0) {
if (rm == 6) {
base = -1;
- disp = cpu_lduw_code(env, s->pc);
- s->pc += 2;
+ disp = x86_lduw_code(env, s);
break;
}
} else if (mod == 1) {
- disp = (int8_t)cpu_ldub_code(env, s->pc++);
+ disp = (int8_t)x86_ldub_code(env, s);
} else {
- disp = (int16_t)cpu_lduw_code(env, s->pc);
- s->pc += 2;
+ disp = (int16_t)x86_lduw_code(env, s);
}
switch (rm) {
@@ -2103,19 +2134,16 @@ static inline uint32_t insn_get(CPUX86State *env, DisasContext *s, TCGMemOp ot)
switch (ot) {
case MO_8:
- ret = cpu_ldub_code(env, s->pc);
- s->pc++;
+ ret = x86_ldub_code(env, s);
break;
case MO_16:
- ret = cpu_lduw_code(env, s->pc);
- s->pc += 2;
+ ret = x86_lduw_code(env, s);
break;
case MO_32:
#ifdef TARGET_X86_64
case MO_64:
#endif
- ret = cpu_ldl_code(env, s->pc);
- s->pc += 4;
+ ret = x86_ldl_code(env, s);
break;
default:
tcg_abort();
@@ -3041,7 +3069,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
gen_helper_enter_mmx(cpu_env);
}
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7);
if (is_xmm)
reg |= rex_r;
@@ -3250,8 +3278,8 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
if (b1 == 1 && reg != 0)
goto illegal_op;
- field_length = cpu_ldub_code(env, s->pc++) & 0x3F;
- bit_index = cpu_ldub_code(env, s->pc++) & 0x3F;
+ field_length = x86_ldub_code(env, s) & 0x3F;
+ bit_index = x86_ldub_code(env, s) & 0x3F;
tcg_gen_addi_ptr(cpu_ptr0, cpu_env,
offsetof(CPUX86State,xmm_regs[reg]));
if (b1 == 1)
@@ -3380,7 +3408,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
if (b1 >= 2) {
goto unknown_op;
}
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
if (is_xmm) {
tcg_gen_movi_tl(cpu_T0, val);
tcg_gen_st32_tl(cpu_T0, cpu_env, offsetof(CPUX86State,xmm_t0.ZMM_L(0)));
@@ -3537,7 +3565,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
case 0x1c4:
s->rip_offset = 1;
gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
if (b1) {
val &= 7;
tcg_gen_st16_tl(cpu_T0, cpu_env,
@@ -3553,7 +3581,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
if (mod != 3)
goto illegal_op;
ot = mo_64_32(s->dflag);
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
if (b1) {
val &= 7;
rm = (modrm & 7) | REX_B(s);
@@ -3616,7 +3644,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
if ((b & 0xf0) == 0xf0) {
goto do_0f_38_fx;
}
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
rm = modrm & 7;
reg = ((modrm >> 3) & 7) | rex_r;
mod = (modrm >> 6) & 3;
@@ -3693,7 +3721,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
do_0f_38_fx:
/* Various integer extensions at 0f 38 f[0-f]. */
b = modrm | (b1 << 8);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
switch (b) {
@@ -4054,7 +4082,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
case 0x03a:
case 0x13a:
b = modrm;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
rm = modrm & 7;
reg = ((modrm >> 3) & 7) | rex_r;
mod = (modrm >> 6) & 3;
@@ -4077,7 +4105,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
if (mod != 3)
gen_lea_modrm(env, s, modrm);
reg = ((modrm >> 3) & 7) | rex_r;
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
switch (b) {
case 0x14: /* pextrb */
tcg_gen_ld8u_tl(cpu_T0, cpu_env, offsetof(CPUX86State,
@@ -4225,7 +4253,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
gen_ldq_env_A0(s, op2_offset);
}
}
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
if ((b & 0xfc) == 0x60) { /* pcmpXstrX */
set_cc_op(s, CC_OP_EFLAGS);
@@ -4244,7 +4272,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
case 0x33a:
/* Various integer extensions at 0f 3a f[0-f]. */
b = modrm | (b1 << 8);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
switch (b) {
@@ -4256,7 +4284,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
}
ot = mo_64_32(s->dflag);
gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
- b = cpu_ldub_code(env, s->pc++);
+ b = x86_ldub_code(env, s);
if (ot == MO_64) {
tcg_gen_rotri_tl(cpu_T0, cpu_T0, b & 63);
} else {
@@ -4351,7 +4379,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
}
switch(b) {
case 0x0f: /* 3DNow! data insns */
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
sse_fn_epp = sse_op_table5[val];
if (!sse_fn_epp) {
goto unknown_op;
@@ -4365,7 +4393,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
break;
case 0x70: /* pshufx insn */
case 0xc6: /* pshufx insn */
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
tcg_gen_addi_ptr(cpu_ptr0, cpu_env, op1_offset);
tcg_gen_addi_ptr(cpu_ptr1, cpu_env, op2_offset);
/* XXX: introduce a new table? */
@@ -4374,7 +4402,7 @@ static void gen_sse(CPUX86State *env, DisasContext *s, int b,
break;
case 0xc2:
/* compare insns */
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
if (val >= 8)
goto unknown_op;
sse_fn_epp = sse_op_table4[val][b1];
@@ -4443,8 +4471,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
if (s->pc - pc_start > 14) {
goto illegal_op;
}
- b = cpu_ldub_code(env, s->pc);
- s->pc++;
+ b = x86_ldub_code(env, s);
/* Collect prefixes. */
switch (b) {
case 0xf3:
@@ -4501,7 +4528,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
static const int pp_prefix[4] = {
0, PREFIX_DATA, PREFIX_REPZ, PREFIX_REPNZ
};
- int vex3, vex2 = cpu_ldub_code(env, s->pc);
+ int vex3, vex2 = x86_ldub_code(env, s);
if (!CODE64(s) && (vex2 & 0xc0) != 0xc0) {
/* 4.1.4.6: In 32-bit mode, bits [7:6] must be 11b,
@@ -4523,17 +4550,17 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
rex_r = (~vex2 >> 4) & 8;
if (b == 0xc5) {
vex3 = vex2;
- b = cpu_ldub_code(env, s->pc++);
+ b = x86_ldub_code(env, s);
} else {
#ifdef TARGET_X86_64
s->rex_x = (~vex2 >> 3) & 8;
s->rex_b = (~vex2 >> 2) & 8;
#endif
- vex3 = cpu_ldub_code(env, s->pc++);
+ vex3 = x86_ldub_code(env, s);
rex_w = (vex3 >> 7) & 1;
switch (vex2 & 0x1f) {
case 0x01: /* Implied 0f leading opcode bytes. */
- b = cpu_ldub_code(env, s->pc++) | 0x100;
+ b = x86_ldub_code(env, s) | 0x100;
break;
case 0x02: /* Implied 0f 38 leading opcode bytes. */
b = 0x138;
@@ -4585,7 +4612,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0x0f:
/**************************/
/* extended op code */
- b = cpu_ldub_code(env, s->pc++) | 0x100;
+ b = x86_ldub_code(env, s) | 0x100;
goto reswitch;
/**************************/
@@ -4607,7 +4634,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
switch(f) {
case 0: /* OP Ev, Gv */
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
mod = (modrm >> 6) & 3;
rm = (modrm & 7) | REX_B(s);
@@ -4628,7 +4655,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_op(s, op, ot, opreg);
break;
case 1: /* OP Gv, Ev */
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
reg = ((modrm >> 3) & 7) | rex_r;
rm = (modrm & 7) | REX_B(s);
@@ -4662,7 +4689,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
rm = (modrm & 7) | REX_B(s);
op = (modrm >> 3) & 7;
@@ -4708,7 +4735,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0xf7:
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
rm = (modrm & 7) | REX_B(s);
op = (modrm >> 3) & 7;
@@ -4940,7 +4967,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0xff: /* GRP5 */
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
rm = (modrm & 7) | REX_B(s);
op = (modrm >> 3) & 7;
@@ -5048,7 +5075,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0x85:
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
@@ -5120,7 +5147,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0x69: /* imul Gv, Ev, I */
case 0x6b:
ot = dflag;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
if (b == 0x69)
s->rip_offset = insn_const_size(ot);
@@ -5172,7 +5199,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0x1c0:
case 0x1c1: /* xadd Ev, Gv */
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
mod = (modrm >> 6) & 3;
gen_op_mov_v_reg(ot, cpu_T0, reg);
@@ -5204,7 +5231,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
TCGv oldv, newv, cmpv;
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
mod = (modrm >> 6) & 3;
oldv = tcg_temp_new();
@@ -5256,7 +5283,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
}
break;
case 0x1c7: /* cmpxchg8b */
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
if ((mod == 3) || ((modrm & 0x38) != 0x8))
goto illegal_op;
@@ -5318,7 +5345,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_push_v(s, cpu_T0);
break;
case 0x8f: /* pop Ev */
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
ot = gen_pop_T0(s);
if (mod == 3) {
@@ -5337,9 +5364,8 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0xc8: /* enter */
{
int level;
- val = cpu_lduw_code(env, s->pc);
- s->pc += 2;
- level = cpu_ldub_code(env, s->pc++);
+ val = x86_lduw_code(env, s);
+ level = x86_ldub_code(env, s);
gen_enter(s, val, level);
}
break;
@@ -5396,7 +5422,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0x88:
case 0x89: /* mov Gv, Ev */
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
/* generate a generic store */
@@ -5405,7 +5431,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0xc6:
case 0xc7: /* mov Ev, Iv */
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
if (mod != 3) {
s->rip_offset = insn_const_size(ot);
@@ -5422,14 +5448,14 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0x8a:
case 0x8b: /* mov Ev, Gv */
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
gen_op_mov_reg_v(ot, reg, cpu_T0);
break;
case 0x8e: /* mov seg, Gv */
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = (modrm >> 3) & 7;
if (reg >= 6 || reg == R_CS)
goto illegal_op;
@@ -5447,7 +5473,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
}
break;
case 0x8c: /* mov Gv, seg */
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = (modrm >> 3) & 7;
mod = (modrm >> 6) & 3;
if (reg >= 6)
@@ -5472,7 +5498,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
/* s_ot is the sign+size of source */
s_ot = b & 8 ? MO_SIGN | ot : ot;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
mod = (modrm >> 6) & 3;
rm = (modrm & 7) | REX_B(s);
@@ -5508,7 +5534,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
break;
case 0x8d: /* lea */
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
if (mod == 3)
goto illegal_op;
@@ -5532,8 +5558,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
switch (s->aflag) {
#ifdef TARGET_X86_64
case MO_64:
- offset_addr = cpu_ldq_code(env, s->pc);
- s->pc += 8;
+ offset_addr = x86_ldq_code(env, s);
break;
#endif
default:
@@ -5570,8 +5595,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
if (dflag == MO_64) {
uint64_t tmp;
/* 64 bit case */
- tmp = cpu_ldq_code(env, s->pc);
- s->pc += 8;
+ tmp = x86_ldq_code(env, s);
reg = (b & 7) | REX_B(s);
tcg_gen_movi_tl(cpu_T0, tmp);
gen_op_mov_reg_v(MO_64, reg, cpu_T0);
@@ -5595,7 +5619,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0x86:
case 0x87: /* xchg Ev, Gv */
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
mod = (modrm >> 6) & 3;
if (mod == 3) {
@@ -5632,7 +5656,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
op = R_GS;
do_lxx:
ot = dflag != MO_16 ? MO_32 : MO_16;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
mod = (modrm >> 6) & 3;
if (mod == 3)
@@ -5660,7 +5684,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
grp2:
{
ot = mo_b_d(b, dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
op = (modrm >> 3) & 7;
@@ -5679,7 +5703,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_shift(s, op, ot, opreg, OR_ECX);
} else {
if (shift == 2) {
- shift = cpu_ldub_code(env, s->pc++);
+ shift = x86_ldub_code(env, s);
}
gen_shifti(s, op, ot, opreg, shift);
}
@@ -5713,7 +5737,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
shift = 0;
do_shiftd:
ot = dflag;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
rm = (modrm & 7) | REX_B(s);
reg = ((modrm >> 3) & 7) | rex_r;
@@ -5726,7 +5750,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_op_mov_v_reg(ot, cpu_T1, reg);
if (shift) {
- TCGv imm = tcg_const_tl(cpu_ldub_code(env, s->pc++));
+ TCGv imm = tcg_const_tl(x86_ldub_code(env, s));
gen_shiftd_rm_T1(s, ot, opreg, op, imm);
tcg_temp_free(imm);
} else {
@@ -5743,7 +5767,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_exception(s, EXCP07_PREX, pc_start - s->cs_base);
break;
}
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
rm = modrm & 7;
op = ((b & 7) << 3) | ((modrm >> 3) & 7);
@@ -6328,7 +6352,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0xe4:
case 0xe5:
ot = mo_b_d32(b, dflag);
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
tcg_gen_movi_tl(cpu_T0, val);
gen_check_io(s, ot, pc_start - s->cs_base,
SVM_IOIO_TYPE_MASK | svm_is_rep(prefixes));
@@ -6347,7 +6371,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0xe6:
case 0xe7:
ot = mo_b_d32(b, dflag);
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
tcg_gen_movi_tl(cpu_T0, val);
gen_check_io(s, ot, pc_start - s->cs_base,
svm_is_rep(prefixes));
@@ -6407,8 +6431,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
/************************/
/* control */
case 0xc2: /* ret im */
- val = cpu_ldsw_code(env, s->pc);
- s->pc += 2;
+ val = x86_ldsw_code(env, s);
ot = gen_pop_T0(s);
gen_stack_update(s, val + (1 << ot));
/* Note that gen_pop_T0 uses a zero-extending load. */
@@ -6425,8 +6448,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_jr(s, cpu_T0);
break;
case 0xca: /* lret im */
- val = cpu_ldsw_code(env, s->pc);
- s->pc += 2;
+ val = x86_ldsw_code(env, s);
do_lret:
if (s->pe && !s->vm86) {
gen_update_cc_op(s);
@@ -6563,7 +6585,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
break;
case 0x190 ... 0x19f: /* setcc Gv */
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
gen_setcc1(s, b, cpu_T0);
gen_ldst_modrm(env, s, modrm, MO_8, OR_TMP0, 1);
break;
@@ -6572,7 +6594,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
goto illegal_op;
}
ot = dflag;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
gen_cmovcc1(env, s, ot, b, modrm, reg);
break;
@@ -6689,7 +6711,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
/* bit operations */
case 0x1ba: /* bt/bts/btr/btc Gv, im */
ot = dflag;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
op = (modrm >> 3) & 7;
mod = (modrm >> 6) & 3;
rm = (modrm & 7) | REX_B(s);
@@ -6703,7 +6725,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_op_mov_v_reg(ot, cpu_T0, rm);
}
/* load shift */
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
tcg_gen_movi_tl(cpu_T1, val);
if (op < 4)
goto unknown_op;
@@ -6722,7 +6744,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
op = 3;
do_btx:
ot = dflag;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
mod = (modrm >> 6) & 3;
rm = (modrm & 7) | REX_B(s);
@@ -6827,7 +6849,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0x1bc: /* bsf / tzcnt */
case 0x1bd: /* bsr / lzcnt */
ot = dflag;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
gen_ldst_modrm(env, s, modrm, ot, OR_TMP0, 0);
gen_extu(ot, cpu_T0);
@@ -6907,7 +6929,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0xd4: /* aam */
if (CODE64(s))
goto illegal_op;
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
if (val == 0) {
gen_exception(s, EXCP00_DIVZ, pc_start - s->cs_base);
} else {
@@ -6918,7 +6940,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
case 0xd5: /* aad */
if (CODE64(s))
goto illegal_op;
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
gen_helper_aad(cpu_env, tcg_const_i32(val));
set_cc_op(s, CC_OP_LOGICB);
break;
@@ -6952,7 +6974,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_interrupt(s, EXCP03_INT3, pc_start - s->cs_base, s->pc - s->cs_base);
break;
case 0xcd: /* int N */
- val = cpu_ldub_code(env, s->pc++);
+ val = x86_ldub_code(env, s);
if (s->vm86 && s->iopl != 3) {
gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
} else {
@@ -7007,7 +7029,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
if (CODE64(s))
goto illegal_op;
ot = dflag;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = (modrm >> 3) & 7;
mod = (modrm >> 6) & 3;
if (mod == 3)
@@ -7186,7 +7208,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
}
break;
case 0x100:
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
op = (modrm >> 3) & 7;
switch(op) {
@@ -7251,7 +7273,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
break;
case 0x101:
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
switch (modrm) {
CASE_MODRM_MEM_OP(0): /* sgdt */
gen_svm_check_intercept(s, pc_start, SVM_EXIT_GDTR_READ);
@@ -7596,7 +7618,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
/* d_ot is the size of destination */
d_ot = dflag;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
mod = (modrm >> 6) & 3;
rm = (modrm & 7) | REX_B(s);
@@ -7625,7 +7647,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
t1 = tcg_temp_local_new();
t2 = tcg_temp_local_new();
ot = MO_16;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = (modrm >> 3) & 7;
mod = (modrm >> 6) & 3;
rm = modrm & 7;
@@ -7670,7 +7692,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
if (!s->pe || s->vm86)
goto illegal_op;
ot = dflag != MO_16 ? MO_32 : MO_16;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
gen_ldst_modrm(env, s, modrm, MO_16, OR_TMP0, 0);
t0 = tcg_temp_local_new();
@@ -7690,7 +7712,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
}
break;
case 0x118:
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
op = (modrm >> 3) & 7;
switch(op) {
@@ -7709,7 +7731,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
}
break;
case 0x11a:
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
if (s->flags & HF_MPX_EN_MASK) {
mod = (modrm >> 6) & 3;
reg = ((modrm >> 3) & 7) | rex_r;
@@ -7799,7 +7821,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_nop_modrm(env, s, modrm);
break;
case 0x11b:
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
if (s->flags & HF_MPX_EN_MASK) {
mod = (modrm >> 6) & 3;
reg = ((modrm >> 3) & 7) | rex_r;
@@ -7901,7 +7923,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_nop_modrm(env, s, modrm);
break;
case 0x119: case 0x11c ... 0x11f: /* nop (multi byte) */
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
gen_nop_modrm(env, s, modrm);
break;
case 0x120: /* mov reg, crN */
@@ -7909,7 +7931,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
if (s->cpl != 0) {
gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
} else {
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
/* Ignore the mod bits (assume (modrm&0xc0)==0xc0).
* AMD documentation (24594.pdf) and testing of
* intel 386 and 486 processors all show that the mod bits
@@ -7966,7 +7988,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
if (s->cpl != 0) {
gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
} else {
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
/* Ignore the mod bits (assume (modrm&0xc0)==0xc0).
* AMD documentation (24594.pdf) and testing of
* intel 386 and 486 processors all show that the mod bits
@@ -8012,7 +8034,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
if (!(s->cpuid_features & CPUID_SSE2))
goto illegal_op;
ot = mo_64_32(dflag);
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
if (mod == 3)
goto illegal_op;
@@ -8021,7 +8043,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
gen_ldst_modrm(env, s, modrm, ot, reg, 1);
break;
case 0x1ae:
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
switch (modrm) {
CASE_MODRM_MEM_OP(0): /* fxsave */
if (!(s->cpuid_features & CPUID_FXSR)
@@ -8219,7 +8241,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
break;
case 0x10d: /* 3DNow! prefetch(w) */
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
mod = (modrm >> 6) & 3;
if (mod == 3)
goto illegal_op;
@@ -8241,7 +8263,7 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
if (!(s->cpuid_ext_features & CPUID_EXT_POPCNT))
goto illegal_op;
- modrm = cpu_ldub_code(env, s->pc++);
+ modrm = x86_ldub_code(env, s);
reg = ((modrm >> 3) & 7) | rex_r;
if (s->prefix & PREFIX_DATA) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 15/29] target/i386: trap on instructions longer than >15 bytes
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (13 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 14/29] target/i386: introduce x86_ld*_code Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 16/29] memory: call log_start after region_add Paolo Bonzini
` (15 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel
Besides being more correct, arbitrarily long instruction allow the
generation of a translation block that spans three pages. This
confuses the generator and even allows ring 3 code to poison the
translation block cache and inject code into other processes that are
in guest ring 3.
This is an improved (and more invasive) fix for commit 30663fd ("tcg/i386:
Check the size of instruction being translated", 2017-03-24). In addition
to being more precise (and generating the right exception, which is #GP
rather than #UD), it distinguishes better between page faults and too long
instructions, as shown by this test case:
#include <sys/mman.h>
#include <string.h>
#include <stdio.h>
int main()
{
char *x = mmap(NULL, 8192, PROT_READ|PROT_WRITE|PROT_EXEC,
MAP_PRIVATE|MAP_ANON, -1, 0);
memset(x, 0x66, 4096);
x[4096] = 0x90;
x[4097] = 0xc3;
char *i = x + 4096 - 15;
mprotect(x + 4096, 4096, PROT_READ|PROT_WRITE);
((void(*)(void)) i) ();
}
... which produces a #GP without the mprotect, and a #PF with it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/translate.c | 29 ++++++++++++++++++++++-------
1 file changed, 22 insertions(+), 7 deletions(-)
diff --git a/target/i386/translate.c b/target/i386/translate.c
index 4a938c2..5f24a2d 100644
--- a/target/i386/translate.c
+++ b/target/i386/translate.c
@@ -136,6 +136,7 @@ typedef struct DisasContext {
int cpuid_ext3_features;
int cpuid_7_0_ebx_features;
int cpuid_xsave_features;
+ sigjmp_buf jmpbuf;
} DisasContext;
static void gen_eob(DisasContext *s);
@@ -1863,11 +1864,27 @@ static void gen_shifti(DisasContext *s1, int op, TCGMemOp ot, int d, int c)
}
}
+#define X86_MAX_INSN_LENGTH 15
+
static uint64_t advance_pc(CPUX86State *env, DisasContext *s, int num_bytes)
{
uint64_t pc = s->pc;
s->pc += num_bytes;
+ if (unlikely(s->pc - s->pc_start > X86_MAX_INSN_LENGTH)) {
+ /* If the instruction's 16th byte is on a different page than the 1st, a
+ * page fault on the second page wins over the general protection fault
+ * caused by the instruction being too long.
+ * This can happen even if the operand is only one byte long!
+ */
+ if (((s->pc - 1) ^ (pc - 1)) & TARGET_PAGE_MASK) {
+ volatile uint8_t unused =
+ cpu_ldub_code(env, (s->pc - 1) & TARGET_PAGE_MASK);
+ (void) unused;
+ }
+ siglongjmp(s->jmpbuf, 1);
+ }
+
return pc;
}
@@ -4463,14 +4480,12 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
s->rip_offset = 0; /* for relative ip address */
s->vex_l = 0;
s->vex_v = 0;
- next_byte:
- /* x86 has an upper limit of 15 bytes for an instruction. Since we
- * do not want to decode and generate IR for an illegal
- * instruction, the following check limits the instruction size to
- * 25 bytes: 14 prefix + 1 opc + 6 (modrm+sib+ofs) + 4 imm */
- if (s->pc - pc_start > 14) {
- goto illegal_op;
+ if (sigsetjmp(s->jmpbuf, 0) != 0) {
+ gen_exception(s, EXCP0D_GPF, pc_start - s->cs_base);
+ return s->pc;
}
+
+ next_byte:
b = x86_ldub_code(env, s);
/* Collect prefixes. */
switch (b) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 16/29] memory: call log_start after region_add
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (14 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 15/29] target/i386: trap on instructions longer than >15 bytes Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 17/29] kvm: fix alignment of ram address Paolo Bonzini
` (14 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: David Hildenbrand
From: David Hildenbrand <david@redhat.com>
It might be confusing for some listener implementations that implement
both, region_add and log_start (e.g. KVM) if we call log_start before an
actual region was added using region_add.
This makes current KVM code trigger an assertion
("kvm_section_update_flags: error finding slot"). So let's just reverse
the order instead of tolerating log_start on yet unknown regions.
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20171016144302.24284-2-david@redhat.com>
Tested-by: Joe Clifford <joeclifford@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
memory.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/memory.c b/memory.c
index b637c12..3e1558a 100644
--- a/memory.c
+++ b/memory.c
@@ -2607,12 +2607,12 @@ static void listener_add_address_space(MemoryListener *listener,
.offset_within_address_space = int128_get64(fr->addr.start),
.readonly = fr->readonly,
};
- if (fr->dirty_log_mask && listener->log_start) {
- listener->log_start(listener, §ion, 0, fr->dirty_log_mask);
- }
if (listener->region_add) {
listener->region_add(listener, §ion);
}
+ if (fr->dirty_log_mask && listener->log_start) {
+ listener->log_start(listener, §ion, 0, fr->dirty_log_mask);
+ }
}
if (listener->commit) {
listener->commit(listener);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 17/29] kvm: fix alignment of ram address
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (15 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 16/29] memory: call log_start after region_add Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 18/29] kvm: tolerate non-existing slot for log_start/log_stop/log_sync Paolo Bonzini
` (13 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: David Hildenbrand
From: David Hildenbrand <david@redhat.com>
Fix the wrong calculation of the delta, used to align the ram address.
This only strikes if alignment has to be done.
Reported-by: Joe Clifford <joeclifford@gmail.com>
Fixes: 5ea69c2e3614 ("kvm: factor out alignment of memory section")
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20171016144302.24284-3-david@redhat.com>
Tested-by: Joe Clifford <joeclifford@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
accel/kvm/kvm-all.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 90c88b5..fae1eca 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -717,8 +717,9 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
return;
}
+ /* use aligned delta to align the ram address */
ram = memory_region_get_ram_ptr(mr) + section->offset_within_region +
- (section->offset_within_address_space - start_addr);
+ (start_addr - section->offset_within_address_space);
mem = kvm_lookup_matching_slot(kml, start_addr, size);
if (!add) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 18/29] kvm: tolerate non-existing slot for log_start/log_stop/log_sync
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (16 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 17/29] kvm: fix alignment of ram address Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 19/29] kvm: fix error message when failing to unregister slot Paolo Bonzini
` (12 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: David Hildenbrand
From: David Hildenbrand <david@redhat.com>
If we want to trap every access to a section, we might not have a
slot. So let's just tolerate if we don't have one.
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20171016144302.24284-4-david@redhat.com>
Tested-by: Joe Clifford <joeclifford@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
accel/kvm/kvm-all.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index fae1eca..f5fa3e2 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -394,8 +394,8 @@ static int kvm_section_update_flags(KVMMemoryListener *kml,
mem = kvm_lookup_matching_slot(kml, start_addr, size);
if (!mem) {
- fprintf(stderr, "%s: error finding slot\n", __func__);
- abort();
+ /* We don't have a slot if we want to trap every access. */
+ return 0;
}
return kvm_slot_update_flags(kml, mem, section->mr);
@@ -470,8 +470,8 @@ static int kvm_physical_sync_dirty_bitmap(KVMMemoryListener *kml,
if (size) {
mem = kvm_lookup_matching_slot(kml, start_addr, size);
if (!mem) {
- fprintf(stderr, "%s: error finding slot\n", __func__);
- abort();
+ /* We don't have a slot if we want to trap every access. */
+ return 0;
}
/* XXX bad kernel interface alert
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 19/29] kvm: fix error message when failing to unregister slot
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (17 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 18/29] kvm: tolerate non-existing slot for log_start/log_stop/log_sync Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 20/29] kvm: region_add and region_del is not called on updates Paolo Bonzini
` (11 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: David Hildenbrand
From: David Hildenbrand <david@redhat.com>
"overlapping" is a leftover, let's drop it.
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20171016144302.24284-5-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
accel/kvm/kvm-all.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index f5fa3e2..559c544 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -734,7 +734,7 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
mem->memory_size = 0;
err = kvm_set_user_memory_region(kml, mem);
if (err) {
- fprintf(stderr, "%s: error unregistering overlapping slot: %s\n",
+ fprintf(stderr, "%s: error unregistering slot: %s\n",
__func__, strerror(-err));
abort();
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 20/29] kvm: region_add and region_del is not called on updates
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (18 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 19/29] kvm: fix error message when failing to unregister slot Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 21/29] kvm: simplify kvm_align_section() Paolo Bonzini
` (10 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: David Hildenbrand
From: David Hildenbrand <david@redhat.com>
Attributes are not updated via region_add()/region_del(). Attribute changes
lead to a delete first, followed by a new add.
If this would ever not be the case, we would get an error when trying to
register the new slot.
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20171016144302.24284-6-david@redhat.com>
Tested-by: Joe Clifford <joeclifford@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
accel/kvm/kvm-all.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 559c544..2835bb3 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -721,8 +721,8 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
ram = memory_region_get_ram_ptr(mr) + section->offset_within_region +
(start_addr - section->offset_within_address_space);
- mem = kvm_lookup_matching_slot(kml, start_addr, size);
if (!add) {
+ mem = kvm_lookup_matching_slot(kml, start_addr, size);
if (!mem) {
return;
}
@@ -741,12 +741,6 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
return;
}
- if (mem) {
- /* update the slot */
- kvm_slot_update_flags(kml, mem, mr);
- return;
- }
-
/* register the new slot */
mem = kvm_alloc_slot(kml);
mem->memory_size = size;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 21/29] kvm: simplify kvm_align_section()
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (19 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 20/29] kvm: region_add and region_del is not called on updates Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 22/29] memory: reuse section_from_flat_range() Paolo Bonzini
` (9 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: David Hildenbrand
From: David Hildenbrand <david@redhat.com>
Use ROUND_UP and simplify the code a bit.
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20171016144302.24284-7-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
accel/kvm/kvm-all.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 2835bb3..f290f48 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -197,26 +197,20 @@ static hwaddr kvm_align_section(MemoryRegionSection *section,
hwaddr *start)
{
hwaddr size = int128_get64(section->size);
- hwaddr delta;
-
- *start = section->offset_within_address_space;
+ hwaddr delta, aligned;
/* kvm works in page size chunks, but the function may be called
with sub-page size and unaligned start address. Pad the start
address to next and truncate size to previous page boundary. */
- delta = qemu_real_host_page_size - (*start & ~qemu_real_host_page_mask);
- delta &= ~qemu_real_host_page_mask;
- *start += delta;
+ aligned = ROUND_UP(section->offset_within_address_space,
+ qemu_real_host_page_size);
+ delta = aligned - section->offset_within_address_space;
+ *start = aligned;
if (delta > size) {
return 0;
}
- size -= delta;
- size &= qemu_real_host_page_mask;
- if (*start & ~qemu_real_host_page_mask) {
- return 0;
- }
- return size;
+ return (size - delta) & qemu_real_host_page_mask;
}
int kvm_physical_memory_addr_from_host(KVMState *s, void *ram,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 22/29] memory: reuse section_from_flat_range()
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (20 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 21/29] kvm: simplify kvm_align_section() Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 23/29] notdirty_mem_write: implement 8-byte accesses Paolo Bonzini
` (8 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: David Hildenbrand
From: David Hildenbrand <david@redhat.com>
We can use section_from_flat_range() instead of manually initializing.
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20171016144302.24284-8-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
memory.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/memory.c b/memory.c
index 3e1558a..e26e5a3 100644
--- a/memory.c
+++ b/memory.c
@@ -2599,14 +2599,8 @@ static void listener_add_address_space(MemoryListener *listener,
view = address_space_get_flatview(as);
FOR_EACH_FLAT_RANGE(fr, view) {
- MemoryRegionSection section = {
- .mr = fr->mr,
- .fv = view,
- .offset_within_region = fr->offset_in_region,
- .size = fr->addr.size,
- .offset_within_address_space = int128_get64(fr->addr.start),
- .readonly = fr->readonly,
- };
+ MemoryRegionSection section = section_from_flat_range(fr, view);
+
if (listener->region_add) {
listener->region_add(listener, §ion);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 23/29] notdirty_mem_write: implement 8-byte accesses
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (21 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 22/29] memory: reuse section_from_flat_range() Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 24/29] watch_mem_write: " Paolo Bonzini
` (7 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Andrew Baumann
From: Andrew Baumann <Andrew.Baumann@microsoft.com>
Aligned 8-byte memory writes by a 64-bit target on a 64-bit host should
always turn into atomic 8-byte writes on the host, however if we missed
in the softmmu, and the TLB line was marked as not dirty, then we
would end up tearing the 8-byte write into two 4-byte writes in
access_with_adjusted_size().
Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
Message-Id: <20171013181913.7556-1-Andrew.Baumann@microsoft.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/exec.c b/exec.c
index 6579e7a..b58bc4e 100644
--- a/exec.c
+++ b/exec.c
@@ -2376,6 +2376,9 @@ static void notdirty_mem_write(void *opaque, hwaddr ram_addr,
case 4:
stl_p(qemu_map_ram_ptr(NULL, ram_addr), val);
break;
+ case 8:
+ stq_p(qemu_map_ram_ptr(NULL, ram_addr), val);
+ break;
default:
abort();
}
@@ -2406,6 +2409,16 @@ static const MemoryRegionOps notdirty_mem_ops = {
.write = notdirty_mem_write,
.valid.accepts = notdirty_mem_accepts,
.endianness = DEVICE_NATIVE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 8,
+ .unaligned = false,
+ },
+ .impl = {
+ .min_access_size = 1,
+ .max_access_size = 8,
+ .unaligned = false,
+ },
};
/* Generate a debug exception if a watchpoint has been hit. */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 24/29] watch_mem_write: implement 8-byte accesses
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (22 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 23/29] notdirty_mem_write: implement 8-byte accesses Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 25/29] qemu-pr-helper: use new libmultipath API Paolo Bonzini
` (6 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel
Aligned 8-byte memory writes by a 64-bit target on a 64-bit host should
always turn into atomic 8-byte writes on the host, however a write
write watchpoint would end up tearing the 8-byte write into two 4-byte
writes in access_with_adjusted_size().
Reported-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/exec.c b/exec.c
index b58bc4e..db5ae23 100644
--- a/exec.c
+++ b/exec.c
@@ -2503,6 +2503,9 @@ static MemTxResult watch_mem_read(void *opaque, hwaddr addr, uint64_t *pdata,
case 4:
data = address_space_ldl(as, addr, attrs, &res);
break;
+ case 8:
+ data = address_space_ldq(as, addr, attrs, &res);
+ break;
default: abort();
}
*pdata = data;
@@ -2528,6 +2531,9 @@ static MemTxResult watch_mem_write(void *opaque, hwaddr addr,
case 4:
address_space_stl(as, addr, val, attrs, &res);
break;
+ case 8:
+ address_space_stq(as, addr, val, attrs, &res);
+ break;
default: abort();
}
return res;
@@ -2537,6 +2543,16 @@ static const MemoryRegionOps watch_mem_ops = {
.read_with_attrs = watch_mem_read,
.write_with_attrs = watch_mem_write,
.endianness = DEVICE_NATIVE_ENDIAN,
+ .valid = {
+ .min_access_size = 1,
+ .max_access_size = 8,
+ .unaligned = false,
+ },
+ .impl = {
+ .min_access_size = 1,
+ .max_access_size = 8,
+ .unaligned = false,
+ },
};
static MemTxResult flatview_write(FlatView *fv, hwaddr addr, MemTxAttrs attrs,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 25/29] qemu-pr-helper: use new libmultipath API
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (23 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 24/29] watch_mem_write: " Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 26/29] qdev: store DeviceState's canonical path to use when unparenting Paolo Bonzini
` (5 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel
libmultipath has recently changed its API. The new API supports multi-threaded
clients better. Unfortunately there is no backwards-compatibility, so we just
switch to the new one. Running QEMU compiled with the new library on the old
library will likely crash, while doing the opposite will cause QEMU not to
start at all (because udev, get_multipath_config and put_multipath_config
are undefined).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
configure | 12 ++++++++++--
scsi/qemu-pr-helper.c | 17 ++++++++++++++---
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/configure b/configure
index 6587e80..7766e74 100755
--- a/configure
+++ b/configure
@@ -3312,9 +3312,17 @@ if test "$mpath" != "no" ; then
#include <mpath_persist.h>
unsigned mpath_mx_alloc_len = 1024;
int logsink;
+static struct config *multipath_conf;
+extern struct udev *udev;
+extern struct config *get_multipath_config(void);
+extern void put_multipath_config(struct config *conf);
+struct udev *udev;
+struct config *get_multipath_config(void) { return multipath_conf; }
+void put_multipath_config(struct config *conf) { }
+
int main(void) {
- struct udev *udev = udev_new();
- mpath_lib_init(udev);
+ udev = udev_new();
+ multipath_conf = mpath_lib_init();
return 0;
}
EOF
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index d581848..dd97851 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -276,15 +276,26 @@ static void dm_init(void)
/* Variables required by libmultipath and libmpathpersist. */
QEMU_BUILD_BUG_ON(PR_HELPER_DATA_SIZE > MPATH_MAX_PARAM_LEN);
+static struct config *multipath_conf;
unsigned mpath_mx_alloc_len = PR_HELPER_DATA_SIZE;
int logsink;
+struct udev *udev;
-static void multipath_pr_init(void)
+extern struct config *get_multipath_config(void);
+struct config *get_multipath_config(void)
{
- static struct udev *udev;
+ return multipath_conf;
+}
+extern void put_multipath_config(struct config *conf);
+void put_multipath_config(struct config *conf)
+{
+}
+
+static void multipath_pr_init(void)
+{
udev = udev_new();
- mpath_lib_init(udev);
+ multipath_conf = mpath_lib_init();
}
static int is_mpath(int fd)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 26/29] qdev: store DeviceState's canonical path to use when unparenting
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (24 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 25/29] qemu-pr-helper: use new libmultipath API Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 27/29] Revert "qdev: Free QemuOpts when the QOM path goes away" Paolo Bonzini
` (4 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth, Michael S. Tsirkin, Greg Kurz
From: Michael Roth <mdroth@linux.vnet.ibm.com>
device_unparent(dev, ...) is called when a device is unparented,
either directly, or as a result of a parent device being
finalized, and handles some final cleanup for the device. Part
of this includes emiting a DEVICE_DELETED QMP event to notify
management, which includes the device's path in the composition
tree as provided by object_get_canonical_path().
object_get_canonical_path() assumes the device is still connected
to the machine/root container, and will assert otherwise, but
in some situations this isn't the case:
If the parent is finalized as a result of object_unparent(), it
will still be attached to the composition tree at the time any
children are unparented as a result of that same call to
object_unparent(). However, in some cases, object_unparent()
will complete without finalizing the parent device, due to
lingering references that won't be released till some time later.
One such example is if the parent has MemoryRegion children (which
take a ref on their parent), who in turn have AddressSpace's (which
take a ref on their regions), since those AddressSpaces get cleaned
up asynchronously by the RCU thread.
In this case qdev:device_unparent() may be called for a child Device
that no longer has a path to the root/machine container, causing
object_get_canonical_path() to assert.
Fix this by storing the canonical path during realize() so the
information will still be available for device_unparent() in such
cases.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Message-Id: <20171016222315.407-2-mdroth@linux.vnet.ibm.com>
[Clear dev->canonical_path at the post_realize_fail label, which is
cleaner. Suggested by David Gibson. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/qdev.c | 17 ++++++++++++++---
include/hw/qdev-core.h | 1 +
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 606ab53..0019a49 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -928,6 +928,13 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
goto post_realize_fail;
}
+ /*
+ * always free/re-initialize here since the value cannot be cleaned up
+ * in device_unrealize due to its usage later on in the unplug path
+ */
+ g_free(dev->canonical_path);
+ dev->canonical_path = object_get_canonical_path(OBJECT(dev));
+
if (qdev_get_vmsd(dev)) {
if (vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
dev->instance_id_alias,
@@ -984,6 +991,8 @@ child_realize_fail:
}
post_realize_fail:
+ g_free(dev->canonical_path);
+ dev->canonical_path = NULL;
if (dc->unrealize) {
dc->unrealize(dev, NULL);
}
@@ -1102,10 +1111,12 @@ static void device_unparent(Object *obj)
/* Only send event if the device had been completely realized */
if (dev->pending_deleted_event) {
- gchar *path = object_get_canonical_path(OBJECT(dev));
+ g_assert(dev->canonical_path);
- qapi_event_send_device_deleted(!!dev->id, dev->id, path, &error_abort);
- g_free(path);
+ qapi_event_send_device_deleted(!!dev->id, dev->id, dev->canonical_path,
+ &error_abort);
+ g_free(dev->canonical_path);
+ dev->canonical_path = NULL;
}
qemu_opts_del(dev->opts);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 0891461..0a71bf8 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -153,6 +153,7 @@ struct DeviceState {
/*< public >*/
const char *id;
+ char *canonical_path;
bool realized;
bool pending_deleted_event;
QemuOpts *opts;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 27/29] Revert "qdev: Free QemuOpts when the QOM path goes away"
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (25 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 26/29] qdev: store DeviceState's canonical path to use when unparenting Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 28/29] qdev: defer DEVICE_DEL event until instance_finalize() Paolo Bonzini
` (3 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth
From: Michael Roth <mdroth@linux.vnet.ibm.com>
This reverts commit abed886ec60cf239a03515cf0b30fb11fa964c44.
This patch originally addressed an issue where a DEVICE_DELETED
event could be emitted (in device_unparent()) before a Device's
QemuOpts were cleaned up (in device_finalize()), leading to a
"duplicate ID" error if management attempted to immediately add
a device with the same ID in response to the DEVICE_DELETED event.
An alternative will be implemented in a subsequent patch where we
defer the DEVICE_DELETED event until device_finalize(), which would
also prevent the race, so we revert the original fix in preparation.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Message-Id: <20171016222315.407-3-mdroth@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/qdev.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 0019a49..418cdac 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1069,6 +1069,7 @@ static void device_finalize(Object *obj)
NamedGPIOList *ngl, *next;
DeviceState *dev = DEVICE(obj);
+ qemu_opts_del(dev->opts);
QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) {
QLIST_REMOVE(ngl, node);
@@ -1118,9 +1119,6 @@ static void device_unparent(Object *obj)
g_free(dev->canonical_path);
dev->canonical_path = NULL;
}
-
- qemu_opts_del(dev->opts);
- dev->opts = NULL;
}
static void device_class_init(ObjectClass *class, void *data)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 28/29] qdev: defer DEVICE_DEL event until instance_finalize()
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (26 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 27/29] Revert "qdev: Free QemuOpts when the QOM path goes away" Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 29/29] scsi: reject configurations with logical block size > physical block size Paolo Bonzini
` (2 subsequent siblings)
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Roth
From: Michael Roth <mdroth@linux.vnet.ibm.com>
DEVICE_DEL is currently emitted when a Device is unparented, as
opposed to when it is finalized. The main design motivation for this
seems to be that after unparent()/unrealize(), the Device is no
longer visible to the guest, and thus the operation is complete
from the perspective of management.
However, there are cases where remaining host-side cleanup is also
pertinent to management. The is generally handled by treating these
resources as aspects of the "backend", which can be managed via
separate interfaces/events, such as blockdev_add/del, netdev_add/del,
object_add/del, etc, but some devices do not have this level of
compartmentalization, namely vfio-pci, and possibly to lend themselves
well to it.
In the case of vfio-pci, the "backend" cleanup happens as part of
the finalization of the vfio-pci device itself, in particular the
cleanup of the VFIO group FD. Failing to wait for this cleanup can
result in tools like libvirt attempting to rebind the device to
the host while it's still being used by VFIO, which can result in
host crashes or other misbehavior depending on the host driver.
Deferring DEVICE_DEL still affords us the ability to manage backends
explicitly, while also addressing cases like vfio-pci's, so we
implement that approach here.
An alternative proposal involving having VFIO emit a separate event
to denote completion of host-side cleanup was discussed, but the
prevailing opinion seems to be that it is not worth the added
complexity, and leaves the issue open for other Device implementations
to solve in the future.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Message-Id: <20171016222315.407-4-mdroth@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/qdev.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 418cdac..1111295 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1069,7 +1069,6 @@ static void device_finalize(Object *obj)
NamedGPIOList *ngl, *next;
DeviceState *dev = DEVICE(obj);
- qemu_opts_del(dev->opts);
QLIST_FOREACH_SAFE(ngl, &dev->gpios, node, next) {
QLIST_REMOVE(ngl, node);
@@ -1080,6 +1079,18 @@ static void device_finalize(Object *obj)
* here
*/
}
+
+ /* Only send event if the device had been completely realized */
+ if (dev->pending_deleted_event) {
+ g_assert(dev->canonical_path);
+
+ qapi_event_send_device_deleted(!!dev->id, dev->id, dev->canonical_path,
+ &error_abort);
+ g_free(dev->canonical_path);
+ dev->canonical_path = NULL;
+ }
+
+ qemu_opts_del(dev->opts);
}
static void device_class_base_init(ObjectClass *class, void *data)
@@ -1109,16 +1120,6 @@ static void device_unparent(Object *obj)
object_unref(OBJECT(dev->parent_bus));
dev->parent_bus = NULL;
}
-
- /* Only send event if the device had been completely realized */
- if (dev->pending_deleted_event) {
- g_assert(dev->canonical_path);
-
- qapi_event_send_device_deleted(!!dev->id, dev->id, dev->canonical_path,
- &error_abort);
- g_free(dev->canonical_path);
- dev->canonical_path = NULL;
- }
}
static void device_class_init(ObjectClass *class, void *data)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* [Qemu-devel] [PULL 29/29] scsi: reject configurations with logical block size > physical block size
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (27 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 28/29] qdev: defer DEVICE_DEL event until instance_finalize() Paolo Bonzini
@ 2017-10-18 16:12 ` Paolo Bonzini
2017-10-18 17:13 ` [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 no-reply
2017-10-19 15:47 ` Peter Maydell
30 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-10-18 16:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Mark Kanda
From: Mark Kanda <mark.kanda@oracle.com>
Logical block size of a SCSI disk should never be larger than
physical block size. From an ATA/SCSI perspective, it makes no sense
to have the logical block size greater than the physical block size,
and it cannot even be effectively expressed in the command set. The
whole point of adding the physical block size to the ATA/SCSI command
set was to communicate a desire for a larger block size (than logical),
while maintaining backwards compatibility with legacy 512 byte block
size.
When setting logical_block_size > physical_block_size, QEMU cannot express
it in READ CAPACITY(16) output, and all it can do is set the physical
block exponent to 0 (i.e. logical_block_size == physical_block_size).
Reporting the error properly, however, is better.
Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Message-Id: <1508185024-5840-1-git-send-email-mark.kanda@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/scsi/scsi-disk.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index a518080..1243117 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2347,6 +2347,14 @@ static void scsi_realize(SCSIDevice *dev, Error **errp)
blkconf_serial(&s->qdev.conf, &s->serial);
blkconf_blocksizes(&s->qdev.conf);
+
+ if (s->qdev.conf.logical_block_size >
+ s->qdev.conf.physical_block_size) {
+ error_setg(errp,
+ "logical_block_size > physical_block_size not supported");
+ return;
+ }
+
if (dev->type == TYPE_DISK) {
blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, &err);
if (err) {
--
1.8.3.1
^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (28 preceding siblings ...)
2017-10-18 16:12 ` [Qemu-devel] [PULL 29/29] scsi: reject configurations with logical block size > physical block size Paolo Bonzini
@ 2017-10-18 17:13 ` no-reply
2017-10-19 15:47 ` Peter Maydell
30 siblings, 0 replies; 32+ messages in thread
From: no-reply @ 2017-10-18 17:13 UTC (permalink / raw)
To: pbonzini; +Cc: famz, qemu-devel
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 1508343141-31835-1-git-send-email-pbonzini@redhat.com
Subject: [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20171018170138.19078-1-dgilbert@redhat.com -> patchew/20171018170138.19078-1-dgilbert@redhat.com
Switched to a new branch 'test'
ddf5f0fd53 scsi: reject configurations with logical block size > physical block size
d6c780ef4a qdev: defer DEVICE_DEL event until instance_finalize()
59877522d9 Revert "qdev: Free QemuOpts when the QOM path goes away"
262daf3cfb qdev: store DeviceState's canonical path to use when unparenting
7e71e1069a qemu-pr-helper: use new libmultipath API
eb4c2f258b watch_mem_write: implement 8-byte accesses
1e543aec92 notdirty_mem_write: implement 8-byte accesses
cfdda4621b memory: reuse section_from_flat_range()
49cfdcaf2f kvm: simplify kvm_align_section()
2436b118fd kvm: region_add and region_del is not called on updates
105a4b17f7 kvm: fix error message when failing to unregister slot
ffbfafda7f kvm: tolerate non-existing slot for log_start/log_stop/log_sync
f2e1adbc75 kvm: fix alignment of ram address
c950b87b75 memory: call log_start after region_add
b396744e40 target/i386: trap on instructions longer than >15 bytes
8ff591fc32 target/i386: introduce x86_ld*_code
005c3cd495 tco: add trace events
337d49e940 docs/devel/loads-stores.rst: Document our various load and store APIs
2afb7e8244 nios2: define tcg_env
14589aebe5 build: remove CONFIG_LIBDECNUMBER
52d6bdeec1 disas: Always initialize read_memory_inner_func properly
39d88c4a9e pc: make sure that plugged CPUs are of the same type
aacc1d66c0 memory: fix off-by-one error in memory_region_notify_one()
aa23b0ddd4 exec: simplify address_space_get_iotlb_entry
de39bf7eee exec: add page_mask for flatview_do_translate
94a9bb400c char: don't skip client cleanup if 'connected' flag is unset
eabec1e383 ide: support reporting of rotation rate
dfbb026ca5 scsi-disk: support reporting of rotation rate
f19f1e6276 checkpatch: refine mode selection
=== OUTPUT BEGIN ===
Checking PATCH 1/29: checkpatch: refine mode selection...
Checking PATCH 2/29: scsi-disk: support reporting of rotation rate...
Checking PATCH 3/29: ide: support reporting of rotation rate...
Checking PATCH 4/29: char: don't skip client cleanup if 'connected' flag is unset...
Checking PATCH 5/29: exec: add page_mask for flatview_do_translate...
Checking PATCH 6/29: exec: simplify address_space_get_iotlb_entry...
Checking PATCH 7/29: memory: fix off-by-one error in memory_region_notify_one()...
Checking PATCH 8/29: pc: make sure that plugged CPUs are of the same type...
ERROR: space required before the open parenthesis '('
#35: FILE: hw/i386/pc.c:1882:
+ if(!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) {
total: 1 errors, 0 warnings, 15 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 9/29: disas: Always initialize read_memory_inner_func properly...
Checking PATCH 10/29: build: remove CONFIG_LIBDECNUMBER...
Checking PATCH 11/29: nios2: define tcg_env...
Checking PATCH 12/29: docs/devel/loads-stores.rst: Document our various load and store APIs...
Checking PATCH 13/29: tco: add trace events...
Checking PATCH 14/29: target/i386: introduce x86_ld*_code...
Checking PATCH 15/29: target/i386: trap on instructions longer than >15 bytes...
ERROR: Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt
#69: FILE: target/i386/translate.c:1881:
+ volatile uint8_t unused =
total: 1 errors, 0 warnings, 53 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 16/29: memory: call log_start after region_add...
Checking PATCH 17/29: kvm: fix alignment of ram address...
Checking PATCH 18/29: kvm: tolerate non-existing slot for log_start/log_stop/log_sync...
Checking PATCH 19/29: kvm: fix error message when failing to unregister slot...
Checking PATCH 20/29: kvm: region_add and region_del is not called on updates...
Checking PATCH 21/29: kvm: simplify kvm_align_section()...
Checking PATCH 22/29: memory: reuse section_from_flat_range()...
Checking PATCH 23/29: notdirty_mem_write: implement 8-byte accesses...
Checking PATCH 24/29: watch_mem_write: implement 8-byte accesses...
Checking PATCH 25/29: qemu-pr-helper: use new libmultipath API...
ERROR: externs should be avoided in .c files
#54: FILE: scsi/qemu-pr-helper.c:284:
+extern struct config *get_multipath_config(void);
ERROR: externs should be avoided in .c files
#61: FILE: scsi/qemu-pr-helper.c:290:
+extern void put_multipath_config(struct config *conf);
total: 2 errors, 0 warnings, 48 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 26/29: qdev: store DeviceState's canonical path to use when unparenting...
Checking PATCH 27/29: Revert "qdev: Free QemuOpts when the QOM path goes away"...
Checking PATCH 28/29: qdev: defer DEVICE_DEL event until instance_finalize()...
Checking PATCH 29/29: scsi: reject configurations with logical block size > physical block size...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
` (29 preceding siblings ...)
2017-10-18 17:13 ` [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 no-reply
@ 2017-10-19 15:47 ` Peter Maydell
30 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2017-10-19 15:47 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU Developers
On 18 October 2017 at 17:11, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit a0b261db8c030813e30a39eae47359ac2a37f7e2:
>
> Merge remote-tracking branch 'remotes/ehabkost/tags/python-next-pull-request' into staging (2017-10-12 10:02:09 +0100)
>
> are available in the git repository at:
>
>
> git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 3da023b5827543ee4c022986ea2ad9d1274410b2:
>
> scsi: reject configurations with logical block size > physical block size (2017-10-18 11:56:14 +0200)
>
> ----------------------------------------------------------------
> * TCG 8-byte atomic accesses bugfix (Andrew)
> * Report disk rotation rate (Daniel)
> * Report invalid scsi-disk block size configuration (Mark)
> * KVM and memory API MemoryListener fixes (David, Maxime, Peter Xu)
> * x86 CPU hotplug crash fix (Igor)
> * Load/store API documentation (Peter Maydell)
> * Small fixes by myself and Thomas
> * qdev DEVICE_DELETED deferral (Michael)
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2017-10-19 15:48 UTC | newest]
Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-10-18 16:11 [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 01/29] checkpatch: refine mode selection Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 02/29] scsi-disk: support reporting of rotation rate Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 03/29] ide: " Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 04/29] char: don't skip client cleanup if 'connected' flag is unset Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 05/29] exec: add page_mask for flatview_do_translate Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 06/29] exec: simplify address_space_get_iotlb_entry Paolo Bonzini
2017-10-18 16:11 ` [Qemu-devel] [PULL 07/29] memory: fix off-by-one error in memory_region_notify_one() Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 08/29] pc: make sure that plugged CPUs are of the same type Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 09/29] disas: Always initialize read_memory_inner_func properly Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 10/29] build: remove CONFIG_LIBDECNUMBER Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 11/29] nios2: define tcg_env Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 12/29] docs/devel/loads-stores.rst: Document our various load and store APIs Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 13/29] tco: add trace events Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 14/29] target/i386: introduce x86_ld*_code Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 15/29] target/i386: trap on instructions longer than >15 bytes Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 16/29] memory: call log_start after region_add Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 17/29] kvm: fix alignment of ram address Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 18/29] kvm: tolerate non-existing slot for log_start/log_stop/log_sync Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 19/29] kvm: fix error message when failing to unregister slot Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 20/29] kvm: region_add and region_del is not called on updates Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 21/29] kvm: simplify kvm_align_section() Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 22/29] memory: reuse section_from_flat_range() Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 23/29] notdirty_mem_write: implement 8-byte accesses Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 24/29] watch_mem_write: " Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 25/29] qemu-pr-helper: use new libmultipath API Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 26/29] qdev: store DeviceState's canonical path to use when unparenting Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 27/29] Revert "qdev: Free QemuOpts when the QOM path goes away" Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 28/29] qdev: defer DEVICE_DEL event until instance_finalize() Paolo Bonzini
2017-10-18 16:12 ` [Qemu-devel] [PULL 29/29] scsi: reject configurations with logical block size > physical block size Paolo Bonzini
2017-10-18 17:13 ` [Qemu-devel] [PULL 00/29] Misc patches for 2017-10-18 no-reply
2017-10-19 15:47 ` Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).