Re: [Qemu-devel] [PATCH v2 2/4] compiler: drop ; after BUILD_BUG_ON
"Michael S. Tsirkin" writes: > All users include the trailing ;, let's require that > so that uses such as if (a) QEMU_BUILD_BUG_ON(); do not > produce unexpected results. > > Not a huge problem for QEMU since our style requires the use > of {} but seems cleaner nevertheless. I think the actual problem is that it sets a bad example. > Signed-off-by: Michael S. Tsirkin > --- > include/qemu/compiler.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > index 157698b..2882470 100644 > --- a/include/qemu/compiler.h > +++ b/include/qemu/compiler.h > @@ -86,7 +86,8 @@ > #define type_check(t1,t2) ((t1*)0 - (t2*)0) > > #define QEMU_BUILD_BUG_ON(x) \ > -typedef char glue(qemu_build_bug_on__,__LINE__)[(x)?-1:1] > __attribute__((unused)); > +typedef char glue(qemu_build_bug_on__,__LINE__)[(x) ? -1 : 1] \ > +__attribute__((unused)) > > #if defined __GNUC__ > # if !QEMU_GNUC_PREREQ(4, 4) Preferably with the commit message clarified a bit: Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH v2 3/4] compiler: expression version of QEMU_BUILD_BUG_ON
"Michael S. Tsirkin" writes: > QEMU_BUILD_BUG_ON uses a typedef in order to be safe > to use outside functions, but sometimes it's useful > to have a version that can be used within an expression. > Following what Linux does, introduce QEMU_BUILD_BUG_ON_ZERO > that return zero after checking condition at build time. Following Linux's example makes sense, but I can't help but wonder whether we need both QEMU_BUILD_BUG_ON_ZERO() and QEMU_BUILD_BUG_ON(). > Signed-off-by: Michael S. Tsirkin > --- > include/qemu/compiler.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > index 2882470..f4cf13b 100644 > --- a/include/qemu/compiler.h > +++ b/include/qemu/compiler.h > @@ -89,6 +89,8 @@ > typedef char glue(qemu_build_bug_on__,__LINE__)[(x) ? -1 : 1] \ > __attribute__((unused)) > > +#define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(int[(x) ? -1 : 1]) - sizeof(int)) > + > #if defined __GNUC__ > # if !QEMU_GNUC_PREREQ(4, 4) > /* gcc versions before 4.4.x don't support gnu_printf, so use printf. */ More so since your QEMU_BUILD_BUG_ON_ZERO seems easier to understand: no token pasting. Anyway, Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH v2 4/4] ARRAY_SIZE: check that argument is an array
"Michael S. Tsirkin" writes: > It's a familiar pattern: some code uses ARRAY_SIZE, then refactoring > changes the argument from an array to a pointer to a dynamically > allocated buffer. Code keeps compiling but any ARRAY_SIZE calls now > return the size of the pointer divided by element size. > > Let's add build time checks to ARRAY_SIZE before we allow more > of these in the code-base. Yes, please! > Signed-off-by: Michael S. Tsirkin > --- > include/qemu/osdep.h | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index 689f253..24bfda0 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -199,7 +199,13 @@ extern int daemon(int, int); > #endif > > #ifndef ARRAY_SIZE > -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) > +/* > + * &(x)[0] is always a pointer - if it's same type as x then the argument is > a > + * pointer, not an array as expected. > + */ > +#define ARRAY_SIZE(x) ((sizeof(x) / sizeof((x)[0])) + > QEMU_BUILD_BUG_ON_ZERO( \ > +__builtin_types_compatible_p(typeof(x), \ > + typeof(&(x)[0] Please break the line near the operator, not within one of its operands: #define ARRAY_SIZE(x) ((sizeof(x) / sizeof((x)[0])) \ + QEMU_BUILD_BUG_ON_ZERO( \ __builtin_types_compatible_p(typeof(x), \ typeof(&(x)[0] > #endif > > int qemu_daemon(int nochdir, int noclose); With the confusing line break tiedied up: Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH v2 23/25] console: make screendump async
On Mi, 2017-01-18 at 20:03 +0400, Marc-André Lureau wrote: > +surface = qemu_console_surface(con); > + > +/* FIXME: async save with coroutine? it would have to copy or > lock > + * the surface. */ > +ppm_save(filename, surface, &err); > + No need to lock or copy. ppm_save() uses the pixman image (surface->image) only anyway, so changing it to accept a pixman image instead of the surface is easy. pixman images are reference counted, so you can just grab a reference using pixman_image_ref() and run with it, without risking it'll be released underneath your feet, then unref when done. cheers, Gerd
Re: [Qemu-devel] [PATCH RFC v3 13/14] intel_iommu: allow dynamic switch of IOMMU region
On Wed, Jan 18, 2017 at 03:49:44PM +0800, Peter Xu wrote: [...] > I was trying to invalidate the entire address space by sending a big > IOTLB notification to vfio-pci, which looks like: > > IOMMUTLBEntry entry = { > .target_as = &address_space_memory, > .iova = 0, > .translated_addr = 0, > .addr_mask = (1 << 63) - 1, > .perm = IOMMU_NONE, /* UNMAP */ > }; > > Then I feed this entry to vfio-pci IOMMU notifier. > > However, this was blocked in vfio_iommu_map_notify(), with error: > > qemu-system-x86_64: iommu has granularity incompatible with target AS > > Since we have: > > /* >* The IOMMU TLB entry we have just covers translation through >* this IOMMU to its immediate target. We need to translate >* it the rest of the way through to memory. >*/ > rcu_read_lock(); > mr = address_space_translate(&address_space_memory, >iotlb->translated_addr, >&xlat, &len, iotlb->perm & IOMMU_WO); > if (!memory_region_is_ram(mr)) { > error_report("iommu map to non memory area %"HWADDR_PRIx"", >xlat); > goto out; > } > /* >* Translation truncates length to the IOMMU page size, >* check that it did not truncate too much. >*/ > if (len & iotlb->addr_mask) { > error_report("iommu has granularity incompatible with target AS"); > goto out; > } > > In my case len == 0xa (that's the translation result), and > iotlb->addr_mask == (1<<63)-1. So looks like the translation above > splitted the big region and a simple big UNMAP doesn't work for me. > > Do you have any suggestion on how I can solve this? In what case will > we need the above address_space_translate()? Hmm... it should be checking that the translated address range is RAM. However if with this, IOMMU notifiers won't be able to leverage the vfio driver feature to unmap a very big region. IMHO the check should only be meaningful for map operations. I'll try to post a RFC patch for vfio-pci to allow unmap of very big regions, to see whether that'll be a workable approach. Thanks, -- peterx
[Qemu-devel] [Bug 665743] Re: Cocoa video corruption when guest uses RGB565 mode
Can you still reproduce this problem with the latest version of QEMU? If so, could you please rebase your patch to the latest git version and send it to the mailing list (see http://qemu- project.org/Contribute/SubmitAPatch for details)? Sorry, we do not take patches from the bugtracker, they always have got to be discussed on the qemu-devel mailing list first. ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/665743 Title: Cocoa video corruption when guest uses RGB565 mode Status in QEMU: Incomplete Bug description: The cocoa video driver doesn't currently support when the guest uses RGB565 or HighColor mode resulting in corrupted video. The initial graphics screen of recent Ubuntu installs is an example. The attached patch against 0.13.0-release seems to fix the problem by introducing an indirect data provider that translates from RGB565 to RGB888, a mode that core graphics supports. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/665743/+subscriptions
[Qemu-devel] [Bug 618533] Re: OpenSolaris guest fails to see the Solaris partitions of a physical disk in qemu-kvm-9999 (GIT)
Is this issue still reproducible with the latest version of QEMU, or could we close this issue nowadays? ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/618533 Title: OpenSolaris guest fails to see the Solaris partitions of a physical disk in qemu-kvm- (GIT) Status in QEMU: Incomplete Bug description: # qemu-kvm --version QEMU emulator version 0.13.50 (qemu-kvm-devel), Copyright (c) 2003-2008 Fabrice Bellard The following disk is presented to guest as IDE disk with /dev/sdd as path. # fdisk -l /dev/sdd Disk /dev/sdd: 750.2 GB, 750156374016 bytes 255 heads, 63 sectors/track, 91201 cylinders, total 1465149168 sectors Units = sectors of 1 * 512 = 512 bytes Sector size (logical/physical): 512 bytes / 512 bytes I/O size (minimum/optimal): 512 bytes / 512 bytes Disk identifier: 0x7f3a03aa Device Boot Start End Blocks Id System /dev/sdd1 63 7887914 3943926 1b Hidden W95 FAT32 /dev/sdd2 7887915 980736119 486424102+ 83 Linux /dev/sdd3 * 980736120 108315049451207187+ bf Solaris /dev/sdd4 1083150495 1465144064 1909967855 Extended /dev/sdd5 1083150558 110774600912297726 83 Linux /dev/sdd6 1107746073 1465144064 1786989967 HPFS/NTFS The prtvtoc output is attached from both VirtualBox and Qemu-KVM. As can be seen in the screenshots, a valid Solaris partition table (which is inside the /dev/sdd3, marked 'bf' in Linux fdisk output) is not seen in Qemu but seen in Virtualbox. This makes the guest unbootable in Qemu because the root FS is not found but its bootable in Virtualbox. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/618533/+subscriptions
[Qemu-devel] [Bug 645662] Re: Python 3.1.2 math errors with Qemu 0.12.5
Can you still reproduce this problem with the latest version of QEMU? As far as I know, the FPU emulation has been completely switched to softfloat with the latest versions, so I assume your problem might be fixed in recent releases... ** Changed in: qemu Status: New => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/645662 Title: Python 3.1.2 math errors with Qemu 0.12.5 Status in QEMU: Incomplete Bug description: When doing the regression tests for Python 3.1.2 with Qemu 0.12.5, (Linux version 2.6.26-2-686 (Debian 2.6.26-25lenny1)), gcc (Debian 4.3.2-1.1) 4.3.2, Python compiled from sources within qemu, 3 math tests fail, apparently because the floating point unit is buggy. Qmeu was compiled from original sources on Debian Lenny with kernel 2.6.34.6 from kernel.org, gcc (Debian 4.3.2-1.1) 4.3. Regression testing errors: test_cmath test test_cmath failed -- Traceback (most recent call last): File "/root/tools/python3/Python-3.1.2/Lib/test/test_cmath.py", line 364, in self.fail(error_message) AssertionError: acos0034: acos(complex(-1.0002, 0.0)) Expected: complex(3.141592653589793, -2.1073424255447014e-08) Received: complex(3.141592653589793, -2.1073424338879928e-08) Received value insufficiently close to expected value. test_float test test_float failed -- Traceback (most recent call last): File "/root/tools/python3/Python-3.1.2/Lib/test/test_float.py", line 479, in self.assertEqual(s, repr(float(s))) AssertionError: '8.72293771110361e+25' != '8.722937711103609e+25' test_math test test_math failed -- multiple errors occurred; run in verbose mode for deta => runtests.sh -v test_math le01:~/tools/python3/Python-3.1.2# ./runtests.sh -v test_math test_math BAD 1 BAD 0 GOOD 0 SKIPPED 1 total le01:~/tools/python3/Python-3.1.2# To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/645662/+subscriptions
[Qemu-devel] [Bug 1256432] Re: qemu mingw 32bit windows crash
** Changed in: qemu Status: Incomplete => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1256432 Title: qemu mingw 32bit windows crash Status in QEMU: Fix Released Bug description: is it a known bug that in windows that even if you do ./configure --disable-coroutine-pool it still builds the coroutine stuff so when you try and run the build after its compiled it crashses? you have to actually edit config-host.mak and change the c flags around. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1256432/+subscriptions
Re: [Qemu-devel] [PULL 04/41] virtio: convert to use DMA api
On 18/01/2017 20:10, Michael S. Tsirkin wrote: >> Coverity reports that ARRAY_SIZE(elem->out_sg) (and all the others too) >> is wrong because elem->out_sg is a pointer. >> >> However, the check is not in the right place and the max_size argument >> of virtqueue_map_iovec can be removed. The check on in_num/out_num can >> be moved to qemu_get_virtqueue_element instead, before the call to >> virtqueue_alloc_element. > > I guess the effect of this bug is basically false-positive asserts, correct? Yes, migration is probably broken in the case where the stream includes VirtQueueElements. Paolo
[Qemu-devel] [PATCH] build-sys: Minor qapi doc generation target cleanups
Move makeinfo flags from MAKEINFO to MAKEINFOFLAGS. Fix the call of quiet-command for target qemu-ga-qapi.texi. Both messed up in commit 56e8bdd. Cc: Marc-André Lureau Signed-off-by: Markus Armbruster --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 9f8968d..f35c422 100644 --- a/Makefile +++ b/Makefile @@ -542,8 +542,8 @@ ui/console-gl.o: $(SRC_PATH)/ui/console-gl.c \ ui/shader/texture-blit-vert.h ui/shader/texture-blit-frag.h # documentation -MAKEINFO=makeinfo -D 'VERSION $(VERSION)' -MAKEINFOFLAGS=--no-split --number-sections +MAKEINFO=makeinfo +MAKEINFOFLAGS=--no-split --number-sections -D 'VERSION $(VERSION)' TEXIFLAG=$(if $(V),,--quiet) --command='@set VERSION $(VERSION)' %.html: %.texi @@ -573,7 +573,7 @@ qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > $@,"GEN","$@") qemu-qapi.texi: $(qapi-modules) $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN" "$@") + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py) $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > $@,"GEN","$@") -- 2.7.4
Re: [Qemu-devel] [PULL 08/41] intel_iommu: support device iotlb descriptor
On 19/01/2017 04:32, Jason Wang wrote: >> >>> +addr &= ~(sz - 1); +} else { +sz = VTD_PAGE_SIZE; +} +entry.target_as = &vtd_dev_as->as; +entry.addr_mask = sz - 1; +entry.iova = addr; >>> If S=1, entry.iova must mask away the 1 bits that specified the size. >>> For example, >>> >>> addr = 0xabcd1000 >>> >>> has cto64(0xabcd1) == 1, so it indicates a 16K invalidation from >>> 0xabcd to 0xabcd3fff. The "1" must be masked away with "addr & -sz" >>> or "addr & ~entry.addr_mask". >>> >>> Thanks, >>> >>> Paolo >> >> Good catch. > > It should be addr & ~(sz - 1) I think? And it has been done above :) Oh, of course! Paolo
Re: [Qemu-devel] [PATCH v2 6/6] qapi: Promote blockdev-change-medium arguments to QAPI type
Hi On Wed, Jan 18, 2017 at 9:02 PM Eric Blake wrote: > Having a named rather than anonymous C type will make it easier > to improve the testsuite in a later patch. No semantic change, > to any of the existing code or to the introspection output. > > Signed-off-by: Eric Blake > > You should move the "Example:" back to blockdev-change-medium command. Otherwise, Reviewed-by: Marc-André Lureau > --- > v2: rebase to master > --- > qapi/block-core.json | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 1b3e6eb..0e31d25 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -3119,6 +3119,15 @@ > # combines blockdev-open-tray, x-blockdev-remove-medium, > # x-blockdev-insert-medium and blockdev-close-tray). > # > +# Since: 2.5 > +## > +{ 'command': 'blockdev-change-medium', > + 'data': 'BlockdevChangeMedium' } > + > + > +## > +# @BlockdevChangeMedium: > +# > # @device: #optional Block device name (deprecated, use @id > instead) > # > # @id: #optional The name or QOM path of the guest device > @@ -3165,7 +3174,7 @@ > # <- { "return": {} } > # > ## > -{ 'command': 'blockdev-change-medium', > +{ 'struct': 'BlockdevChangeMedium', >'data': { '*device': 'str', > '*id': 'str', > 'filename': 'str', > -- > 2.9.3 > > > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH v2 1/6] pci: Use struct instead of QDict to pass back parameters
On Wed, Jan 18, 2017 at 8:58 PM Eric Blake wrote: > It's simpler to just use a C struct than it is to bundle things > into a QDict in one function just to pull them back out in the > caller. Plus, doing this gets rid of one more user of dynamic > JSON through qobject_from_jsonf(). > and it looks like it removes a leak > > Signed-off-by: Eric Blake > Acked-by: Michael S. Tsirkin > Reviewed-by: Marc-André Lureau > --- > hw/pci/pcie_aer.c | 36 +--- > 1 file changed, 17 insertions(+), 19 deletions(-) > > diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c > index daf1f65..78fd2c3 100644 > --- a/hw/pci/pcie_aer.c > +++ b/hw/pci/pcie_aer.c > @@ -44,6 +44,13 @@ > #define PCI_ERR_SRC_COR_OFFS0 > #define PCI_ERR_SRC_UNCOR_OFFS 2 > > +typedef struct PCIEErrorInject { > +const char *id; > +const char *root_bus; > +int bus; > +int devfn; > +} PCIEErrorInject; > + > /* From 6.2.7 Error Listing and Rules. Table 6-2, 6-3 and 6-4 */ > static uint32_t pcie_aer_uncor_default_severity(uint32_t status) > { > @@ -943,7 +950,8 @@ static int pcie_aer_parse_error_string(const char > *error_name, > } > > static int do_pcie_aer_inject_error(Monitor *mon, > -const QDict *qdict, QObject > **ret_data) > +const QDict *qdict, > +PCIEErrorInject *ret_data) > { > const char *id = qdict_get_str(qdict, "id"); > const char *error_name; > @@ -1004,34 +1012,24 @@ static int do_pcie_aer_inject_error(Monitor *mon, > err.prefix[2] = qdict_get_try_int(qdict, "prefix2", 0); > err.prefix[3] = qdict_get_try_int(qdict, "prefix3", 0); > > -ret = pcie_aer_inject_error(dev, &err); > -*ret_data = qobject_from_jsonf("{'id': %s, " > - "'root_bus': %s, 'bus': %d, 'devfn': > %d, " > - "'ret': %d}", > - id, pci_root_bus_path(dev), > - pci_bus_num(dev->bus), dev->devfn, > - ret); > -assert(*ret_data); > +pcie_aer_inject_error(dev, &err); > +ret_data->id = id; > +ret_data->root_bus = pci_root_bus_path(dev); > +ret_data->bus = pci_bus_num(dev->bus); > +ret_data->devfn = dev->devfn; > > return 0; > } > > void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict) > { > -QObject *data; > -int devfn; > +PCIEErrorInject data; > > if (do_pcie_aer_inject_error(mon, qdict, &data) < 0) { > return; > } > > -assert(qobject_type(data) == QTYPE_QDICT); > -qdict = qobject_to_qdict(data); > - > -devfn = (int)qdict_get_int(qdict, "devfn"); > monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n", > - qdict_get_str(qdict, "id"), > - qdict_get_str(qdict, "root_bus"), > - (int) qdict_get_int(qdict, "bus"), > - PCI_SLOT(devfn), PCI_FUNC(devfn)); > + data.id, data.root_bus, data.bus, > + PCI_SLOT(data.devfn), PCI_FUNC(data.devfn)); > } > -- > 2.9.3 > > > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH] hw/usb/dev-hid: add a Mac guest compatibility option to usb-tablet
On Mi, 2017-01-18 at 15:30 +0100, Phil Dennis-Jordan wrote: > Darwin/OS X/macOS's HID driver stack does not correctly drive Qemu's > simulated USB Tablet. This adds a boolean option "mac_compat" which > subtly changes the device so it behaves in a way that Mac guests can > handle. > > The specific incompatibilities with the regular Qemu USB tablet are: > > 1. Absolute pointing devices with HID Report Descriptor usage page of > 0x01 (pointing) are handled by the macOS HID driver as analog sticks, > so the movement of the cursor ends up being the cumulative deviance > from the centre position. > 2. The bInterfaceProtocol of 0x02 enables a particular macOS HID > driver mode which only works properly with mice (relative motion) not > absolute pointing devices, so spurious events with relative > coordinates are generated in addition to absolute ones. This manifests > as a very jittery cursor. > > The workaround is to report a usage page of 0x02 (mouse) and a > bInterfaceProtocol value of 0x00. Waded through the specs. Setting bInterfaceProtocol to 0x00 should be done anyway. Only boot protocol devices (which the tablet isn't) should set this to 1 (kbd) or 2 (mouse). No need to hide that behind a macos option, we can do that for everybody. The other one is more tricky. Declaring the device as "mouse" is clearly wrong according to the specs. There is no explicit "tablet" device. "pointing" comes closest. Does the issue still show up with bInterfaceProtocol being fixed? cheers, Gerd
Re: [Qemu-devel] [PATCH v2 3/6] qlist: Add convenience helpers for wrapped appends
Hi On Wed, Jan 18, 2017 at 8:34 PM Eric Blake wrote: > Similar to the qdict additions of the previous patch, although > this time there are not as many clients. > > Signed-off-by: Eric Blake > Not very useful, but why not: Reviewed-by: Marc-André Lureau > --- > include/qapi/qmp/qlist.h | 8 > tests/check-qdict.c | 10 +- > tests/check-qlist.c | 2 +- > 3 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h > index a84117e..659325a 100644 > --- a/include/qapi/qmp/qlist.h > +++ b/include/qapi/qmp/qlist.h > @@ -29,6 +29,14 @@ typedef struct QList { > #define qlist_append(qlist, obj) \ > qlist_append_obj(qlist, QOBJECT(obj)) > > +/* Helpers for int, bool, and const char*. */ > +#define qlist_append_int(qlist, value) \ > +qlist_append(qlist, qint_from_int(value)) > +#define qlist_append_bool(qlist, value) \ > +qlist_append(qlist, qbool_from_bool(value)) > +#define qlist_append_str(qlist, value) \ > +qlist_append(qlist, qstring_from_str(value)) > + > #define QLIST_FOREACH_ENTRY(qlist, var) \ > for ((var) = ((qlist)->head.tqh_first); \ > (var); \ > diff --git a/tests/check-qdict.c b/tests/check-qdict.c > index d00b411..6d4f8a7 100644 > --- a/tests/check-qdict.c > +++ b/tests/check-qdict.c > @@ -297,11 +297,11 @@ static void qdict_flatten_test(void) > qdict_put_int(dict1, "a", 0); > qdict_put_int(dict1, "b", 1); > > -qlist_append_obj(list1, QOBJECT(qint_from_int(23))); > -qlist_append_obj(list1, QOBJECT(qint_from_int(66))); > -qlist_append_obj(list1, QOBJECT(dict1)); > -qlist_append_obj(list2, QOBJECT(qint_from_int(42))); > -qlist_append_obj(list2, QOBJECT(list1)); > +qlist_append_int(list1, 23); > +qlist_append_int(list1, 66); > +qlist_append(list1, dict1); > +qlist_append_int(list2, 42); > +qlist_append(list2, list1); > > qdict_put_int(dict2, "c", 2); > qdict_put_int(dict2, "d", 3); > diff --git a/tests/check-qlist.c b/tests/check-qlist.c > index e16da5e..38463f1 100644 > --- a/tests/check-qlist.c > +++ b/tests/check-qlist.c > @@ -74,7 +74,7 @@ static void qlist_destroy_test(void) > qlist = qlist_new(); > > for (i = 0; i < 42; i++) > -qlist_append(qlist, qint_from_int(i)); > +qlist_append_int(qlist, i); > > QDECREF(qlist); > } > -- > 2.9.3 > > > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH v2 2/6] qdict: Add convenience helpers for wrapped puts
Hi On Wed, Jan 18, 2017 at 8:44 PM Eric Blake wrote: > Quite a few users of qdict_put() were manually wrapping a > non-QObject. We can make such call-sites shorter, by providing > common macros to do the tedious work. Also shorten nearby > qdict_put_obj(,,QOBJECT()) sequences. > > Signed-off-by: Eric Blake > Reviewed-by: Alberto Garcia > > --- > > v2: rebase to current master > > I'm okay if you want me to break this patch into smaller pieces. > --- > include/qapi/qmp/qdict.h| 8 +++ > block.c | 59 +++- > block/archipelago.c | 4 +- > block/blkdebug.c| 6 +- > block/blkverify.c | 11 ++- > block/curl.c| 2 +- > block/file-posix.c | 8 +-- > block/file-win32.c | 4 +- > block/iscsi.c | 2 +- > block/nbd.c | 41 ++- > block/nfs.c | 43 +--- > block/null.c| 2 +- > block/qcow2.c | 4 +- > block/quorum.c | 13 ++-- > block/ssh.c | 16 ++--- > block/vvfat.c | 10 +-- > blockdev.c | 28 > hw/block/xen_disk.c | 2 +- > hw/usb/xen-usb.c| 12 ++-- > monitor.c | 18 ++--- > qapi/qmp-event.c| 2 +- > qemu-img.c | 6 +- > qemu-io.c | 2 +- > qemu-nbd.c | 2 +- > qobject/qdict.c | 2 +- > target/s390x/cpu_models.c | 4 +- > tests/check-qdict.c | 132 > ++-- > tests/test-qmp-commands.c | 30 > tests/test-qmp-event.c | 30 > tests/test-qobject-output-visitor.c | 6 +- > util/qemu-option.c | 6 +- > 31 files changed, 245 insertions(+), 270 deletions(-) > > diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h > index fe9a4c5..9d9f9a3 100644 > --- a/include/qapi/qmp/qdict.h > +++ b/include/qapi/qmp/qdict.h > @@ -52,6 +52,14 @@ void qdict_destroy_obj(QObject *obj); > #define qdict_put(qdict, key, obj) \ > qdict_put_obj(qdict, key, QOBJECT(obj)) > > +/* Helpers for int, bool, and const char*. */ > +#define qdict_put_int(qdict, key, value) \ > +qdict_put(qdict, key, qint_from_int(value)) > +#define qdict_put_bool(qdict, key, value) \ > +qdict_put(qdict, key, qbool_from_bool(value)) > +#define qdict_put_str(qdict, key, value) \ > +qdict_put(qdict, key, qstring_from_str(value)) > + > /* High level helpers */ > double qdict_get_double(const QDict *qdict, const char *key); > int64_t qdict_get_int(const QDict *qdict, const char *key); > diff --git a/block.c b/block.c > index 39ddea3..e816657 100644 > --- a/block.c > +++ b/block.c > @@ -876,16 +876,16 @@ static void update_flags_from_options(int *flags, > QemuOpts *opts) > static void update_options_from_flags(QDict *options, int flags) > { > if (!qdict_haskey(options, BDRV_OPT_CACHE_DIRECT)) { > -qdict_put(options, BDRV_OPT_CACHE_DIRECT, > - qbool_from_bool(flags & BDRV_O_NOCACHE)); > +qdict_put_bool(options, BDRV_OPT_CACHE_DIRECT, > + flags & BDRV_O_NOCACHE); > } > if (!qdict_haskey(options, BDRV_OPT_CACHE_NO_FLUSH)) { > -qdict_put(options, BDRV_OPT_CACHE_NO_FLUSH, > - qbool_from_bool(flags & BDRV_O_NO_FLUSH)); > +qdict_put_bool(options, BDRV_OPT_CACHE_NO_FLUSH, > + flags & BDRV_O_NO_FLUSH); > } > if (!qdict_haskey(options, BDRV_OPT_READ_ONLY)) { > -qdict_put(options, BDRV_OPT_READ_ONLY, > - qbool_from_bool(!(flags & BDRV_O_RDWR))); > +qdict_put_bool(options, BDRV_OPT_READ_ONLY, > + !(flags & BDRV_O_RDWR)); > } > } > > @@ -1244,7 +1244,7 @@ static int bdrv_fill_options(QDict **options, const > char *filename, > /* Fetch the file name from the options QDict if necessary */ > if (protocol && filename) { > if (!qdict_haskey(*options, "filename")) { > -qdict_put(*options, "filename", qstring_from_str(filename)); > +qdict_put_str(*options, "filename", filename); > parse_filename = true; > } else { > error_setg(errp, "Can't specify 'file' and 'filename' options > at " > @@ -1264,7 +1264,7 @@ static int bdrv_fill_options(QDict **options, const > char *filename, > } > > drvname = drv->format_name; > -qdict_put(*options, "driver", qstring_from_str(drvname)); > +qdict_put_str(*options, "driver", drvname); > } else { > error_setg(errp, "Must specify either driver or fil
Re: [Qemu-devel] [PATCH v2 5/6] test-qga: Actually test 0xff sync bytes
Hi On Wed, Jan 18, 2017 at 8:39 PM Eric Blake wrote: > Commit 62c39b3 introduced test-qga, and at face value, appears > to be testing the 'guest-sync' behavior that is recommended for > guests in sending 0xff to QGA to force the parser to reset. But > this aspect of the test has never actually done anything: the > qmp_fd() call chain converts its string argument into QObject, > then converts that QObject back to the actual string that is > sent over the wire - and the conversion process silently drops > the 0xff byte from the string sent to QGA, thus never resetting > the QGA parser. > > An upcoming patch will get rid of the wasteful round trip > through QObject, at which point the string in test-qga will be > directly sent over the wire. > > But fixing qmp_fd() to actually send 0xff over the wire is not > all we have to do - the actual QMP parser loudly complains that > 0xff is not valid JSON, and sends an error message _prior_ to > actually parsing the 'guest-sync' or 'guest-sync-delimited' > command. With 'guest-sync', we cannot easily tell if this error > message is a result of our command - which is WHY we invented > the 'guest-sync-delimited' command. So for the testsuite, fix > things to only check 0xff behavior on 'guest-sync-delimited', > and to loop until we've consumed all garbage prior to the > requested delimiter, which matches the documented actions that > a real QGA client is supposed to do. > > Ideally, we'd fix the QGA JSON parser to silently ignore 0xff > rather than sending an error message back, at which point we > could enhance this test for 'guest-sync' as well as for > 'guest-sync-delimited'. But for the sake of this patch, our > testing of 'guest-sync' is no worse than it was pre-patch, > because we have never been sending 0xff over the wire in the > first place. > > Signed-off-by: Eric Blake > Reviewed-by: Marc-André Lureau > --- > tests/libqtest.c | 8 > tests/test-qga.c | 12 +++- > 2 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/tests/libqtest.c b/tests/libqtest.c > index d8fba66..3912e3e 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -430,6 +430,14 @@ void qmp_fd_sendv(int fd, const char *fmt, va_list ap) > va_list ap_copy; > QObject *qobj; > > +/* qobject_from_jsonv() silently eats leading 0xff as invalid > + * JSON, but we want to test sending them over the wire to force > + * resyncs */ > +if (*fmt == '\377') { > +socket_send(fd, fmt, 1); > +fmt++; > +} > + > /* Going through qobject ensures we escape strings properly. > * This seemingly unnecessary copy is required in case va_list > * is an array type. > diff --git a/tests/test-qga.c b/tests/test-qga.c > index 868b02a..4b64630 100644 > --- a/tests/test-qga.c > +++ b/tests/test-qga.c > @@ -151,9 +151,11 @@ static void test_qga_sync_delimited(gconstpointer fix) > qmp_fd_send(fixture->fd, cmd); > g_free(cmd); > > -v = read(fixture->fd, &c, 1); > -g_assert_cmpint(v, ==, 1); > -g_assert_cmpint(c, ==, 0xff); > +/* Read and ignore garbage until resynchronized */ > +do { > +v = read(fixture->fd, &c, 1); > +g_assert_cmpint(v, ==, 1); > +} while (c != 0xff); > > ret = qmp_fd_receive(fixture->fd); > g_assert_nonnull(ret); > @@ -172,8 +174,8 @@ static void test_qga_sync(gconstpointer fix) > QDict *ret; > gchar *cmd; > > -cmd = g_strdup_printf("%c{'execute': 'guest-sync'," > - " 'arguments': {'id': %u } }", 0xff, r); > +cmd = g_strdup_printf("{'execute': 'guest-sync'," > + " 'arguments': {'id': %u } }", r); > ret = qmp_fd(fixture->fd, cmd); > g_free(cmd); > > -- > 2.9.3 > > > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH] build-sys: Minor qapi doc generation target cleanups
On Thu, Jan 19, 2017 at 1:07 PM Markus Armbruster wrote: > Move makeinfo flags from MAKEINFO to MAKEINFOFLAGS. Fix the call of > quiet-command for target qemu-ga-qapi.texi. Both messed up in commit > 56e8bdd. > > Cc: Marc-André Lureau > Signed-off-by: Markus Armbruster > Yes, I thought you touched that before the merge: Reviewed-by: Marc-André Lureau > --- > Makefile | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Makefile b/Makefile > index 9f8968d..f35c422 100644 > --- a/Makefile > +++ b/Makefile > @@ -542,8 +542,8 @@ ui/console-gl.o: $(SRC_PATH)/ui/console-gl.c \ > ui/shader/texture-blit-vert.h ui/shader/texture-blit-frag.h > > # documentation > -MAKEINFO=makeinfo -D 'VERSION $(VERSION)' > -MAKEINFOFLAGS=--no-split --number-sections > +MAKEINFO=makeinfo > +MAKEINFOFLAGS=--no-split --number-sections -D 'VERSION $(VERSION)' > TEXIFLAG=$(if $(V),,--quiet) --command='@set VERSION $(VERSION)' > > %.html: %.texi > @@ -573,7 +573,7 @@ qemu-img-cmds.texi: $(SRC_PATH)/qemu-img-cmds.hx > $(SRC_PATH)/scripts/hxtool > $(call quiet-command,sh $(SRC_PATH)/scripts/hxtool -t < $< > > $@,"GEN","$@") > > qemu-qapi.texi: $(qapi-modules) $(qapi-py) > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > > $@,"GEN" "$@") > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > > $@,"GEN","$@") > > qemu-ga-qapi.texi: $(SRC_PATH)/qga/qapi-schema.json $(qapi-py) > $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi2texi.py $< > > $@,"GEN","$@") > -- > 2.7.4 > > > -- Marc-André Lureau
Re: [Qemu-devel] [PATCH v2 1/6] pci: Use struct instead of QDict to pass back parameters
Eric Blake writes: > It's simpler to just use a C struct than it is to bundle things > into a QDict in one function just to pull them back out in the > caller. Plus, doing this gets rid of one more user of dynamic > JSON through qobject_from_jsonf(). > > Signed-off-by: Eric Blake > Acked-by: Michael S. Tsirkin > --- > hw/pci/pcie_aer.c | 36 +--- > 1 file changed, 17 insertions(+), 19 deletions(-) > > diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c > index daf1f65..78fd2c3 100644 > --- a/hw/pci/pcie_aer.c > +++ b/hw/pci/pcie_aer.c > @@ -44,6 +44,13 @@ > #define PCI_ERR_SRC_COR_OFFS0 > #define PCI_ERR_SRC_UNCOR_OFFS 2 > > +typedef struct PCIEErrorInject { > +const char *id; > +const char *root_bus; > +int bus; > +int devfn; > +} PCIEErrorInject; > + > /* From 6.2.7 Error Listing and Rules. Table 6-2, 6-3 and 6-4 */ > static uint32_t pcie_aer_uncor_default_severity(uint32_t status) > { Aside: both pcie_aer_inject_error() and pcie_aer_msg() could be made static. > @@ -943,7 +950,8 @@ static int pcie_aer_parse_error_string(const char > *error_name, > } > > static int do_pcie_aer_inject_error(Monitor *mon, > -const QDict *qdict, QObject **ret_data) > +const QDict *qdict, > +PCIEErrorInject *ret_data) Your chance to pick a better name than @ret_data, if you like. > { > const char *id = qdict_get_str(qdict, "id"); > const char *error_name; > @@ -1004,34 +1012,24 @@ static int do_pcie_aer_inject_error(Monitor *mon, > err.prefix[2] = qdict_get_try_int(qdict, "prefix2", 0); > err.prefix[3] = qdict_get_try_int(qdict, "prefix3", 0); > > -ret = pcie_aer_inject_error(dev, &err); > -*ret_data = qobject_from_jsonf("{'id': %s, " > - "'root_bus': %s, 'bus': %d, 'devfn': %d, " > - "'ret': %d}", > - id, pci_root_bus_path(dev), > - pci_bus_num(dev->bus), dev->devfn, > - ret); > -assert(*ret_data); > +pcie_aer_inject_error(dev, &err); Ignores failure (assuming it can happen here). Turns out this is no change: before, we passed the return code to the caller, and ignored it there. So this is an improvement of sorts. Still, is ignoring errors correct? For what it's worth, the other caller of pcie_aer_inject_error() asserts it succeeds. > +ret_data->id = id; > +ret_data->root_bus = pci_root_bus_path(dev); > +ret_data->bus = pci_bus_num(dev->bus); > +ret_data->devfn = dev->devfn; > > return 0; > } > > void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict) > { > -QObject *data; > -int devfn; > +PCIEErrorInject data; Your chance to pick a better name than @data, if you like. > > if (do_pcie_aer_inject_error(mon, qdict, &data) < 0) { > return; > } > > -assert(qobject_type(data) == QTYPE_QDICT); > -qdict = qobject_to_qdict(data); > - > -devfn = (int)qdict_get_int(qdict, "devfn"); > monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n", > - qdict_get_str(qdict, "id"), > - qdict_get_str(qdict, "root_bus"), > - (int) qdict_get_int(qdict, "bus"), > - PCI_SLOT(devfn), PCI_FUNC(devfn)); Here's where we ignore the return code. > + data.id, data.root_bus, data.bus, > + PCI_SLOT(data.devfn), PCI_FUNC(data.devfn)); > } Since the fishy error ignore is pre-existing: Reviewed-by: Markus Armbruster
[Qemu-devel] [PATCH RFC 2/3] vfio: introduce vfio_get_vaddr()
A cleanup for vfio_iommu_map_notify(). Should have no functional change, just to make the function shorter and easier to understand. Signed-off-by: Peter Xu --- hw/vfio/common.c | 58 +--- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 174f351..ce55dff 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -294,25 +294,14 @@ static bool vfio_listener_skipped_section(MemoryRegionSection *section) section->offset_within_address_space & (1ULL << 63); } -static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb) +static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void **vaddr, + bool *read_only) { -VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n); -VFIOContainer *container = giommu->container; -hwaddr iova = iotlb->iova + giommu->iommu_offset; MemoryRegion *mr; hwaddr xlat; hwaddr len = iotlb->addr_mask + 1; -void *vaddr; -int ret; - -trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP", -iova, iova + iotlb->addr_mask); - -if (iotlb->target_as != &address_space_memory) { -error_report("Wrong target AS \"%s\", only system memory is allowed", - iotlb->target_as->name ? iotlb->target_as->name : "none"); -return; -} +bool ret = false; +bool writable = iotlb->perm & IOMMU_WO; /* * The IOMMU TLB entry we have just covers translation through @@ -322,12 +311,13 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb) rcu_read_lock(); mr = address_space_translate(&address_space_memory, iotlb->translated_addr, - &xlat, &len, iotlb->perm & IOMMU_WO); + &xlat, &len, writable); if (!memory_region_is_ram(mr)) { error_report("iommu map to non memory area %"HWADDR_PRIx"", xlat); goto out; } + /* * Translation truncates length to the IOMMU page size, * check that it did not truncate too much. @@ -337,11 +327,41 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb) goto out; } +*vaddr = memory_region_get_ram_ptr(mr) + xlat; +*read_only = !writable || mr->readonly; +ret = true; + +out: +rcu_read_unlock(); +return ret; +} + +static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb) +{ +VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n); +VFIOContainer *container = giommu->container; +hwaddr iova = iotlb->iova + giommu->iommu_offset; +bool read_only; +void *vaddr; +int ret; + +trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP", +iova, iova + iotlb->addr_mask); + +if (iotlb->target_as != &address_space_memory) { +error_report("Wrong target AS \"%s\", only system memory is allowed", + iotlb->target_as->name ? iotlb->target_as->name : "none"); +return; +} + +if (!vfio_get_vaddr(iotlb, &vaddr, &read_only)) { +return; +} + if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) { -vaddr = memory_region_get_ram_ptr(mr) + xlat; ret = vfio_dma_map(container, iova, iotlb->addr_mask + 1, vaddr, - !(iotlb->perm & IOMMU_WO) || mr->readonly); + read_only); if (ret) { error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", " "0x%"HWADDR_PRIx", %p) = %d (%m)", @@ -357,8 +377,6 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb) iotlb->addr_mask + 1, ret); } } -out: -rcu_read_unlock(); } static void vfio_listener_region_add(MemoryListener *listener, -- 2.7.4
[Qemu-devel] [PATCH RFC 1/3] vfio: trace map/unmap for notify as well
We traces its range, but we don't know whether it's a MAP/UNMAP. Let's dump it as well. Signed-off-by: Peter Xu --- hw/vfio/common.c | 3 ++- hw/vfio/trace-events | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index 801578b..174f351 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -305,7 +305,8 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb) void *vaddr; int ret; -trace_vfio_iommu_map_notify(iova, iova + iotlb->addr_mask); +trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP", +iova, iova + iotlb->addr_mask); if (iotlb->target_as != &address_space_memory) { error_report("Wrong target AS \"%s\", only system memory is allowed", diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events index ef81609..7ae8233 100644 --- a/hw/vfio/trace-events +++ b/hw/vfio/trace-events @@ -84,7 +84,7 @@ vfio_pci_igd_lpc_bridge_enabled(const char *name) "%s" # hw/vfio/common.c vfio_region_write(const char *name, int index, uint64_t addr, uint64_t data, unsigned size) " (%s:region%d+0x%"PRIx64", 0x%"PRIx64 ", %d)" vfio_region_read(char *name, int index, uint64_t addr, unsigned size, uint64_t data) " (%s:region%d+0x%"PRIx64", %d) = 0x%"PRIx64 -vfio_iommu_map_notify(uint64_t iova_start, uint64_t iova_end) "iommu map @ %"PRIx64" - %"PRIx64 +vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "iommu %s @ %"PRIx64" - %"PRIx64 vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING region_add %"PRIx64" - %"PRIx64 vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add [iommu] %"PRIx64" - %"PRIx64 vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr) "region_add [ram] %"PRIx64" - %"PRIx64" [%p]" -- 2.7.4
Re: [Qemu-devel] [PATCH v2 2/6] qdict: Add convenience helpers for wrapped puts
Eric Blake writes: > Quite a few users of qdict_put() were manually wrapping a > non-QObject. We can make such call-sites shorter, by providing > common macros to do the tedious work. Also shorten nearby > qdict_put_obj(,,QOBJECT()) sequences. > > Signed-off-by: Eric Blake > Reviewed-by: Alberto Garcia > > --- > > v2: rebase to current master > > I'm okay if you want me to break this patch into smaller pieces. I guess I'm okay with a single piece, but I'd like to know how you did the conversion. Coccinelle? Manually?
[Qemu-devel] [PATCH RFC 0/3] vfio: allow to notify unmap for very big region
This requirement originates from the VT-d vfio series: https://lists.nongnu.org/archive/html/qemu-devel/2017-01/msg03495.html The goal of this series is to allow IOMMU to notify unmap with very big IOTLB range, for example, with base=0 and size=2^63-1 (to unmap the whole address space). The first patch is a good to have, for traces. The second one is a cleanup of existing code, only. The third one moves the further RAM translation and check into map operation logic, so that it'll free unmap operations. The series is marked as RFC since I am not sure whether this is a workable way. Anyway, please review to help confirm it. Thanks. Peter Xu (3): vfio: trace map/unmap for notify as well vfio: introduce vfio_get_vaddr() vfio: allow to notify unmap for very large region hw/vfio/common.c | 56 ++-- hw/vfio/trace-events | 2 +- 2 files changed, 38 insertions(+), 20 deletions(-) -- 2.7.4
Re: [Qemu-devel] Virtual Machine Generation ID
On 01/19/17 08:09, Ben Warren wrote: > >> On Jan 18, 2017, at 4:02 PM, Ben Warren >> wrote: >> >> Hi Michael, >>> On Jan 17, 2017, at 9:45 AM, Michael S. Tsirkin >>> wrote: >>> >>> On Mon, Jan 16, 2017 at 10:57:42AM -0800, Ben Warren wrote: I think we have a misunderstanding here. I'm storing the VM Generation ID __data__ (a GUID) in a fw_cfg blob, not the address. >>> >>> Yes, I think I gathered this much from the discussion. This is what >>> I'm saying - don't. Have guest loader reserve guest memory and write >>> the address into a fw cfg blob, and have qemu write the id at that >>> address. This way you can update guest memory at any time. >>> >> So I've gone down the path of creating a writeable fw_cfg blob to >> hold the VGID address, but it doesn't seem to be getting updated. >> >> Here's the code I've added: >> >> #define VMGENID_FW_CFG_FILE "etc/vmgenid" >> #define VMGENID_FW_CFG_ADDR_FILE "etc/vmgenid_addr" >> >> // Create writeable fw_cfg blob, vas->vgia is a GArray of size 8 and element >> size 1 >> fw_cfg_add_file_callback(s, VMGENID_FW_CFG_ADDR_FILE, NULL, NULL, >> vms->vgia->data, 8, false); >> >> // Request BIOS to allocate memory for the read-only DATA file: >> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_FILE, guid, 0,false); >> >> // Request BIOS to allocate memory for the writeable ADDRESS file: >> bios_linker_loader_alloc(linker, VMGENID_FW_CFG_ADDR_FILE, s->vgia, 0, >> false); >> >> // Request BIOS to write the address of the DATA file into the ADDRESS file: >> bios_linker_loader_add_pointer(linker, VMGENID_FW_CFG_ADDR_FILE, 0, >> sizeof(uint64_t), VMGENID_FW_CFG_FILE, 0); >> >> I've instrumented SeaBIOS and see the requests being made and memcpy >> to the file happening, but don't see any changes in QEMU in the >> memory pointed to by VMGENID_FW_CFG_ADDR_FILE. Is this how writeable >> fw_cfg is supposed to work? >> > Ed explained it to me and I think I get it now. I realize that you > and Igor have explained this throughout the e-mail chain, but I'm a > little bit slower. > > Anyway, is this understanding correct? BIOS is in fact patching > fw_cfg data, but it's in a copy of the fw_cfg file that is in guest > space. In order to get this to work we would need to add a new > command to the linker/loader protocol to write back changes to QEMU > after patching, and of course implement the change in BIOS and UEFI > projects. (1) It's not enough to create just the "etc/vmgenid_addr" file; the "etc/vmgenid" file must exist too, so that the firmware can download it. (2) I don't understand why you need the ADD_POINTER command here. I think it's unnecessary. (3) The ALLOCATE command for "etc/vmgenid_addr" is also unnecessary. (4) A new ALLOCATE_RETURN_ADDR command type should be introduced, with the following contents: struct { char file[BIOS_LINKER_LOADER_FILESZ]; uint32_t align; uint8_t zone; char addr_file[BIOS_LINKER_LOADER_FILESZ]; } alloc_return_addr; The first three fields have identical meanings to those of the current ALLOCATE command. The last field instructs the firmware as to what fw_cfg file to write the 8-byte physical address back to, in little endian byte order, of the actual allocation. (5) The linker/loader script should then use one command in total, namely ALLOCATE_RETURN_ADDR, with file = etc/vmgenid addr_file = etc/vmgenid_addr This will allow both SeaBIOS and OVMF to do the right thing. In summary: - create the read-only "etc/vmgenid" fw_cfg file - create the writeable "etc/vmgenid_addr" fw_cfg file - use one ALLOCATE_RETURN_ADDR command, and nothing else. I hereby volunteer to write the OVMF patch for the ALLOCATE_RETURN_ADDR command type. If someone is interested, I'll remind us that EFI_ACPI_TABLE_PROTOCOL installs *copies* of ACPI tables, out of the fw_cfg blobs that were downloaded. Therefore OVMF tracks all ADD_POINTER commands that point into fw_cfg blobs, and if an fw_cfg blob is not pointed-to by *any* ADD_POINTER command, or else *all* ADD_POINTER commands that point into it point to ACPI table headers, then OVMF will ultimately free the originally downloaded fw_cfg blob. If there is at least one referring ADD_POINTER command that points to non-ACPI-table data within the blob, then OVMF marks the blob to be preserved permanently, in AcpiNVS type memory. By introducing the above ALLOCATE_RETURN_ADDR command type, OVMF can do two things: (a) immediately mark the blob for permanent preservation (i.e., it won't be released after the script processing is complete), (b) write the actual allocation address back to the appropriate fw_cfg file. For SeaBIOS, only (b) matters -- because it doesn't install *copies* of ACPI tables; it rather keeps everything in place, as originally allocated, and it just links things together --, but ALLOCATE_RETURN_ADDR as proposed above should be suitable for SeaBIOS just as well. Thanks Laszlo
Re: [Qemu-devel] [PATCH v2 4/6] fdc-test: Avoid deprecated 'change' command
Eric Blake writes: > Use the preferred blockdev-change-medium command instead. > > Signed-off-by: Eric Blake > Reviewed-by: John Snow > --- > tests/fdc-test.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/tests/fdc-test.c b/tests/fdc-test.c > index 738c6b4..f5ff68d 100644 > --- a/tests/fdc-test.c > +++ b/tests/fdc-test.c > @@ -298,8 +298,9 @@ static void test_media_insert(void) > > /* Insert media in drive. DSKCHK should not be reset until a step pulse > * is sent. */ > -qmp_discard_response("{'execute':'change', 'arguments':{" > - " 'device':'floppy0', 'target': %s, 'arg': 'raw' > }}", > +qmp_discard_response("{'execute':'blockdev-change-medium', 'arguments':{" > + " 'device':'floppy0', 'filename': %s, " > + "'format': 'raw' }}", > test_image); > > dir = inb(FLOPPY_BASE + reg_dir); 'device' is deprecated. Can we use 'id' here?
[Qemu-devel] [PATCH RFC 3/3] vfio: allow to notify unmap for very large region
Linux vfio driver supports to do VFIO_IOMMU_UNMAP_DMA for a very big region. This can be leveraged by QEMU IOMMU implementation to cleanup existing page mappings for an entire iova address space (by notifying with an IOTLB with extremely huge addr_mask). However current vfio_iommu_map_notify() does not allow that. It make sure that all the translated address in IOTLB is falling into RAM range. The check makes sense, but it should only be a sensible checker for mapping operations, and mean little for unmap operations. This patch moves this check into map logic only, so that we'll get faster unmap handling (no need to translate again), and also we can then better support unmapping a very big region when it covers non-ram ranges or even not-existing ranges. Signed-off-by: Peter Xu --- hw/vfio/common.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/hw/vfio/common.c b/hw/vfio/common.c index ce55dff..4d90844 100644 --- a/hw/vfio/common.c +++ b/hw/vfio/common.c @@ -354,11 +354,10 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb) return; } -if (!vfio_get_vaddr(iotlb, &vaddr, &read_only)) { -return; -} - if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) { +if (!vfio_get_vaddr(iotlb, &vaddr, &read_only)) { +return; +} ret = vfio_dma_map(container, iova, iotlb->addr_mask + 1, vaddr, read_only); -- 2.7.4
Re: [Qemu-devel] [PATCH RFC v3 14/14] intel_iommu: enable vfio devices
On Thu, Jan 19, 2017 at 06:22:48AM +, Tian, Kevin wrote: [...] > still copy response from spec owner here:-) > > Explicit invalidation is anytime software is explicitly invalidating > something ( > through any descriptor) as opposed to something hardware does > implicitly. > The only time hardware does implicit invalidation is during the > handling of a page > request (recoverable page-fault) from an endpoint device. Thanks for the confirmation! -- peterx
Re: [Qemu-devel] [PATCH v1 11/15] qcow2: convert QCow2 to use QCryptoBlock for encryption
On Wed, Jan 18, 2017 at 07:13:19PM +0100, Max Reitz wrote: > On 03.01.2017 19:27, Daniel P. Berrange wrote: > > This converts the qcow2 driver to make use of the QCryptoBlock > > APIs for encrypting image content, using the legacyy QCow2 AES > > scheme. > > > > With this change it is now required to use the QCryptoSecret > > object for providing passwords, instead of the current block > > password APIs / interactive prompting. > > > > $QEMU \ > > -object secret,id=sec0,filename=/home/berrange/encrypted.pw \ > > -drive file=/home/berrange/encrypted.qcow2,aes-key-secret=sec0 > > > > Signed-off-by: Daniel P. Berrange > > --- > > block/qcow2-cluster.c | 47 +-- > > block/qcow2.c | 190 > > + > > block/qcow2.h | 5 +- > > qapi/block-core.json | 7 +- > > tests/qemu-iotests/049 | 2 +- > > tests/qemu-iotests/049.out | 4 +- > > tests/qemu-iotests/082.out | 27 +++ > > tests/qemu-iotests/087 | 28 ++- > > tests/qemu-iotests/087.out | 6 +- > > tests/qemu-iotests/134 | 18 +++-- > > tests/qemu-iotests/134.out | 10 +-- > > tests/qemu-iotests/158 | 19 +++-- > > tests/qemu-iotests/158.out | 14 +--- > > 13 files changed, 219 insertions(+), 158 deletions(-) > > [...] > > > diff --git a/block/qcow2.c b/block/qcow2.c > > index 3c14c86..5c9e196 100644 > > --- a/block/qcow2.c > > +++ b/block/qcow2.c > > [...] > > > @@ -1122,6 +1144,24 @@ static int qcow2_open(BlockDriverState *bs, QDict > > *options, int flags, > > goto fail; > > } > > > > +if (s->crypt_method_header == QCOW_CRYPT_AES) { > > +unsigned int cflags = 0; > > +if (flags & BDRV_O_NO_IO) { > > +cflags |= QCRYPTO_BLOCK_OPEN_NO_IO; > > +} > > +/* XXX how do we pass the same crypto opts down to the > > I think a TODO instead of an XXX would have been sufficient, but it's > your call. Sure, I can put TODO. > > + * backing file by default, so we don't have to manually > > + * provide the same key-secret property against the full > > + * backing chain > > + */ > > +s->crypto = qcrypto_block_open(s->crypto_opts, NULL, NULL, > > + cflags, errp); > > +if (!s->crypto) { > > +ret = -EINVAL; > > +goto fail; > > +} > > [...] > > > @@ -2022,6 +2027,44 @@ static int > > qcow2_change_backing_file(BlockDriverState *bs, > > return qcow2_update_header(bs); > > } > > > > + > > +static int qcow2_change_encryption(BlockDriverState *bs, QemuOpts *opts, > > + Error **errp) > > I think this name is not quite appropriate, since this doesn't change > the format of the file if it is already encrypted (and it will not > encrypt any existing data). > > Maybe set_up instead of change? Yep, will change to that > > diff --git a/block/qcow2.h b/block/qcow2.h > > index 033d8c0..f4cb171 100644 > > --- a/block/qcow2.h > > +++ b/block/qcow2.h > > @@ -25,7 +25,7 @@ > > #ifndef BLOCK_QCOW2_H > > #define BLOCK_QCOW2_H > > > > -#include "crypto/cipher.h" > > +#include "crypto/block.h" > > #include "qemu/coroutine.h" > > > > //#define DEBUG_ALLOC > > @@ -256,7 +256,8 @@ typedef struct BDRVQcow2State { > > > > CoMutex lock; > > > > -QCryptoCipher *cipher; /* current cipher, NULL if no key yet */ > > +QCryptoBlockOpenOptions *crypto_opts; /* Disk encryption runtime > > options */ > > +QCryptoBlock *crypto; /* Disk encryption format driver */ > > uint32_t crypt_method_header; > > uint64_t snapshots_offset; > > int snapshots_size; > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > index c2b70e8..2ca5674 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -1935,6 +1935,9 @@ > > # @cache-clean-interval: #optional clean unused entries in the L2 and > > refcount > > # caches. The interval is in seconds. The default > > value > > # is 0 and it disables this feature (since 2.5) > > +# @aes-key-secret:#optional the ID of a QCryptoSecret object > > providing > > +# the AES decryption key (since 2.9) Mandatory > > except > > Missing full stop after the closing parenthesis. > > Also, it's mandatory only for encrypted images. I know it's obvious but > that's not what it says here. True, I'll clarify > > > +# when doing a metadata-only probe of the image. > > # > > # Since: 1.7 > > ## > > @@ -1948,8 +1951,8 @@ > > '*cache-size': 'int', > > '*l2-cache-size': 'int', > > '*refcount-cache-size': 'int', > > -'*cache-clean-interval': 'int' } } > > - > > +'*cache-clean-interval': 'int', > > +'*aes-key-secret': 'str' } } > > > > ## > > # @BlockdevOptionsArchipelago: > > d
Re: [Qemu-devel] [RFC 00/13] numa: add '-numa cpu' option
On Wed, Jan 18, 2017 at 06:13:16PM +0100, Igor Mammedov wrote: > > Series introduces a new CLI option to allow mapping cpus to numa > nodes using public properties [socket|core|thread]-ids instead of > internal cpu-index and moving cpu<->node mapping from global bitmaps > to PCMachineState struct. What is the benefit of this change to apps ? Obviously libvirt uses the current syntax, but I'm not aware of what problems that has - why would libvirt want to use this new syntax instead ? Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :|
Re: [Qemu-devel] [PATCH v2 5/6] test-qga: Actually test 0xff sync bytes
Eric Blake writes: > Commit 62c39b3 introduced test-qga, and at face value, appears > to be testing the 'guest-sync' behavior that is recommended for > guests in sending 0xff to QGA to force the parser to reset. But > this aspect of the test has never actually done anything: the > qmp_fd() call chain converts its string argument into QObject, > then converts that QObject back to the actual string that is > sent over the wire - and the conversion process silently drops > the 0xff byte from the string sent to QGA, This isn't immediately obvious from the code, but I assume you observed it carefully. >thus never resetting > the QGA parser. > > An upcoming patch will get rid of the wasteful round trip > through QObject, at which point the string in test-qga will be > directly sent over the wire. > > But fixing qmp_fd() to actually send 0xff over the wire is not > all we have to do - the actual QMP parser loudly complains that > 0xff is not valid JSON, and sends an error message _prior_ to > actually parsing the 'guest-sync' or 'guest-sync-delimited' > command. With 'guest-sync', we cannot easily tell if this error > message is a result of our command - which is WHY we invented > the 'guest-sync-delimited' command. So for the testsuite, fix > things to only check 0xff behavior on 'guest-sync-delimited', > and to loop until we've consumed all garbage prior to the > requested delimiter, which matches the documented actions that > a real QGA client is supposed to do. > > Ideally, we'd fix the QGA JSON parser to silently ignore 0xff > rather than sending an error message back, at which point we > could enhance this test for 'guest-sync' as well as for > 'guest-sync-delimited'. But for the sake of this patch, our > testing of 'guest-sync' is no worse than it was pre-patch, > because we have never been sending 0xff over the wire in the > first place. Is this worth a TODO comment, perhaps in test_qga_sync()? > > Signed-off-by: Eric Blake > --- > tests/libqtest.c | 8 > tests/test-qga.c | 12 +++- > 2 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/tests/libqtest.c b/tests/libqtest.c > index d8fba66..3912e3e 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -430,6 +430,14 @@ void qmp_fd_sendv(int fd, const char *fmt, va_list ap) > va_list ap_copy; > QObject *qobj; > > +/* qobject_from_jsonv() silently eats leading 0xff as invalid > + * JSON, but we want to test sending them over the wire to force > + * resyncs */ > +if (*fmt == '\377') { > +socket_send(fd, fmt, 1); > +fmt++; > +} > + > /* Going through qobject ensures we escape strings properly. > * This seemingly unnecessary copy is required in case va_list > * is an array type. > diff --git a/tests/test-qga.c b/tests/test-qga.c > index 868b02a..4b64630 100644 > --- a/tests/test-qga.c > +++ b/tests/test-qga.c > @@ -151,9 +151,11 @@ static void test_qga_sync_delimited(gconstpointer fix) cmd = g_strdup_printf("%c{'execute': 'guest-sync-delimited'," " 'arguments': {'id': %u } }", 0xff, r); Simplification opportunity: cmd = g_strdup_printf("\xff{'execute': 'guest-sync-delimited'," " 'arguments': {'id': %u } }", r); > qmp_fd_send(fixture->fd, cmd); > g_free(cmd); > > -v = read(fixture->fd, &c, 1); > -g_assert_cmpint(v, ==, 1); > -g_assert_cmpint(c, ==, 0xff); > +/* Read and ignore garbage until resynchronized */ > +do { > +v = read(fixture->fd, &c, 1); > +g_assert_cmpint(v, ==, 1); > +} while (c != 0xff); Slow as molasses, but it'll do for this test. > > ret = qmp_fd_receive(fixture->fd); > g_assert_nonnull(ret); > @@ -172,8 +174,8 @@ static void test_qga_sync(gconstpointer fix) > QDict *ret; > gchar *cmd; > > -cmd = g_strdup_printf("%c{'execute': 'guest-sync'," > - " 'arguments': {'id': %u } }", 0xff, r); > +cmd = g_strdup_printf("{'execute': 'guest-sync'," > + " 'arguments': {'id': %u } }", r); > ret = qmp_fd(fixture->fd, cmd); > g_free(cmd); Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH v2 1/4] virtio: fix up max size checks
On Wed, 18 Jan 2017 22:55:40 +0200 "Michael S. Tsirkin" wrote: > Coverity reports that ARRAY_SIZE(elem->out_sg) (and all the others too) > is wrong because elem->out_sg is a pointer. > > However, the check is not in the right place and the max_size argument > of virtqueue_map_iovec can be removed. The check on in_num/out_num > should be moved to qemu_get_virtqueue_element instead, before the call > to virtqueue_alloc_element. > > Cc: qemu-sta...@nongnu.org > Reported-by: Paolo Bonzini > Fixes: 3724650db07057333879484c8bc7d900b5c1bf8e ("virtio: introduce > virtqueue_alloc_element") > Signed-off-by: Michael S. Tsirkin > --- > hw/virtio/virtio.c | 33 + > 1 file changed, 13 insertions(+), 20 deletions(-) Reviewed-by: Cornelia Huck
Re: [Qemu-devel] [PATCH v2 6/6] qapi: Promote blockdev-change-medium arguments to QAPI type
Eric Blake writes: > Having a named rather than anonymous C type will make it easier > to improve the testsuite in a later patch. Post it together with said later patch then. >No semantic change, > to any of the existing code or to the introspection output. > > Signed-off-by: Eric Blake > > --- > v2: rebase to master > --- > qapi/block-core.json | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 1b3e6eb..0e31d25 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -3119,6 +3119,15 @@ > # combines blockdev-open-tray, x-blockdev-remove-medium, > # x-blockdev-insert-medium and blockdev-close-tray). > # > +# Since: 2.5 > +## > +{ 'command': 'blockdev-change-medium', > + 'data': 'BlockdevChangeMedium' } > + > + > +## > +# @BlockdevChangeMedium: > +# > # @device: #optional Block device name (deprecated, use @id instead) > # > # @id: #optional The name or QOM path of the guest device # (since: 2.8) # # @filename:filename of the new image to be loaded # # @format: #optional, format to open the new image with (defaults to # the probed format) # # @read-only-mode: #optional, change the read-only mode of the device; defaults # to 'retain' # # Since: 2.5 Isn't Since: 2.5 misleading? The anonymous type goes back to 2.5, but the name doesn't. > @@ -3165,7 +3174,7 @@ > # <- { "return": {} } > # > ## > -{ 'command': 'blockdev-change-medium', > +{ 'struct': 'BlockdevChangeMedium', >'data': { '*device': 'str', > '*id': 'str', > 'filename': 'str',
Re: [Qemu-devel] [PATCH V6 0/2] Add new qmp commands to suppurt Xen COLO
Hi~~ Anyone have any comments? Ping Thanks Zhang Chen On 01/05/2017 02:08 PM, Zhang Chen wrote: Xen COLO depend on qemu COLO replication function. So, We need new qmp commands for Xen to use qemu replication. Corresponding libxl patches already in xen.git. Commit ID: ed37ef1f91c20f0ab162ce60f8c38400b917fa64 COLO: introduce new API to prepare/start/do/get_error/stop replication a0ddc0b359375b2418213966dfbdbfab593ecc6f tools/libxl: introduction of libxl__qmp_restore to load qemu state Zhang Chen (2): Add a new qmp command to start/stop replication Add a new qmp command to do checkpoint, query xen replication status docs/qmp-commands.txt | 42 +++ migration/colo.c | 40 + qapi-schema.json | 69 +++ 3 files changed, 151 insertions(+) -- Thanks Zhang Chen
Re: [Qemu-devel] [PATCH v2 3/4] compiler: expression version of QEMU_BUILD_BUG_ON
On 19/01/2017 09:12, Markus Armbruster wrote: > "Michael S. Tsirkin" writes: > >> QEMU_BUILD_BUG_ON uses a typedef in order to be safe >> to use outside functions, but sometimes it's useful >> to have a version that can be used within an expression. >> Following what Linux does, introduce QEMU_BUILD_BUG_ON_ZERO >> that return zero after checking condition at build time. > > Following Linux's example makes sense, but I can't help but wonder > whether we need both QEMU_BUILD_BUG_ON_ZERO() and QEMU_BUILD_BUG_ON(). I think so, most notably QEMU_BUILD_BUG_ON was added to C11 as _Static_assert but QEMU_BUILD_BUG_ON_ZERO wasn't. But we can indeed redefine QEMU_BUILD_BUG_ON to (void)QEMU_BUILD_BUG_ON_ZERO(x) like Linux does, until we add optional support for _Static_assert. >> Signed-off-by: Michael S. Tsirkin >> --- >> include/qemu/compiler.h | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h >> index 2882470..f4cf13b 100644 >> --- a/include/qemu/compiler.h >> +++ b/include/qemu/compiler.h >> @@ -89,6 +89,8 @@ >> typedef char glue(qemu_build_bug_on__,__LINE__)[(x) ? -1 : 1] \ >> __attribute__((unused)) >> >> +#define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(int[(x) ? -1 : 1]) - sizeof(int)) Linux here uses: #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); })) and the issue is that sizeof(int[(x) ? -1 : 1]) could be runtime-evaluated (the type is a variable-length array). Paolo >> #if defined __GNUC__ >> # if !QEMU_GNUC_PREREQ(4, 4) >> /* gcc versions before 4.4.x don't support gnu_printf, so use printf. */ > > More so since your QEMU_BUILD_BUG_ON_ZERO seems easier to understand: no > token pasting. > > Anyway, > Reviewed-by: Markus Armbruster >
Re: [Qemu-devel] [PATCH] libvhost-user: Start VQs on SET_VRING_CALL
On 17/01/2017 20:00, Michael S. Tsirkin wrote: >>> vhost user is not supposed to use maskfd at all. >>> >>> We have this code: >>>if (net->nc->info->type == NET_CLIENT_DRIVER_VHOST_USER) { >>>dev->use_guest_notifier_mask = false; >>>} >>> >>> isn't it effective? >> >> I'm observing this problem when using vhost-user-scsi, not -net. So the code >> above is not in effect. Anyway, I'd expect the race I described to also >> happen on vhost-scsi. > > maskfd is just racy for vhost-user ATM. I'm guessing vhost-scsi should > just set use_guest_notifier_mask, that will fix it. Alternatively, > rework masking to support sync with the backend - but I doubt it's > useful. I agree with Michael that this is a QEMU bug, not a libvhost-user bug. Thanks, Paolo >>> >>> >>> Qemu sends the new callfd to the application It's not hard to repro. How could this situation be avoided? Cheers, Felipe > >> >>> > > Perhaps it's best for now to delay the callfd notification with a > flag until it is received? The other idea is to always kick when we receive the callfd. I remember discussing that alternative with you before libvhost-user went in. The protocol says both the driver and the backend must handle spurious kicks. This approach also fixes the bug. I'm happy with whatever alternative you want, as long it makes libvhost-user usable for storage devices. Thanks, Felipe > > >> Signed-off-by: Felipe Franciosi >> --- >> contrib/libvhost-user/libvhost-user.c | 26 +- >> 1 file changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/contrib/libvhost-user/libvhost-user.c >> b/contrib/libvhost-user/libvhost-user.c >> index af4faad..a46ef90 100644 >> --- a/contrib/libvhost-user/libvhost-user.c >> +++ b/contrib/libvhost-user/libvhost-user.c >> @@ -607,19 +607,6 @@ vu_set_vring_kick_exec(VuDev *dev, VhostUserMsg >> *vmsg) >> DPRINT("Got kick_fd: %d for vq: %d\n", vmsg->fds[0], index); >> } >> >> -dev->vq[index].started = true; >> -if (dev->iface->queue_set_started) { >> -dev->iface->queue_set_started(dev, index, true); >> -} >> - >> -if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) { >> -dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN, >> - vu_kick_cb, (void *)(long)index); >> - >> -DPRINT("Waiting for kicks on fd: %d for vq: %d\n", >> - dev->vq[index].kick_fd, index); >> -} >> - >> return false; >> } >> >> @@ -661,6 +648,19 @@ vu_set_vring_call_exec(VuDev *dev, VhostUserMsg >> *vmsg) >> >> DPRINT("Got call_fd: %d for vq: %d\n", vmsg->fds[0], index); >> >> +dev->vq[index].started = true; >> +if (dev->iface->queue_set_started) { >> +dev->iface->queue_set_started(dev, index, true); >> +} >> + >> +if (dev->vq[index].kick_fd != -1 && dev->vq[index].handler) { >> +dev->set_watch(dev, dev->vq[index].kick_fd, VU_WATCH_IN, >> + vu_kick_cb, (void *)(long)index); >> + >> +DPRINT("Waiting for kicks on fd: %d for vq: %d\n", >> + dev->vq[index].kick_fd, index); >> +} >> + >> return false; >> } >> >> -- >> 1.9.4 >> >> > >
Re: [Qemu-devel] [PULL 0/4] Tracing patches
On 16 January 2017 at 13:44, Stefan Hajnoczi wrote: > The following changes since commit 2ccede18bd24fce5db83fef3674563a1f256717b: > > Merge remote-tracking branch > 'remotes/vivier/tags/m68k-for-2.9-pull-request' into staging (2017-01-16 > 12:41:35 +) > > are available in the git repository at: > > git://github.com/stefanha/qemu.git tags/tracing-pull-request > > for you to fetch changes up to a47e87151e785977d34e7b726495e7781860ca9f: > > trace: Add event "guest_cpu_exit" (2017-01-16 13:40:56 +) > > > > > Applied, thanks. -- PMM
Re: [Qemu-devel] [libvirt] [PATCH 0/9] i386: query-cpu-model-expansion test script
On Wed, Jan 18, 2017 at 08:18:40PM +0100, David Hildenbrand wrote: > Am 18.01.2017 um 18:34 schrieb Eduardo Habkost: > > On Wed, Jan 18, 2017 at 12:09:28PM -0500, Jason J. Herne wrote: > >> On 01/18/2017 12:00 PM, Eduardo Habkost wrote: > >>> On Tue, Jan 17, 2017 at 10:22:10AM -0500, Jason J. Herne wrote: > On 01/16/2017 08:01 PM, Eduardo Habkost wrote: > > This is a follow-up to the series that implements > > query-cpu-model-expansion. Before including the test script, the > > series has some fixes to allow the results of > > query-cpu-model-expansion to be used in the QEMU command-line. > > > > The script probably will work on s390x too, but I couldn't test > > it yet. > > > > Eduardo, > > This test seems to mostly work on s390. The only issue I ran into is > querying host model using tcg only. s390 requires kvm to query the host > model. Perhaps we could just skip the tcg host test case on s390? > >>> > >>> We could still try to test "host", but add it to a greylist where > >>> errors returned by query-cpu-model-expansion can be non-fatal. > >>> query-cpu-model-expansion model="host" can also fail with KVM if > >>> the host doesn't support CPU models. > >>> > >> > >> David had the idea to just support -cpu host for tcg. We could do that. > >> In the meantime, I'm ok with your greylist idea too. This would allow the > >> script to work properly on s390 right from the start, and we can remove the > >> greylist when s390 supports specifying -cpu host for tcg. > > > > I believe we will still need to ignore query-cpu-model-expansion > > errors on some cases, otherwise the test script will fail on > > hosts where KVM doesn't support CPU models in KVM. > > That is indeed true. For "host" + KVM there would have to be an extra > check. non-fatal error sound right for this case (e.g. a warning) > > > > > But we probably don't need a hardcoded greylist, anyway: we could > > just make the error non-fatal in case the CPU model is not > > reported as migration-safe in query-cpu-definitions. > > > > But I was wondering: > > > > 1) Isn't "-cpu host" the default CPU model on s390x on KVM, > >even if the host doesn't support CPU models? > > Yes, it has an inbuilt compatibility mode when specified. If KVM support > for cpu models is missing, using "-cpu host" will work (as it worked on > QEMU versions without cpu model support), doing something like "-cpu > host,vx=on" will not work, as modifying features is not possible (as the > interface for query/config is missing). > > Using "host" for all query-cpu-model is forbiden, as we can't tell what > this model looks like. > > > > > 2) Is it really correct to return an error on > >"query-cpu-model-expansion model=host type=full" if the host > >doesn't support CPU models? > > > > Yes it is, because there is no way to tell which features there are. > Returning { name: "host", props: {} } would be misleading, as it > would mean that there are no features. Which is wrong. We just can't > tell. It is up to the caller to handle this. E.g. ignoring and > continuing, doing compatibility stuff or simply reporting an error. Well, if there's a risk { props: {} } will be interpreted as "all features disabled" instead of "we don't know the real value for any feature", then I agree that returning an error is better and safer. > > Also think about "query-cpu-model-expansion model=host type=static", > which will primarily be used by libvirt on s390x. There is no way to > expand this into a static cpu model. Faking anything will just hide errors. Yes, static expansion of host model must always return an error if it's not possible to expand. > > If "host" can't be expanded, QEMU has to be treated like there is no CPU > model support (as for older QEMU versions). OK. I will propose a patch updating the query-cpu-model-expansion documentation to be more explicit about it. > > >What if it just returned { name: "host", props: {} } > >on those cases, meaning that the CPU model is valid and > >usable, but QEMU is unable to provide extra information about > >it. > > > > > -- > > David -- Eduardo
Re: [Qemu-devel] [RFC 00/13] numa: add '-numa cpu' option
On Thu, Jan 19, 2017 at 09:45:11AM +, Daniel P. Berrange wrote: > On Wed, Jan 18, 2017 at 06:13:16PM +0100, Igor Mammedov wrote: > > > > Series introduces a new CLI option to allow mapping cpus to numa > > nodes using public properties [socket|core|thread]-ids instead of > > internal cpu-index and moving cpu<->node mapping from global bitmaps > > to PCMachineState struct. > > What is the benefit of this change to apps ? Obviously libvirt uses > the current syntax, but I'm not aware of what problems that has - why > would libvirt want to use this new syntax instead ? The current interface is based on a "CPU index", and the mapping from CPU index to a specific CPU object can be non-trivial, especially if "-device *-cpu", CPU hotplug, or more complex CPU topologies are used. -- Eduardo
Re: [Qemu-devel] [PATCH v2 4/4] ARRAY_SIZE: check that argument is an array
On 19 January 2017 at 08:20, Markus Armbruster wrote: > "Michael S. Tsirkin" writes: > >> It's a familiar pattern: some code uses ARRAY_SIZE, then refactoring >> changes the argument from an array to a pointer to a dynamically >> allocated buffer. Code keeps compiling but any ARRAY_SIZE calls now >> return the size of the pointer divided by element size. >> >> Let's add build time checks to ARRAY_SIZE before we allow more >> of these in the code-base. > > Yes, please! > >> Signed-off-by: Michael S. Tsirkin >> --- >> include/qemu/osdep.h | 8 +++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h >> index 689f253..24bfda0 100644 >> --- a/include/qemu/osdep.h >> +++ b/include/qemu/osdep.h >> @@ -199,7 +199,13 @@ extern int daemon(int, int); >> #endif >> >> #ifndef ARRAY_SIZE >> -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) >> +/* >> + * &(x)[0] is always a pointer - if it's same type as x then the argument >> is a >> + * pointer, not an array as expected. >> + */ >> +#define ARRAY_SIZE(x) ((sizeof(x) / sizeof((x)[0])) + >> QEMU_BUILD_BUG_ON_ZERO( \ >> +__builtin_types_compatible_p(typeof(x), \ >> + typeof(&(x)[0] > > Please break the line near the operator, not within one of its operands: > >#define ARRAY_SIZE(x) ((sizeof(x) / sizeof((x)[0])) \ > + QEMU_BUILD_BUG_ON_ZERO( \ >__builtin_types_compatible_p(typeof(x), \ > typeof(&(x)[0] The other possible approach to the long-lines issue would be to do what the Linux kernel does and abstract out a MUST_BE_ARRAY() macro. thanks -- PMM
Re: [Qemu-devel] [PATCH 0/7] target-m68k: implement 680x0 FPU
Le 18/01/2017 à 22:42, Richard Henderson a écrit : > On 01/18/2017 01:05 PM, Laurent Vivier wrote: >> This series modifies the original ColdFire FPU implementation >> to use floatx80 instead of float64 internally as this >> is the native datatype for 680x0. I didn't keep the float64 >> type for ColdFire, but if someone thinks it's required I >> can update this series in this way. > > I think at minimum you'll need to round to float64. But this control is > also needed for 68040 FPU, so probably you can use the same helpers. > > I'm thinking something like > > void helper_foo_d(args); > { > ... > r80 = floatx80_foo(args); > r64 = floatx80_to_float64(r80, &env->fp_status); > r80 = float64_to_floatx80(r64, &env->fp_status); > [store r80] > ... > } > > since iirc 68040 has fadd.d, fadd.s to go with fadd. OK, I will modify this part. Thanks, Laurent
Re: [Qemu-devel] [PATCH 3/4] compiler: expression version of QEMU_BUILD_BUG_ON
* Michael S. Tsirkin (m...@redhat.com) wrote: > QEMU_BUILD_BUG_ON uses a typedef in order to be safe > to use outside functions, but sometimes it's useful > to have a version that can be used within an expression. > Following what Linux does, introduce QEMU_BUILD_BUG_ON_ZERO > that return zero after checking condition at build time. > > Signed-off-by: Michael S. Tsirkin > --- > include/qemu/compiler.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h > index 1a9eeb9..d2b05dd 100644 > --- a/include/qemu/compiler.h > +++ b/include/qemu/compiler.h > @@ -88,6 +88,8 @@ > #define QEMU_BUILD_BUG_ON(x) \ > typedef char glue(qemu_build_bug_on__,__LINE__)[(x)?-1:1] > __attribute__((unused)) > > +#define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(int[(x)?-1:1]) - sizeof(int)) > + Yes, pretty much the same as: https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07090.html I'd used -sizeof(int[1]) in the right hand size to match the left, but I think it should always be the same. Reviewed-by: Dr. David Alan Gilbert > #if defined __GNUC__ > # if !QEMU_GNUC_PREREQ(4, 4) > /* gcc versions before 4.4.x don't support gnu_printf, so use printf. */ > -- > MST > > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 0/3] cpu: numa: Fix the mapping initialization of VCPUs and NUMA nodes
Hi, Igor, At 01/18/2017 09:46 PM, Igor Mammedov wrote: On Wed, 18 Jan 2017 20:40:04 +0800 Dou Liyang wrote: As we fixed a bug(Bug 1) in below links, Named "Method-A": https://lists.nongnu.org/archive/html/qemu-devel/2017-01/msg03354.html Then, Eduardo gave us many suggests. Thanks very much! when we try them, we also find another bugs named "Bug 2". I have an alternative fix for both issues for which I've been writing cover letter when I saw this series. My fix series more intrusive though as it's goal isn't just to fix 'info numa' but rather switch away from cpu-index based mapping to socket/core/thread based mapping and stop using bitmaps for mapping. And as 'info numa' was getting in the way, I fixed both issues in a slightly different way. So pls wait a bit, once travis build is completed, I'll post series here. It doesn't matter. I am learning about your patches. I think my method is temporary. your fix is better than me. Thanks, Liyang.
Re: [Qemu-devel] [Qemu-stable] Data corruption in Qemu 2.7.1
On Wed, Jan 18, 2017 at 05:30:17PM +0100, Paolo Bonzini wrote: > > > On 18/01/2017 17:19, Fabian Grünbichler wrote: > > Jan 18 17:07:51 ubuntu kernel: sd 2:0:0:0: [sda] tag#109 FAILED Result: > > hostbyte=DID_OK driverbyte=DRIVER_SENSE > > Jan 18 17:07:51 ubuntu kernel: sd 2:0:0:0: [sda] tag#109 Sense Key : > > Illegal Request [current] > > Jan 18 17:07:51 ubuntu kernel: sd 2:0:0:0: [sda] tag#109 Add. Sense: > > Invalid field in cdb > > Jan 18 17:07:51 ubuntu kernel: sd 2:0:0:0: [sda] tag#109 CDB: Write(10) 2a > > 00 0d d6 51 48 00 08 00 00 > > Jan 18 17:07:51 ubuntu kernel: blk_update_request: critical target error, > > dev sda, sector 232149320 > > Jan 18 17:07:51 ubuntu kernel: EXT4-fs warning (device sda1): > > ext4_end_bio:329: I/O error -121 writing to inode 125 (offset 0 size 0 > > starting block 29018921) > > Jan 18 17:07:51 ubuntu kernel: Buffer I/O error on device sda1, logical > > block 29018409 > > Jan 18 17:07:51 ubuntu kernel: Buffer I/O error on device sda1, logical > > block 29018410 > > Jan 18 17:07:51 ubuntu kernel: Buffer I/O error on device sda1, logical > > block 29018411 > > Jan 18 17:07:51 ubuntu kernel: Buffer I/O error on device sda1, logical > > block 29018412 > > Jan 18 17:07:51 ubuntu kernel: Buffer I/O error on device sda1, logical > > block 29018413 > > Jan 18 17:07:51 ubuntu kernel: Buffer I/O error on device sda1, logical > > block 29018414 > > Jan 18 17:07:51 ubuntu kernel: Buffer I/O error on device sda1, logical > > block 29018415 > > Jan 18 17:07:51 ubuntu kernel: Buffer I/O error on device sda1, logical > > block 29018416 > > Jan 18 17:07:51 ubuntu kernel: Buffer I/O error on device sda1, logical > > block 29018417 > > Jan 18 17:07:51 ubuntu kernel: Buffer I/O error on device sda1, logical > > block 29018418 > > Jan 18 17:07:51 ubuntu kernel: sd 2:0:0:0: [sda] tag#106 FAILED Result: > > hostbyte=DID_OK driverbyte=DRIVER_SENSE > > Jan 18 17:07:51 ubuntu kernel: sd 2:0:0:0: [sda] tag#106 Sense Key : > > Illegal Request [current] > > Jan 18 17:07:51 ubuntu kernel: sd 2:0:0:0: [sda] tag#106 Add. Sense: > > Invalid field in cdb > > Jan 18 17:07:51 ubuntu kernel: sd 2:0:0:0: [sda] tag#106 CDB: Write(10) 2a > > 00 0d d6 59 48 00 08 00 00 > > Jan 18 17:07:51 ubuntu kernel: blk_update_request: critical target error, > > dev sda, sector 232151368 > > Jan 18 17:07:51 ubuntu kernel: EXT4-fs warning (device sda1): > > ext4_end_bio:329: I/O error -121 writing to inode 125 (offset 0 size 0 > > starting block 29019177) > > Jan 18 17:07:52 ubuntu kernel: JBD2: Detected IO errors while flushing file > > data on sda1-8 > > Jan 18 17:07:58 ubuntu kernel: JBD2: Detected IO errors while flushing file > > data on sda1-8 > > > > > > strace (with some random grep-ing): > > [pid 1794] ioctl(19, SG_IO, {'S', SG_DXFER_TO_DEV, cmd[10]=[2a, 00, 0d, > > d6, 51, 48, 00, 08, 00, 00], mx_sb_len=252, iovec_count=17, > > dxfer_len=1048576, timeout=4294967295, flags=0x1, > > data[1048576]=["\0`\235=c\177\0\0\0\0\1\0\0\0\0\0\0`\236=c\177\0\0\0\0\1\0\0\0\0\0"...]}) > > = -1 EINVAL (Invalid argument) > > [pid 1794] ioctl(19, SG_IO, {'S', SG_DXFER_TO_DEV, cmd[10]=[2a, 00, 0d, > > d6, 59, 48, 00, 08, 00, 00], mx_sb_len=252, iovec_count=16, > > dxfer_len=1048576, timeout=4294967295, flags=0x1, > > data[1048576]=["\0`-=c\177\0\0\0\0\1\0\0\0\0\0\0`.=c\177\0\0\0\0\1\0\0\0\0\0"...]}) > > = -1 EINVAL (Invalid argument) > > This is useful, thanks. I suspect blk_rq_map_user_iov is failing, > meaning that the scatter/gather list has too many segments for the HBA > in the host. (The limit can be found in /sys/block/sda/queue/max_segments). limit is 168 for all the disks I tested with. > > This is consistent with your finding here: > > > disabling THP on the hypervisor host with > > > > # echo 'never' > /sys/kernel/mm/transparent_hugepage/enabled > > > > allows reproducing the bug very reliably, shutting the VM down, then > > enabling THP (with 'always') and trying again makes it go away. > > because no THP means more memory fragmentation and thus more segments. it is also very easily reproducible with both THP enable and defrag set to madvise or always, if tested in fragmented- or low-memory conditions. my test host has 64G of memory, my test VM 4G, huge pages are 2k big if I simulate some memory load by repeatedly reserving 50G memory using stress-ng: # stress-ng --vm 50 --vm-bytes=1G --vm-hang 30 and then start the test VM and the dd-ing, I can see the big chunk of AnonHugePages allocated to the VM system grow: # grep -E 'AnonHugePages:[[:space:]]+[0-9]{5,} kB' /proc/$(pidof qemu-system-x86_64)/smaps up to about 3G (of 4G), and hit the issue. without the additional load and fragmentation using stress-ng, the AnonHugePages allocated to the qemu process grow to the expected 4G, and the issue does not occur. > > I'm not sure how to fix it, unfortunately. :( so this means either use non-transparent huge pages when using scsi-block (haven't verified but should wo
Re: [Qemu-devel] [RFC PATCH v0] softfloat: Add round-to-odd rounding mode
On 19 January 2017 at 05:14, Bharata B Rao wrote: > Power ISA 3.0 introduces a few quadruple precision floating point > instructions that support round-to-add rounding mode. The > round-to-odd mode is explained as under: > > Let Z be the intermediate arithmetic result or the operand of a convert > operation. If Z can be represented exactly in the target format, the > result is Z. Otherwise the result is either Z1 or Z2 whichever is odd. > Here Z1 and Z2 are the next larger and smaller numbers representable > in the target format respectively. > > Signed-off-by: Bharata B Rao > --- > - I am not fully sure if this the correct implementation for the above > described round-to-odd rounding method. Any help is appreciated. > - Didn't bother to add round-to-odd to other floating point precision > variants as round-to-odd option is currently supported only for some > instructions that work on quad precision. ARM has a couple of instructions that also want round-to-odd, specifically for 32-bit results. So we should probably extend it to those at some point (at the moment the target/arm code just treats it like nearest-even, so gets inaccurate results). > fpu/softfloat.c | 6 ++ > include/fpu/softfloat.h | 1 + > 2 files changed, 7 insertions(+) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index c295f31..05932a9 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -1149,6 +1149,9 @@ static float128 roundAndPackFloat128(flag zSign, > int32_t zExp, > case float_round_down: > increment = zSign && zSig2; > break; > +case float_round_to_odd: > +increment = !(zSig1 & 0x1) && zSig2; > +break; I think this is the right calculation, yes. The ARM spec expresses the rounding operation in terms of "force the LS bit of the mantissa to 1", but I think doing it by setting increment like this helps us get the corner cases right (like tininess detection). The remaining part that your patch doesn't address is the handling of rounding to infinity if the precise result is too large to fit in the float128. For ARM this is clearly defined in the spec: the rounding pseudocode doesn't permit overflow-to-infinity. This means the condition if (( roundingMode == float_round_to_zero ) || ( zSign && ( roundingMode == float_round_up ) ) || ( ! zSign && ( roundingMode == float_round_down ) ) which controls "do we return the maximum-float-value?" needs an extra (roundingMode == float_round_to_odd) clause. Hopefully those semantics work for the PPC case too? > default: > abort(); > } > @@ -1215,6 +1218,9 @@ static float128 roundAndPackFloat128(flag zSign, > int32_t zExp, > case float_round_down: > increment = zSign && zSig2; > break; > +case float_round_to_odd: > +increment = !(zSig1 & 0x1) && zSig2; > +break; > default: > abort(); > } > diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h > index 842ec6b..1463062 100644 > --- a/include/fpu/softfloat.h > +++ b/include/fpu/softfloat.h > @@ -180,6 +180,7 @@ enum { > float_round_up = 2, > float_round_to_zero = 3, > float_round_ties_away= 4, > +float_round_to_odd = 5, Maybe worth a comment: /* Not an IEEE rounding mode: round to the closest odd mantissa value */ since all our other rounding modes are as defined in the IEEE spec. > }; > > > /* > -- thanks -- PMM
Re: [Qemu-devel] [PATCH 0/3] cpu: numa: Fix the mapping initialization of VCPUs and NUMA nodes
Hi, Eduardo At 01/19/2017 01:06 AM, Eduardo Habkost wrote: On Wed, Jan 18, 2017 at 09:26:36PM +0800, Dou Liyang wrote: Hi, All ** ERROR:/tmp/qemu-test/src/tests/vhost-user-test.c:668:test_migrate: assertion failed: (qdict_haskey(rsp, "return")) GTester: last random seed: R02Sf52546c4daff8087416f43fa7c146db8 ftruncate: Permission denied ftruncate: Permission denied qemu-system-aarch64: /tmp/qemu-test/src/qom/cpu.c:346: cpu_common_map_numa_node: Assertion `cpu->cpu_index < max_cpus' failed. Broken pipe I don't know What's the meaning of this log ? Is the qemu-system-aarch64 can't recognize the qom/cpu.c:346: assert(cpu->cpu_index < max_cpus); This means the assert() line is being triggered for some reason, and cpu_index is >= max_cpus when we cpu_common_map_numa_node() gets called. We need to investigate why. I have investigated the reason why it is failed. Because not all targets(aarch64-linux-user, x86_64-linux-user ...) need to compile the vl.c(include the max_cpus) and numa.c, but the all may compile the ./qom/cpu.c. So, when we Link those targets, we may can't find the vl.o or numa.o that we want. Add "#ifdef CONFIG_NUMA" to fix it. +static void cpu_common_map_numa_node(CPUState *cpu) +{ +#ifdef CONFIG_NUMA +int i; + +assert(cpu->cpu_index < max_cpus); +for (i = 0; i < nb_numa_nodes; i++) { +if (test_bit(cpu->cpu_index, numa_info[i].node_cpu)) { +cpu->numa_node = i; +return; +} +} +#endif +} + And I am not sure if it is necessary to resend this patch for fixing the bug before Igor's patches is OK completely? Thanks, Liyang.
Re: [Qemu-devel] [PATCH v9 04/11] msix: check msix_init's return value
On 01/18/2017 11:21 PM, Michael S. Tsirkin wrote: > On Wed, Jan 18, 2017 at 02:29:19PM +0800, Cao jin wrote: >> >> >> On 01/18/2017 12:01 AM, Michael S. Tsirkin wrote: >>> On Tue, Jan 17, 2017 at 02:50:38PM +0800, Cao jin wrote: forget to cc maintainers in this new patch On 01/17/2017 02:18 PM, Cao jin wrote: > Doesn't do it for megasas & hcd-xhci, later patches will fix them. > > Signed-off-by: Cao jin >>> >>> I don't like this one, frankly. That's a bunch of code duplication. >> >> Yes, code duplication, seems inevitable if move the asserts into a >> separate patch. >> >>> I suspect vfio is the only one who might reasonably get EINVAL here. >>> So how about e.g. msix_validate_and_init that doesn't assert and use that >>> from vfio, then switch msix_init to assert instead? >>> >> >> Not sure if I get your idea. Do you mean: do param check via assert in >> msix_init(), so that no need check its returned error outside, and >> introduce new api msix_validate_and_init(same content as msix_init, >> except param check) dedicated to vfio? > > Something like this. > >> If I understand you right, the way we do param check for msi_init[*] & >> msix_init will be inconsistent. > > Right, we should consolidate these for msi too. > > I got confused: for msi_init, convert assert to return -errno is a choice from a long discussion: http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg08215.html then now we will revert again? And IIRC, I did use assert in msix_init to do sanity test, and revert as suggest. And this is the way we have done for msi_init: assert the return error outside. And if it need to be modified as your suggestion, I see lots of place need to be taken care, does that worth the trouble? I see there is a simpler way helping us: drop this one from the patchset, at least there is no regression, just a few devices doesn't assert the return error while other(megasas, hcd-xhci) does. What would you say? -- Sincerely, Cao jin
Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/18] arm: Add virtualization to GICv3, and enable EL2 on 64-bit CPUs
On 18 January 2017 at 09:17, Edgar E. Iglesias wrote: > Hi Peter, I'm on holidays and won't have time to review for a couple of > weeks at least... Do you want me to hold off on committing the GICv3 patches until you get back and have had a chance to review? thanks -- PMM
Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/18] arm: Add virtualization to GICv3, and enable EL2 on 64-bit CPUs
Nah, if I find something we can always fix it with follow up patches... Cheers, Edgar On Jan 19, 2017 1:59 PM, "Peter Maydell" wrote: > On 18 January 2017 at 09:17, Edgar E. Iglesias > wrote: > > Hi Peter, I'm on holidays and won't have time to review for a couple of > > weeks at least... > > Do you want me to hold off on committing the GICv3 patches > until you get back and have had a chance to review? > > thanks > -- PMM >
Re: [Qemu-devel] [PATCH 01/18] tcg: add support for 128bit vector type
On Wed, 18 Jan 2017, Richard Henderson wrote: > On 01/17/2017 01:07 AM, Kirill Batuzov wrote: > > +static inline TCGv_v128 tcg_global_mem_new_v128(TCGv_ptr reg, intptr_t > > offset, > > +const char *name) > > +{ > > +int idx = tcg_global_mem_new_internal(TCG_TYPE_V128, reg, offset, > > name); > > +return MAKE_TCGV_V128(idx); > > +} > > You shouldn't allow a v128 type to be created if the host doesn't support it. The idea here was to create it either way, but make sure no operation will ever be issued if host does not support it (tcg_gen_* wrappers take care of this). > > You may want to treat v128 as a pair of v64 if the host supports that. > Although there's limited applicability there, since only minor hosts (MIPS, > Sparc, ia64) have 64-bit-only vector extensions. > > That said, treating v128 as 2 x v64 scales nicely when we add v256. Which, if > we've already gone this far, is clearly how avx2 guest support should be > implemented. > > For hosts that have had no vector support added, you may want to represent > v128 as 2 x i64, for the purpose of intermediate expansion. > I'm not sure about this last part. The host may not have i64, so there should be another case - 4 x i32. So we'll get 4 cases for v128: v128 2 x v64 2 x i64 4 x i32 3 cases will need to be added to tcg_temp_new_internal and tcg_global_new_mem_internal, two of which are rather useless (2 x i64, 4 x i32). Introduction of v256 will add 4 more cases two of which will be useless again. This sounds like too much code that serves no purpose to me. Maybe we can only adapt 2 x v64 (and later 2 x v128 and may be 4 x v64) cases and just generate v128 temp that'll never be used if none of these worked? -- Kirill
[Qemu-devel] [PATCH 1/2] qapi: Tweak error message of bdrv_query_image_info
@bs doesn't always have a device name, such as when it comes from "qemu-img info". Report file name instead. Signed-off-by: Fam Zheng --- block/qapi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index a62e862..6329735 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -237,8 +237,8 @@ void bdrv_query_image_info(BlockDriverState *bs, size = bdrv_getlength(bs); if (size < 0) { -error_setg_errno(errp, -size, "Can't get size of device '%s'", - bdrv_get_device_name(bs)); +error_setg_errno(errp, -size, "Can't get image size '%s'", + bs->exact_filename); goto out; } -- 2.9.3
[Qemu-devel] [PATCH 0/2] block: Fix iotests 059
The 059.out went out of sync, bring it back in line. Fam Zheng (2): qapi: Tweak error message of bdrv_query_image_info iotests: Fix reference output for 059 block/qapi.c | 4 ++-- tests/qemu-iotests/059.out | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.9.3
[Qemu-devel] [PATCH 2/2] iotests: Fix reference output for 059
It was broken by efaa7c4eeb7 when it dropped the device name "image" from BB API. Now this error message text is updated again, sync it up. Signed-off-by: Fam Zheng --- tests/qemu-iotests/059.out | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out index 678adb4..19bd50d 100644 --- a/tests/qemu-iotests/059.out +++ b/tests/qemu-iotests/059.out @@ -2361,5 +2361,5 @@ Offset Length Mapped to File 0x14000 0x1 0x5 TEST_DIR/iotest-version3-s003.vmdk === Testing afl image with a very large capacity === -qemu-img: Can't get size of device 'image': File too large +qemu-img: Can't get image size 'TEST_DIR/afl9.IMGFMT': File too large *** done -- 2.9.3
Re: [Qemu-devel] [PATCH v2] hw/arm/virt-acpi - reserve ECAM space as PNP0C02 device
On 18 January 2017 at 17:26, Laszlo Ersek wrote: > In brief, for one data point, I'd be fine if we didn't tie this change > to machine types. We seem to have arrived at a consensus that we don't need to version-constrain this change, so I'm applying Ard's patch to target-arm.next. thanks -- PMM
Re: [Qemu-devel] [PATCH] hw/usb/dev-hid: add a Mac guest compatibility option to usb-tablet
Thanks for looking into this, Gerd. On 19 January 2017 at 10:06, Gerd Hoffmann wrote: > Waded through the specs. Setting bInterfaceProtocol to 0x00 should be > done anyway. Only boot protocol devices (which the tablet isn't) should > set this to 1 (kbd) or 2 (mouse). No need to hide that behind a macos > option, we can do that for everybody. > OK, that shortens the patch considerably! > The other one is more tricky. Declaring the device as "mouse" is > clearly wrong according to the specs. There is no explicit "tablet" > device. "pointing" comes closest. > > Does the issue still show up with bInterfaceProtocol being fixed? > I just tested it with macOS 10.12, and yes, unfortunately it does. Or rather, the cursor now zips off (I think) the bottom right corner of the screen as soon as you try to move it for the first time, never to be seen again. I don't know if the "mouse" usage is strictly correct for a tablet; the spec does appear somewhat ambiguous on this - none of the examples it gives for "pointer" usage seem to match Qemu's usb-tablet. It would appear that OSX/macOS follows the following sentence from chapter 2's overview: "For example, a gaming device driver might look for Joystick and Game Pad usages, while a system mouse driver might look for Mouse, Digitizer Tablet and Touch Screen usages." Note that "Pointer" usage is not listed for the system mouse driver; I'm not sure Digitizer Tablet and Touch Screen are any more accurate though, considering we have more mouse-like button events. I do know for certain that VMWare also uses 0x02 for its absolute pointing device. WinXP, Win7, and Win10 certainly appear to have no problem with mac_compat from my patch enabled. I haven't checked other windowing systems yet. (So far I've been hacking around this issue with a special guest driver, https://github.com/pmj/QemuUSBTablet-OSX but having a workaround in mainline Qemu would be much more convenient, particularly as it'll work during OS install etc.) Any thoughts on how to proceed? Thanks, Phil
Re: [Qemu-devel] [Qemu-arm] [PATCH v2 00/18] arm: Add virtualization to GICv3, and enable EL2 on 64-bit CPUs
On 19 January 2017 at 13:02, Edgar E. Iglesias wrote: > Nah, if I find something we can always fix it with follow up patches... Cool. I've applied this series to target-arm.next. thanks -- PMM
Re: [Qemu-devel] [PATCH v2 3/4] compiler: expression version of QEMU_BUILD_BUG_ON
Paolo Bonzini writes: > On 19/01/2017 09:12, Markus Armbruster wrote: >> "Michael S. Tsirkin" writes: >> >>> QEMU_BUILD_BUG_ON uses a typedef in order to be safe >>> to use outside functions, but sometimes it's useful >>> to have a version that can be used within an expression. >>> Following what Linux does, introduce QEMU_BUILD_BUG_ON_ZERO >>> that return zero after checking condition at build time. >> >> Following Linux's example makes sense, but I can't help but wonder >> whether we need both QEMU_BUILD_BUG_ON_ZERO() and QEMU_BUILD_BUG_ON(). > > I think so, most notably QEMU_BUILD_BUG_ON was added to C11 as > _Static_assert but QEMU_BUILD_BUG_ON_ZERO wasn't. Okay. > But we can indeed redefine QEMU_BUILD_BUG_ON to > (void)QEMU_BUILD_BUG_ON_ZERO(x) like Linux does, until we add optional > support for _Static_assert. Yes, please. >>> Signed-off-by: Michael S. Tsirkin >>> --- >>> include/qemu/compiler.h | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h >>> index 2882470..f4cf13b 100644 >>> --- a/include/qemu/compiler.h >>> +++ b/include/qemu/compiler.h >>> @@ -89,6 +89,8 @@ >>> typedef char glue(qemu_build_bug_on__,__LINE__)[(x) ? -1 : 1] \ >>> __attribute__((unused)) >>> >>> +#define QEMU_BUILD_BUG_ON_ZERO(x) (sizeof(int[(x) ? -1 : 1]) - sizeof(int)) > > Linux here uses: > > #define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); })) > > and the issue is that sizeof(int[(x) ? -1 : 1]) could be > runtime-evaluated (the type is a variable-length array). Let's copy both macros from Linux.
Re: [Qemu-devel] [PULL v2 00/32] Misc patches for 2017-01-11
On 16 January 2017 at 16:58, Paolo Bonzini wrote: > The following changes since commit 2ccede18bd24fce5db83fef3674563a1f256717b: > > Merge remote-tracking branch > 'remotes/vivier/tags/m68k-for-2.9-pull-request' into staging (2017-01-16 > 12:41:35 +) > > are available in the git repository at: > > git://github.com/bonzini/qemu.git tags/for-upstream > > for you to fetch changes up to 91c7e9f6b82804c6b53ecc5057d6b305b766ba49: > > Revert "win32: don't run subprocess tests on Mingw32 platform" (2017-01-16 > 17:55:25 +0100) Fails to build, ppc64be host: In file included from /home/pm215/qemu/linux-headers/linux/kvm.h:13:0, from /home/pm215/qemu/include/sysemu/kvm.h:23, from /home/pm215/qemu/include/sysemu/hw_accel.h:16, from /home/pm215/qemu/target/ppc/translate_init.c:27, from /home/pm215/qemu/target/ppc/translate.c:6786: /home/pm215/qemu/build/all/linux-headers/asm/kvm.h:320:0: error: "KVM_INTERRUPT_SET" redefined [-Wer ror] #define KVM_INTERRUPT_SET -1U ^ In file included from /home/pm215/qemu/target/ppc/translate_init.c:24:0, from /home/pm215/qemu/target/ppc/translate.c:6786: /home/pm215/qemu/target/ppc/kvm_ppc.h:319:0: note: this is the location of the previous definition #define KVM_INTERRUPT_SET -1 ^ In file included from /home/pm215/qemu/linux-headers/linux/kvm.h:13:0, from /home/pm215/qemu/include/sysemu/kvm.h:23, from /home/pm215/qemu/include/sysemu/hw_accel.h:16, from /home/pm215/qemu/target/ppc/translate_init.c:27, from /home/pm215/qemu/target/ppc/translate.c:6786: /home/pm215/qemu/build/all/linux-headers/asm/kvm.h:321:0: error: "KVM_INTERRUPT_UNSET" redefined [-Werror] #define KVM_INTERRUPT_UNSET -2U ^ thanks -- PMM
[Qemu-devel] [Bug 641118] Re: NetBSD guest only supports network without ACPI
I've just tried, and it is OK using NetBSD7 as the guest. I no longer have NetBSD5 so I am unable to check if the problem still exists on that. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/641118 Title: NetBSD guest only supports network without ACPI Status in QEMU: Incomplete Bug description: Git commit: abdfd9500e07fab7d6ffd4385fa30a065c329a39 Host: Linux 64bit Debian Guest: NetBSD5.0.2/i386 Networking works only when ACPI is disabled in the guest. Without it the network card (wm0) is not detected. Boot: qemu -hda netbsd5.0.2-i386 -boot c -enable-kvm Configure: --enable-linux-aio --enable-io-thread --enable-kvm To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/641118/+subscriptions
Re: [Qemu-devel] [RFC 00/13] numa: add '-numa cpu' option
On Thu, 19 Jan 2017 09:45:11 + "Daniel P. Berrange" wrote: > On Wed, Jan 18, 2017 at 06:13:16PM +0100, Igor Mammedov wrote: > > > > Series introduces a new CLI option to allow mapping cpus to numa > > nodes using public properties [socket|core|thread]-ids instead of > > internal cpu-index and moving cpu<->node mapping from global bitmaps > > to PCMachineState struct. > > What is the benefit of this change to apps ? Obviously libvirt uses > the current syntax, but I'm not aware of what problems that has - why > would libvirt want to use this new syntax instead ? current syntax -numa cpus=1,2,3... depends on cpu-index which is internal to QEMU. External users wouldn't actually know which cpu is associated with which cpu-index without re-implementing cpu-index assignment which is qemu-version/target/machine/topology dependent. New '-numa cpu' provides mapping of cpus to numa nodes in CPU terms that are used with device_add/-device commands. For management there is query-hotpluggble-cpus command that allows to get a list of possible cpus with their socket-id/core-id/thread-id property values. for example without numa mapping CLI could look like: $QEMU -M pc -smp 1,sockets=3,maxcpus=3 \ -device qemu64-x86_64-cpu,socket-id=1,core-id=0,thread-id=0 (qemu) info hotpluggable-cpus Hotpluggable CPUs: type: "qemu64-x86_64-cpu" vcpus_count: "1" CPUInstance Properties: socket-id: "2" core-id: "0" thread-id: "0" type: "qemu64-x86_64-cpu" vcpus_count: "1" qom_path: "/machine/peripheral-anon/device[0]" CPUInstance Properties: socket-id: "1" core-id: "0" thread-id: "0" type: "qemu64-x86_64-cpu" vcpus_count: "1" qom_path: "/machine/unattached/device[0]" CPUInstance Properties: socket-id: "0" core-id: "0" thread-id: "0" based on that list the one could extend CLI with numa mapping: $QEMU -M pc -smp 1,sockets=3,maxcpus=3 \ -device qemu64-x86_64-cpu,socket-id=1,core-id=0,thread-id=0 \ -numa cpu,socket-id=0,core-id=0,thread-id=0,node-id=0 \ -numa cpu,socket-id=1,core-id=0,thread-id=0,node-id=1 \ -numa cpu,socket-id=2,core-id=0,thread-id=0,node-id=2 (qemu) info hotpluggable-cpus Hotpluggable CPUs: type: "qemu64-x86_64-cpu" vcpus_count: "1" CPUInstance Properties: node-id: "2" socket-id: "2" core-id: "0" thread-id: "0" type: "qemu64-x86_64-cpu" vcpus_count: "1" qom_path: "/machine/peripheral-anon/device[0]" CPUInstance Properties: node-id: "1" socket-id: "1" core-id: "0" thread-id: "0" type: "qemu64-x86_64-cpu" vcpus_count: "1" qom_path: "/machine/unattached/device[0]" CPUInstance Properties: node-id: "0" socket-id: "0" core-id: "0" thread-id: "0" As it's been discussed previous times, there is chicken/egg issue where management has to get query-hotpluggble-cpus result for a specific combination of machine type and -smp option. It would need to be done at most one time when creating a configuration and then CLI numa mapping could be used for starting VM without doing query-hotpluggable-cpus. '-numa cpu' CLI option is also needed for setting known mapping on target side of migration. Previous consensus has been that the only way to avoid 2 stage discovery/configuration is starting QEMU in paused mode and than do query + mapping at runtime via QMP before allowing guest run with 'continue' command. But this part hasn't been implemented in this series yet. > > > Regards, > Daniel
[Qemu-devel] [PULL 00/36] target-arm queue
ARM queue -- the big bit here is the virtualization stuff for the GIC and correspondingly enabling it in the virt board. thanks -- PMM The following changes since commit ab4b92760498e097ff668f0e9c83aa87a2ec1128: Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2017-01-17 16:54:09 +) are available in the git repository at: git://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20170119 for you to fetch changes up to 245c8cbc4e4f0f8b6c3dfaab2090d1d55f94d360: hw/arm/virt: Add board property to enable EL2 (2017-01-19 13:40:23 +) target-arm queue: * support virtualization in GICv3 * enable EL2 in AArch64 CPU models * allow EL2 to be enabled on 'virt' board via -machine virtualization=on * aspeed: SMC improvements * m25p80: support die erase command * m25p80: Add Quad Page Program 4byte * m25p80: Improve 1GiB Micron flash definition * arm: Uniquely name imx25 I2C buses Alastair D'Silva (1): arm: Uniquely name imx25 I2C buses. Andrew Jones (1): hw/arm/virt-acpi-build: use SMC if booting in EL2 Ard Biesheuvel (1): hw/arm/virt-acpi - reserve ECAM space as PNP0C02 device Cédric Le Goater (10): aspeed/smc: remove call to reset in realize function aspeed/smc: remove call to aspeed_smc_update_cs() in reset function aspeed/smc: rework the prototype of the AspeedSMCFlash helper routines aspeed/smc: autostrap CE0/1 configuration aspeed/smc: unfold the AspeedSMCController array aspeed/smc: adjust the size of the register region aspeed/smc: handle SPI flash Command mode aspeed/smc: reset flash after each test aspeed/smc: extend tests for Command mode aspeed: use first FMC flash as a boot ROM Marcin Krzeminski (3): block: m25p80: Add Quad Page Program 4byte block: m25p80: Introduce die erase command block: m25p80: Improve 1GiB Micron flash definition Peter Maydell (19): target/arm: Handle VIRQ and VFIQ in arm_cpu_do_interrupt_aarch32() target/arm: Implement DBGVCR32_EL2 system register hw/intc/arm_gicv3: Add external IRQ lines for VIRQ and VFIQ hw/intc/arm_gic: Add external IRQ lines for VIRQ and VFIQ target-arm: Expose output GPIO line for VCPU maintenance interrupt hw/arm/virt: Wire VIRQ, VFIQ, maintenance irq lines from GIC to CPU target-arm: Add ARMCPU fields for GIC CPU i/f config hw/intc/gicv3: Add defines for ICH system register fields hw/intc/gicv3: Add data fields for virtualization support hw/intc/arm_gicv3: Add accessors for ICH_ system registers hw/intc/arm_gicv3: Implement ICV_ registers which are just accessors hw/intc/arm_gicv3: Implement ICV_ HPPIR, DIR and RPR registers hw/intc/arm_gicv3: Implement ICV_ registers EOIR and IAR hw/intc/arm_gicv3: Implement gicv3_cpuif_virt_update() hw/intc/arm_gicv3: Implement EL2 traps for CPU i/f regs hw/arm/virt: Support using SMC for PSCI target/arm/psci.c: If EL2 implemented, start CPUs in EL2 target-arm: Enable EL2 feature bit on A53 and A57 hw/arm/virt: Add board property to enable EL2 Shannon Zhao (1): arm: virt: Fix segmentation fault when specifying an unsupported CPU hw/intc/gicv3_internal.h | 79 +++ include/hw/arm/virt.h |5 +- include/hw/intc/arm_gic_common.h |2 + include/hw/intc/arm_gicv3_common.h | 21 + include/hw/ssi/aspeed_smc.h|4 +- target/arm/cpu.h |9 + hw/arm/aspeed.c| 41 ++ hw/arm/imx25_pdk.c |2 +- hw/arm/virt-acpi-build.c | 36 +- hw/arm/virt.c | 88 ++- hw/arm/xlnx-zynqmp.c |2 + hw/block/m25p80.c | 51 +- hw/i2c/imx_i2c.c |2 +- hw/intc/arm_gic_common.c |6 + hw/intc/arm_gicv3_common.c | 31 + hw/intc/arm_gicv3_cpuif.c | 1351 +++- hw/ssi/aspeed_smc.c| 330 +++-- target/arm/cpu.c | 15 + target/arm/cpu64.c |8 + target/arm/helper.c| 21 + target/arm/psci.c | 25 +- tests/m25p80-test.c| 133 hw/intc/trace-events | 33 + 23 files changed, 2154 insertions(+), 141 deletions(-)
[Qemu-devel] [PULL 21/36] target-arm: Expose output GPIO line for VCPU maintenance interrupt
The GICv3 support for virtualization includes an outbound maintenance interrupt signal which is asserted when the CPU interface wants to signal to the hypervisor that it needs attention. Expose this as an outbound GPIO line from the CPU object which can be wired up as a physical interrupt line by the board code (as we do already for the CPU timers). Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Alistair Francis Message-id: 1483977924-14522-4-git-send-email-peter.mayd...@linaro.org --- target/arm/cpu.h | 2 ++ target/arm/cpu.c | 3 +++ 2 files changed, 5 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 7bd16ee..fa09498 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -558,6 +558,8 @@ struct ARMCPU { QEMUTimer *gt_timer[NUM_GTIMERS]; /* GPIO outputs for generic timer */ qemu_irq gt_timer_outputs[NUM_GTIMERS]; +/* GPIO output for GICv3 maintenance interrupt signal */ +qemu_irq gicv3_maintenance_interrupt; /* MemoryRegion to use for secure physical accesses */ MemoryRegion *secure_memory; diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 9104611..93ebbc9 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -465,6 +465,9 @@ static void arm_cpu_initfn(Object *obj) arm_gt_stimer_cb, cpu); qdev_init_gpio_out(DEVICE(cpu), cpu->gt_timer_outputs, ARRAY_SIZE(cpu->gt_timer_outputs)); + +qdev_init_gpio_out_named(DEVICE(cpu), &cpu->gicv3_maintenance_interrupt, + "gicv3-maintenance-interrupt", 1); #endif /* DTB consumers generally don't in fact care what the 'compatible' -- 2.7.4
[Qemu-devel] [PULL 27/36] hw/intc/arm_gicv3: Implement ICV_ registers which are just accessors
If the HCR_EL2.IMO or FMO bits are set, accesses to ICC_ system registers are redirected to be accesses to ICV_ registers (the guest-visible interface to the virtual interrupt controller). Implement this behaviour for the ICV_ registers which are simple accessors to the underlying register state. Signed-off-by: Peter Maydell Message-id: 1483977924-14522-10-git-send-email-peter.mayd...@linaro.org --- hw/intc/arm_gicv3_cpuif.c | 239 ++ hw/intc/trace-events | 10 ++ 2 files changed, 249 insertions(+) diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index 585c91c..40a52ce 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -13,6 +13,7 @@ */ #include "qemu/osdep.h" +#include "qemu/bitops.h" #include "trace.h" #include "gicv3_internal.h" #include "cpu.h" @@ -48,6 +49,26 @@ static int ich_lr_state(uint64_t lr) return extract64(lr, ICH_LR_EL2_STATE_SHIFT, ICH_LR_EL2_STATE_LENGTH); } +static bool icv_access(CPUARMState *env, int hcr_flags) +{ +/* Return true if this ICC_ register access should really be + * directed to an ICV_ access. hcr_flags is a mask of + * HCR_EL2 bits to check: we treat this as an ICV_ access + * if we are in NS EL1 and at least one of the specified + * HCR_EL2 bits is set. + * + * ICV registers fall into four categories: + * * access if NS EL1 and HCR_EL2.FMO == 1: + *all ICV regs with '0' in their name + * * access if NS EL1 and HCR_EL2.IMO == 1: + *all ICV regs with '1' in their name + * * access if NS EL1 and either IMO or FMO == 1: + *CTLR, DIR, PMR, RPR + */ +return (env->cp15.hcr_el2 & hcr_flags) && arm_current_el(env) == 1 +&& !arm_is_secure_below_el3(env); +} + static int read_vbpr(GICv3CPUState *cs, int grp) { /* Read VBPR value out of the VMCR field (caller must handle @@ -84,6 +105,16 @@ static void write_vbpr(GICv3CPUState *cs, int grp, int value) } } +static uint32_t icv_fullprio_mask(GICv3CPUState *cs) +{ +/* Return a mask word which clears the unimplemented priority bits + * from a priority value for a virtual interrupt. (Not to be confused + * with the group priority, whose mask depends on the value of VBPR + * for the interrupt group.) + */ +return ~0U << (8 - cs->vpribits); +} + static uint32_t eoi_maintenance_interrupt_state(GICv3CPUState *cs, uint32_t *misr) { @@ -168,6 +199,170 @@ static void gicv3_cpuif_virt_update(GICv3CPUState *cs) { } +static uint64_t icv_ap_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ +GICv3CPUState *cs = icc_cs_from_env(env); +int regno = ri->opc2 & 3; +int grp = ri->crm & 1 ? GICV3_G0 : GICV3_G1NS; +uint64_t value = cs->ich_apr[grp][regno]; + +trace_gicv3_icv_ap_read(ri->crm & 1, regno, gicv3_redist_affid(cs), value); +return value; +} + +static void icv_ap_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ +GICv3CPUState *cs = icc_cs_from_env(env); +int regno = ri->opc2 & 3; +int grp = ri->crm & 1 ? GICV3_G0 : GICV3_G1NS; + +trace_gicv3_icv_ap_write(ri->crm & 1, regno, gicv3_redist_affid(cs), value); + +cs->ich_apr[grp][regno] = value & 0xU; + +gicv3_cpuif_virt_update(cs); +return; +} + +static uint64_t icv_bpr_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ +GICv3CPUState *cs = icc_cs_from_env(env); +int grp = (ri->crm == 8) ? GICV3_G0 : GICV3_G1NS; +uint64_t bpr; +bool satinc = false; + +if (grp == GICV3_G1NS && (cs->ich_vmcr_el2 & ICH_VMCR_EL2_VCBPR)) { +/* reads return bpr0 + 1 saturated to 7, writes ignored */ +grp = GICV3_G0; +satinc = true; +} + +bpr = read_vbpr(cs, grp); + +if (satinc) { +bpr++; +bpr = MIN(bpr, 7); +} + +trace_gicv3_icv_bpr_read(ri->crm == 8 ? 0 : 1, gicv3_redist_affid(cs), bpr); + +return bpr; +} + +static void icv_bpr_write(CPUARMState *env, const ARMCPRegInfo *ri, + uint64_t value) +{ +GICv3CPUState *cs = icc_cs_from_env(env); +int grp = (ri->crm == 8) ? GICV3_G0 : GICV3_G1NS; + +trace_gicv3_icv_bpr_write(ri->crm == 8 ? 0 : 1, + gicv3_redist_affid(cs), value); + +if (grp == GICV3_G1NS && (cs->ich_vmcr_el2 & ICH_VMCR_EL2_VCBPR)) { +/* reads return bpr0 + 1 saturated to 7, writes ignored */ +return; +} + +write_vbpr(cs, grp, value); + +gicv3_cpuif_virt_update(cs); +} + +static uint64_t icv_pmr_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ +GICv3CPUState *cs = icc_cs_from_env(env); +uint64_t value; + +value = extract64(cs->ich_vmcr_el2, ICH_VMCR_EL2_VPMR_SHIFT, + ICH_VMCR_EL2_VPMR_LENGTH); + +trace_gicv3_icv_pmr_read(gicv3_redist_affid(cs), value); +return value; +} + +static void icv_pmr_write(CPUARMState
[Qemu-devel] [PULL 34/36] target/arm/psci.c: If EL2 implemented, start CPUs in EL2
The PSCI spec states that a CPU_ON call should cause the new CPU to be started in the highest implemented Non-secure exception level. We were incorrectly starting it at the exception level of the caller, which happens to be correct if EL2 is not implemented. Implement the correct logic as described in the PSCI 1.0 spec section 6.4: * if EL2 exists and SCR_EL3.HCE is set: start in EL2 * otherwise start in EL1 Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Andrew Jones Tested-by: Andrew Jones Message-id: 1483977924-14522-17-git-send-email-peter.mayd...@linaro.org --- target/arm/psci.c | 25 ++--- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/target/arm/psci.c b/target/arm/psci.c index 14316eb..64bf82e 100644 --- a/target/arm/psci.c +++ b/target/arm/psci.c @@ -148,17 +148,28 @@ void arm_handle_psci_call(ARMCPU *cpu) case QEMU_PSCI_0_1_FN_CPU_ON: case QEMU_PSCI_0_2_FN_CPU_ON: case QEMU_PSCI_0_2_FN64_CPU_ON: +{ +/* The PSCI spec mandates that newly brought up CPUs start + * in the highest exception level which exists and is enabled + * on the calling CPU. Since the QEMU PSCI implementation is + * acting as a "fake EL3" or "fake EL2" firmware, this for us + * means that we want to start at the highest NS exception level + * that we are providing to the guest. + * The execution mode should be that which is currently in use + * by the same exception level on the calling CPU. + * The CPU should be started with the context_id value + * in x0 (if AArch64) or r0 (if AArch32). + */ +int target_el = arm_feature(env, ARM_FEATURE_EL2) ? 2 : 1; +bool target_aarch64 = arm_el_is_aa64(env, target_el); + mpidr = param[1]; entry = param[2]; context_id = param[3]; -/* - * The PSCI spec mandates that newly brought up CPUs enter the - * exception level of the caller in the same execution mode as - * the caller, with context_id in x0/r0, respectively. - */ -ret = arm_set_cpu_on(mpidr, entry, context_id, arm_current_el(env), - is_a64(env)); +ret = arm_set_cpu_on(mpidr, entry, context_id, + target_el, target_aarch64); break; +} case QEMU_PSCI_0_1_FN_CPU_OFF: case QEMU_PSCI_0_2_FN_CPU_OFF: goto cpu_off; -- 2.7.4
[Qemu-devel] [PULL 20/36] hw/intc/arm_gic: Add external IRQ lines for VIRQ and VFIQ
Augment the GIC's QOM device interface by adding two new sets of sysbus IRQ lines, to signal VIRQ and VFIQ to each CPU. We never use these, but it's helpful to keep the v2-and-earlier GIC's external interface in line with that of the GICv3 to avoid board code having to add extra code conditional on which version of the GIC is in use. Signed-off-by: Peter Maydell Reviewed-by: Alistair Francis Reviewed-by: Edgar E. Iglesias Message-id: 1483977924-14522-3-git-send-email-peter.mayd...@linaro.org --- include/hw/intc/arm_gic_common.h | 2 ++ hw/intc/arm_gic_common.c | 6 ++ 2 files changed, 8 insertions(+) diff --git a/include/hw/intc/arm_gic_common.h b/include/hw/intc/arm_gic_common.h index f4c349a..af3ca18 100644 --- a/include/hw/intc/arm_gic_common.h +++ b/include/hw/intc/arm_gic_common.h @@ -55,6 +55,8 @@ typedef struct GICState { qemu_irq parent_irq[GIC_NCPU]; qemu_irq parent_fiq[GIC_NCPU]; +qemu_irq parent_virq[GIC_NCPU]; +qemu_irq parent_vfiq[GIC_NCPU]; /* GICD_CTLR; for a GIC with the security extensions the NS banked version * of this register is just an alias of bit 1 of the S banked version. */ diff --git a/hw/intc/arm_gic_common.c b/hw/intc/arm_gic_common.c index 0a1f56a..4a8df44 100644 --- a/hw/intc/arm_gic_common.c +++ b/hw/intc/arm_gic_common.c @@ -110,6 +110,12 @@ void gic_init_irqs_and_mmio(GICState *s, qemu_irq_handler handler, for (i = 0; i < s->num_cpu; i++) { sysbus_init_irq(sbd, &s->parent_fiq[i]); } +for (i = 0; i < s->num_cpu; i++) { +sysbus_init_irq(sbd, &s->parent_virq[i]); +} +for (i = 0; i < s->num_cpu; i++) { +sysbus_init_irq(sbd, &s->parent_vfiq[i]); +} /* Distributor */ memory_region_init_io(&s->iomem, OBJECT(s), ops, s, "gic_dist", 0x1000); -- 2.7.4
[Qemu-devel] [PULL 22/36] hw/arm/virt: Wire VIRQ, VFIQ, maintenance irq lines from GIC to CPU
Wire the new VIRQ, VFIQ and maintenance interrupt lines from the GIC to each CPU. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Message-id: 1483977924-14522-5-git-send-email-peter.mayd...@linaro.org --- include/hw/arm/virt.h | 2 ++ hw/arm/virt.c | 14 +++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index eb1c63d..b8a19ec 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -39,6 +39,8 @@ #define NUM_GICV2M_SPIS 64 #define NUM_VIRTIO_TRANSPORTS 32 +#define ARCH_GICV3_MAINT_IRQ 9 + #define ARCH_TIMER_VIRT_IRQ 11 #define ARCH_TIMER_S_EL1_IRQ 13 #define ARCH_TIMER_NS_EL1_IRQ 14 diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 95ac585..d931d17 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -546,9 +546,9 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) sysbus_mmio_map(gicbusdev, 1, vms->memmap[VIRT_GIC_CPU].base); } -/* Wire the outputs from each CPU's generic timer to the - * appropriate GIC PPI inputs, and the GIC's IRQ output to - * the CPU's IRQ input. +/* Wire the outputs from each CPU's generic timer and the GICv3 + * maintenance interrupt signal to the appropriate GIC PPI inputs, + * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs. */ for (i = 0; i < smp_cpus; i++) { DeviceState *cpudev = DEVICE(qemu_get_cpu(i)); @@ -570,9 +570,17 @@ static void create_gic(VirtMachineState *vms, qemu_irq *pic) ppibase + timer_irq[irq])); } +qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0, +qdev_get_gpio_in(gicdev, ppibase + + ARCH_GICV3_MAINT_IRQ)); + sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); sysbus_connect_irq(gicbusdev, i + smp_cpus, qdev_get_gpio_in(cpudev, ARM_CPU_FIQ)); +sysbus_connect_irq(gicbusdev, i + 2 * smp_cpus, + qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ)); +sysbus_connect_irq(gicbusdev, i + 3 * smp_cpus, + qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ)); } for (i = 0; i < NUM_IRQS; i++) { -- 2.7.4
[Qemu-devel] [PULL 23/36] target-arm: Add ARMCPU fields for GIC CPU i/f config
Add fields to the ARMCPU structure to allow CPU classes to specify the configurable aspects of their GIC CPU interface. In particular, the virtualization support allows different values for number of list registers, priority bits and preemption bits. Signed-off-by: Peter Maydell Acked-by: Alistair Francis Message-id: 1483977924-14522-6-git-send-email-peter.mayd...@linaro.org --- target/arm/cpu.h | 5 + target/arm/cpu64.c | 6 ++ 2 files changed, 11 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index fa09498..16c7c10 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -662,6 +662,11 @@ struct ARMCPU { uint32_t dcz_blocksize; uint64_t rvbar; +/* Configurable aspects of GIC cpu interface (which is part of the CPU) */ +int gic_num_lrs; /* number of list registers */ +int gic_vpribits; /* number of virtual priority bits */ +int gic_vprebits; /* number of virtual preemption bits */ + ARMELChangeHook *el_change_hook; void *el_change_hook_opaque; }; diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 549cb1e..73c7f31 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -147,6 +147,9 @@ static void aarch64_a57_initfn(Object *obj) cpu->ccsidr[1] = 0x201fe012; /* 48KB L1 icache */ cpu->ccsidr[2] = 0x70ffe07a; /* 2048KB L2 cache */ cpu->dcz_blocksize = 4; /* 64 bytes */ +cpu->gic_num_lrs = 4; +cpu->gic_vpribits = 5; +cpu->gic_vprebits = 5; define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo); } @@ -201,6 +204,9 @@ static void aarch64_a53_initfn(Object *obj) cpu->ccsidr[1] = 0x201fe00a; /* 32KB L1 icache */ cpu->ccsidr[2] = 0x707fe07a; /* 1024KB L2 cache */ cpu->dcz_blocksize = 4; /* 64 bytes */ +cpu->gic_num_lrs = 4; +cpu->gic_vpribits = 5; +cpu->gic_vprebits = 5; define_arm_cp_regs(cpu, cortex_a57_a53_cp_reginfo); } -- 2.7.4
[Qemu-devel] [PULL 19/36] hw/intc/arm_gicv3: Add external IRQ lines for VIRQ and VFIQ
Augment the GICv3's QOM device interface by adding two new sets of sysbus IRQ lines, to signal VIRQ and VFIQ to each CPU. Signed-off-by: Peter Maydell Reviewed-by: Alistair Francis Message-id: 1483977924-14522-2-git-send-email-peter.mayd...@linaro.org --- include/hw/intc/arm_gicv3_common.h | 2 ++ hw/intc/arm_gicv3_common.c | 6 ++ 2 files changed, 8 insertions(+) diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h index 341a311..beb2c77 100644 --- a/include/hw/intc/arm_gicv3_common.h +++ b/include/hw/intc/arm_gicv3_common.h @@ -145,6 +145,8 @@ struct GICv3CPUState { CPUState *cpu; qemu_irq parent_irq; qemu_irq parent_fiq; +qemu_irq parent_virq; +qemu_irq parent_vfiq; /* Redistributor */ uint32_t level; /* Current IRQ level */ diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c index 0aa9b9c..0ee67a4 100644 --- a/hw/intc/arm_gicv3_common.c +++ b/hw/intc/arm_gicv3_common.c @@ -126,6 +126,12 @@ void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler, for (i = 0; i < s->num_cpu; i++) { sysbus_init_irq(sbd, &s->cpu[i].parent_fiq); } +for (i = 0; i < s->num_cpu; i++) { +sysbus_init_irq(sbd, &s->cpu[i].parent_virq); +} +for (i = 0; i < s->num_cpu; i++) { +sysbus_init_irq(sbd, &s->cpu[i].parent_vfiq); +} memory_region_init_io(&s->iomem_dist, OBJECT(s), ops, s, "gicv3_dist", 0x1); -- 2.7.4
[Qemu-devel] [PULL 28/36] hw/intc/arm_gicv3: Implement ICV_ HPPIR, DIR and RPR registers
Implement the the ICV_ registers HPPIR, DIR and RPR. Signed-off-by: Peter Maydell Message-id: 1483977924-14522-11-git-send-email-peter.mayd...@linaro.org --- hw/intc/arm_gicv3_cpuif.c | 235 +- hw/intc/trace-events | 3 + 2 files changed, 235 insertions(+), 3 deletions(-) diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index 40a52ce..e069082 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -44,6 +44,21 @@ static inline int icv_min_vbpr(GICv3CPUState *cs) } /* Simple accessor functions for LR fields */ +static uint32_t ich_lr_vintid(uint64_t lr) +{ +return extract64(lr, ICH_LR_EL2_VINTID_SHIFT, ICH_LR_EL2_VINTID_LENGTH); +} + +static uint32_t ich_lr_pintid(uint64_t lr) +{ +return extract64(lr, ICH_LR_EL2_PINTID_SHIFT, ICH_LR_EL2_PINTID_LENGTH); +} + +static uint32_t ich_lr_prio(uint64_t lr) +{ +return extract64(lr, ICH_LR_EL2_PRIORITY_SHIFT, ICH_LR_EL2_PRIORITY_LENGTH); +} + static int ich_lr_state(uint64_t lr) { return extract64(lr, ICH_LR_EL2_STATE_SHIFT, ICH_LR_EL2_STATE_LENGTH); @@ -115,6 +130,79 @@ static uint32_t icv_fullprio_mask(GICv3CPUState *cs) return ~0U << (8 - cs->vpribits); } +static int ich_highest_active_virt_prio(GICv3CPUState *cs) +{ +/* Calculate the current running priority based on the set bits + * in the ICH Active Priority Registers. + */ +int i; +int aprmax = 1 << (cs->vprebits - 5); + +assert(aprmax <= ARRAY_SIZE(cs->ich_apr[0])); + +for (i = 0; i < aprmax; i++) { +uint32_t apr = cs->ich_apr[GICV3_G0][i] | +cs->ich_apr[GICV3_G1NS][i]; + +if (!apr) { +continue; +} +return (i * 32 + ctz32(apr)) << (icv_min_vbpr(cs) + 1); +} +/* No current active interrupts: return idle priority */ +return 0xff; +} + +static int hppvi_index(GICv3CPUState *cs) +{ +/* Return the list register index of the highest priority pending + * virtual interrupt, as per the HighestPriorityVirtualInterrupt + * pseudocode. If no pending virtual interrupts, return -1. + */ +int idx = -1; +int i; +/* Note that a list register entry with a priority of 0xff will + * never be reported by this function; this is the architecturally + * correct behaviour. + */ +int prio = 0xff; + +if (!(cs->ich_vmcr_el2 & (ICH_VMCR_EL2_VENG0 | ICH_VMCR_EL2_VENG1))) { +/* Both groups disabled, definitely nothing to do */ +return idx; +} + +for (i = 0; i < cs->num_list_regs; i++) { +uint64_t lr = cs->ich_lr_el2[i]; +int thisprio; + +if (ich_lr_state(lr) != ICH_LR_EL2_STATE_PENDING) { +/* Not Pending */ +continue; +} + +/* Ignore interrupts if relevant group enable not set */ +if (lr & ICH_LR_EL2_GROUP) { +if (!(cs->ich_vmcr_el2 & ICH_VMCR_EL2_VENG1)) { +continue; +} +} else { +if (!(cs->ich_vmcr_el2 & ICH_VMCR_EL2_VENG0)) { +continue; +} +} + +thisprio = ich_lr_prio(lr); + +if (thisprio < prio) { +prio = thisprio; +idx = i; +} +} + +return idx; +} + static uint32_t eoi_maintenance_interrupt_state(GICv3CPUState *cs, uint32_t *misr) { @@ -363,6 +451,35 @@ static void icv_ctlr_write(CPUARMState *env, const ARMCPRegInfo *ri, gicv3_cpuif_virt_update(cs); } +static uint64_t icv_rpr_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ +GICv3CPUState *cs = icc_cs_from_env(env); +int prio = ich_highest_active_virt_prio(cs); + +trace_gicv3_icv_rpr_read(gicv3_redist_affid(cs), prio); +return prio; +} + +static uint64_t icv_hppir_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ +GICv3CPUState *cs = icc_cs_from_env(env); +int grp = ri->crm == 8 ? GICV3_G0 : GICV3_G1NS; +int idx = hppvi_index(cs); +uint64_t value = INTID_SPURIOUS; + +if (idx >= 0) { +uint64_t lr = cs->ich_lr_el2[idx]; +int thisgrp = (lr & ICH_LR_EL2_GROUP) ? GICV3_G1NS : GICV3_G0; + +if (grp == thisgrp) { +value = ich_lr_vintid(lr); +} +} + +trace_gicv3_icv_hppir_read(grp, gicv3_redist_affid(cs), value); +return value; +} + static int icc_highest_active_prio(GICv3CPUState *cs) { /* Calculate the current running priority based on the set bits @@ -781,6 +898,97 @@ static void icc_deactivate_irq(GICv3CPUState *cs, int irq) } } +static bool icv_eoi_split(CPUARMState *env, GICv3CPUState *cs) +{ +/* Return true if we should split priority drop and interrupt + * deactivation, ie whether the virtual EOIMode bit is set. + */ +return cs->ich_vmcr_el2 & ICH_VMCR_EL2_VEOIM; +} + +static int icv_find_active(GICv3CPUState *cs, int irq) +{ +/* Given an interrupt number for an active interrupt, return the index
[Qemu-devel] [PULL 25/36] hw/intc/gicv3: Add data fields for virtualization support
As the first step in adding support for the virtualization extensions to the GICv3 emulation: * add the necessary data fields to the state structures * add the fields to the migration state, as a subsection which is only present if virtualization is enabled The use of a subsection means we retain migration compatibility as EL2 is not enabled on any CPUs currently. Signed-off-by: Peter Maydell Acked-by: Alistair Francis Message-id: 1483977924-14522-8-git-send-email-peter.mayd...@linaro.org --- include/hw/intc/arm_gicv3_common.h | 18 ++ hw/intc/arm_gicv3_common.c | 25 + hw/intc/arm_gicv3_cpuif.c | 13 + 3 files changed, 56 insertions(+) diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h index beb2c77..665d3f8 100644 --- a/include/hw/intc/arm_gicv3_common.h +++ b/include/hw/intc/arm_gicv3_common.h @@ -38,6 +38,9 @@ /* Number of SGI target-list bits */ #define GICV3_TARGETLIST_BITS 16 +/* Maximum number of list registers (architectural limit) */ +#define GICV3_LR_MAX 16 + /* Minimum BPR for Secure, or when security not enabled */ #define GIC_MIN_BPR 0 /* Minimum BPR for Nonsecure when security is enabled */ @@ -175,6 +178,21 @@ struct GICv3CPUState { uint64_t icc_igrpen[3]; uint64_t icc_ctlr_el3; +/* Virtualization control interface */ +uint64_t ich_apr[3][4]; /* ich_apr[GICV3_G1][x] never used */ +uint64_t ich_hcr_el2; +uint64_t ich_lr_el2[GICV3_LR_MAX]; +uint64_t ich_vmcr_el2; + +/* Properties of the CPU interface. These are initialized from + * the settings in the CPU proper. + * If the number of implemented list registers is 0 then the + * virtualization support is not implemented. + */ +int num_list_regs; +int vpribits; /* number of virtual priority bits */ +int vprebits; /* number of virtual preemption bits */ + /* Current highest priority pending interrupt for this CPU. * This is cached information that can be recalculated from the * real state above; it doesn't need to be migrated. diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c index 0ee67a4..16b9b0f 100644 --- a/hw/intc/arm_gicv3_common.c +++ b/hw/intc/arm_gicv3_common.c @@ -49,6 +49,27 @@ static int gicv3_post_load(void *opaque, int version_id) return 0; } +static bool virt_state_needed(void *opaque) +{ +GICv3CPUState *cs = opaque; + +return cs->num_list_regs != 0; +} + +static const VMStateDescription vmstate_gicv3_cpu_virt = { +.name = "arm_gicv3_cpu/virt", +.version_id = 1, +.minimum_version_id = 1, +.needed = virt_state_needed, +.fields = (VMStateField[]) { +VMSTATE_UINT64_2DARRAY(ich_apr, GICv3CPUState, 3, 4), +VMSTATE_UINT64(ich_hcr_el2, GICv3CPUState), +VMSTATE_UINT64_ARRAY(ich_lr_el2, GICv3CPUState, GICV3_LR_MAX), +VMSTATE_UINT64(ich_vmcr_el2, GICv3CPUState), +VMSTATE_END_OF_LIST() +} +}; + static const VMStateDescription vmstate_gicv3_cpu = { .name = "arm_gicv3_cpu", .version_id = 1, @@ -75,6 +96,10 @@ static const VMStateDescription vmstate_gicv3_cpu = { VMSTATE_UINT64_ARRAY(icc_igrpen, GICv3CPUState, 3), VMSTATE_UINT64(icc_ctlr_el3, GICv3CPUState), VMSTATE_END_OF_LIST() +}, +.subsections = (const VMStateDescription * []) { +&vmstate_gicv3_cpu_virt, +NULL } }; diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index 35e8eb3..d2f859c 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -36,6 +36,12 @@ static bool gicv3_use_ns_bank(CPUARMState *env) return !arm_is_secure_below_el3(env); } +/* The minimum BPR for the virtual interface is a configurable property */ +static inline int icv_min_vbpr(GICv3CPUState *cs) +{ +return 7 - cs->vprebits; +} + static int icc_highest_active_prio(GICv3CPUState *cs) { /* Calculate the current running priority based on the set bits @@ -1081,6 +1087,13 @@ static void icc_reset(CPUARMState *env, const ARMCPRegInfo *ri) cs->icc_ctlr_el3 = ICC_CTLR_EL3_NDS | ICC_CTLR_EL3_A3V | (1 << ICC_CTLR_EL3_IDBITS_SHIFT) | (7 << ICC_CTLR_EL3_PRIBITS_SHIFT); + +memset(cs->ich_apr, 0, sizeof(cs->ich_apr)); +cs->ich_hcr_el2 = 0; +memset(cs->ich_lr_el2, 0, sizeof(cs->ich_lr_el2)); +cs->ich_vmcr_el2 = ICH_VMCR_EL2_VFIQEN | +(icv_min_vbpr(cs) << ICH_VMCR_EL2_VBPR1_SHIFT) | +(icv_min_vbpr(cs) << ICH_VMCR_EL2_VBPR0_SHIFT); } static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { -- 2.7.4
[Qemu-devel] [PULL 08/36] aspeed/smc: remove call to aspeed_smc_update_cs() in reset function
From: Cédric Le Goater Instead, we can simply set the irq level when unselecting the slave devices. This change prepares ground for a subsequent cleanup of the aspeed_smc_update_cs() routine which uselessly loops on all slaves to update their status. Signed-off-by: Cédric Le Goater Message-id: 1483979087-32663-3-git-send-email-...@kaod.org Signed-off-by: Peter Maydell --- hw/ssi/aspeed_smc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 8a7217d..205c0ab 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -424,6 +424,7 @@ static void aspeed_smc_reset(DeviceState *d) /* Unselect all slaves */ for (i = 0; i < s->num_cs; ++i) { s->regs[s->r_ctrl0 + i] |= CTRL_CE_STOP_ACTIVE; +qemu_set_irq(s->cs_lines[i], true); } /* setup default segment register values for all */ @@ -431,8 +432,6 @@ static void aspeed_smc_reset(DeviceState *d) s->regs[R_SEG_ADDR0 + i] = aspeed_smc_segment_to_reg(&s->ctrl->segments[i]); } - -aspeed_smc_update_cs(s); } static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size) -- 2.7.4
[Qemu-devel] [PULL 18/36] hw/arm/virt-acpi - reserve ECAM space as PNP0C02 device
From: Ard Biesheuvel Linux for arm64 v4.10 and later will complain if the ECAM config space is not reserved in the ACPI namespace: acpi PNP0A08:00: [Firmware Bug]: ECAM area [mem 0x3f00-0x3fff] not reserved in ACPI namespace The rationale is that OSes that don't consume the MCFG table should still be able to infer that the PCI config space MMIO region is occupied. So update the ACPI table generation routine to add this reservation. Signed-off-by: Ard Biesheuvel Message-id: 1484328738-21149-1-git-send-email-ard.biesheu...@linaro.org Signed-off-by: Peter Maydell --- hw/arm/virt-acpi-build.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 085a611..50d52f6 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -310,6 +310,13 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, Aml *dev_rp0 = aml_device("%s", "RP0"); aml_append(dev_rp0, aml_name_decl("_ADR", aml_int(0))); aml_append(dev, dev_rp0); + +Aml *dev_res0 = aml_device("%s", "RES0"); +aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02"))); +crs = aml_resource_template(); +aml_append(crs, aml_memory32_fixed(base_ecam, size_ecam, AML_READ_WRITE)); +aml_append(dev_res0, aml_name_decl("_CRS", crs)); +aml_append(dev, dev_res0); aml_append(scope, dev); } -- 2.7.4
[Qemu-devel] [PULL 31/36] hw/intc/arm_gicv3: Implement EL2 traps for CPU i/f regs
Implement the architecturally required traps from NS EL1 to EL2 for the CPU interface registers. These fall into several different groups: * group-0-only registers all trap if ICH_HRC_EL2.TALL0 is set (exactly the registers covered by gicv3_fiq_access()) * group-1-only registers all trap if ICH_HRC_EL2.TALL1 is set (exactly the registers covered by gicv3_irq_access()) * DIR traps if ICH_HCR_EL2.TC or ICH_HCR_EL2.TDIR are set * PMR, RPR, CTLR trap if ICH_HCR_EL2.TC is set * SGI0R, SGI1R, ASGI1R trap if ICH_HCR_EL2.TC is set or if HCR_EL2.IMO or HCR_EL2.FMO are set We split DIR and the SGI registers out into their own access functions, leaving the existing gicv3_irqfiq_access() just handling PMR, RPR and CTLR. This commit doesn't implement support for trapping on HSTR_EL2.T12 for the 32-bit registers, as we don't implement any of those per-coprocessor trap bits currently and probably will want to do those in some more centralized way. Signed-off-by: Peter Maydell Message-id: 1483977924-14522-14-git-send-email-peter.mayd...@linaro.org --- hw/intc/arm_gicv3_cpuif.c | 70 --- 1 file changed, 60 insertions(+), 10 deletions(-) diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index e05f6b3..a9ee7fd 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -1833,9 +1833,17 @@ static CPAccessResult gicv3_irqfiq_access(CPUARMState *env, const ARMCPRegInfo *ri, bool isread) { CPAccessResult r = CP_ACCESS_OK; +GICv3CPUState *cs = icc_cs_from_env(env); +int el = arm_current_el(env); + +if ((cs->ich_hcr_el2 & ICH_HCR_EL2_TC) && +el == 1 && !arm_is_secure_below_el3(env)) { +/* Takes priority over a possible EL3 trap */ +return CP_ACCESS_TRAP_EL2; +} if ((env->cp15.scr_el3 & (SCR_FIQ | SCR_IRQ)) == (SCR_FIQ | SCR_IRQ)) { -switch (arm_current_el(env)) { +switch (el) { case 1: if (arm_is_secure_below_el3(env) || ((env->cp15.hcr_el2 & (HCR_IMO | HCR_FMO)) == 0)) { @@ -1861,13 +1869,47 @@ static CPAccessResult gicv3_irqfiq_access(CPUARMState *env, return r; } +static CPAccessResult gicv3_dir_access(CPUARMState *env, + const ARMCPRegInfo *ri, bool isread) +{ +GICv3CPUState *cs = icc_cs_from_env(env); + +if ((cs->ich_hcr_el2 & ICH_HCR_EL2_TDIR) && +arm_current_el(env) == 1 && !arm_is_secure_below_el3(env)) { +/* Takes priority over a possible EL3 trap */ +return CP_ACCESS_TRAP_EL2; +} + +return gicv3_irqfiq_access(env, ri, isread); +} + +static CPAccessResult gicv3_sgi_access(CPUARMState *env, + const ARMCPRegInfo *ri, bool isread) +{ +if ((env->cp15.hcr_el2 & (HCR_IMO | HCR_FMO)) && +arm_current_el(env) == 1 && !arm_is_secure_below_el3(env)) { +/* Takes priority over a possible EL3 trap */ +return CP_ACCESS_TRAP_EL2; +} + +return gicv3_irqfiq_access(env, ri, isread); +} + static CPAccessResult gicv3_fiq_access(CPUARMState *env, const ARMCPRegInfo *ri, bool isread) { CPAccessResult r = CP_ACCESS_OK; +GICv3CPUState *cs = icc_cs_from_env(env); +int el = arm_current_el(env); + +if ((cs->ich_hcr_el2 & ICH_HCR_EL2_TALL0) && +el == 1 && !arm_is_secure_below_el3(env)) { +/* Takes priority over a possible EL3 trap */ +return CP_ACCESS_TRAP_EL2; +} if (env->cp15.scr_el3 & SCR_FIQ) { -switch (arm_current_el(env)) { +switch (el) { case 1: if (arm_is_secure_below_el3(env) || ((env->cp15.hcr_el2 & HCR_FMO) == 0)) { @@ -1897,9 +1939,17 @@ static CPAccessResult gicv3_irq_access(CPUARMState *env, const ARMCPRegInfo *ri, bool isread) { CPAccessResult r = CP_ACCESS_OK; +GICv3CPUState *cs = icc_cs_from_env(env); +int el = arm_current_el(env); + +if ((cs->ich_hcr_el2 & ICH_HCR_EL2_TALL1) && +el == 1 && !arm_is_secure_below_el3(env)) { +/* Takes priority over a possible EL3 trap */ +return CP_ACCESS_TRAP_EL2; +} if (env->cp15.scr_el3 & SCR_IRQ) { -switch (arm_current_el(env)) { +switch (el) { case 1: if (arm_is_secure_below_el3(env) || ((env->cp15.hcr_el2 & HCR_IMO) == 0)) { @@ -2055,7 +2105,7 @@ static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { { .name = "ICC_DIR_EL1", .state = ARM_CP_STATE_BOTH, .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 11, .opc2 = 1, .type = ARM_CP_IO | ARM_CP_NO_RAW, - .access = PL1_W, .accessfn = gicv3_irqfiq_access, + .access = PL1_W, .accessfn = gicv3_dir_access, .writefn = icc_dir_write, }, { .name = "ICC_RPR_EL1", .state = ARM_CP_STATE_BOTH, @@ -2067,37 +2117,37 @@ static const ARMCPR
[Qemu-devel] [PULL 32/36] hw/arm/virt: Support using SMC for PSCI
If we are giving the guest a CPU with EL2, it is likely to want to use the HVC instruction itself, for instance for providing PSCI to inner guest VMs. This makes using HVC as the PSCI conduit for the outer QEMU a bad idea. We will want to use SMC instead is this case: this makes sense because QEMU's PSCI implementation is effectively an emulation of functionality provided by EL3 firmware. Add code to support selecting the PSCI conduit to use, rather than hardcoding use of HVC. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias Reviewed-by: Andrew Jones Message-id: 1483977924-14522-15-git-send-email-peter.mayd...@linaro.org --- include/hw/arm/virt.h | 2 +- hw/arm/virt.c | 27 +-- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index b8a19ec..53507e6 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -103,7 +103,7 @@ typedef struct { uint32_t clock_phandle; uint32_t gic_phandle; uint32_t msi_phandle; -bool using_psci; +int psci_conduit; } VirtMachineState; #define TYPE_VIRT_MACHINE MACHINE_TYPE_NAME("virt") diff --git a/hw/arm/virt.c b/hw/arm/virt.c index d931d17..3a6f895 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -229,9 +229,19 @@ static void fdt_add_psci_node(const VirtMachineState *vms) uint32_t migrate_fn; void *fdt = vms->fdt; ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(0)); +const char *psci_method; -if (!vms->using_psci) { +switch (vms->psci_conduit) { +case QEMU_PSCI_CONDUIT_DISABLED: return; +case QEMU_PSCI_CONDUIT_HVC: +psci_method = "hvc"; +break; +case QEMU_PSCI_CONDUIT_SMC: +psci_method = "smc"; +break; +default: +g_assert_not_reached(); } qemu_fdt_add_subnode(fdt, "/psci"); @@ -263,7 +273,7 @@ static void fdt_add_psci_node(const VirtMachineState *vms) * However, the device tree binding uses 'method' instead, so that is * what we should use here. */ -qemu_fdt_setprop_string(fdt, "/psci", "method", "hvc"); +qemu_fdt_setprop_string(fdt, "/psci", "method", psci_method); qemu_fdt_setprop_cell(fdt, "/psci", "cpu_suspend", cpu_suspend_fn); qemu_fdt_setprop_cell(fdt, "/psci", "cpu_off", cpu_off_fn); @@ -365,7 +375,8 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) qemu_fdt_setprop_string(vms->fdt, nodename, "compatible", armcpu->dtb_compatible); -if (vms->using_psci && vms->smp_cpus > 1) { +if (vms->psci_conduit != QEMU_PSCI_CONDUIT_DISABLED +&& vms->smp_cpus > 1) { qemu_fdt_setprop_string(vms->fdt, nodename, "enable-method", "psci"); } @@ -1230,7 +1241,11 @@ static void machvirt_init(MachineState *machine) * let the boot ROM sort them out. * The usual case is that we do use QEMU's PSCI implementation. */ -vms->using_psci = !(vms->secure && firmware_loaded); +if (vms->secure && firmware_loaded) { +vms->psci_conduit = QEMU_PSCI_CONDUIT_DISABLED; +} else { +vms->psci_conduit = QEMU_PSCI_CONDUIT_HVC; +} /* The maximum number of CPUs depends on the GIC version, or on how * many redistributors we can fit into the memory map. @@ -1313,8 +1328,8 @@ static void machvirt_init(MachineState *machine) object_property_set_bool(cpuobj, false, "has_el3", NULL); } -if (vms->using_psci) { -object_property_set_int(cpuobj, QEMU_PSCI_CONDUIT_HVC, +if (vms->psci_conduit != QEMU_PSCI_CONDUIT_DISABLED) { +object_property_set_int(cpuobj, vms->psci_conduit, "psci-conduit", NULL); /* Secondary CPUs start in PSCI powered-down state */ -- 2.7.4
[Qemu-devel] [PULL 17/36] arm: virt: Fix segmentation fault when specifying an unsupported CPU
From: Shannon Zhao Using -cpu cortex-a9 (or any other unsupported CPU) with the virt board will cause QEMU to segmentation fault. This bug was introduced in commit 9ac4ef77, which incorrectly added a NULL terminator when converting the VirtBoardInfo array into a simple array of strings defining the valid CPUs. The cpuname_valid() loop already has a termination condition based on ARRAY_SIZE, so the NULL is spurious and causes the strcmp() to segfault if we reach it. Delete the NULL. Signed-off-by: Shannon Zhao Message-id: 1484619334-10488-1-git-send-email-zhaoshengl...@huawei.com [PMM: expanded commit message] Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/arm/virt.c | 1 - 1 file changed, 1 deletion(-) diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 7a03f84..95ac585 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -167,7 +167,6 @@ static const char *valid_cpus[] = { "cortex-a53", "cortex-a57", "host", -NULL }; static bool cpuname_valid(const char *cpu) -- 2.7.4
[Qemu-devel] [PULL 26/36] hw/intc/arm_gicv3: Add accessors for ICH_ system registers
The GICv3 virtualization interface includes system registers accessible only to the hypervisor which form the control interface for interrupt virtualization. Implement these registers. The function gicv3_cpuif_virt_update() which determines whether it needs to signal vIRQ, vFIQ or a maintenance interrupt is introduced here as a stub function -- its implementation will be added in a subsequent commit. Signed-off-by: Peter Maydell Message-id: 1483977924-14522-9-git-send-email-peter.mayd...@linaro.org --- hw/intc/arm_gicv3_cpuif.c | 477 ++ hw/intc/trace-events | 16 ++ 2 files changed, 493 insertions(+) diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index d2f859c..585c91c 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -42,6 +42,132 @@ static inline int icv_min_vbpr(GICv3CPUState *cs) return 7 - cs->vprebits; } +/* Simple accessor functions for LR fields */ +static int ich_lr_state(uint64_t lr) +{ +return extract64(lr, ICH_LR_EL2_STATE_SHIFT, ICH_LR_EL2_STATE_LENGTH); +} + +static int read_vbpr(GICv3CPUState *cs, int grp) +{ +/* Read VBPR value out of the VMCR field (caller must handle + * VCBPR effects if required) + */ +if (grp == GICV3_G0) { +return extract64(cs->ich_vmcr_el2, ICH_VMCR_EL2_VBPR0_SHIFT, + ICH_VMCR_EL2_VBPR0_LENGTH); +} else { +return extract64(cs->ich_vmcr_el2, ICH_VMCR_EL2_VBPR1_SHIFT, + ICH_VMCR_EL2_VBPR1_LENGTH); +} +} + +static void write_vbpr(GICv3CPUState *cs, int grp, int value) +{ +/* Write new VBPR1 value, handling the "writing a value less than + * the minimum sets it to the minimum" semantics. + */ +int min = icv_min_vbpr(cs); + +if (grp != GICV3_G0) { +min++; +} + +value = MAX(value, min); + +if (grp == GICV3_G0) { +cs->ich_vmcr_el2 = deposit64(cs->ich_vmcr_el2, ICH_VMCR_EL2_VBPR0_SHIFT, + ICH_VMCR_EL2_VBPR0_LENGTH, value); +} else { +cs->ich_vmcr_el2 = deposit64(cs->ich_vmcr_el2, ICH_VMCR_EL2_VBPR1_SHIFT, + ICH_VMCR_EL2_VBPR1_LENGTH, value); +} +} + +static uint32_t eoi_maintenance_interrupt_state(GICv3CPUState *cs, +uint32_t *misr) +{ +/* Return a set of bits indicating the EOI maintenance interrupt status + * for each list register. The EOI maintenance interrupt status is + * 1 if LR.State == 0 && LR.HW == 0 && LR.EOI == 1 + * (see the GICv3 spec for the ICH_EISR_EL2 register). + * If misr is not NULL then we should also collect the information + * about the MISR.EOI, MISR.NP and MISR.U bits. + */ +uint32_t value = 0; +int validcount = 0; +bool seenpending = false; +int i; + +for (i = 0; i < cs->num_list_regs; i++) { +uint64_t lr = cs->ich_lr_el2[i]; + +if ((lr & (ICH_LR_EL2_STATE_MASK | ICH_LR_EL2_HW | ICH_LR_EL2_EOI)) +== ICH_LR_EL2_EOI) { +value |= (1 << i); +} +if ((lr & ICH_LR_EL2_STATE_MASK)) { +validcount++; +} +if (ich_lr_state(lr) == ICH_LR_EL2_STATE_PENDING) { +seenpending = true; +} +} + +if (misr) { +if (validcount < 2 && (cs->ich_hcr_el2 & ICH_HCR_EL2_UIE)) { +*misr |= ICH_MISR_EL2_U; +} +if (!seenpending && (cs->ich_hcr_el2 & ICH_HCR_EL2_NPIE)) { +*misr |= ICH_MISR_EL2_NP; +} +if (value) { +*misr |= ICH_MISR_EL2_EOI; +} +} +return value; +} + +static uint32_t maintenance_interrupt_state(GICv3CPUState *cs) +{ +/* Return a set of bits indicating the maintenance interrupt status + * (as seen in the ICH_MISR_EL2 register). + */ +uint32_t value = 0; + +/* Scan list registers and fill in the U, NP and EOI bits */ +eoi_maintenance_interrupt_state(cs, &value); + +if (cs->ich_hcr_el2 & (ICH_HCR_EL2_LRENPIE | ICH_HCR_EL2_EOICOUNT_MASK)) { +value |= ICH_MISR_EL2_LRENP; +} + +if ((cs->ich_hcr_el2 & ICH_HCR_EL2_VGRP0EIE) && +(cs->ich_vmcr_el2 & ICH_VMCR_EL2_VENG0)) { +value |= ICH_MISR_EL2_VGRP0E; +} + +if ((cs->ich_hcr_el2 & ICH_HCR_EL2_VGRP0DIE) && +!(cs->ich_vmcr_el2 & ICH_VMCR_EL2_VENG1)) { +value |= ICH_MISR_EL2_VGRP0D; +} +if ((cs->ich_hcr_el2 & ICH_HCR_EL2_VGRP1EIE) && +(cs->ich_vmcr_el2 & ICH_VMCR_EL2_VENG1)) { +value |= ICH_MISR_EL2_VGRP1E; +} + +if ((cs->ich_hcr_el2 & ICH_HCR_EL2_VGRP1DIE) && +!(cs->ich_vmcr_el2 & ICH_VMCR_EL2_VENG1)) { +value |= ICH_MISR_EL2_VGRP1D; +} + +return value; +} + +static void gicv3_cpuif_virt_update(GICv3CPUState *cs) +{ +} + static int icc_highest_active_prio(GICv3CPUState *cs) { /* Calculate the current running priority based on the set bits @@ -1334,6 +1460,306 @@ s
[Qemu-devel] [PULL 24/36] hw/intc/gicv3: Add defines for ICH system register fields
Add defines to gicv3_internal.h for fields in the ICH_* system registers which form the GIC virtualization control interface. Signed-off-by: Peter Maydell Message-id: 1483977924-14522-7-git-send-email-peter.mayd...@linaro.org --- hw/intc/gicv3_internal.h | 79 1 file changed, 79 insertions(+) diff --git a/hw/intc/gicv3_internal.h b/hw/intc/gicv3_internal.h index 8f3567e..aeb801d 100644 --- a/hw/intc/gicv3_internal.h +++ b/hw/intc/gicv3_internal.h @@ -159,6 +159,85 @@ #define ICC_CTLR_EL3_A3V (1U << 15) #define ICC_CTLR_EL3_NDS (1U << 17) +#define ICH_VMCR_EL2_VENG0_SHIFT 0 +#define ICH_VMCR_EL2_VENG0 (1U << ICH_VMCR_EL2_VENG0_SHIFT) +#define ICH_VMCR_EL2_VENG1_SHIFT 1 +#define ICH_VMCR_EL2_VENG1 (1U << ICH_VMCR_EL2_VENG1_SHIFT) +#define ICH_VMCR_EL2_VACKCTL (1U << 2) +#define ICH_VMCR_EL2_VFIQEN (1U << 3) +#define ICH_VMCR_EL2_VCBPR_SHIFT 4 +#define ICH_VMCR_EL2_VCBPR (1U << ICH_VMCR_EL2_VCBPR_SHIFT) +#define ICH_VMCR_EL2_VEOIM_SHIFT 9 +#define ICH_VMCR_EL2_VEOIM (1U << ICH_VMCR_EL2_VEOIM_SHIFT) +#define ICH_VMCR_EL2_VBPR1_SHIFT 18 +#define ICH_VMCR_EL2_VBPR1_LENGTH 3 +#define ICH_VMCR_EL2_VBPR1_MASK (0x7U << ICH_VMCR_EL2_VBPR1_SHIFT) +#define ICH_VMCR_EL2_VBPR0_SHIFT 21 +#define ICH_VMCR_EL2_VBPR0_LENGTH 3 +#define ICH_VMCR_EL2_VBPR0_MASK (0x7U << ICH_VMCR_EL2_VBPR0_SHIFT) +#define ICH_VMCR_EL2_VPMR_SHIFT 24 +#define ICH_VMCR_EL2_VPMR_LENGTH 8 +#define ICH_VMCR_EL2_VPMR_MASK (0xffU << ICH_VMCR_EL2_VPMR_SHIFT) + +#define ICH_HCR_EL2_EN (1U << 0) +#define ICH_HCR_EL2_UIE (1U << 1) +#define ICH_HCR_EL2_LRENPIE (1U << 2) +#define ICH_HCR_EL2_NPIE (1U << 3) +#define ICH_HCR_EL2_VGRP0EIE (1U << 4) +#define ICH_HCR_EL2_VGRP0DIE (1U << 5) +#define ICH_HCR_EL2_VGRP1EIE (1U << 6) +#define ICH_HCR_EL2_VGRP1DIE (1U << 7) +#define ICH_HCR_EL2_TC (1U << 10) +#define ICH_HCR_EL2_TALL0 (1U << 11) +#define ICH_HCR_EL2_TALL1 (1U << 12) +#define ICH_HCR_EL2_TSEI (1U << 13) +#define ICH_HCR_EL2_TDIR (1U << 14) +#define ICH_HCR_EL2_EOICOUNT_SHIFT 27 +#define ICH_HCR_EL2_EOICOUNT_LENGTH 5 +#define ICH_HCR_EL2_EOICOUNT_MASK (0x1fU << ICH_HCR_EL2_EOICOUNT_SHIFT) + +#define ICH_LR_EL2_VINTID_SHIFT 0 +#define ICH_LR_EL2_VINTID_LENGTH 32 +#define ICH_LR_EL2_VINTID_MASK (0xULL << ICH_LR_EL2_VINTID_SHIFT) +#define ICH_LR_EL2_PINTID_SHIFT 32 +#define ICH_LR_EL2_PINTID_LENGTH 10 +#define ICH_LR_EL2_PINTID_MASK (0x3ffULL << ICH_LR_EL2_PINTID_SHIFT) +/* Note that EOI shares with the top bit of the pINTID field */ +#define ICH_LR_EL2_EOI (1ULL << 41) +#define ICH_LR_EL2_PRIORITY_SHIFT 48 +#define ICH_LR_EL2_PRIORITY_LENGTH 8 +#define ICH_LR_EL2_PRIORITY_MASK (0xffULL << ICH_LR_EL2_PRIORITY_SHIFT) +#define ICH_LR_EL2_GROUP (1ULL << 60) +#define ICH_LR_EL2_HW (1ULL << 61) +#define ICH_LR_EL2_STATE_SHIFT 62 +#define ICH_LR_EL2_STATE_LENGTH 2 +#define ICH_LR_EL2_STATE_MASK (3ULL << ICH_LR_EL2_STATE_SHIFT) +/* values for the state field: */ +#define ICH_LR_EL2_STATE_INVALID 0 +#define ICH_LR_EL2_STATE_PENDING 1 +#define ICH_LR_EL2_STATE_ACTIVE 2 +#define ICH_LR_EL2_STATE_ACTIVE_PENDING 3 +#define ICH_LR_EL2_STATE_PENDING_BIT (1ULL << ICH_LR_EL2_STATE_SHIFT) +#define ICH_LR_EL2_STATE_ACTIVE_BIT (2ULL << ICH_LR_EL2_STATE_SHIFT) + +#define ICH_MISR_EL2_EOI (1U << 0) +#define ICH_MISR_EL2_U (1U << 1) +#define ICH_MISR_EL2_LRENP (1U << 2) +#define ICH_MISR_EL2_NP (1U << 3) +#define ICH_MISR_EL2_VGRP0E (1U << 4) +#define ICH_MISR_EL2_VGRP0D (1U << 5) +#define ICH_MISR_EL2_VGRP1E (1U << 6) +#define ICH_MISR_EL2_VGRP1D (1U << 7) + +#define ICH_VTR_EL2_LISTREGS_SHIFT 0 +#define ICH_VTR_EL2_TDS (1U << 19) +#define ICH_VTR_EL2_NV4 (1U << 20) +#define ICH_VTR_EL2_A3V (1U << 21) +#define ICH_VTR_EL2_SEIS (1U << 22) +#define ICH_VTR_EL2_IDBITS_SHIFT 23 +#define ICH_VTR_EL2_PREBITS_SHIFT 26 +#define ICH_VTR_EL2_PRIBITS_SHIFT 29 + /* Special interrupt IDs */ #define INTID_SECURE 1020 #define INTID_NONSECURE 1021 -- 2.7.4
[Qemu-devel] [PULL 05/36] target/arm: Handle VIRQ and VFIQ in arm_cpu_do_interrupt_aarch32()
To run a VM in 32-bit EL1 our AArch32 interrupt handling code needs to be able to cope with VIRQ and VFIQ exceptions. These behave like IRQ and FIQ except that we don't need to try to route them to Monitor mode. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias --- target/arm/helper.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/target/arm/helper.c b/target/arm/helper.c index b3875c7..ba72ebb 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -6399,6 +6399,20 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs) } offset = 4; break; +case EXCP_VIRQ: +new_mode = ARM_CPU_MODE_IRQ; +addr = 0x18; +/* Disable IRQ and imprecise data aborts. */ +mask = CPSR_A | CPSR_I; +offset = 4; +break; +case EXCP_VFIQ: +new_mode = ARM_CPU_MODE_FIQ; +addr = 0x1c; +/* Disable FIQ, IRQ and imprecise data aborts. */ +mask = CPSR_A | CPSR_I | CPSR_F; +offset = 4; +break; case EXCP_SMC: new_mode = ARM_CPU_MODE_MON; addr = 0x08; -- 2.7.4
[Qemu-devel] [PULL 03/36] block: m25p80: Introduce die erase command
From: Marcin Krzeminski Modern big flash NOR devices consist of more than one die. Some of them do not support chip erase and instead have a die erase command that can erase one die only. This commit adds support for defining the number of dies in the chip, and adds support for die erase command. The NOR flash model is not strict, so no option to disable chip erase has been added. Signed-off-by: Marcin Krzeminski Reviewed-by: Cédric Le Goater Reviewed-by: Edgar E. Iglesias Message-id: 20170108083854.5006-3-mar.krzemin...@gmail.com Signed-off-by: Peter Maydell --- hw/block/m25p80.c | 41 - 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index 4ad67ac..74c6e39 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -74,6 +74,12 @@ typedef struct FlashPartInfo { uint32_t n_sectors; uint32_t page_size; uint16_t flags; +/* + * Big sized spi nor are often stacked devices, thus sometime + * replace chip erase with die erase. + * This field inform how many die is in the chip. + */ +uint8_t die_cnt; } FlashPartInfo; /* adapted from linux */ @@ -91,7 +97,8 @@ typedef struct FlashPartInfo { .sector_size = (_sector_size),\ .n_sectors = (_n_sectors),\ .page_size = 256,\ -.flags = (_flags), +.flags = (_flags),\ +.die_cnt = 0 #define INFO6(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors, _flags)\ .part_name = _part_name,\ @@ -108,6 +115,24 @@ typedef struct FlashPartInfo { .n_sectors = (_n_sectors),\ .page_size = 256,\ .flags = (_flags),\ +.die_cnt = 0 + +#define INFO_STACKED(_part_name, _jedec_id, _ext_id, _sector_size, _n_sectors,\ +_flags, _die_cnt)\ +.part_name = _part_name,\ +.id = {\ +((_jedec_id) >> 16) & 0xff,\ +((_jedec_id) >> 8) & 0xff,\ +(_jedec_id) & 0xff,\ +((_ext_id) >> 8) & 0xff,\ +(_ext_id) & 0xff,\ + },\ +.id_len = (!(_jedec_id) ? 0 : (3 + ((_ext_id) ? 2 : 0))),\ +.sector_size = (_sector_size),\ +.n_sectors = (_n_sectors),\ +.page_size = 256,\ +.flags = (_flags),\ +.die_cnt = _die_cnt #define JEDEC_NUMONYX 0x20 #define JEDEC_WINBOND 0xEF @@ -360,6 +385,8 @@ typedef enum { REVCR = 0x65, WEVCR = 0x61, + +DIE_ERASE = 0xC4, } FlashCMD; typedef enum { @@ -517,6 +544,16 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd) case BULK_ERASE: len = s->size; break; +case DIE_ERASE: +if (s->pi->die_cnt) { +len = s->size / s->pi->die_cnt; +offset = offset & (~(len - 1)); +} else { +qemu_log_mask(LOG_GUEST_ERROR, "M25P80: die erase is not supported" + " by device\n"); +return; +} +break; default: abort(); } @@ -638,6 +675,7 @@ static void complete_collecting_data(Flash *s) case ERASE4_32K: case ERASE_SECTOR: case ERASE4_SECTOR: +case DIE_ERASE: flash_erase(s, s->cur_addr, s->cmd_in_progress); break; case WRSR: @@ -884,6 +922,7 @@ static void decode_new_cmd(Flash *s, uint32_t value) case PP: case PP4: case PP4_4: +case DIE_ERASE: s->needed_bytes = get_addr_length(s); s->pos = 0; s->len = 0; -- 2.7.4
[Qemu-devel] [PULL 15/36] aspeed/smc: extend tests for Command mode
From: Cédric Le Goater The Aspeed SMC controllers have a mode (Command mode) in which accesses to the flash content are no different than doing MMIOs. The controller generates all the necessary commands to load (or store) data in memory. So add a couple of tests doing direct reads and writes on the AHB bus. Signed-off-by: Cédric Le Goater Reviewed-by: Andrew Jeffery Message-id: 1483979087-32663-10-git-send-email-...@kaod.org Signed-off-by: Peter Maydell --- tests/m25p80-test.c | 102 1 file changed, 102 insertions(+) diff --git a/tests/m25p80-test.c b/tests/m25p80-test.c index 8dd550d..244aa33 100644 --- a/tests/m25p80-test.c +++ b/tests/m25p80-test.c @@ -36,6 +36,9 @@ #define CRTL_EXTENDED0 0 /* 32 bit addressing for SPI */ #define R_CTRL0 0x10 #define CTRL_CE_STOP_ACTIVE (1 << 2) +#define CTRL_READMODE0x0 +#define CTRL_FREADMODE 0x1 +#define CTRL_WRITEMODE 0x2 #define CTRL_USERMODE0x3 #define ASPEED_FMC_BASE0x1E62 @@ -86,6 +89,22 @@ static void spi_conf_remove(uint32_t value) writel(ASPEED_FMC_BASE + R_CONF, conf); } +static void spi_ce_ctrl(uint32_t value) +{ +uint32_t conf = readl(ASPEED_FMC_BASE + R_CE_CTRL); + +conf |= value; +writel(ASPEED_FMC_BASE + R_CE_CTRL, conf); +} + +static void spi_ctrl_setmode(uint8_t mode, uint8_t cmd) +{ +uint32_t ctrl = readl(ASPEED_FMC_BASE + R_CTRL0); +ctrl &= ~(CTRL_USERMODE | 0xff << 16); +ctrl |= mode | (cmd << 16); +writel(ASPEED_FMC_BASE + R_CTRL0, ctrl); +} + static void spi_ctrl_start_user(void) { uint32_t ctrl = readl(ASPEED_FMC_BASE + R_CTRL0); @@ -152,6 +171,18 @@ static void read_page(uint32_t addr, uint32_t *page) spi_ctrl_stop_user(); } +static void read_page_mem(uint32_t addr, uint32_t *page) +{ +int i; + +/* move out USER mode to use direct reads from the AHB bus */ +spi_ctrl_setmode(CTRL_READMODE, READ); + +for (i = 0; i < PAGE_SIZE / 4; i++) { +page[i] = make_be32(readl(ASPEED_FLASH_BASE + addr + i * 4)); +} +} + static void test_erase_sector(void) { uint32_t some_page_addr = 0x600 * PAGE_SIZE; @@ -248,6 +279,75 @@ static void test_write_page(void) flash_reset(); } +static void test_read_page_mem(void) +{ +uint32_t my_page_addr = 0x14000 * PAGE_SIZE; /* beyond 16MB */ +uint32_t some_page_addr = 0x15000 * PAGE_SIZE; +uint32_t page[PAGE_SIZE / 4]; +int i; + +/* Enable 4BYTE mode for controller. This is should be strapped by + * HW for CE0 anyhow. + */ +spi_ce_ctrl(1 << CRTL_EXTENDED0); + +/* Enable 4BYTE mode for flash. */ +spi_conf(CONF_ENABLE_W0); +spi_ctrl_start_user(); +writeb(ASPEED_FLASH_BASE, EN_4BYTE_ADDR); +spi_ctrl_stop_user(); +spi_conf_remove(CONF_ENABLE_W0); + +/* Check what was written */ +read_page_mem(my_page_addr, page); +for (i = 0; i < PAGE_SIZE / 4; i++) { +g_assert_cmphex(page[i], ==, my_page_addr + i * 4); +} + +/* Check some other page. It should be full of 0xff */ +read_page_mem(some_page_addr, page); +for (i = 0; i < PAGE_SIZE / 4; i++) { +g_assert_cmphex(page[i], ==, 0x); +} + +flash_reset(); +} + +static void test_write_page_mem(void) +{ +uint32_t my_page_addr = 0x15000 * PAGE_SIZE; +uint32_t page[PAGE_SIZE / 4]; +int i; + +/* Enable 4BYTE mode for controller. This is should be strapped by + * HW for CE0 anyhow. + */ +spi_ce_ctrl(1 << CRTL_EXTENDED0); + +/* Enable 4BYTE mode for flash. */ +spi_conf(CONF_ENABLE_W0); +spi_ctrl_start_user(); +writeb(ASPEED_FLASH_BASE, EN_4BYTE_ADDR); +writeb(ASPEED_FLASH_BASE, WREN); +spi_ctrl_stop_user(); + +/* move out USER mode to use direct writes to the AHB bus */ +spi_ctrl_setmode(CTRL_WRITEMODE, PP); + +for (i = 0; i < PAGE_SIZE / 4; i++) { +writel(ASPEED_FLASH_BASE + my_page_addr + i * 4, + make_be32(my_page_addr + i * 4)); +} + +/* Check what was written */ +read_page_mem(my_page_addr, page); +for (i = 0; i < PAGE_SIZE / 4; i++) { +g_assert_cmphex(page[i], ==, my_page_addr + i * 4); +} + +flash_reset(); +} + static char tmp_path[] = "/tmp/qtest.m25p80.XX"; int main(int argc, char **argv) @@ -273,6 +373,8 @@ int main(int argc, char **argv) qtest_add_func("/m25p80/erase_sector", test_erase_sector); qtest_add_func("/m25p80/erase_all", test_erase_all); qtest_add_func("/m25p80/write_page", test_write_page); +qtest_add_func("/m25p80/read_page_mem", test_read_page_mem); +qtest_add_func("/m25p80/write_page_mem", test_write_page_mem); ret = g_test_run(); -- 2.7.4
[Qemu-devel] [PULL 33/36] hw/arm/virt-acpi-build: use SMC if booting in EL2
From: Andrew Jones Signed-off-by: Andrew Jones Acked-by: Alistair Francis Signed-off-by: Peter Maydell Message-id: 1483977924-14522-16-git-send-email-peter.mayd...@linaro.org [PMM: look at vms->psci_conduit rather than vms->virt to decide whether to use HVC or SMC, and report no PSCI support at all for the 'PSCI disabled' case] Signed-off-by: Peter Maydell --- hw/arm/virt-acpi-build.c | 26 -- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 50d52f6..73aca9e 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -650,16 +650,30 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) } /* FADT */ -static void -build_fadt(GArray *table_data, BIOSLinker *linker, unsigned dsdt_tbl_offset) +static void build_fadt(GArray *table_data, BIOSLinker *linker, + VirtMachineState *vms, unsigned dsdt_tbl_offset) { AcpiFadtDescriptorRev5_1 *fadt = acpi_data_push(table_data, sizeof(*fadt)); unsigned dsdt_entry_offset = (char *)&fadt->dsdt - table_data->data; +uint16_t bootflags; + +switch (vms->psci_conduit) { +case QEMU_PSCI_CONDUIT_DISABLED: +bootflags = 0; +break; +case QEMU_PSCI_CONDUIT_HVC: +bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT | ACPI_FADT_ARM_PSCI_USE_HVC; +break; +case QEMU_PSCI_CONDUIT_SMC: +bootflags = ACPI_FADT_ARM_PSCI_COMPLIANT; +break; +default: +g_assert_not_reached(); +} -/* Hardware Reduced = 1 and use PSCI 0.2+ and with HVC */ +/* Hardware Reduced = 1 and use PSCI 0.2+ */ fadt->flags = cpu_to_le32(1 << ACPI_FADT_F_HW_REDUCED_ACPI); -fadt->arm_boot_flags = cpu_to_le16(ACPI_FADT_ARM_PSCI_COMPLIANT | - ACPI_FADT_ARM_PSCI_USE_HVC); +fadt->arm_boot_flags = cpu_to_le16(bootflags); /* ACPI v5.1 (fadt->revision.fadt->minor_revision) */ fadt->minor_revision = 0x1; @@ -745,7 +759,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables) /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */ acpi_add_table(table_offsets, tables_blob); -build_fadt(tables_blob, tables->linker, dsdt); +build_fadt(tables_blob, tables->linker, vms, dsdt); acpi_add_table(table_offsets, tables_blob); build_madt(tables_blob, tables->linker, vms); -- 2.7.4
[Qemu-devel] [PULL 01/36] arm: Uniquely name imx25 I2C buses.
From: Alastair D'Silva The imx25 chip provides 3 i2c buses, but they have all been named "i2c", which makes it difficult to predict which bus a device will be connected to when specified on the command line. This patch addresses the issue by naming the buses uniquely: i2c-bus.0 i2c-bus.1 i2c-bus.2 Signed-off-by: Alastair D'Silva Message-id: 20170105043430.3176-2-alast...@au1.ibm.com Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- hw/arm/imx25_pdk.c | 2 +- hw/i2c/imx_i2c.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c index 025b608..44e741f 100644 --- a/hw/arm/imx25_pdk.c +++ b/hw/arm/imx25_pdk.c @@ -139,7 +139,7 @@ static void imx25_pdk_init(MachineState *machine) * of simple qtest. See "make check" for details. */ i2c_create_slave((I2CBus *)qdev_get_child_bus(DEVICE(&s->soc.i2c[0]), - "i2c"), + "i2c-bus.0"), "ds1338", 0x68); } } diff --git a/hw/i2c/imx_i2c.c b/hw/i2c/imx_i2c.c index 37e5a62..6c81b98 100644 --- a/hw/i2c/imx_i2c.c +++ b/hw/i2c/imx_i2c.c @@ -310,7 +310,7 @@ static void imx_i2c_realize(DeviceState *dev, Error **errp) IMX_I2C_MEM_SIZE); sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem); sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq); -s->bus = i2c_init_bus(DEVICE(dev), "i2c"); +s->bus = i2c_init_bus(DEVICE(dev), NULL); } static void imx_i2c_class_init(ObjectClass *klass, void *data) -- 2.7.4
[Qemu-devel] [PULL 36/36] hw/arm/virt: Add board property to enable EL2
Add a board level property to the virt board which will enable EL2 on the CPU if the user asks for it. The default is not to provide EL2. If EL2 is enabled then we will use SMC as our PSCI conduit, and report the virtualization support in the GICv3 device tree node and the ACPI tables. Signed-off-by: Peter Maydell Reviewed-by: Andrew Jones Reviewed-by: Edgar E. Iglesias Message-id: 1483977924-14522-19-git-send-email-peter.mayd...@linaro.org --- include/hw/arm/virt.h| 1 + hw/arm/virt-acpi-build.c | 3 +++ hw/arm/virt.c| 44 ++-- 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index 53507e6..58ce74e 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -93,6 +93,7 @@ typedef struct { FWCfgState *fw_cfg; bool secure; bool highmem; +bool virt; int32_t gic_version; struct arm_boot_info bootinfo; const MemMapEntry *memmap; diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 73aca9e..d0a8a0f 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -614,6 +614,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) { gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ)); } +if (vms->virt && vms->gic_version == 3) { +gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GICV3_MAINT_IRQ)); +} } if (vms->gic_version == 3) { diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 769afa0..6c9e898 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -443,6 +443,11 @@ static void fdt_add_gic_node(VirtMachineState *vms) 2, vms->memmap[VIRT_GIC_DIST].size, 2, vms->memmap[VIRT_GIC_REDIST].base, 2, vms->memmap[VIRT_GIC_REDIST].size); +if (vms->virt) { +qemu_fdt_setprop_cells(vms->fdt, "/intc", "interrupts", + GIC_FDT_IRQ_TYPE_PPI, ARCH_GICV3_MAINT_IRQ, + GIC_FDT_IRQ_FLAGS_LEVEL_HI); +} } else { /* 'cortex-a15-gic' means 'GIC v2' */ qemu_fdt_setprop_string(vms->fdt, "/intc", "compatible", @@ -1239,10 +1244,15 @@ static void machvirt_init(MachineState *machine) * so it doesn't get in the way. Instead of starting secondary * CPUs in PSCI powerdown state we will start them all running and * let the boot ROM sort them out. - * The usual case is that we do use QEMU's PSCI implementation. + * The usual case is that we do use QEMU's PSCI implementation; + * if the guest has EL2 then we will use SMC as the conduit, + * and otherwise we will use HVC (for backwards compatibility and + * because if we're using KVM then we must use HVC). */ if (vms->secure && firmware_loaded) { vms->psci_conduit = QEMU_PSCI_CONDUIT_DISABLED; +} else if (vms->virt) { +vms->psci_conduit = QEMU_PSCI_CONDUIT_SMC; } else { vms->psci_conduit = QEMU_PSCI_CONDUIT_HVC; } @@ -1272,6 +1282,12 @@ static void machvirt_init(MachineState *machine) exit(1); } +if (vms->virt && kvm_enabled()) { +error_report("mach-virt: KVM does not support providing " + "Virtualization extensions to the guest CPU"); +exit(1); +} + if (vms->secure) { if (kvm_enabled()) { error_report("mach-virt: KVM does not support Security extensions"); @@ -1328,7 +1344,7 @@ static void machvirt_init(MachineState *machine) object_property_set_bool(cpuobj, false, "has_el3", NULL); } -if (object_property_find(cpuobj, "has_el2", NULL)) { +if (!vms->virt && object_property_find(cpuobj, "has_el2", NULL)) { object_property_set_bool(cpuobj, false, "has_el2", NULL); } @@ -1434,6 +1450,20 @@ static void virt_set_secure(Object *obj, bool value, Error **errp) vms->secure = value; } +static bool virt_get_virt(Object *obj, Error **errp) +{ +VirtMachineState *vms = VIRT_MACHINE(obj); + +return vms->virt; +} + +static void virt_set_virt(Object *obj, bool value, Error **errp) +{ +VirtMachineState *vms = VIRT_MACHINE(obj); + +vms->virt = value; +} + static bool virt_get_highmem(Object *obj, Error **errp) { VirtMachineState *vms = VIRT_MACHINE(obj); @@ -1521,6 +1551,16 @@ static void virt_2_9_instance_init(Object *obj) "Security Extensions (TrustZone)", NULL); +/* EL2 is also disabled by default, for similar reasons */ +vms->virt = false; +object_property_add_bool(obj, "virtualization", virt_get_virt, + virt_set_virt, NULL); +object_property_set_description(obj, "virtualiz
[Qemu-devel] [PULL 29/36] hw/intc/arm_gicv3: Implement ICV_ registers EOIR and IAR
Implement the two remaining ICV_ registers: EOIR and IAR. Signed-off-by: Peter Maydell Message-id: 1483977924-14522-12-git-send-email-peter.mayd...@linaro.org --- hw/intc/arm_gicv3_cpuif.c | 220 ++ hw/intc/trace-events | 2 + 2 files changed, 222 insertions(+) diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index e069082..f3845a6 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -203,6 +203,73 @@ static int hppvi_index(GICv3CPUState *cs) return idx; } +static uint32_t icv_gprio_mask(GICv3CPUState *cs, int group) +{ +/* Return a mask word which clears the subpriority bits from + * a priority value for a virtual interrupt in the specified group. + * This depends on the VBPR value: + * a BPR of 0 means the group priority bits are [7:1]; + * a BPR of 1 means they are [7:2], and so on down to + * a BPR of 7 meaning no group priority bits at all. + * Which BPR to use depends on the group of the interrupt and + * the current ICH_VMCR_EL2.VCBPR settings. + */ +if (group == GICV3_G1NS && cs->ich_vmcr_el2 & ICH_VMCR_EL2_VCBPR) { +group = GICV3_G0; +} + +return ~0U << (read_vbpr(cs, group) + 1); +} + +static bool icv_hppi_can_preempt(GICv3CPUState *cs, uint64_t lr) +{ +/* Return true if we can signal this virtual interrupt defined by + * the given list register value; see the pseudocode functions + * CanSignalVirtualInterrupt and CanSignalVirtualInt. + * Compare also icc_hppi_can_preempt() which is the non-virtual + * equivalent of these checks. + */ +int grp; +uint32_t mask, prio, rprio, vpmr; + +if (!(cs->ich_hcr_el2 & ICH_HCR_EL2_EN)) { +/* Virtual interface disabled */ +return false; +} + +/* We don't need to check that this LR is in Pending state because + * that has already been done in hppvi_index(). + */ + +prio = ich_lr_prio(lr); +vpmr = extract64(cs->ich_vmcr_el2, ICH_VMCR_EL2_VPMR_SHIFT, + ICH_VMCR_EL2_VPMR_LENGTH); + +if (prio >= vpmr) { +/* Priority mask masks this interrupt */ +return false; +} + +rprio = ich_highest_active_virt_prio(cs); +if (rprio == 0xff) { +/* No running interrupt so we can preempt */ +return true; +} + +grp = (lr & ICH_LR_EL2_GROUP) ? GICV3_G1NS : GICV3_G0; + +mask = icv_gprio_mask(cs, grp); + +/* We only preempt a running interrupt if the pending interrupt's + * group priority is sufficient (the subpriorities are not considered). + */ +if ((prio & mask) < (rprio & mask)) { +return true; +} + +return false; +} + static uint32_t eoi_maintenance_interrupt_state(GICv3CPUState *cs, uint32_t *misr) { @@ -480,6 +547,53 @@ static uint64_t icv_hppir_read(CPUARMState *env, const ARMCPRegInfo *ri) return value; } +static void icv_activate_irq(GICv3CPUState *cs, int idx, int grp) +{ +/* Activate the interrupt in the specified list register + * by moving it from Pending to Active state, and update the + * Active Priority Registers. + */ +uint32_t mask = icv_gprio_mask(cs, grp); +int prio = ich_lr_prio(cs->ich_lr_el2[idx]) & mask; +int aprbit = prio >> (8 - cs->vprebits); +int regno = aprbit / 32; +int regbit = aprbit % 32; + +cs->ich_lr_el2[idx] &= ~ICH_LR_EL2_STATE_PENDING_BIT; +cs->ich_lr_el2[idx] |= ICH_LR_EL2_STATE_ACTIVE_BIT; +cs->ich_apr[grp][regno] |= (1 << regbit); +} + +static uint64_t icv_iar_read(CPUARMState *env, const ARMCPRegInfo *ri) +{ +GICv3CPUState *cs = icc_cs_from_env(env); +int grp = ri->crm == 8 ? GICV3_G0 : GICV3_G1NS; +int idx = hppvi_index(cs); +uint64_t intid = INTID_SPURIOUS; + +if (idx >= 0) { +uint64_t lr = cs->ich_lr_el2[idx]; +int thisgrp = (lr & ICH_LR_EL2_GROUP) ? GICV3_G1NS : GICV3_G0; + +if (thisgrp == grp && icv_hppi_can_preempt(cs, lr)) { +intid = ich_lr_vintid(lr); +if (intid < INTID_SECURE) { +icv_activate_irq(cs, idx, grp); +} else { +/* Interrupt goes from Pending to Invalid */ +cs->ich_lr_el2[idx] &= ~ICH_LR_EL2_STATE_PENDING_BIT; +/* We will now return the (bogus) ID from the list register, + * as per the pseudocode. + */ +} +} +} + +trace_gicv3_icv_iar_read(ri->crm == 8 ? 0 : 1, + gicv3_redist_affid(cs), intid); +return intid; +} + static int icc_highest_active_prio(GICv3CPUState *cs) { /* Calculate the current running priority based on the set bits @@ -773,6 +887,10 @@ static uint64_t icc_iar0_read(CPUARMState *env, const ARMCPRegInfo *ri) GICv3CPUState *cs = icc_cs_from_env(env); uint64_t intid; +if (icv_access(env, HCR_FMO)) { +ret
[Qemu-devel] [PULL 09/36] aspeed/smc: rework the prototype of the AspeedSMCFlash helper routines
From: Cédric Le Goater Change the routines prototype to use a 'AspeedSMCFlash *' instead of 'AspeedSMCState *'. The result will help in making future changes clearer. Also change aspeed_smc_update_cs() which uselessly loops on all slave devices to update their status. Signed-off-by: Cédric Le Goater Reviewed-by: Joel Stanley Reviewed-by: Andrew Jeffery Message-id: 1483979087-32663-4-git-send-email-...@kaod.org Signed-off-by: Peter Maydell --- hw/ssi/aspeed_smc.c | 39 ++- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 205c0ab..9b31d5d 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -328,19 +328,23 @@ static const MemoryRegionOps aspeed_smc_flash_default_ops = { }, }; -static inline int aspeed_smc_flash_mode(const AspeedSMCState *s, int cs) +static inline int aspeed_smc_flash_mode(const AspeedSMCFlash *fl) { -return s->regs[s->r_ctrl0 + cs] & CTRL_CMD_MODE_MASK; +const AspeedSMCState *s = fl->controller; + +return s->regs[s->r_ctrl0 + fl->id] & CTRL_CMD_MODE_MASK; } -static inline bool aspeed_smc_is_usermode(const AspeedSMCState *s, int cs) +static inline bool aspeed_smc_is_usermode(const AspeedSMCFlash *fl) { -return aspeed_smc_flash_mode(s, cs) == CTRL_USERMODE; +return aspeed_smc_flash_mode(fl) == CTRL_USERMODE; } -static inline bool aspeed_smc_is_writable(const AspeedSMCState *s, int cs) +static inline bool aspeed_smc_is_writable(const AspeedSMCFlash *fl) { -return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + cs)); +const AspeedSMCState *s = fl->controller; + +return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + fl->id)); } static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) @@ -350,7 +354,7 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size) uint64_t ret = 0; int i; -if (aspeed_smc_is_usermode(s, fl->id)) { +if (aspeed_smc_is_usermode(fl)) { for (i = 0; i < size; i++) { ret |= ssi_transfer(s->spi, 0x0) << (8 * i); } @@ -370,13 +374,13 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data, const AspeedSMCState *s = fl->controller; int i; -if (!aspeed_smc_is_writable(s, fl->id)) { +if (!aspeed_smc_is_writable(fl)) { qemu_log_mask(LOG_GUEST_ERROR, "%s: flash is not writable at 0x%" HWADDR_PRIx "\n", __func__, addr); return; } -if (!aspeed_smc_is_usermode(s, fl->id)) { +if (!aspeed_smc_is_usermode(fl)) { qemu_log_mask(LOG_UNIMP, "%s: usermode not implemented\n", __func__); return; @@ -397,18 +401,18 @@ static const MemoryRegionOps aspeed_smc_flash_ops = { }, }; -static bool aspeed_smc_is_ce_stop_active(const AspeedSMCState *s, int cs) +static inline bool aspeed_smc_is_ce_stop_active(const AspeedSMCFlash *fl) { -return s->regs[s->r_ctrl0 + cs] & CTRL_CE_STOP_ACTIVE; +const AspeedSMCState *s = fl->controller; + +return s->regs[s->r_ctrl0 + fl->id] & CTRL_CE_STOP_ACTIVE; } -static void aspeed_smc_update_cs(const AspeedSMCState *s) +static void aspeed_smc_flash_update_cs(AspeedSMCFlash *fl) { -int i; +const AspeedSMCState *s = fl->controller; -for (i = 0; i < s->num_cs; ++i) { -qemu_set_irq(s->cs_lines[i], aspeed_smc_is_ce_stop_active(s, i)); -} +qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl)); } static void aspeed_smc_reset(DeviceState *d) @@ -481,8 +485,9 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data, addr == s->r_ce_ctrl) { s->regs[addr] = value; } else if (addr >= s->r_ctrl0 && addr < s->r_ctrl0 + s->num_cs) { +int cs = addr - s->r_ctrl0; s->regs[addr] = value; -aspeed_smc_update_cs(s); +aspeed_smc_flash_update_cs(&s->flashes[cs]); } else if (addr >= R_SEG_ADDR0 && addr < R_SEG_ADDR0 + s->ctrl->max_slaves) { int cs = addr - R_SEG_ADDR0; -- 2.7.4
[Qemu-devel] [PULL 30/36] hw/intc/arm_gicv3: Implement gicv3_cpuif_virt_update()
Implement the function which signals virtual interrupts to the CPU as appropriate following CPU interface state changes. Signed-off-by: Peter Maydell Message-id: 1483977924-14522-13-git-send-email-peter.mayd...@linaro.org --- include/hw/intc/arm_gicv3_common.h | 1 + hw/intc/arm_gicv3_cpuif.c | 49 ++ hw/intc/trace-events | 2 ++ 3 files changed, 52 insertions(+) diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h index 665d3f8..4156051 100644 --- a/include/hw/intc/arm_gicv3_common.h +++ b/include/hw/intc/arm_gicv3_common.h @@ -150,6 +150,7 @@ struct GICv3CPUState { qemu_irq parent_fiq; qemu_irq parent_virq; qemu_irq parent_vfiq; +qemu_irq maintenance_irq; /* Redistributor */ uint32_t level; /* Current IRQ level */ diff --git a/hw/intc/arm_gicv3_cpuif.c b/hw/intc/arm_gicv3_cpuif.c index f3845a6..e05f6b3 100644 --- a/hw/intc/arm_gicv3_cpuif.c +++ b/hw/intc/arm_gicv3_cpuif.c @@ -352,6 +352,53 @@ static uint32_t maintenance_interrupt_state(GICv3CPUState *cs) static void gicv3_cpuif_virt_update(GICv3CPUState *cs) { +/* Tell the CPU about any pending virtual interrupts or + * maintenance interrupts, following a change to the state + * of the CPU interface relevant to virtual interrupts. + * + * CAUTION: this function will call qemu_set_irq() on the + * CPU maintenance IRQ line, which is typically wired up + * to the GIC as a per-CPU interrupt. This means that it + * will recursively call back into the GIC code via + * gicv3_redist_set_irq() and thus into the CPU interface code's + * gicv3_cpuif_update(). It is therefore important that this + * function is only called as the final action of a CPU interface + * register write implementation, after all the GIC state + * fields have been updated. gicv3_cpuif_update() also must + * not cause this function to be called, but that happens + * naturally as a result of there being no architectural + * linkage between the physical and virtual GIC logic. + */ +int idx; +int irqlevel = 0; +int fiqlevel = 0; +int maintlevel = 0; + +idx = hppvi_index(cs); +trace_gicv3_cpuif_virt_update(gicv3_redist_affid(cs), idx); +if (idx >= 0) { +uint64_t lr = cs->ich_lr_el2[idx]; + +if (icv_hppi_can_preempt(cs, lr)) { +/* Virtual interrupts are simple: G0 are always FIQ, and G1 IRQ */ +if (lr & ICH_LR_EL2_GROUP) { +irqlevel = 1; +} else { +fiqlevel = 1; +} +} +} + +if (cs->ich_hcr_el2 & ICH_HCR_EL2_EN) { +maintlevel = maintenance_interrupt_state(cs); +} + +trace_gicv3_cpuif_virt_set_irqs(gicv3_redist_affid(cs), fiqlevel, +irqlevel, maintlevel); + +qemu_set_irq(cs->parent_vfiq, fiqlevel); +qemu_set_irq(cs->parent_virq, irqlevel); +qemu_set_irq(cs->maintenance_irq, maintlevel); } static uint64_t icv_ap_read(CPUARMState *env, const ARMCPRegInfo *ri) @@ -2480,6 +2527,8 @@ void gicv3_init_cpuif(GICv3State *s) && cpu->gic_num_lrs) { int j; +cs->maintenance_irq = cpu->gicv3_maintenance_interrupt; + cs->num_list_regs = cpu->gic_num_lrs; cs->vpribits = cpu->gic_vpribits; cs->vprebits = cpu->gic_vprebits; diff --git a/hw/intc/trace-events b/hw/intc/trace-events index 1dcc830..6116df5 100644 --- a/hw/intc/trace-events +++ b/hw/intc/trace-events @@ -138,6 +138,8 @@ gicv3_icv_hppir_read(int grp, uint32_t cpu, uint64_t val) "GICv3 ICV_HPPIR%d rea gicv3_icv_dir_write(uint32_t cpu, uint64_t val) "GICv3 ICV_DIR write cpu %x value 0x%" PRIx64 gicv3_icv_iar_read(int grp, uint32_t cpu, uint64_t val) "GICv3 ICV_IAR%d read cpu %x value 0x%" PRIx64 gicv3_icv_eoir_write(int grp, uint32_t cpu, uint64_t val) "GICv3 ICV_EOIR%d write cpu %x value 0x%" PRIx64 +gicv3_cpuif_virt_update(uint32_t cpuid, int idx) "GICv3 CPU i/f %x virt HPPI update LR index %d" +gicv3_cpuif_virt_set_irqs(uint32_t cpuid, int fiqlevel, int irqlevel, int maintlevel) "GICv3 CPU i/f %x virt HPPI update: setting FIQ %d IRQ %d maintenance-irq %d" # hw/intc/arm_gicv3_dist.c gicv3_dist_read(uint64_t offset, uint64_t data, unsigned size, bool secure) "GICv3 distributor read: offset 0x%" PRIx64 " data 0x%" PRIx64 " size %u secure %d" -- 2.7.4
[Qemu-devel] [PULL 35/36] target-arm: Enable EL2 feature bit on A53 and A57
Enable the ARM_FEATURE_EL2 bit on Cortex-A52 and Cortex-A57, since this is all now sufficiently implemented to work with the GICv3. We provide the usual CPU property to disable it for backwards compatibility with the older virt boards. In this commit, we disable the EL2 feature on the virt and ZynpMP boards, so there is no overall effect. Another commit will expose a board-level property to allow the user to enable EL2. Signed-off-by: Peter Maydell Reviewed-by: Andrew Jones Reviewed-by: Edgar E. Iglesias Reviewed-by: Alistair Francis Message-id: 1483977924-14522-18-git-send-email-peter.mayd...@linaro.org --- target/arm/cpu.h | 2 ++ hw/arm/virt.c| 4 hw/arm/xlnx-zynqmp.c | 2 ++ target/arm/cpu.c | 12 target/arm/cpu64.c | 2 ++ 5 files changed, 22 insertions(+) diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 16c7c10..151a5d7 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -577,6 +577,8 @@ struct ARMCPU { bool start_powered_off; /* CPU currently in PSCI powered-off state */ bool powered_off; +/* CPU has virtualization extension */ +bool has_el2; /* CPU has security extension */ bool has_el3; /* CPU has PMU (Performance Monitor Unit) */ diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 3a6f895..769afa0 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1328,6 +1328,10 @@ static void machvirt_init(MachineState *machine) object_property_set_bool(cpuobj, false, "has_el3", NULL); } +if (object_property_find(cpuobj, "has_el2", NULL)) { +object_property_set_bool(cpuobj, false, "has_el2", NULL); +} + if (vms->psci_conduit != QEMU_PSCI_CONDUIT_DISABLED) { object_property_set_int(cpuobj, vms->psci_conduit, "psci-conduit", NULL); diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c index 0d86ba3..bc4e66b 100644 --- a/hw/arm/xlnx-zynqmp.c +++ b/hw/arm/xlnx-zynqmp.c @@ -258,6 +258,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp) object_property_set_bool(OBJECT(&s->apu_cpu[i]), s->secure, "has_el3", NULL); +object_property_set_bool(OBJECT(&s->apu_cpu[i]), + false, "has_el2", NULL); object_property_set_int(OBJECT(&s->apu_cpu[i]), GIC_BASE_ADDR, "reset-cbar", &error_abort); object_property_set_bool(OBJECT(&s->apu_cpu[i]), true, "realized", diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 93ebbc9..3f2cdb6 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -496,6 +496,9 @@ static Property arm_cpu_reset_hivecs_property = static Property arm_cpu_rvbar_property = DEFINE_PROP_UINT64("rvbar", ARMCPU, rvbar, 0); +static Property arm_cpu_has_el2_property = +DEFINE_PROP_BOOL("has_el2", ARMCPU, has_el2, true); + static Property arm_cpu_has_el3_property = DEFINE_PROP_BOOL("has_el3", ARMCPU, has_el3, true); @@ -546,6 +549,11 @@ static void arm_cpu_post_init(Object *obj) #endif } +if (arm_feature(&cpu->env, ARM_FEATURE_EL2)) { +qdev_property_add_static(DEVICE(obj), &arm_cpu_has_el2_property, + &error_abort); +} + if (arm_feature(&cpu->env, ARM_FEATURE_PMU)) { qdev_property_add_static(DEVICE(obj), &arm_cpu_has_pmu_property, &error_abort); @@ -694,6 +702,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) cpu->id_aa64pfr0 &= ~0xf000; } +if (!cpu->has_el2) { +unset_feature(env, ARM_FEATURE_EL2); +} + if (!cpu->has_pmu || !kvm_enabled()) { cpu->has_pmu = false; unset_feature(env, ARM_FEATURE_PMU); diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 73c7f31..670c07a 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -110,6 +110,7 @@ static void aarch64_a57_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_V8_SHA256); set_feature(&cpu->env, ARM_FEATURE_V8_PMULL); set_feature(&cpu->env, ARM_FEATURE_CRC); +set_feature(&cpu->env, ARM_FEATURE_EL2); set_feature(&cpu->env, ARM_FEATURE_EL3); set_feature(&cpu->env, ARM_FEATURE_PMU); cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57; @@ -169,6 +170,7 @@ static void aarch64_a53_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_V8_SHA256); set_feature(&cpu->env, ARM_FEATURE_V8_PMULL); set_feature(&cpu->env, ARM_FEATURE_CRC); +set_feature(&cpu->env, ARM_FEATURE_EL2); set_feature(&cpu->env, ARM_FEATURE_EL3); set_feature(&cpu->env, ARM_FEATURE_PMU); cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A53; -- 2.7.4
[Qemu-devel] [PULL 14/36] aspeed/smc: reset flash after each test
From: Cédric Le Goater Let's make sure when each test is run that the flash object is in an initial state and did not keep configuration from the previous tests. Signed-off-by: Cédric Le Goater Message-id: 1483979087-32663-9-git-send-email-...@kaod.org Signed-off-by: Peter Maydell --- tests/m25p80-test.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/tests/m25p80-test.c b/tests/m25p80-test.c index cb7ec81..8dd550d 100644 --- a/tests/m25p80-test.c +++ b/tests/m25p80-test.c @@ -50,6 +50,8 @@ enum { READ = 0x03, PP = 0x02, WREN = 0x6, +RESET_ENABLE = 0x66, +RESET_MEMORY = 0x99, EN_4BYTE_ADDR = 0xB7, ERASE_SECTOR = 0xd8, }; @@ -76,6 +78,14 @@ static void spi_conf(uint32_t value) writel(ASPEED_FMC_BASE + R_CONF, conf); } +static void spi_conf_remove(uint32_t value) +{ +uint32_t conf = readl(ASPEED_FMC_BASE + R_CONF); + +conf &= ~value; +writel(ASPEED_FMC_BASE + R_CONF, conf); +} + static void spi_ctrl_start_user(void) { uint32_t ctrl = readl(ASPEED_FMC_BASE + R_CTRL0); @@ -95,6 +105,18 @@ static void spi_ctrl_stop_user(void) writel(ASPEED_FMC_BASE + R_CTRL0, ctrl); } +static void flash_reset(void) +{ +spi_conf(CONF_ENABLE_W0); + +spi_ctrl_start_user(); +writeb(ASPEED_FLASH_BASE, RESET_ENABLE); +writeb(ASPEED_FLASH_BASE, RESET_MEMORY); +spi_ctrl_stop_user(); + +spi_conf_remove(CONF_ENABLE_W0); +} + static void test_read_jedec(void) { uint32_t jedec = 0x0; @@ -108,6 +130,8 @@ static void test_read_jedec(void) jedec |= readb(ASPEED_FLASH_BASE); spi_ctrl_stop_user(); +flash_reset(); + g_assert_cmphex(jedec, ==, FLASH_JEDEC); } @@ -155,6 +179,8 @@ static void test_erase_sector(void) for (i = 0; i < PAGE_SIZE / 4; i++) { g_assert_cmphex(page[i], ==, 0x); } + +flash_reset(); } static void test_erase_all(void) @@ -182,6 +208,8 @@ static void test_erase_all(void) for (i = 0; i < PAGE_SIZE / 4; i++) { g_assert_cmphex(page[i], ==, 0x); } + +flash_reset(); } static void test_write_page(void) @@ -195,6 +223,7 @@ static void test_write_page(void) spi_ctrl_start_user(); writeb(ASPEED_FLASH_BASE, EN_4BYTE_ADDR); +writeb(ASPEED_FLASH_BASE, WREN); writeb(ASPEED_FLASH_BASE, PP); writel(ASPEED_FLASH_BASE, make_be32(my_page_addr)); @@ -215,6 +244,8 @@ static void test_write_page(void) for (i = 0; i < PAGE_SIZE / 4; i++) { g_assert_cmphex(page[i], ==, 0x); } + +flash_reset(); } static char tmp_path[] = "/tmp/qtest.m25p80.XX"; -- 2.7.4
[Qemu-devel] [PULL 12/36] aspeed/smc: adjust the size of the register region
From: Cédric Le Goater The SPI controller of the AST2400 SoC has less registers. So we can adjust the size of the memory region holding the registers depending on the controller type. We can also remove the guest_error logging which is useless as the range of the region is strict enough. Signed-off-by: Cédric Le Goater Reviewed-by: Joel Stanley Message-id: 1483979087-32663-7-git-send-email-...@kaod.org Signed-off-by: Peter Maydell --- include/hw/ssi/aspeed_smc.h | 1 + hw/ssi/aspeed_smc.c | 25 ++--- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h index 861120b..e811742 100644 --- a/include/hw/ssi/aspeed_smc.h +++ b/include/hw/ssi/aspeed_smc.h @@ -45,6 +45,7 @@ typedef struct AspeedSMCController { hwaddr flash_window_base; uint32_t flash_window_size; bool has_dma; +uint32_t nregs; } AspeedSMCController; typedef struct AspeedSMCFlash { diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index d8287ab..8d8a62e 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -130,6 +130,9 @@ #define R_SPI_MISC_CTRL (0x10 / 4) #define R_SPI_TIMINGS (0x14 / 4) +#define ASPEED_SMC_R_SPI_MAX (0x20 / 4) +#define ASPEED_SMC_R_SMC_MAX (0x20 / 4) + #define ASPEED_SOC_SMC_FLASH_BASE 0x1000 #define ASPEED_SOC_FMC_FLASH_BASE 0x2000 #define ASPEED_SOC_SPI_FLASH_BASE 0x3000 @@ -185,6 +188,7 @@ static const AspeedSMCController controllers[] = { .flash_window_base = ASPEED_SOC_SMC_FLASH_BASE, .flash_window_size = 0x600, .has_dma = false, +.nregs = ASPEED_SMC_R_SMC_MAX, }, { .name = "aspeed.smc.fmc", .r_conf= R_CONF, @@ -197,6 +201,7 @@ static const AspeedSMCController controllers[] = { .flash_window_base = ASPEED_SOC_FMC_FLASH_BASE, .flash_window_size = 0x1000, .has_dma = true, +.nregs = ASPEED_SMC_R_MAX, }, { .name = "aspeed.smc.spi", .r_conf= R_SPI_CONF, @@ -209,6 +214,7 @@ static const AspeedSMCController controllers[] = { .flash_window_base = ASPEED_SOC_SPI_FLASH_BASE, .flash_window_size = 0x1000, .has_dma = false, +.nregs = ASPEED_SMC_R_SPI_MAX, }, { .name = "aspeed.smc.ast2500-fmc", .r_conf= R_CONF, @@ -221,6 +227,7 @@ static const AspeedSMCController controllers[] = { .flash_window_base = ASPEED_SOC_FMC_FLASH_BASE, .flash_window_size = 0x1000, .has_dma = true, +.nregs = ASPEED_SMC_R_MAX, }, { .name = "aspeed.smc.ast2500-spi1", .r_conf= R_CONF, @@ -233,6 +240,7 @@ static const AspeedSMCController controllers[] = { .flash_window_base = ASPEED_SOC_SPI_FLASH_BASE, .flash_window_size = 0x800, .has_dma = false, +.nregs = ASPEED_SMC_R_MAX, }, { .name = "aspeed.smc.ast2500-spi2", .r_conf= R_CONF, @@ -245,6 +253,7 @@ static const AspeedSMCController controllers[] = { .flash_window_base = ASPEED_SOC_SPI2_FLASH_BASE, .flash_window_size = 0x800, .has_dma = false, +.nregs = ASPEED_SMC_R_MAX, }, }; @@ -521,13 +530,6 @@ static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size) addr >>= 2; -if (addr >= ARRAY_SIZE(s->regs)) { -qemu_log_mask(LOG_GUEST_ERROR, - "%s: Out-of-bounds read at 0x%" HWADDR_PRIx "\n", - __func__, addr); -return 0; -} - if (addr == s->r_conf || addr == s->r_timings || addr == s->r_ce_ctrl || @@ -550,13 +552,6 @@ static void aspeed_smc_write(void *opaque, hwaddr addr, uint64_t data, addr >>= 2; -if (addr >= ARRAY_SIZE(s->regs)) { -qemu_log_mask(LOG_GUEST_ERROR, - "%s: Out-of-bounds write at 0x%" HWADDR_PRIx "\n", - __func__, addr); -return; -} - if (addr == s->r_conf || addr == s->r_timings || addr == s->r_ce_ctrl) { @@ -624,7 +619,7 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp) /* The memory region for the controller registers */ memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s, - s->ctrl->name, ASPEED_SMC_R_MAX * 4); + s->ctrl->name, s->ctrl->nregs * 4); sysbus_init_mmio(sbd, &s->mmio); /* -- 2.7.4
[Qemu-devel] [PULL 06/36] target/arm: Implement DBGVCR32_EL2 system register
The DBGVCR_EL2 system register is needed to run a 32-bit EL1 guest under a Linux EL2 64-bit hypervisor. Its only purpose is to provide AArch64 with access to the state of the DBGVCR AArch32 register. Since we only have a dummy DBGVCR, implement a corresponding dummy DBGVCR32_EL2. Signed-off-by: Peter Maydell Reviewed-by: Edgar E. Iglesias --- target/arm/helper.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/target/arm/helper.c b/target/arm/helper.c index ba72ebb..7111c8c 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -4066,6 +4066,13 @@ static const ARMCPRegInfo debug_cp_reginfo[] = { .cp = 14, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 0, .access = PL1_RW, .accessfn = access_tda, .type = ARM_CP_NOP }, +/* Dummy DBGVCR32_EL2 (which is only for a 64-bit hypervisor + * to save and restore a 32-bit guest's DBGVCR) + */ +{ .name = "DBGVCR32_EL2", .state = ARM_CP_STATE_AA64, + .opc0 = 2, .opc1 = 4, .crn = 0, .crm = 7, .opc2 = 0, + .access = PL2_RW, .accessfn = access_tda, + .type = ARM_CP_NOP }, /* Dummy MDCCINT_EL1, since we don't implement the Debug Communications * Channel but Linux may try to access this register. The 32-bit * alias is DBGDCCINT. -- 2.7.4
[Qemu-devel] [PATCH v3 1/2] linux-user: fix settime old value location
From: Marc-André Lureau old_value is the 4th argument of timer_settime(), not the 2nd. Signed-off-by: Marc-André Lureau Signed-off-by: Pranith Kumar --- linux-user/syscall.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 7b77503f94..a393764a17 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -12024,10 +12024,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, timer_t htimer = g_posix_timers[timerid]; struct itimerspec hspec_new = {{0},}, hspec_old = {{0},}; -target_to_host_itimerspec(&hspec_new, arg3); +if (arg3 && target_to_host_itimerspec(&hspec_new, arg3)) { +goto efault; +} ret = get_errno( timer_settime(htimer, arg2, &hspec_new, &hspec_old)); -host_to_target_itimerspec(arg2, &hspec_old); +if (arg4 && host_to_target_itimerspec(arg4, &hspec_old)) { +goto efault; +} } break; } -- 2.11.0
[Qemu-devel] [PULL 13/36] aspeed/smc: handle SPI flash Command mode
From: Cédric Le Goater The Aspeed SMC controllers have a mode (Command mode) in which accesses to the flash content are no different than doing MMIOs. The controller generates all the necessary commands to load (or store) data in memory. However, accesses are restricted to the segment window assigned the the flash module by the controller. This window is defined by the Segment Address Register. Signed-off-by: Cédric Le Goater Reviewed-by: Andrew Jeffery Message-id: 1483979087-32663-8-git-send-email-...@kaod.org Signed-off-by: Peter Maydell --- include/hw/ssi/aspeed_smc.h | 2 +- hw/ssi/aspeed_smc.c | 152 ++-- 2 files changed, 132 insertions(+), 22 deletions(-) diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h index e811742..1f55731 100644 --- a/include/hw/ssi/aspeed_smc.h +++ b/include/hw/ssi/aspeed_smc.h @@ -49,7 +49,7 @@ typedef struct AspeedSMCController { } AspeedSMCController; typedef struct AspeedSMCFlash { -const struct AspeedSMCState *controller; +struct AspeedSMCState *controller; uint8_t id; uint32_t size; diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 8d8a62e..a0a8164 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -69,6 +69,7 @@ #define R_CTRL0 (0x10 / 4) #define CTRL_CMD_SHIFT 16 #define CTRL_CMD_MASK0xff +#define CTRL_AST2400_SPI_4BYTE (1 << 13) #define CTRL_CE_STOP_ACTIVE (1 << 2) #define CTRL_CMD_MODE_MASK 0x3 #define CTRL_READMODE 0x0 @@ -138,6 +139,9 @@ #define ASPEED_SOC_SPI_FLASH_BASE 0x3000 #define ASPEED_SOC_SPI2_FLASH_BASE 0x3800 +/* Flash opcodes. */ +#define SPI_OP_READ 0x03/* Read data bytes (low frequency) */ + /* * Default segments mapping addresses and size for each slave per * controller. These can be changed when board is initialized with the @@ -414,21 +418,123 @@ static inline bool aspeed_smc_is_writable(const AspeedSMCFlash *fl) return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + fl->id)); } +static inline int aspeed_smc_flash_cmd(const AspeedSMCFlash *fl) +{ +const AspeedSMCState *s = fl->controller; +int cmd = (s->regs[s->r_ctrl0 + fl->id] >> CTRL_CMD_SHIFT) & CTRL_CMD_MASK; + +/* In read mode, the default SPI command is READ (0x3). In other + * modes, the command should necessarily be defined */ +if (aspeed_smc_flash_mode(fl) == CTRL_READMODE) { +cmd = SPI_OP_READ; +} + +if (!cmd) { +qemu_log_mask(LOG_GUEST_ERROR, "%s: no command defined for mode %d\n", + __func__, aspeed_smc_flash_mode(fl)); +} + +return cmd; +} + +static inline int aspeed_smc_flash_is_4byte(const AspeedSMCFlash *fl) +{ +const AspeedSMCState *s = fl->controller; + +if (s->ctrl->segments == aspeed_segments_spi) { +return s->regs[s->r_ctrl0] & CTRL_AST2400_SPI_4BYTE; +} else { +return s->regs[s->r_ce_ctrl] & (1 << (CTRL_EXTENDED0 + fl->id)); +} +} + +static inline bool aspeed_smc_is_ce_stop_active(const AspeedSMCFlash *fl) +{ +const AspeedSMCState *s = fl->controller; + +return s->regs[s->r_ctrl0 + fl->id] & CTRL_CE_STOP_ACTIVE; +} + +static void aspeed_smc_flash_select(AspeedSMCFlash *fl) +{ +AspeedSMCState *s = fl->controller; + +s->regs[s->r_ctrl0 + fl->id] &= ~CTRL_CE_STOP_ACTIVE; +qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl)); +} + +static void aspeed_smc_flash_unselect(AspeedSMCFlash *fl) +{ +AspeedSMCState *s = fl->controller; + +s->regs[s->r_ctrl0 + fl->id] |= CTRL_CE_STOP_ACTIVE; +qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl)); +} + +static uint32_t aspeed_smc_check_segment_addr(const AspeedSMCFlash *fl, + uint32_t addr) +{ +const AspeedSMCState *s = fl->controller; +AspeedSegments seg; + +aspeed_smc_reg_to_segment(s->regs[R_SEG_ADDR0 + fl->id], &seg); +if ((addr & (seg.size - 1)) != addr) { +qemu_log_mask(LOG_GUEST_ERROR, + "%s: invalid address 0x%08x for CS%d segment : " + "[ 0x%"HWADDR_PRIx" - 0x%"HWADDR_PRIx" ]\n", + s->ctrl->name, addr, fl->id, seg.addr, + seg.addr + seg.size); +} + +addr &= seg.size - 1; +return addr; +} + +static void aspeed_smc_flash_send_addr(AspeedSMCFlash *fl, uint32_t addr) +{ +const AspeedSMCState *s = fl->controller; +uint8_t cmd = aspeed_smc_flash_cmd(fl); + +/* Flash access can not exceed CS segment */ +addr = aspeed_smc_check_segment_addr(fl, addr); + +ssi_transfer(s->spi, cmd); + +if (aspeed_smc_flash_is_4byte(fl)) { +ssi_transfer(s->spi, (addr >> 24) & 0xff); +} +ssi_transfer(s->spi, (addr >> 16) & 0xff); +ssi_transfer(s->spi, (addr >> 8) & 0xff); +ssi_transfer(s->spi, (addr & 0xff)); +} + static uint64_t aspeed_smc_flash_read(vo
[Qemu-devel] [PULL 02/36] block: m25p80: Add Quad Page Program 4byte
From: Marcin Krzeminski Some flash chips have additional page program opcode that takes only 4 byte address. This commit adds support for such command in Qemu. Signed-off-by: Marcin Krzeminski Reviewed-by: Edgar E. Iglesias Message-id: 20170108083854.5006-2-mar.krzemin...@gmail.com Signed-off-by: Peter Maydell --- hw/block/m25p80.c | 4 1 file changed, 4 insertions(+) diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index 4c5f8c3..4ad67ac 100644 --- a/hw/block/m25p80.c +++ b/hw/block/m25p80.c @@ -327,6 +327,7 @@ typedef enum { PP4_4 = 0x3e, DPP = 0xa2, QPP = 0x32, +QPP_4 = 0x34, ERASE_4K = 0x20, ERASE4_4K = 0x21, @@ -577,6 +578,7 @@ static inline int get_addr_length(Flash *s) switch (s->cmd_in_progress) { case PP4: case PP4_4: + case QPP_4: case READ4: case QIOR4: case ERASE4_4K: @@ -610,6 +612,7 @@ static void complete_collecting_data(Flash *s) switch (s->cmd_in_progress) { case DPP: case QPP: +case QPP_4: case PP: case PP4: case PP4_4: @@ -877,6 +880,7 @@ static void decode_new_cmd(Flash *s, uint32_t value) case READ4: case DPP: case QPP: +case QPP_4: case PP: case PP4: case PP4_4: -- 2.7.4
[Qemu-devel] [PATCH v3 2/2] linux-user: fix tcg/mmap test
From: Marc-André Lureau tests/tcg/mmap test fails with values other than default target page size. When creating a map beyond EOF, extra anonymous pages are added up to the target page boundary. Currently, this operation is performed only when qemu_real_host_page_size < TARGET_PAGE_SIZE, but it should be performed if the configured page size (qemu -p) is larger than qemu_real_host_page_size too. Signed-off-by: Marc-André Lureau [pranith: dropped checkpatch changes] Signed-off-by: Pranith Kumar Reviewed-by: Alex Bennée Reviewed-by: Laurent Vivier Signed-off-by: Pranith Kumar --- linux-user/mmap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 61685bf79e..76905cc9fd 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -429,9 +429,9 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot, may need to truncate file maps at EOF and add extra anonymous pages up to the targets page boundary. */ -if ((qemu_real_host_page_size < TARGET_PAGE_SIZE) -&& !(flags & MAP_ANONYMOUS)) { - struct stat sb; +if ((qemu_real_host_page_size < qemu_host_page_size) && +!(flags & MAP_ANONYMOUS)) { +struct stat sb; if (fstat (fd, &sb) == -1) goto fail; -- 2.11.0
[Qemu-devel] [PULL 07/36] aspeed/smc: remove call to reset in realize function
From: Cédric Le Goater This is useless as reset will be called later on. Signed-off-by: Cédric Le Goater Acked-by: Marcin Krzemiński Message-id: 1483979087-32663-2-git-send-email-...@kaod.org Signed-off-by: Peter Maydell --- hw/ssi/aspeed_smc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 78f5aed..8a7217d 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -541,8 +541,6 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp) sysbus_init_irq(sbd, &s->cs_lines[i]); } -aspeed_smc_reset(dev); - /* The memory region for the controller registers */ memory_region_init_io(&s->mmio, OBJECT(s), &aspeed_smc_ops, s, s->ctrl->name, ASPEED_SMC_R_MAX * 4); -- 2.7.4
[Qemu-devel] [PULL 10/36] aspeed/smc: autostrap CE0/1 configuration
From: Cédric Le Goater On the AST2500 SoC, the FMC controller flash type is fixed to SPI for CE0 and CE1 and 4BYTE mode is autodetected for CE0. On the AST2400 SoC, the FMC controller flash type and 4BYTE mode are strapped with register SCU70. We use the default settings from the palmetto-bmc machine for now. Signed-off-by: Cédric Le Goater Reviewed-by: Joel Stanley Reviewed-by: Andrew Jeffery Message-id: 1483979087-32663-5-git-send-email-...@kaod.org Signed-off-by: Peter Maydell --- hw/ssi/aspeed_smc.c | 32 +++- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 9b31d5d..3bd381b 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -39,11 +39,14 @@ #define CONF_ENABLE_W2 18 #define CONF_ENABLE_W1 17 #define CONF_ENABLE_W0 16 -#define CONF_FLASH_TYPE4 9 -#define CONF_FLASH_TYPE3 7 -#define CONF_FLASH_TYPE2 5 -#define CONF_FLASH_TYPE1 3 -#define CONF_FLASH_TYPE0 1 +#define CONF_FLASH_TYPE4 8 +#define CONF_FLASH_TYPE3 6 +#define CONF_FLASH_TYPE2 4 +#define CONF_FLASH_TYPE1 2 +#define CONF_FLASH_TYPE0 0 +#define CONF_FLASH_TYPE_NOR 0x0 +#define CONF_FLASH_TYPE_NAND 0x1 +#define CONF_FLASH_TYPE_SPI 0x2 /* CE Control Register */ #define R_CE_CTRL(0x04 / 4) @@ -436,6 +439,25 @@ static void aspeed_smc_reset(DeviceState *d) s->regs[R_SEG_ADDR0 + i] = aspeed_smc_segment_to_reg(&s->ctrl->segments[i]); } + +/* HW strapping for AST2500 FMC controllers */ +if (s->ctrl->segments == aspeed_segments_ast2500_fmc) { +/* flash type is fixed to SPI for CE0 and CE1 */ +s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE0); +s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE1); + +/* 4BYTE mode is autodetected for CE0. Let's force it to 1 for + * now */ +s->regs[s->r_ce_ctrl] |= (1 << (CTRL_EXTENDED0)); +} + +/* HW strapping for AST2400 FMC controllers (SCU70). Let's use the + * configuration of the palmetto-bmc machine */ +if (s->ctrl->segments == aspeed_segments_fmc) { +s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE0); + +s->regs[s->r_ce_ctrl] |= (1 << (CTRL_EXTENDED0)); +} } static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size) -- 2.7.4
[Qemu-devel] [PULL 11/36] aspeed/smc: unfold the AspeedSMCController array
From: Cédric Le Goater This is getting difficult to read. Also add a 'has_dma' field for each controller type. Signed-off-by: Cédric Le Goater Reviewed-by: Joel Stanley Reviewed-by: Andrew Jeffery Message-id: 1483979087-32663-6-git-send-email-...@kaod.org Signed-off-by: Peter Maydell --- include/hw/ssi/aspeed_smc.h | 1 + hw/ssi/aspeed_smc.c | 91 - 2 files changed, 74 insertions(+), 18 deletions(-) diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h index bdfbcc0..861120b 100644 --- a/include/hw/ssi/aspeed_smc.h +++ b/include/hw/ssi/aspeed_smc.h @@ -44,6 +44,7 @@ typedef struct AspeedSMCController { const AspeedSegments *segments; hwaddr flash_window_base; uint32_t flash_window_size; +bool has_dma; } AspeedSMCController; typedef struct AspeedSMCFlash { diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c index 3bd381b..d8287ab 100644 --- a/hw/ssi/aspeed_smc.c +++ b/hw/ssi/aspeed_smc.c @@ -173,24 +173,79 @@ static const AspeedSegments aspeed_segments_ast2500_spi2[] = { }; static const AspeedSMCController controllers[] = { -{ "aspeed.smc.smc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS, - CONF_ENABLE_W0, 5, aspeed_segments_legacy, - ASPEED_SOC_SMC_FLASH_BASE, 0x600 }, -{ "aspeed.smc.fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS, - CONF_ENABLE_W0, 5, aspeed_segments_fmc, - ASPEED_SOC_FMC_FLASH_BASE, 0x1000 }, -{ "aspeed.smc.spi", R_SPI_CONF, 0xff, R_SPI_CTRL0, R_SPI_TIMINGS, - SPI_CONF_ENABLE_W0, 1, aspeed_segments_spi, - ASPEED_SOC_SPI_FLASH_BASE, 0x1000 }, -{ "aspeed.smc.ast2500-fmc", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS, - CONF_ENABLE_W0, 3, aspeed_segments_ast2500_fmc, - ASPEED_SOC_FMC_FLASH_BASE, 0x1000 }, -{ "aspeed.smc.ast2500-spi1", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS, - CONF_ENABLE_W0, 2, aspeed_segments_ast2500_spi1, - ASPEED_SOC_SPI_FLASH_BASE, 0x800 }, -{ "aspeed.smc.ast2500-spi2", R_CONF, R_CE_CTRL, R_CTRL0, R_TIMINGS, - CONF_ENABLE_W0, 2, aspeed_segments_ast2500_spi2, - ASPEED_SOC_SPI2_FLASH_BASE, 0x800 }, +{ +.name = "aspeed.smc.smc", +.r_conf= R_CONF, +.r_ce_ctrl = R_CE_CTRL, +.r_ctrl0 = R_CTRL0, +.r_timings = R_TIMINGS, +.conf_enable_w0= CONF_ENABLE_W0, +.max_slaves= 5, +.segments = aspeed_segments_legacy, +.flash_window_base = ASPEED_SOC_SMC_FLASH_BASE, +.flash_window_size = 0x600, +.has_dma = false, +}, { +.name = "aspeed.smc.fmc", +.r_conf= R_CONF, +.r_ce_ctrl = R_CE_CTRL, +.r_ctrl0 = R_CTRL0, +.r_timings = R_TIMINGS, +.conf_enable_w0= CONF_ENABLE_W0, +.max_slaves= 5, +.segments = aspeed_segments_fmc, +.flash_window_base = ASPEED_SOC_FMC_FLASH_BASE, +.flash_window_size = 0x1000, +.has_dma = true, +}, { +.name = "aspeed.smc.spi", +.r_conf= R_SPI_CONF, +.r_ce_ctrl = 0xff, +.r_ctrl0 = R_SPI_CTRL0, +.r_timings = R_SPI_TIMINGS, +.conf_enable_w0= SPI_CONF_ENABLE_W0, +.max_slaves= 1, +.segments = aspeed_segments_spi, +.flash_window_base = ASPEED_SOC_SPI_FLASH_BASE, +.flash_window_size = 0x1000, +.has_dma = false, +}, { +.name = "aspeed.smc.ast2500-fmc", +.r_conf= R_CONF, +.r_ce_ctrl = R_CE_CTRL, +.r_ctrl0 = R_CTRL0, +.r_timings = R_TIMINGS, +.conf_enable_w0= CONF_ENABLE_W0, +.max_slaves= 3, +.segments = aspeed_segments_ast2500_fmc, +.flash_window_base = ASPEED_SOC_FMC_FLASH_BASE, +.flash_window_size = 0x1000, +.has_dma = true, +}, { +.name = "aspeed.smc.ast2500-spi1", +.r_conf= R_CONF, +.r_ce_ctrl = R_CE_CTRL, +.r_ctrl0 = R_CTRL0, +.r_timings = R_TIMINGS, +.conf_enable_w0= CONF_ENABLE_W0, +.max_slaves= 2, +.segments = aspeed_segments_ast2500_spi1, +.flash_window_base = ASPEED_SOC_SPI_FLASH_BASE, +.flash_window_size = 0x800, +.has_dma = false, +}, { +.name = "aspeed.smc.ast2500-spi2", +.r_conf= R_CONF, +.r_ce_ctrl = R_CE_CTRL, +.r_ctrl0 = R_CTRL0, +.r_timings = R_TIMINGS, +.conf_enable_w0= CONF_ENABLE_W0, +.max_slaves= 2, +.segments = aspeed_segments_ast2500_spi2, +.f
[Qemu-devel] [Bug 641118] Re: NetBSD guest only supports network without ACPI
OK, thanks for checking! ... so let's close this bug. ** Changed in: qemu Status: Incomplete => Won't Fix -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/641118 Title: NetBSD guest only supports network without ACPI Status in QEMU: Won't Fix Bug description: Git commit: abdfd9500e07fab7d6ffd4385fa30a065c329a39 Host: Linux 64bit Debian Guest: NetBSD5.0.2/i386 Networking works only when ACPI is disabled in the guest. Without it the network card (wm0) is not detected. Boot: qemu -hda netbsd5.0.2-i386 -boot c -enable-kvm Configure: --enable-linux-aio --enable-io-thread --enable-kvm To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/641118/+subscriptions
[Qemu-devel] [PULL 16/36] aspeed: use first FMC flash as a boot ROM
From: Cédric Le Goater Create a ROM region, using the default size of the mapping window for the CE0 FMC flash module, and fill it with the flash content. This is a little hacky but until we can boot from a MMIO region, it seems difficult to do anything else. Signed-off-by: Cédric Le Goater Reviewed-by: Joel Stanley Reviewed-by: Andrew Jeffery Message-id: 1483979087-32663-11-git-send-email-...@kaod.org Signed-off-by: Peter Maydell --- hw/arm/aspeed.c | 41 + 1 file changed, 41 insertions(+) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index 40c1383..a92c2f1 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -20,6 +20,8 @@ #include "qemu/log.h" #include "sysemu/block-backend.h" #include "sysemu/blockdev.h" +#include "hw/loader.h" +#include "qemu/error-report.h" static struct arm_boot_info aspeed_board_binfo = { .board_id = -1, /* device-tree-only board */ @@ -104,6 +106,28 @@ static const AspeedBoardConfig aspeed_boards[] = { }, }; +#define FIRMWARE_ADDR 0x0 + +static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size, + Error **errp) +{ +BlockBackend *blk = blk_by_legacy_dinfo(dinfo); +uint8_t *storage; + +if (rom_size > blk_getlength(blk)) { +rom_size = blk_getlength(blk); +} + +storage = g_new0(uint8_t, rom_size); +if (blk_pread(blk, 0, storage, rom_size) < 0) { +error_setg(errp, "failed to read the initial flash content"); +return; +} + +rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr); +g_free(storage); +} + static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype, Error **errp) { @@ -135,6 +159,7 @@ static void aspeed_board_init(MachineState *machine, { AspeedBoardState *bmc; AspeedSoCClass *sc; +DriveInfo *drive0 = drive_get(IF_MTD, 0, 0); bmc = g_new0(AspeedBoardState, 1); object_initialize(&bmc->soc, (sizeof(bmc->soc)), cfg->soc_name); @@ -168,6 +193,22 @@ static void aspeed_board_init(MachineState *machine, aspeed_board_init_flashes(&bmc->soc.fmc, cfg->fmc_model, &error_abort); aspeed_board_init_flashes(&bmc->soc.spi[0], cfg->spi_model, &error_abort); +/* Install first FMC flash content as a boot rom. */ +if (drive0) { +AspeedSMCFlash *fl = &bmc->soc.fmc.flashes[0]; +MemoryRegion *boot_rom = g_new(MemoryRegion, 1); + +/* + * create a ROM region using the default mapping window size of + * the flash module. + */ +memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom", + fl->size, &error_abort); +memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR, +boot_rom); +write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort); +} + aspeed_board_binfo.kernel_filename = machine->kernel_filename; aspeed_board_binfo.initrd_filename = machine->initrd_filename; aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline; -- 2.7.4
[Qemu-devel] [PATCH v10 01/16] osdep: Add qemu_lock_fd and qemu_unlock_fd
They are wrappers of POSIX fcntl "file private locking", with a convenient "try lock" wrapper implemented with F_OFD_GETLK. Signed-off-by: Fam Zheng --- include/qemu/osdep.h | 3 +++ util/osdep.c | 48 2 files changed, 51 insertions(+) diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 689f253..e864fe8 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -297,6 +297,9 @@ int qemu_close(int fd); #ifndef _WIN32 int qemu_dup(int fd); #endif +int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive); +int qemu_unlock_fd(int fd, int64_t start, int64_t len); +int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive); #if defined(__HAIKU__) && defined(__i386__) #define FMT_pid "%ld" diff --git a/util/osdep.c b/util/osdep.c index 06fb1cf..3de4a18 100644 --- a/util/osdep.c +++ b/util/osdep.c @@ -140,6 +140,54 @@ static int qemu_parse_fdset(const char *param) { return qemu_parse_fd(param); } + +static int qemu_lock_fcntl(int fd, int64_t start, int64_t len, int fl_type) +{ +#ifdef F_OFD_SETLK +int ret; +struct flock fl = { +.l_whence = SEEK_SET, +.l_start = start, +.l_len= len, +.l_type = fl_type, +}; +ret = fcntl(fd, F_OFD_SETLK, &fl); +return ret == -1 ? -errno : 0; +#else +return -ENOTSUP; +#endif +} + +int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive) +{ +return qemu_lock_fcntl(fd, start, len, exclusive ? F_WRLCK : F_RDLCK); +} + +int qemu_unlock_fd(int fd, int64_t start, int64_t len) +{ +return qemu_lock_fcntl(fd, start, len, F_UNLCK); +} + +int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive) +{ +#ifdef F_OFD_SETLK +int ret; +struct flock fl = { +.l_whence = SEEK_SET, +.l_start = start, +.l_len= len, +.l_type = exclusive ? F_WRLCK : F_RDLCK, +}; +ret = fcntl(fd, F_OFD_GETLK, &fl); +if (ret == -1) { +return -errno; +} else { +return fl.l_type == F_UNLCK ? 0 : -EAGAIN; +} +#else +return -ENOTSUP; +#endif +} #endif /* -- 2.9.3