Re: [Qemu-devel] QEMU without X11 support
On Thu, 2017-10-12 at 16:25 -0400, hanji unit wrote: > Hello, is it possible to run (or rebuild modifying build flags) QEMU > without support for X11 window system integration? ./configure --disable-gtk --disable-sdl --disable-opengl cheers, Gerd
Re: [Qemu-devel] [RFC v4 00/16] VIRTIO-IOMMU device
> From: Auger Eric [mailto:eric.au...@redhat.com] > Sent: Thursday, October 12, 2017 6:10 PM > > Hi Peter, > > On 12/10/2017 11:54, Peter Maydell wrote: > > On 11 October 2017 at 17:08, Auger Eric wrote: > >> Hi Peter, > >> > >> On 11/10/2017 16:56, Peter Maydell wrote: > >>> On 19 September 2017 at 08:46, Eric Auger > wrote: > This series implements the virtio-iommu device. > > This v4 is an upgrade to v0.4 spec [1] and applies on QEMU v2.10.0. > - probe request support although no reserved region is returned at > the moment > - unmap semantics less strict, as specified in v0.4 > - device registration, attach/detach revisited > - split into smaller patches to ease review > - propose a way to inform the IOMMU mr about the page_size_mask > of underlying HW IOMMU, if any > - remove warning associated with the translation of the MSI doorbell > > The device gets instantiated using the "-device virtio-iommu-device" > option. It currently works with ARM virt machine only, as the machine > must handle the dt binding between the virtio-mmio "iommu" node > and > the PCI host bridge node. > >>> > >>> Could this work on x86, or is it inherently arm-only? > >> > >> Yes this is the goal. At the moment the ACPI probing is not yet properly > >> specified but a Q35 prototype was developed in the Red Hat Virt team. > >> This will be presented at the KVM forum. > > > > Since I have very little familiarity with virtio or iommu code, > > I'd be much happier if this was reviewed as a generic virtio-iommu > > by the x86/virtio devs and then the arm specific parts done second... > > Understood. I was rather expecting you to review the smmuv3 emulation > code which you did, in a comprehensive manner ;-), and many thanks for > that. > > Note sure this is time yet to get this RFC reviewed as > - the v0.4 virtio-iommu driver it relies on was not officially submitted, > - the virtio-iommu specification review has not really been reviewed, > - the ACPI probing method has not been discussed yet. > > Jean-Philippe, please correct me if I am wrong. > > So to me, this is pure RFC at the moment. > > > > I'm also not clear on what we're expecting the recommended or normal > > way to do device passthrough is going to be -- this virtio-mmio, > > or presenting the guest with an SMMUv3 interface? Do we really > > need to implement both ? > > I think the KVM forum is the right place to sync as both approaches will > be presented and some pros/cons + performance figures will be given. > > As we talk about choosing, there is one alternative that was suggested > on the ML by Alex & Michael but never really get considered yet and > maybe should be: using intel iommu emulation code for ARM. I > aknowledge > this deserves a thorough impact study on kernel and FW side but I would > be happy to get your opinion about the QEMU side. Would you have a by- > principle rejection of this idea to instantiate such an Intel device in > mach virt or would it be something you would be ready to consider? > bear posting a link to Alex/Michael's comment? interesting to know the rationale... Thanks Kevin
Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 14/24] fixup! ppc: spapr: use cpu type name directly
On Thu, 12 Oct 2017 17:48:19 +0200 Igor Mammedov wrote: > follow up commit that registers host-spapr-cpu-core type unconditionally > "ppc: spapr: register 'host' core type along with the rest of core types" > > makes 'non' machine crash > ppc64-softmmu/qemu-system-ppc64 -M none -device host-spapr-cpu-core > ERROR:qom/object.c:217:object_type_get_instance_size: assertion failed: > (type != NULL) > Aborted > > before it qemu fails cleanly with > ppc64-softmmu/qemu-system-ppc64 -M none -device host-spapr-cpu-core > qemu-system-ppc64: -device host-spapr-cpu-core: 'host-spapr-cpu-core' is not > a valid device model name > > spapr_cpu_core_realize() already has explicit check for pseries machine, > so move access to host cpu type after it so 'none' machine would fail > cleanly as expected. > > Reported-by: Greg Kurz > Signed-off-by: Igor Mammedov > --- That's much simpler indeed than trying to register the host CPU type at QOM init... which turns out to be quite hairy compared to the gain :) > hw/ppc/spapr_cpu_core.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index b5bbb6a..7dbf9c3 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -151,7 +151,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, > Error **errp) > sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); > sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); > CPUCore *cc = CPU_CORE(OBJECT(dev)); > -size_t size = object_type_get_instance_size(scc->cpu_type); > +size_t size; > Error *local_err = NULL; > void *obj; > int i, j; > @@ -162,6 +162,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, > Error **errp) > return; > } > > +size = object_type_get_instance_size(scc->cpu_type); > sc->threads = g_malloc0(size * cc->nr_threads); > for (i = 0; i < cc->nr_threads; i++) { > char id[32];
Re: [Qemu-devel] [RFC v4 00/16] VIRTIO-IOMMU device
Hi Kevin, On 13/10/2017 09:01, Tian, Kevin wrote: >> From: Auger Eric [mailto:eric.au...@redhat.com] >> Sent: Thursday, October 12, 2017 6:10 PM >> >> Hi Peter, >> >> On 12/10/2017 11:54, Peter Maydell wrote: >>> On 11 October 2017 at 17:08, Auger Eric wrote: Hi Peter, On 11/10/2017 16:56, Peter Maydell wrote: > On 19 September 2017 at 08:46, Eric Auger >> wrote: >> This series implements the virtio-iommu device. >> >> This v4 is an upgrade to v0.4 spec [1] and applies on QEMU v2.10.0. >> - probe request support although no reserved region is returned at >> the moment >> - unmap semantics less strict, as specified in v0.4 >> - device registration, attach/detach revisited >> - split into smaller patches to ease review >> - propose a way to inform the IOMMU mr about the page_size_mask >> of underlying HW IOMMU, if any >> - remove warning associated with the translation of the MSI doorbell >> >> The device gets instantiated using the "-device virtio-iommu-device" >> option. It currently works with ARM virt machine only, as the machine >> must handle the dt binding between the virtio-mmio "iommu" node >> and >> the PCI host bridge node. > > Could this work on x86, or is it inherently arm-only? Yes this is the goal. At the moment the ACPI probing is not yet properly specified but a Q35 prototype was developed in the Red Hat Virt team. This will be presented at the KVM forum. >>> >>> Since I have very little familiarity with virtio or iommu code, >>> I'd be much happier if this was reviewed as a generic virtio-iommu >>> by the x86/virtio devs and then the arm specific parts done second... >> >> Understood. I was rather expecting you to review the smmuv3 emulation >> code which you did, in a comprehensive manner ;-), and many thanks for >> that. >> >> Note sure this is time yet to get this RFC reviewed as >> - the v0.4 virtio-iommu driver it relies on was not officially submitted, >> - the virtio-iommu specification review has not really been reviewed, >> - the ACPI probing method has not been discussed yet. >> >> Jean-Philippe, please correct me if I am wrong. >> >> So to me, this is pure RFC at the moment. >>> >>> I'm also not clear on what we're expecting the recommended or normal >>> way to do device passthrough is going to be -- this virtio-mmio, >>> or presenting the guest with an SMMUv3 interface? Do we really >>> need to implement both ? >> >> I think the KVM forum is the right place to sync as both approaches will >> be presented and some pros/cons + performance figures will be given. >> >> As we talk about choosing, there is one alternative that was suggested >> on the ML by Alex & Michael but never really get considered yet and >> maybe should be: using intel iommu emulation code for ARM. I >> aknowledge >> this deserves a thorough impact study on kernel and FW side but I would >> be happy to get your opinion about the QEMU side. Would you have a by- >> principle rejection of this idea to instantiate such an Intel device in >> mach virt or would it be something you would be ready to consider? >> > > bear posting a link to Alex/Michael's comment? interesting to know > the rationale... This was suggested here for instance: https://lkml.org/lkml/2017/7/12/579 I think rationale was - this is an emulated platform so there is more freedom - vtd emulation code is rather stable - it has pieces missing on smmu: cachine mode, IOTLB invalidation command with addr_mask, - intel iommu driver implements deferred IOTLB Invalidation which boosts perf But problems I foresee are: - MSI handling. ARM MSI doorbells can be anywhere in the GPA address space whereas Intel has MSIs within the APIC window: [FEE0_h – FEF0_000h]. So I suspect the MSI handling may not work at kernel level. - intel IOMMU input/output address ranges and page sizes may be different from ARM ones. - ARM uses ACPI IORT for binding RC <-> IOMMU <-> MSI controller whereas Intel uses other tables - Intel's dmar kernel code must be compiled/enabled on ARM So personally I don't think this solution is viable but I prefer this gets discussed on the ML. Thanks Eric > > Thanks > Kevin >
Re: [Qemu-devel] [PATCH v4 9/9] disas: Add capstone as submodule
Richard Henderson writes: > On 10/02/2017 10:43 AM, Alex Bennée wrote: >>> +mkdir -p capstone >> >> Having a mkdir -p here seems a little out of place. Shouldn't the build >> directory creation be handled by the make recipe? > > *shrug* We do this for the other directories at configure time. fair enough. Reviewed-by: Alex Bennée > > > r~ -- Alex Bennée
Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest
On 10/12/17 17:45 +0200, Paolo Bonzini wrote: > On 12/10/2017 14:45, Haozhong Zhang wrote: > > Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and > > /rom@etc/table-loader. The former is unstructured to guest, and > > contains all data of guest ACPI. The latter is a BIOSLinkerLoader > > organized as a set of commands, which direct the guest (e.g., SeaBIOS > > on KVM/QEMU) to relocate data in the former file, recalculate checksum > > of specified area, and fill guest address in specified ACPI field. > > > > One part of my patches is to implement a mechanism to tell Xen which > > part of ACPI data is a table (NFIT), and which part defines a > > namespace device and what the device name is. I can add two new loader > > commands for them respectively. > > > > Because they just provide information and SeaBIOS in non-xen > > environment ignores unrecognized commands, they will not break SeaBIOS > > in non-xen environment. > > > > On QEMU side, most Xen-specific hacks in ACPI builder could be > > dropped, and replaced by adding the new loader commands (though they > > may be used only by Xen). > > > > On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor > > are needed in, perhaps, hvmloader. > > If Xen has to parse BIOSLinkerLoader, it can use the existing commands > to process a reduced set of ACPI tables. In other words, > etc/acpi/tables would only include the NFIT, the SSDT with namespace > devices, and the XSDT. etc/acpi/rsdp would include the RSDP table as usual. > > hvmloader can then: > > 1) allocate some memory for where the XSDT will go > > 2) process the BIOSLinkerLoader like SeaBIOS would do > > 3) find the RSDP in low memory, since the loader script must have placed > it there. If it cannot find it, allocate some low memory, fill it with > the RSDP header and revision, and and jump to step 6 > > 4) If it found QEMU's RSDP, use it to find QEMU's XSDT > > 5) Copy ACPI table pointers from QEMU to hvmloader's RSDT and/or XSDT. > > 6) build hvmloader tables and link them into the RSDT and/or XSDT as usual. > > 7) overwrite the RSDP in low memory with a pointer to hvmloader's own > RSDT and/or XSDT, and updated the checksums > > QEMU's XSDT remains there somewhere in memory, unused but harmless. > It can work for plan tables which do not contain AML. However, for a namespace device, Xen needs to know its name in order to detect the potential name conflict with those used in Xen built ACPI. Xen does not (and is not going to) introduce an AML parser, so it cannot get those device names from QEMU built ACPI by its own. The idea of either this patch series or the new BIOSLinkerLoader command is to let QEMU tell Xen where the definition body of a namespace device (i.e. that part within the outmost "Device(NAME)") is and what the device name is. Xen, after the name conflict check, can re-package the definition body in a namespace device (w/ minimal AML builder code added in Xen) and then in SSDT. Haozhong
Re: [Qemu-devel] [PATCH v3 00/13] nbd minimal structured read
13.10.2017 01:39, Eric Blake wrote: On 10/12/2017 04:53 AM, Vladimir Sementsov-Ogievskiy wrote: Minimally implement nbd structured read extension. v3: clone: tag up-nbd-minimal-structured-read-v3 from https://src.openvz.org/scm/~vsementsov/qemu.git online: https://src.openvz.org/users/vsementsov/repos/qemu/browse?at=up-nbd-minimal-structured-read-v3 03: rename nbd_co_send_reply here too, remove r-b 04-07: splitted nbd server refactoring, with some changes English does not have 'splitted'; 'split' is one of those irregular verbs that is the same form for present, past, and past participle tenses. I've pushed 1-7 with tweaks to my NBD staging queue: http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/nbd Let me know if I need to redo any of the tweaks I made to your work. I've checked, and I agree with all your changes, thank you for this work! 08: add Eric's r-b 13: rewrite receiving loops by Paolo's suggestion and I will resume review of the rest tomorrow. -- Best regards, Vladimir
Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest
On 10/12/17 13:39 -0400, Konrad Rzeszutek Wilk wrote: > On Thu, Oct 12, 2017 at 08:45:44PM +0800, Haozhong Zhang wrote: > > On 10/10/17 12:05 -0400, Konrad Rzeszutek Wilk wrote: > > > On Tue, Sep 12, 2017 at 11:15:09AM +0800, Haozhong Zhang wrote: > > > > On 09/11/17 11:52 -0700, Stefano Stabellini wrote: > > > > > CC'ing xen-devel, and the Xen tools and x86 maintainers. > > > > > > > > > > On Mon, 11 Sep 2017, Igor Mammedov wrote: > > > > > > On Mon, 11 Sep 2017 12:41:47 +0800 > > > > > > Haozhong Zhang wrote: > > > > > > > > > > > > > This is the QEMU part patches that works with the associated Xen > > > > > > > patches to enable vNVDIMM support for Xen HVM domains. Xen relies > > > > > > > on > > > > > > > QEMU to build guest NFIT and NVDIMM namespace devices, and > > > > > > > allocate > > > > > > > guest address space for vNVDIMM devices. > > > > > > > > > > > > > > All patches can be found at > > > > > > > Xen: https://github.com/hzzhan9/xen.git nvdimm-rfc-v3 > > > > > > > QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3 > > > > > > > > > > > > > > Patch 1 is to avoid dereferencing the NULL pointer to non-existing > > > > > > > label data, as the Xen side support for labels is not implemented > > > > > > > yet. > > > > > > > > > > > > > > Patch 2 & 3 add a memory backend dedicated for Xen usage and a > > > > > > > hotplug > > > > > > > memory region for Xen guest, in order to make the existing nvdimm > > > > > > > device plugging path work on Xen. > > > > > > > > > > > > > > Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when > > > > > > > QEMU is > > > > > > > used as the Xen device model. > > > > > > > > > > > > I've skimmed over patch-set and can't say that I'm happy with > > > > > > number of xen_enabled() invariants it introduced as well as > > > > > > with partial blobs it creates. > > > > > > > > > > I have not read the series (Haozhong, please CC me, Anthony and > > > > > xen-devel to the whole series next time), but yes, indeed. Let's not > > > > > add > > > > > more xen_enabled() if possible. > > > > > > > > > > Haozhong, was there a design document thread on xen-devel about this? > > > > > If > > > > > so, did it reach a conclusion? Was the design accepted? If so, please > > > > > add a link to the design doc in the introductory email, so that > > > > > everybody can read it and be on the same page. > > > > > > > > Yes, there is a design [1] discussed and reviewed. Section 4.3 discussed > > > > the guest ACPI. > > > > > > > > [1] > > > > https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01921.html > > > > > > Igor, did you have a chance to read it? > > > > > > .. see below > > > > > > > > > > > > > > > > > > > > I'd like to reduce above and a way to do this might be making xen > > > > > > 1. use fw_cfg > > > > > > 2. fetch QEMU build acpi tables from fw_cfg > > > > > > 3. extract nvdim tables (which is trivial) and use them > > > > > > > > > > > > looking at xen_load_linux(), it seems possible to use fw_cfg. > > > > > > > > > > > > So what's stopping xen from using it elsewhere?, > > > > > > instead of adding more xen specific code to do 'the same' > > > > > > job and not reusing/sharing common code with tcg/kvm. > > > > > > > > > > So far, ACPI tables have not been generated by QEMU. Xen HVM machines > > > > > rely on a firmware-like application called "hvmloader" that runs in > > > > > guest context and generates the ACPI tables. I have no opinions on > > > > > hvmloader and I'll let the Xen maintainers talk about it. However, > > > > > keep > > > > > in mind that with an HVM guest some devices are emulated by Xen and/or > > > > > by other device emulators that can run alongside QEMU. QEMU doesn't > > > > > have > > > > > a full few of the system. > > > > > > > > > > Here the question is: does it have to be QEMU the one to generate the > > > > > ACPI blobs for the nvdimm? It would be nicer if it was up to hvmloader > > > > > like the rest, instead of introducing this split-brain design about > > > > > ACPI. We need to see a design doc to fully understand this. > > > > > > > > > > > > > hvmloader runs in the guest and is responsible to build/load guest > > > > ACPI. However, it's not capable to build AML at runtime (for the lack > > > > of AML builder). If any guest ACPI object is needed (e.g. by guest > > > > DSDT), it has to be generated from ASL by iasl at Xen compile time and > > > > then be loaded by hvmloader at runtime. > > > > > > > > Xen includes an OperationRegion "BIOS" in the static generated guest > > > > DSDT, whose address is hardcoded and which contains a list of values > > > > filled by hvmloader at runtime. Other ACPI objects can refer to those > > > > values (e.g., the number of vCPUs). But it's not enough for generating > > > > guest NVDIMM ACPI objects at compile time and then being customized > > > > and loaded by hvmload, because its structure (i.e., the number of > > > > namespace devices)
Re: [Qemu-devel] [PATCH 4/4] multiboot: make tests work with clang
On 13.10.2017 01:54, Anatol Pomozov wrote: > * clang 3.8 enables SSE even for 32bit code. Generate code for pentium >CPU to make sure no new instructions are used. > * add memset() implementation. Clang implements array zeroing in >print_num() via memset() function call. > --- > tests/multiboot/Makefile| 2 +- > tests/multiboot/libc.c | 9 + > tests/multiboot/libc.h | 2 ++ > tests/multiboot/run_test.sh | 1 + > 4 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/tests/multiboot/Makefile b/tests/multiboot/Makefile > index b6a5056347..79dfe85afc 100644 > --- a/tests/multiboot/Makefile > +++ b/tests/multiboot/Makefile > @@ -1,5 +1,5 @@ > CC=gcc > -CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc > -fno-builtin > +CCFLAGS=-m32 -Wall -Wextra -Werror -fno-stack-protector -nostdinc > -fno-builtin -march=pentium > ASFLAGS=-m32 > > LD=ld > diff --git a/tests/multiboot/libc.c b/tests/multiboot/libc.c > index 6df9bda96d..512fccd7fa 100644 > --- a/tests/multiboot/libc.c > +++ b/tests/multiboot/libc.c > @@ -33,6 +33,15 @@ void* memcpy(void *dest, const void *src, int n) > > return dest; > } > +void *memset(void *s, int c, size_t n) Add an empty line before the new function? > +{ > +size_t i; > +char *d = s; > +for (i = 0; i < n; i++) { while (n-- > 0) ? ... that way you don't need the i variable. > +*d++ = c; > +} > +return s; > +} Thomas
Re: [Qemu-devel] [PATCH 1/2] MAINTAINERS: Fix scsi path
On 13.10.2017 03:24, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 772ac209e1..da3c78df47 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -982,7 +982,7 @@ S: Supported > F: include/hw/scsi/* > F: include/scsi/* > F: hw/scsi/* > -F: util/scsi* > +F: scsi/* scsi/* is already covered by the new "Block SCSI subsystem" section, so I think you can also simply remove that line instead. Thomas
Re: [Qemu-devel] [PATCH 2/2] MAINTAINERS: Drop Sun4v nonexistent file
On 13.10.2017 03:24, Fam Zheng wrote: > Signed-off-by: Fam Zheng > --- > MAINTAINERS | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index da3c78df47..731c5c7ec2 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -778,7 +778,6 @@ F: pc-bios/openbios-sparc64 > Sun4v > M: Artyom Tarasenko > S: Maintained > -F: hw/sparc64/sun4v.c I guess you should rather replace that one with: hw/sparc64/niagara.c Artyom, is that right? Thomas
[Qemu-devel] [PULL 02/11] docker: don't rely on submodules existing in the main checkout
From: "Daniel P. Berrange" When building the tarball to pass into the docker/vm test image, the code relies on the git submodules being checked out in the main checkout. ie if the developer has not run 'git submodule update --init dtc' many of the docker tests will fail due to the libfdt package not being present in the test images. Patchew manually checks out the dtc submodule in the main git checkout, but this is a bad idea. When running tests we want to have a predictable set of submodules included in the source that's tested. The build environment is completely independent of the developers host OS, so the submodules the developer has checked out should not be considered relevant for the tests. This changes the archive-source.sh script so that it clones the current git checkout into a temporary directory, checks out a fixed set of submodules, builds the tarball and finally removes the temporary git clone. Signed-off-by: Daniel P. Berrange Message-id: 20170929101201.21039-3-berra...@redhat.com Signed-off-by: Gerd Hoffmann --- scripts/archive-source.sh | 34 -- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh index c4e7d98f4d..4029de7b20 100755 --- a/scripts/archive-source.sh +++ b/scripts/archive-source.sh @@ -18,15 +18,37 @@ if test $# -lt 1; then error "Usage: $0 " fi -tar_file="$1" -list_file="$1.list" -submodules=$(git submodule foreach --recursive --quiet 'echo $name') +tar_file=`realpath "$1"` +list_file="${tar_file}.list" +vroot_dir="${tar_file}.vroot" -if test $? -ne 0; then -error "git submodule command failed" +# We want a predictable list of submodules for builds, that is +# independent of what the developer currently has initialized +# in their checkout, because the build environment is completely +# different to the host OS. +submodules="dtc" + +trap "status=$?; rm -rf \"$list_file\" \"$vroot_dir\"; exit \$status" 0 1 2 3 15 + +if git diff-index --quiet HEAD -- &>/dev/null +then +HEAD=HEAD +else +HEAD=`git stash create` fi +git clone --shared . "$vroot_dir" +test $? -ne 0 && error "failed to clone into '$vroot_dir'" -trap "status=$?; rm -f \"$list_file\"; exit \$status" 0 1 2 3 15 +cd "$vroot_dir" +test $? -ne 0 && error "failed to change into '$vroot_dir'" + +git checkout $HEAD +test $? -ne 0 && error "failed to checkout $HEAD revision" + +for sm in $submodules; do +git submodule update --init $sm +test $? -ne 0 && error "failed to init submodule $sm" +done if test -n "$submodules"; then { -- 2.9.3
[Qemu-devel] [PULL 01/11] build: automatically handle GIT submodule checkout for dtc
From: "Daniel P. Berrange" Currently if DTC is required by configure and not available in the host OS install, we exit with an error message telling the user to checkout a git submodule or install the library. This introduces automatic handling of the git submodule checkout process and enables it for dtc. This only runs if building from GIT, so users of release tarballs still need the system library install. The current state of the git checkout is stashed in .git-submodule-status, and a helper program is used to determine if this state matches the desired submodule state. A dependency against 'Makefile' ensures that the submodule state is refreshed at the start of the build process Signed-off-by: Daniel P. Berrange Message-id: 20170929101201.21039-2-berra...@redhat.com [ kraxel: use /bin/sh not bash for scripts/git-submodule.sh ] Signed-off-by: Gerd Hoffmann --- configure| 46 ++ Makefile | 25 - .gitignore | 1 + MAINTAINERS | 6 ++ scripts/git-submodule.sh | 38 ++ 5 files changed, 95 insertions(+), 21 deletions(-) create mode 100755 scripts/git-submodule.sh diff --git a/configure b/configure index 6587e8014b..38f1710c80 100755 --- a/configure +++ b/configure @@ -264,6 +264,7 @@ cc_i386=i386-pc-linux-gnu-gcc libs_qga="" debug_info="yes" stack_protector="" +git_submodules="" # Don't accept a target_list environment variable. unset target_list @@ -3584,27 +3585,30 @@ EOF if compile_prog "" "$fdt_libs" ; then # system DTC is good - use it fdt=yes - elif test -d ${source_path}/dtc/libfdt ; then -# have submodule DTC - use it -fdt=yes -dtc_internal="yes" -mkdir -p dtc -if [ "$pwd_is_source_path" != "y" ] ; then - symlink "$source_path/dtc/Makefile" "dtc/Makefile" - symlink "$source_path/dtc/scripts" "dtc/scripts" -fi -fdt_cflags="-I\$(SRC_PATH)/dtc/libfdt" -fdt_libs="-L\$(BUILD_DIR)/dtc/libfdt $fdt_libs" - elif test "$fdt" = "yes" ; then -# have neither and want - prompt for system/submodule install -error_exit "DTC (libfdt) version >= 1.4.2 not present. Your options:" \ -" (1) Preferred: Install the DTC (libfdt) devel package" \ -" (2) Fetch the DTC submodule, using:" \ -" git submodule update --init dtc" else -# don't have and don't want -fdt_libs= -fdt=no + # have GIT checkout, so activate dtc submodule + if test -e "${source_path}/.git" ; then + git_submodules="${git_submodules} dtc" + fi + if test -d "${source_path}/dtc/libfdt" || test -e "${source_path}/.git" ; then + fdt=yes + dtc_internal="yes" + mkdir -p dtc + if [ "$pwd_is_source_path" != "y" ] ; then + symlink "$source_path/dtc/Makefile" "dtc/Makefile" + symlink "$source_path/dtc/scripts" "dtc/scripts" + fi + fdt_cflags="-I\$(SRC_PATH)/dtc/libfdt" + fdt_libs="-L\$(BUILD_DIR)/dtc/libfdt $fdt_libs" + elif test "$fdt" = "yes" ; then + # Not a git build & no libfdt found, prompt for system install + error_exit "DTC (libfdt) version >= 1.4.2 not present." \ + "Please install the DTC (libfdt) devel package" + else + # don't have and don't want + fdt_libs= + fdt=no + fi fi fi @@ -5295,6 +5299,7 @@ echo "local state directory queried at runtime" echo "Windows SDK $win_sdk" fi echo "Source path $source_path" +echo "GIT submodules$git_submodules" echo "C compiler$cc" echo "Host C compiler $host_cc" echo "C++ compiler $cxx" @@ -5483,6 +5488,7 @@ echo "extra_cxxflags=$EXTRA_CXXFLAGS" >> $config_host_mak echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak echo "qemu_localedir=$qemu_localedir" >> $config_host_mak echo "libs_softmmu=$libs_softmmu" >> $config_host_mak +echo "GIT_SUBMODULES=$git_submodules" >> $config_host_mak echo "ARCH=$ARCH" >> $config_host_mak diff --git a/Makefile b/Makefile index 97b4508d7d..f479e28afb 100644 --- a/Makefile +++ b/Makefile @@ -14,6 +14,29 @@ ifneq ($(wildcard config-host.mak),) all: include config-host.mak +git-submodule-update: + +.PHONY: git-submodule-update + +ifeq (0,$(MAKELEVEL)) + git_module_status := $(shell \ +cd '$(SRC_PATH)' && \ +./scripts/git-submodule.sh status $(GIT_SUBMODULES); \ +echo $$?; \ + ) + +ifeq (1,$(git_module_status)) +git-submodule-update: + $(call quiet-command, \ + (cd $(SRC_PATH) && ./scripts/git-submodule.sh update $(GIT_SUBMODULES)), \ + "GIT","$(GIT_SUBMODULES)") +endif +endif + +.git-submodule-status: git-submodule-update + +Makefile: .git-submodule-status + # Check that we're not trying to do an out-of-tree build from # a tree that's been used for an in-tree build. ifneq ($(realpath $(SRC_PATH)),$(rea
[Qemu-devel] [PULL 06/11] ui: don't export qemu_input_event_new_key
From: "Daniel P. Berrange" All public code should use qemu_input_event_send_key* functions instead of creating an event directly. Signed-off-by: Daniel P. Berrange Message-id: 20170929101201.21039-7-berra...@redhat.com Signed-off-by: Gerd Hoffmann --- include/ui/input.h | 1 - ui/input.c | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/include/ui/input.h b/include/ui/input.h index 479cc46cfc..f8cee43f65 100644 --- a/include/ui/input.h +++ b/include/ui/input.h @@ -38,7 +38,6 @@ void qemu_input_event_send_impl(QemuConsole *src, InputEvent *evt); void qemu_input_event_sync(void); void qemu_input_event_sync_impl(void); -InputEvent *qemu_input_event_new_key(KeyValue *key, bool down); void qemu_input_event_send_key(QemuConsole *src, KeyValue *key, bool down); void qemu_input_event_send_key_number(QemuConsole *src, int num, bool down); void qemu_input_event_send_key_qcode(QemuConsole *src, QKeyCode q, bool down); diff --git a/ui/input.c b/ui/input.c index 4b241aa823..290b47354a 100644 --- a/ui/input.c +++ b/ui/input.c @@ -386,7 +386,7 @@ void qemu_input_event_sync(void) replay_input_sync_event(); } -InputEvent *qemu_input_event_new_key(KeyValue *key, bool down) +static InputEvent *qemu_input_event_new_key(KeyValue *key, bool down) { InputEvent *evt = g_new0(InputEvent, 1); evt->u.key.data = g_new0(InputKeyEvent, 1); -- 2.9.3
[Qemu-devel] [PULL 08/11] Add pc-bios/keymaps/Makefile
Update files where I think I've figured the correct xkb maps. TODO: nl-be sl sv Signed-off-by: Gerd Hoffmann Message-id: 20171005153330.19210-3-kra...@redhat.com --- pc-bios/keymaps/Makefile | 56 1 file changed, 56 insertions(+) create mode 100644 pc-bios/keymaps/Makefile diff --git a/pc-bios/keymaps/Makefile b/pc-bios/keymaps/Makefile new file mode 100644 index 00..f0e44fd110 --- /dev/null +++ b/pc-bios/keymaps/Makefile @@ -0,0 +1,56 @@ + +KEYMAP := $(shell which qemu-keymap 2>/dev/null) + +MAPS := ar bepo cz da de de-ch en-us en-gb es et fi fo \ + fr fr-be fr-ca fr-ch \ + hr hu is it ja lt lv mk nl no pl pt pt-br ru th tr + +ar : MAP_FLAGS := -l ar +bepo : MAP_FLAGS := -l fr -v dvorak +cz : MAP_FLAGS := -l cz +da : MAP_FLAGS := -l dk +de : MAP_FLAGS := -l de +de-ch : MAP_FLAGS := -l ch +en-us : MAP_FLAGS := -l us +en-gb : MAP_FLAGS := -l gb +es : MAP_FLAGS := -l es +et : MAP_FLAGS := -l et +fi : MAP_FLAGS := -l fi +fo : MAP_FLAGS := -l fo +fr : MAP_FLAGS := -l fr +fr-be : MAP_FLAGS := -l be +fr-ca : MAP_FLAGS := -l ca -v fr +fr-ch : MAP_FLAGS := -l ch -v fr +hr : MAP_FLAGS := -l hr +hu : MAP_FLAGS := -l hu +is : MAP_FLAGS := -l is +it : MAP_FLAGS := -l it +ja : MAP_FLAGS := -l jp -m jp106 +lt : MAP_FLAGS := -l lt +lv : MAP_FLAGS := -l lv +mk : MAP_FLAGS := -l mk +nl : MAP_FLAGS := -l nl +no : MAP_FLAGS := -l no +pl : MAP_FLAGS := -l pl +pt : MAP_FLAGS := -l pt +pt-br : MAP_FLAGS := -l br +ru : MAP_FLAGS := -l ru +th : MAP_FLAGS := -l th +tr : MAP_FLAGS := -l tr + +ifeq ($(KEYMAP),) + +all: + @echo "nothing to do (qemu-keymap not found)" + +else + +all: $(MAPS) + +clean: + rm -f $(MAPS) + +$(MAPS): $(KEYMAP) Makefile + $(KEYMAP) -f $@ $(MAP_FLAGS) + +endif -- 2.9.3
[Qemu-devel] [PULL 05/11] ui: convert key events to QKeyCodes immediately
From: "Daniel P. Berrange" Always use QKeyCode in the InputKeyEvent struct, by converting key numbers to QKeyCode at the time the event is created. This allows the code processing / consuming key events to assume QKeyCode is used. The only place we accept a key number in the InputKeyEvent struct is with QMP commands sent by the user. Signed-off-by: Daniel P. Berrange Message-id: 20170929101201.21039-6-berra...@redhat.com Signed-off-by: Gerd Hoffmann --- ui/input.c | 22 -- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/ui/input.c b/ui/input.c index 3422d4a8ef..4b241aa823 100644 --- a/ui/input.c +++ b/ui/input.c @@ -157,9 +157,16 @@ void qmp_input_send_event(bool has_device, const char *device, } for (e = events; e != NULL; e = e->next) { -InputEvent *event = e->value; +InputEvent *evt = e->value; -qemu_input_event_send(con, event); +if (evt->type == INPUT_EVENT_KIND_KEY && +evt->u.key.data->key->type == KEY_VALUE_KIND_NUMBER) { +KeyValue *key = evt->u.key.data->key; +QKeyCode code = qemu_input_key_number_to_qcode(key->u.qcode.data); +qemu_input_event_send_key_qcode(con, code, evt->u.key.data->down); +} else { +qemu_input_event_send(con, evt); +} } qemu_input_event_sync(); @@ -341,6 +348,11 @@ void qemu_input_event_send_impl(QemuConsole *src, InputEvent *evt) void qemu_input_event_send(QemuConsole *src, InputEvent *evt) { +/* Expect all parts of QEMU to send events with QCodes exclusively. + * Key numbers are only supported as end-user input via QMP */ +assert(!(evt->type == INPUT_EVENT_KIND_KEY && + evt->u.key.data->key->type == KEY_VALUE_KIND_NUMBER)); + if (!runstate_is_running() && !runstate_check(RUN_STATE_SUSPENDED)) { return; } @@ -400,10 +412,8 @@ void qemu_input_event_send_key(QemuConsole *src, KeyValue *key, bool down) void qemu_input_event_send_key_number(QemuConsole *src, int num, bool down) { -KeyValue *key = g_new0(KeyValue, 1); -key->type = KEY_VALUE_KIND_NUMBER; -key->u.number.data = num; -qemu_input_event_send_key(src, key, down); +QKeyCode code = qemu_input_key_number_to_qcode(num); +qemu_input_event_send_key_qcode(src, code, down); } void qemu_input_event_send_key_qcode(QemuConsole *src, QKeyCode q, bool down) -- 2.9.3
[Qemu-devel] [PULL 07/11] tools: add qemu-keymap
qemu-keymap generates qemu reverse keymaps from xkb keymaps, which can be used with the qemu "-k" command line switch. Signed-off-by: Gerd Hoffmann Message-id: 20171005153330.19210-2-kra...@redhat.com --- configure | 23 ++ Makefile | 5 ++ qemu-keymap.c | 258 ++ 3 files changed, 286 insertions(+) create mode 100644 qemu-keymap.c diff --git a/configure b/configure index 06f18ea9af..be53b6b104 100755 --- a/configure +++ b/configure @@ -304,6 +304,7 @@ vde="" vnc_sasl="" vnc_jpeg="" vnc_png="" +xkbcommon="" xen="" xen_ctrl_version="" xen_pv_domain_build="no" @@ -2908,6 +2909,21 @@ EOF fi ## +# xkbcommon probe +if test "$xkbcommon" != "no" ; then + if $pkg_config xkbcommon --exists; then +xkbcommon_cflags=$($pkg_config xkbcommon --cflags) +xkbcommon_libs=$($pkg_config xkbcommon --libs) +xkbcommon=yes + else +if test "$xkbcommon" = "yes" ; then + feature_not_found "xkbcommon" "Install libxkbcommon-devel" +fi +xkbcommon=no + fi +fi + +## # fnmatch() probe, used for ACL routines fnmatch="no" cat > $TMPC << EOF @@ -5107,6 +5123,9 @@ if test "$softmmu" = yes ; then mpath=no fi fi +if test "$xkbcommon" = "yes"; then + tools="qemu-keymap\$(EXESUF) $tools" +fi # Probe for guest agent support/options @@ -5606,6 +5625,10 @@ fi if test "$vnc_png" = "yes" ; then echo "CONFIG_VNC_PNG=y" >> $config_host_mak fi +if test "$xkbcommon" = "yes" ; then + echo "XKBCOMMON_CFLAGS=$xkbcommon_cflags" >> $config_host_mak + echo "XKBCOMMON_LIBS=$xkbcommon_libs" >> $config_host_mak +fi if test "$fnmatch" = "yes" ; then echo "CONFIG_FNMATCH=y" >> $config_host_mak fi diff --git a/Makefile b/Makefile index 0ae29531db..e54c4e6232 100644 --- a/Makefile +++ b/Makefile @@ -419,6 +419,8 @@ qemu-io$(EXESUF): qemu-io.o $(block-obj-y) $(crypto-obj-y) $(io-obj-y) $(qom-obj qemu-bridge-helper$(EXESUF): qemu-bridge-helper.o $(COMMON_LDADDS) +qemu-keymap$(EXESUF): qemu-keymap.o ui/input-keymap.o $(COMMON_LDADDS) + fsdev/virtfs-proxy-helper$(EXESUF): fsdev/virtfs-proxy-helper.o fsdev/9p-marshal.o fsdev/9p-iov-marshal.o $(COMMON_LDADDS) fsdev/virtfs-proxy-helper$(EXESUF): LIBS += -lcap @@ -433,6 +435,9 @@ qemu-img-cmds.h: $(SRC_PATH)/qemu-img-cmds.hx $(SRC_PATH)/scripts/hxtool qemu-ga$(EXESUF): LIBS = $(LIBS_QGA) qemu-ga$(EXESUF): QEMU_CFLAGS += -I qga/qapi-generated +qemu-keymap$(EXESUF): LIBS += $(XKBCOMMON_LIBS) +qemu-keymap$(EXESUF): QEMU_CFLAGS += $(XKBCOMMON_CFLAGS) + gen-out-type = $(subst .,-,$(suffix $@)) qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py diff --git a/qemu-keymap.c b/qemu-keymap.c new file mode 100644 index 00..49e9167b86 --- /dev/null +++ b/qemu-keymap.c @@ -0,0 +1,258 @@ +/* + * QEMU keymap utility + * + * Copyright Red Hat, Inc. 2017 + * + * Authors: + * Gerd Hoffmann + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "qapi-types.h" +#include "qemu/notify.h" +#include "ui/input.h" + +#include + +struct xkb_rule_names names = { +.rules = NULL, +.model = "pc105", +.layout = "us", +.variant = NULL, +.options = NULL, +}; + +static xkb_mod_mask_t shift; +static xkb_mod_mask_t ctrl; +static xkb_mod_mask_t altgr; +static xkb_mod_mask_t numlock; + +static FILE *outfile; + +/* */ + +static uint32_t qcode_to_number(uint32_t qcode) +{ +KeyValue keyvalue; +uint32_t number; + +keyvalue.type = KEY_VALUE_KIND_QCODE; +keyvalue.u.qcode.data = qcode; +number = qemu_input_key_value_to_number(&keyvalue); +assert(number != 0); +return number; +} + +static void print_sym(xkb_keysym_t sym, uint32_t qcode, const char *mod) +{ +char name[64]; + +if (sym == XKB_KEY_NoSymbol) { +return; +} +xkb_keysym_get_name(sym, name, sizeof(name)); + +/* TODO: make ui/keymap.c parser accept QKeyCode names */ +fprintf(outfile, "%s 0x%02x%s\n", name, qcode_to_number(qcode), mod); +} + +static void walk_map(struct xkb_keymap *map, xkb_keycode_t code, void *data) +{ +struct xkb_state *state = data; +xkb_keysym_t kbase, knumlock, kshift, kaltgr, kaltgrshift; +uint32_t evdev, qcode; +char name[64]; + +fprintf(outfile, "\n"); + +/* + * map xkb keycode -> QKeyCode + * + * xkb keycode is linux evdev shifted by 8 + */ +evdev = code - 8; +qcode = qemu_input_linux_to_qcode(evdev); +if (qcode == Q_KEY_CODE_UNMAPPED) { +xkb_state_update_mask(state, 0, 0, 0, 0, 0, 0); +kbase = xkb_state_key_get_one_sym(state, code); +xkb_keysym_get_name(kbase, name, sizeof(name)); +fprintf(outfile, "# evdev %d (0x%x): no evdev -
[Qemu-devel] [PULL 10/11] ui/gtk: Fix deprecation of vte_terminal_copy_clipboard
From: Anthony PERARD vte_terminal_copy_clipboard() is deprecated in VTE 0.50. Signed-off-by: Anthony PERARD Reviewed-by: Daniel P. Berrange Signed-off-by: Gerd Hoffmann --- ui/gtk.c | 5 + 1 file changed, 5 insertions(+) diff --git a/ui/gtk.c b/ui/gtk.c index 5bd87c265a..342e96fbe9 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -1702,7 +1702,12 @@ static void gd_menu_copy(GtkMenuItem *item, void *opaque) GtkDisplayState *s = opaque; VirtualConsole *vc = gd_vc_find_current(s); +#if VTE_CHECK_VERSION(0, 50, 0) +vte_terminal_copy_clipboard_format(VTE_TERMINAL(vc->vte.terminal), + VTE_FORMAT_TEXT); +#else vte_terminal_copy_clipboard(VTE_TERMINAL(vc->vte.terminal)); +#endif } static void gd_vc_adjustment_changed(GtkAdjustment *adjustment, void *opaque) -- 2.9.3
[Qemu-devel] [PULL 11/11] gtk: fix wrong id between texture and framebuffer
From: Anthoine Bourgeois The gd_gl_area_scanout_texture must destroy framebuffer if there is no texture id instead of no framebuffer id. The effect was a black screen with "-vga virtio -display gtk,gl=on" options. The bug was introduce by a4f113fd "gtk: use framebuffer helper functions." Signed-off-by: Anthoine Bourgeois Message-id: 20171002124052.13829-1-anthoine.bourge...@gmail.com Signed-off-by: Gerd Hoffmann --- ui/gtk-gl-area.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/gtk-gl-area.c b/ui/gtk-gl-area.c index 01ebf2c7de..7080f4e14f 100644 --- a/ui/gtk-gl-area.c +++ b/ui/gtk-gl-area.c @@ -178,8 +178,7 @@ void gd_gl_area_scanout_texture(DisplayChangeListener *dcl, gtk_gl_area_make_current(GTK_GL_AREA(vc->gfx.drawing_area)); -if (vc->gfx.guest_fb.framebuffer == 0 || -vc->gfx.w == 0 || vc->gfx.h == 0) { +if (backing_id == 0 || vc->gfx.w == 0 || vc->gfx.h == 0) { gtk_gl_area_set_scanout_mode(vc, false); return; } -- 2.9.3
[Qemu-devel] [PULL 03/11] ui: add keycodemapdb repository as a GIT submodule
From: "Daniel P. Berrange" The https://gitlab.com/keycodemap/keycodemapdb/ repo contains a data file mapping between all the different scancode/keycode/keysym sets that are known, and a tool to auto-generate lookup tables for different combinations. It is used by GTK-VNC, SPICE-GTK and libvirt for mapping keys. Using it in QEMU will let us replace many hand written lookup tables with auto-generated tables from a master data source, reducing bugs. Adding new QKeyCodes will now only require the master table to be updated, all ~20 other tables will be automatically updated to follow. Signed-off-by: Daniel P. Berrange Message-id: 20170929101201.21039-4-berra...@redhat.com [ kraxel: fix build ] [ kraxel: switch repo to qemu.git mirror ] Signed-off-by: Gerd Hoffmann --- configure | 8 +++- Makefile | 24 +++- .gitignore| 1 + .gitmodules | 3 +++ scripts/archive-source.sh | 2 +- ui/keycodemapdb | 1 + 6 files changed, 36 insertions(+), 3 deletions(-) create mode 16 ui/keycodemapdb diff --git a/configure b/configure index 38f1710c80..06f18ea9af 100755 --- a/configure +++ b/configure @@ -264,7 +264,13 @@ cc_i386=i386-pc-linux-gnu-gcc libs_qga="" debug_info="yes" stack_protector="" -git_submodules="" + +if test -e "$source_path/.git" +then +git_submodules="ui/keycodemapdb" +else +git_submodules="" +fi # Don't accept a target_list environment variable. unset target_list diff --git a/Makefile b/Makefile index f479e28afb..505d13bd31 100644 --- a/Makefile +++ b/Makefile @@ -33,7 +33,7 @@ git-submodule-update: endif endif -.git-submodule-status: git-submodule-update +.git-submodule-status: git-submodule-update config-host.mak Makefile: .git-submodule-status @@ -214,6 +214,28 @@ trace-dtrace-root.h: trace-dtrace-root.dtrace trace-dtrace-root.o: trace-dtrace-root.dtrace +KEYCODEMAP_GEN = $(SRC_PATH)/ui/keycodemapdb/tools/keymap-gen +KEYCODEMAP_CSV = $(SRC_PATH)/ui/keycodemapdb/data/keymaps.csv + +KEYCODEMAP_FILES = \ +$(NULL) + +GENERATED_FILES += $(KEYCODEMAP_FILES) + +ui/input-keymap-%.c: $(KEYCODEMAP_GEN) $(KEYCODEMAP_CSV) $(SRC_PATH)/ui/Makefile.objs + $(call quiet-command,\ + src=$$(echo $@ | sed -E -e "s,^ui/input-keymap-(.+)-to-(.+)\.c$$,\1,") && \ + dst=$$(echo $@ | sed -E -e "s,^ui/input-keymap-(.+)-to-(.+)\.c$$,\2,") && \ + test -e $(KEYCODEMAP_GEN) && \ + $(PYTHON) $(KEYCODEMAP_GEN) \ + --lang glib2 \ + --varname qemu_input_map_$${src}_to_$${dst} \ + code-map $(KEYCODEMAP_CSV) $${src} $${dst} \ + > $@ || rm -f $@, "GEN", "$@") + +$(KEYCODEMAP_GEN): .git-submodule-status +$(KEYCODEMAP_CSV): .git-submodule-status + # Don't try to regenerate Makefile or configure # We don't generate any of them Makefile: ; diff --git a/.gitignore b/.gitignore index b5e115d96b..620eec6b47 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ /trace/generated-tcg-tracers.h /ui/shader/texture-blit-frag.h /ui/shader/texture-blit-vert.h +/ui/input-keymap-*.c *-timestamp /*-softmmu /*-darwin-user diff --git a/.gitmodules b/.gitmodules index 84c54cdc49..7c981a42b6 100644 --- a/.gitmodules +++ b/.gitmodules @@ -34,3 +34,6 @@ [submodule "roms/QemuMacDrivers"] path = roms/QemuMacDrivers url = git://git.qemu.org/QemuMacDrivers.git +[submodule "ui/keycodemapdb"] + path = ui/keycodemapdb + url = git://git.qemu.org/keycodemapdb.git diff --git a/scripts/archive-source.sh b/scripts/archive-source.sh index 4029de7b20..4e63774f9a 100755 --- a/scripts/archive-source.sh +++ b/scripts/archive-source.sh @@ -26,7 +26,7 @@ vroot_dir="${tar_file}.vroot" # independent of what the developer currently has initialized # in their checkout, because the build environment is completely # different to the host OS. -submodules="dtc" +submodules="dtc ui/keycodemapdb" trap "status=$?; rm -rf \"$list_file\" \"$vroot_dir\"; exit \$status" 0 1 2 3 15 diff --git a/ui/keycodemapdb b/ui/keycodemapdb new file mode 16 index 00..56ce5650d2 --- /dev/null +++ b/ui/keycodemapdb @@ -0,0 +1 @@ +Subproject commit 56ce5650d2c6ea216b4580df44b9a6dd3bc92c3b -- 2.9.3
[Qemu-devel] [PULL 00/11] Ui 20171013 patches
The following changes since commit bac960832015bf4c4c1b873011612e2675e4464c: Merge remote-tracking branch 'remotes/elmarco/tags/vus-pull-request' into staging (2017-10-11 13:10:36 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/ui-20171013-pull-request for you to fetch changes up to 942a35335b2efa2f2997d51eb5142fc9903efe43: gtk: fix wrong id between texture and framebuffer (2017-10-13 10:10:36 +0200) ui: use keycodemapdb for key code mappings, part one (v2) ui: add qemu-keymap, update reverse keymaps (for qemu -k $map) ui: fix for vte 0.50 ui: gtk texture fix Anthoine Bourgeois (1): gtk: fix wrong id between texture and framebuffer Anthony PERARD (1): ui/gtk: Fix deprecation of vte_terminal_copy_clipboard Daniel P. Berrange (6): build: automatically handle GIT submodule checkout for dtc docker: don't rely on submodules existing in the main checkout ui: add keycodemapdb repository as a GIT submodule ui: convert common input code to keycodemapdb ui: convert key events to QKeyCodes immediately ui: don't export qemu_input_event_new_key Gerd Hoffmann (3): tools: add qemu-keymap Add pc-bios/keymaps/Makefile pc-bios/keymaps: keymaps update configure | 75 ++- Makefile | 55 ++- include/ui/input.h| 12 +- qemu-keymap.c | 258 +++ ui/gtk-gl-area.c |3 +- ui/gtk.c |5 + ui/input-keymap.c | 336 +- ui/input.c| 24 +- .gitignore|2 + .gitmodules |3 + MAINTAINERS |6 + pc-bios/keymaps/Makefile | 56 +++ pc-bios/keymaps/ar| 819 + pc-bios/keymaps/bepo | 1108 +++-- pc-bios/keymaps/cz| 861 --- pc-bios/keymaps/da| 732 +- pc-bios/keymaps/de| 767 ++- pc-bios/keymaps/de-ch | 915 - pc-bios/keymaps/en-gb | 724 - pc-bios/keymaps/en-us | 718 - pc-bios/keymaps/es| 744 +- pc-bios/keymaps/et| 818 + pc-bios/keymaps/fi| 814 ++--- pc-bios/keymaps/fo| 881 --- pc-bios/keymaps/fr| 704 +++- pc-bios/keymaps/fr-be | 724 - pc-bios/keymaps/fr-ca | 804 ++-- pc-bios/keymaps/fr-ch | 800 ++-- pc-bios/keymaps/hr| 752 +- pc-bios/keymaps/hu| 887 pc-bios/keymaps/is| 802 +--- pc-bios/keymaps/it| 757 ++- pc-bios/keymaps/ja| 792 +--- pc-bios/keymaps/lt| 844 -- pc-bios/keymaps/lv| 766 +-- pc-bios/keymaps/mk| 814 + pc-bios/keymaps/nl| 794 +++- pc-bios/keymaps/no| 758 ++- pc-bios/keymaps/pl| 789 ++-- pc-bios/keymaps/pt| 737 +- pc-bios/keymaps/pt-br | 775 ++- pc-bios/keymaps/ru| 835 ++ pc-bios/keymaps/th| 878 +-- pc-bios/keymaps/tr| 819 ++--- scripts/archive-source.sh | 34 +- scripts/git-submodule.sh | 38 ++ ui/keycodemapdb |1 + 47 files changed, 24565 insertions(+), 2075 deletions(-) create mode 100644 qemu-keymap.c create mode 100644 pc-bios/keymaps/Makefile create mode 100755 scripts/git-submodule.sh create mode 16 ui/keycodemapdb -- 2.9.3
Re: [Qemu-devel] [PULL 00/10] Ui 20171011 patches
On Wed, 2017-10-11 at 15:37 +0100, Peter Maydell wrote: > On 11 October 2017 at 09:35, Gerd Hoffmann wrote: > > The following changes since commit > > 567d0a19c7998fa366598b83d5a6e5f0759d3ea9: > > > > Merge remote-tracking branch 'remotes/ehabkost/tags/x86-and- > > machine-pull-request' into staging (2017-10-10 13:25:46 +0100) > > > > are available in the git repository at: > > > > git://git.kraxel.org/qemu tags/ui-20171011-pull-request > > > > for you to fetch changes up to > > 92328b7104ffc825f10c33f3fd1866e940a523f7: > > > > ui/gtk: Fix deprecation of vte_terminal_copy_clipboard (2017-10- > > 11 10:03:31 +0200) > > > > > > ui: use keycodemapdb for key code mappings, part one (v2) > > ui: add qemu-keymap, update reverse keymaps (for qemu -k $map) > > ui: fix for vte 0.50 > > > > > > Build failures: > > NetBSD: > CC ui/input-keymap.o > /root/qemu/ui/input-keymap.c:8:44: fatal error: > ui/input-keymap-linux-to-qcode.c: No such file or directory > #include "ui/input-keymap-linux-to-qcode.c" > ^ > > FreeBSD: > > > CC ui/input-keymap.o > /root/qemu/ui/input-keymap.c:8:10: fatal error: > 'ui/input-keymap-linux-to-qcode.c' file not found > #include "ui/input-keymap-linux-to-qcode.c" > ^ > > OpenBSD: > > gmake: Entering directory '/home/qemu/build/all' > /bin/sh: ./scripts/git-submodule.sh: No such file or directory > gmake: Leaving directory '/home/qemu/build/all' > gmake: Entering directory '/home/qemu/build/all' > GIT > /bin/sh: ./scripts/git-submodule.sh: No such file or directory > gmake: *** [Makefile:30: git-submodule-update] Error 1 > gmake: Leaving directory '/home/qemu/build/all' > gmake: *** Waiting for unfinished jobs > gmake: Entering directory '/home/qemu/build/all' > config-host.mak is out-of-date, running configure > Install prefix/usr/local > [configure output snipped] > gmake: Leaving directory '/home/qemu/build/all' > > AArch32 Linux: > > make: Entering directory '/home/peter.maydell/qemu/build/all-a32' > config-host.mak is out-of-date, running configure > Install prefix/usr/local > BIOS directory/usr/local/share/qemu > [...] > VxHS block device no > GEN config-host.h > GEN module_block.h > GEN ui/input-keymap-linux-to-qcode.c > GEN ui/input-keymap-qcode-to-qnum.c > GEN ui/input-keymap-qnum-to-qcode.c > GIT ui/keycodemapdb dtc > GEN trace/generated-tcg-tracers.h > GEN trace/generated-helpers-wrappers.h > GEN trace/generated-helpers.h > GEN trace/generated-helpers.c > Makefile:30: recipe for target 'git-submodule-update' failed > make: *** [git-submodule-update] Error 128 > make: *** Waiting for unfinished jobs > make: Leaving directory '/home/peter.maydell/qemu/build/all-a32' > > > The OpenBSD failure is because make now tries to run a > script that uses #!/bin/bash, and this machine doesn't > have bash installed. Do we really need to add bash to > our build-dependencies? The script doesn't seem like it's > doing anything that critically requires bash... > > thanks > -- PMM New pull out, with bash dropped, repo switched to qemu mirror and one additional gtk fix picked up from the list. cheers, Gerd
Re: [Qemu-devel] [PATCH 1/2] MAINTAINERS: Fix scsi path
On Fri, 10/13 10:04, Thomas Huth wrote: > On 13.10.2017 03:24, Fam Zheng wrote: > > Signed-off-by: Fam Zheng > > --- > > MAINTAINERS | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 772ac209e1..da3c78df47 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -982,7 +982,7 @@ S: Supported > > F: include/hw/scsi/* > > F: include/scsi/* > > F: hw/scsi/* > > -F: util/scsi* > > +F: scsi/* > > scsi/* is already covered by the new "Block SCSI subsystem" section, so > I think you can also simply remove that line instead. I didn't notice that section existing, should the two be merged into one? Fam
[Qemu-devel] [PULL 04/11] ui: convert common input code to keycodemapdb
From: "Daniel P. Berrange" Replace the number_to_qcode, qcode_to_number and linux_to_qcode tables with automatically generated tables. Missing entries in linux_to_qcode now fixed: KEY_LINEFEED -> Q_KEY_CODE_LF KEY_KPEQUAL -> Q_KEY_CODE_KP_EQUALS KEY_COMPOSE -> Q_KEY_CODE_COMPOSE KEY_AGAIN -> Q_KEY_CODE_AGAIN KEY_PROPS -> Q_KEY_CODE_PROPS KEY_UNDO -> Q_KEY_CODE_UNDO KEY_FRONT -> Q_KEY_CODE_FRONT KEY_COPY -> Q_KEY_CODE_COPY KEY_OPEN -> Q_KEY_CODE_OPEN KEY_PASTE -> Q_KEY_CODE_PASTE KEY_CUT -> Q_KEY_CODE_CUT KEY_HELP -> Q_KEY_CODE_HELP KEY_MEDIA -> Q_KEY_CODE_MEDIASELECT In addition, some fixes: - KEY_PLAYPAUSE now maps to Q_KEY_CODE_AUDIOPLAY, instead of KEY_PLAYCD. KEY_PLAYPAUSE is defined across almost all scancodes sets, while KEY_PLAYCD only appears in AT set1, so the former is a more useful mapping. Missing entries in qcode_to_number now fixed: Q_KEY_CODE_AGAIN -> 0x85 Q_KEY_CODE_PROPS -> 0x86 Q_KEY_CODE_UNDO -> 0x87 Q_KEY_CODE_FRONT -> 0x8c Q_KEY_CODE_COPY -> 0xf8 Q_KEY_CODE_OPEN -> 0x64 Q_KEY_CODE_PASTE -> 0x65 Q_KEY_CODE_CUT -> 0xbc Q_KEY_CODE_LF -> 0x5b Q_KEY_CODE_HELP -> 0xf5 Q_KEY_CODE_COMPOSE -> 0xdd Q_KEY_CODE_KP_EQUALS -> 0x59 Q_KEY_CODE_MEDIASELECT -> 0xed In addition, some fixes: - Q_KEY_CODE_MENU was incorrectly mapped to the compose scancode (0xdd) and is now mapped to 0x9e - Q_KEY_CODE_FIND was mapped to 0xe065 (Search) instead of to 0xe041 (Find) - Q_KEY_CODE_HIRAGANA was mapped to 0x70 (Katakanahiragana) instead of of 0x77 (Hirigana) - Q_KEY_CODE_PRINT was mapped to 0xb7 which is not a defined scan code in AT set 1, it is now mapped to 0x54 (sysrq) Signed-off-by: Daniel P. Berrange Message-id: 20170929101201.21039-5-berra...@redhat.com Signed-off-by: Gerd Hoffmann --- Makefile | 3 + include/ui/input.h | 11 +- ui/input-keymap.c | 336 - 3 files changed, 33 insertions(+), 317 deletions(-) diff --git a/Makefile b/Makefile index 505d13bd31..0ae29531db 100644 --- a/Makefile +++ b/Makefile @@ -218,6 +218,9 @@ KEYCODEMAP_GEN = $(SRC_PATH)/ui/keycodemapdb/tools/keymap-gen KEYCODEMAP_CSV = $(SRC_PATH)/ui/keycodemapdb/data/keymaps.csv KEYCODEMAP_FILES = \ +ui/input-keymap-linux-to-qcode.c \ +ui/input-keymap-qcode-to-qnum.c \ +ui/input-keymap-qnum-to-qcode.c \ $(NULL) GENERATED_FILES += $(KEYCODEMAP_FILES) diff --git a/include/ui/input.h b/include/ui/input.h index c488585def..479cc46cfc 100644 --- a/include/ui/input.h +++ b/include/ui/input.h @@ -43,7 +43,7 @@ void qemu_input_event_send_key(QemuConsole *src, KeyValue *key, bool down); void qemu_input_event_send_key_number(QemuConsole *src, int num, bool down); void qemu_input_event_send_key_qcode(QemuConsole *src, QKeyCode q, bool down); void qemu_input_event_send_key_delay(uint32_t delay_ms); -int qemu_input_key_number_to_qcode(uint8_t nr); +int qemu_input_key_number_to_qcode(unsigned int nr); int qemu_input_key_value_to_number(const KeyValue *value); int qemu_input_key_value_to_qcode(const KeyValue *value); int qemu_input_key_value_to_scancode(const KeyValue *value, bool down, @@ -69,4 +69,13 @@ void qemu_input_check_mode_change(void); void qemu_add_mouse_mode_change_notifier(Notifier *notify); void qemu_remove_mouse_mode_change_notifier(Notifier *notify); +extern const guint qemu_input_map_linux_to_qcode_len; +extern const guint16 qemu_input_map_linux_to_qcode[]; + +extern const guint qemu_input_map_qcode_to_qnum_len; +extern const guint16 qemu_input_map_qcode_to_qnum[]; + +extern const guint qemu_input_map_qnum_to_qcode_len; +extern const guint16 qemu_input_map_qnum_to_qcode[]; + #endif /* INPUT_H */ diff --git a/ui/input-keymap.c b/ui/input-keymap.c index cf979c2ce9..3a19a169f5 100644 --- a/ui/input-keymap.c +++ b/ui/input-keymap.c @@ -5,333 +5,37 @@ #include "standard-headers/linux/input.h" -static int linux_to_qcode[KEY_CNT] = { -[KEY_ESC]= Q_KEY_CODE_ESC, -[KEY_1] = Q_KEY_CODE_1, -[KEY_2] = Q_KEY_CODE_2, -[KEY_3] = Q_KEY_CODE_3, -[KEY_4] = Q_KEY_CODE_4, -[KEY_5] = Q_KEY_CODE_5, -[KEY_6] = Q_KEY_CODE_6, -[KEY_7] = Q_KEY_CODE_7, -[KEY_8] = Q_KEY_CODE_8, -[KEY_9] = Q_KEY_CODE_9, -[KEY_0] = Q_KEY_CODE_0, -[KEY_MINUS] = Q_KEY_CODE_MINUS, -[KEY_EQUAL] = Q_KEY_CODE_EQUAL, -[KEY_BACKSPACE] = Q_KEY_CODE_BACKSPACE, -[KEY_TAB]= Q_KEY_CODE_TAB, -[KEY_Q] = Q_KEY_CODE_Q, -[KEY_W] = Q_KEY_CODE_W, -[KEY_E] = Q_KEY_CODE_E, -[KEY_R] = Q_KEY_CODE_R, -[KEY_T] = Q_KEY_CODE_T, -[KEY_Y] = Q_KEY_CODE_Y, -[KEY_U] = Q_KEY_CODE_U, -[KEY_I] = Q_KEY_CODE_I,
Re: [Qemu-devel] [PATCH 2/2] MAINTAINERS: Drop Sun4v nonexistent file
On Fri, Oct 13, 2017 at 10:08 AM, Thomas Huth wrote: > On 13.10.2017 03:24, Fam Zheng wrote: >> Signed-off-by: Fam Zheng >> --- >> MAINTAINERS | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index da3c78df47..731c5c7ec2 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -778,7 +778,6 @@ F: pc-bios/openbios-sparc64 >> Sun4v >> M: Artyom Tarasenko >> S: Maintained >> -F: hw/sparc64/sun4v.c > > I guess you should rather replace that one with: > > hw/sparc64/niagara.c > > Artyom, is that right? Yes, please. Thanks! -- Regards, Artyom Tarasenko SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
Re: [Qemu-devel] [PATCH 1/2] MAINTAINERS: Fix scsi path
On 13.10.2017 10:20, Fam Zheng wrote: > On Fri, 10/13 10:04, Thomas Huth wrote: >> On 13.10.2017 03:24, Fam Zheng wrote: >>> Signed-off-by: Fam Zheng >>> --- >>> MAINTAINERS | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 772ac209e1..da3c78df47 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -982,7 +982,7 @@ S: Supported >>> F: include/hw/scsi/* >>> F: include/scsi/* >>> F: hw/scsi/* >>> -F: util/scsi* >>> +F: scsi/* >> >> scsi/* is already covered by the new "Block SCSI subsystem" section, so >> I think you can also simply remove that line instead. > > I didn't notice that section existing, should the two be merged into one? I don't think so. One section is about the emulated SCSI devices, and the other one about the SCSI block backend code, so that's two different parts. Some other ideas though: 1) I think "include/scsi/*" should be removed from the SCSI devices section, it is already handled in the "Block SCSI subsystem" - and headers that are related to devices should go into include/hw/scsi/ anyway instead. 2) The orphan LSI53C895A section could maybe be removed from the MAINTAINERS file? It's of no use in its current "Orphan" state, and the file is already covered by the generic SCSI devices section. Thomas
[Qemu-devel] [PATCH 0/2] monitor: increase the refcount of the current CPU
If a CPU selected with the "cpu" command is hot-unplugged then "info cpus" causes QEMU to exit: (qemu) device_del cpu1 (qemu) info cpus qemu:qemu_cpu_kick_thread: No such process I could verify that this happens with x86 and ppc, but I guess s390x is also impacted. This series tries to fix the issue by using object_ref() and object_unref() in the monitor code. For this to work on ppc, some preliminary work is needed to let QOM handle the CPU object lifecycle. Please comment. -- Greg --- Greg Kurz (2): spapr_cpu_core: instantiate CPUs separately monitor: add proper reference counting of the current CPU hw/ppc/spapr.c | 10 +++--- hw/ppc/spapr_cpu_core.c | 29 + include/hw/ppc/spapr_cpu_core.h |2 +- monitor.c | 12 4 files changed, 25 insertions(+), 28 deletions(-)
[Qemu-devel] [PATCH 1/2] spapr_cpu_core: instantiate CPUs separately
The current code assumes that only the CPU core object holds a reference on each individual CPU object, and happily frees their allocated memory when the core is unrealized. This is dangerous as some other code can legitimely keep a pointer to a CPU if it calls object_ref(), but it would end up with a dangling pointer. Let's allocate all CPUs with object_new() and let QOM frees them when their reference count reaches zero. Signed-off-by: Greg Kurz --- hw/ppc/spapr.c | 10 +++--- hw/ppc/spapr_cpu_core.c | 29 + include/hw/ppc/spapr_cpu_core.h |2 +- 3 files changed, 13 insertions(+), 28 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index fd9813bde82f..ba391c6ed5de 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3153,12 +3153,10 @@ void spapr_core_release(DeviceState *dev) if (smc->pre_2_10_has_unused_icps) { sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc)); -size_t size = object_type_get_instance_size(scc->cpu_type); int i; for (i = 0; i < cc->nr_threads; i++) { -CPUState *cs = CPU(sc->threads + i * size); +CPUState *cs = CPU(sc->threads[i]); pre_2_10_vmstate_register_dummy_icp(cs->cpu_index); } @@ -3204,7 +3202,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev, sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc); sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev)); CPUCore *cc = CPU_CORE(dev); -CPUState *cs = CPU(core->threads); +CPUState *cs = CPU(core->threads[0]); sPAPRDRConnector *drc; Error *local_err = NULL; int smt = kvmppc_smt_threads(); @@ -3249,13 +3247,11 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev, core_slot->cpu = OBJECT(dev); if (smc->pre_2_10_has_unused_icps) { -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc)); -size_t size = object_type_get_instance_size(scc->cpu_type); int i; for (i = 0; i < cc->nr_threads; i++) { sPAPRCPUCore *sc = SPAPR_CPU_CORE(dev); -void *obj = sc->threads + i * size; +void *obj = sc->threads[i]; cs = CPU(obj); pre_2_10_vmstate_unregister_dummy_icp(cs->cpu_index); diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 3a4c17401226..47c408b52ca1 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -79,21 +79,18 @@ const char *spapr_get_cpu_core_type(const char *cpu_type) static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp) { sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); -size_t size = object_type_get_instance_size(scc->cpu_type); CPUCore *cc = CPU_CORE(dev); int i; for (i = 0; i < cc->nr_threads; i++) { -void *obj = sc->threads + i * size; -DeviceState *dev = DEVICE(obj); +DeviceState *dev = DEVICE(sc->threads[i]); CPUState *cs = CPU(dev); PowerPCCPU *cpu = POWERPC_CPU(cs); spapr_cpu_destroy(cpu); object_unparent(cpu->intc); cpu_remove_sync(cs); -object_unparent(obj); +object_unparent(sc->threads[i]); } g_free(sc->threads); } @@ -146,9 +143,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); CPUCore *cc = CPU_CORE(OBJECT(dev)); -size_t size; Error *local_err = NULL; -void *obj; int i, j; if (!spapr) { @@ -156,17 +151,14 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) return; } -size = object_type_get_instance_size(scc->cpu_type); -sc->threads = g_malloc0(size * cc->nr_threads); +sc->threads = g_new(void *, cc->nr_threads); for (i = 0; i < cc->nr_threads; i++) { char id[32]; CPUState *cs; PowerPCCPU *cpu; -obj = sc->threads + i * size; - -object_initialize(obj, size, scc->cpu_type); -cs = CPU(obj); +sc->threads[i] = object_new(scc->cpu_type); +cs = CPU(sc->threads[i]); cpu = POWERPC_CPU(cs); cs->cpu_index = cc->core_id + i; cpu->vcpu_id = (cc->core_id * spapr->vsmt / smp_threads) + i; @@ -184,17 +176,15 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) cpu->node_id = sc->node_id; snprintf(id, sizeof(id), "thread[%d]", i); -object_property_add_child(OBJECT(sc), id, obj, &local_err); +object_property_add_child(OBJECT(sc), id, sc->threads[i], &local_err); if (local_err) { goto err; } -object_unref(obj); +object_unref(sc->threads[i]); } f
[Qemu-devel] [PATCH 2/2] monitor: add proper reference counting of the current CPU
If a CPU selected with the "cpu" command is hot-unplugged then "info cpus" causes QEMU to exit: (qemu) device_del cpu1 (qemu) info cpus qemu:qemu_cpu_kick_thread: No such process This happens because "cpu" stores the pointer to the selected CPU into the monitor structure. When the CPU is hot-unplugged, we end up with a dangling pointer. The "info cpus" command then does: hmp_info_cpus() monitor_get_cpu_index() mon_get_cpu() cpu_synchronize_state() <--- called with dangling pointer This could cause a QEMU crash as well. This patch switches the monitor to use object_ref() to ensure the CPU object doesn't vanish unexpectedly. The reference is dropped either when "cpu" is used to switch to another CPU, or when the selected CPU is unrealized and cpu_list_remove() sets its cpu_index back to UNASSIGNED_CPU_INDEX. Signed-off-by: Greg Kurz --- monitor.c | 12 1 file changed, 12 insertions(+) diff --git a/monitor.c b/monitor.c index fe0d1bdbb461..1c0b9a2c3ad3 100644 --- a/monitor.c +++ b/monitor.c @@ -579,6 +579,9 @@ static void monitor_data_init(Monitor *mon) static void monitor_data_destroy(Monitor *mon) { +if (mon->mon_cpu) { +object_unref((Object *) mon->mon_cpu); +} qemu_chr_fe_deinit(&mon->chr, false); if (monitor_is_qmp(mon)) { json_message_parser_destroy(&mon->qmp.parser); @@ -1047,12 +1050,21 @@ int monitor_set_cpu(int cpu_index) if (cpu == NULL) { return -1; } +if (cur_mon->mon_cpu) { +object_unref((Object *) cur_mon->mon_cpu); +} cur_mon->mon_cpu = cpu; +object_ref((Object *) cpu); return 0; } CPUState *mon_get_cpu(void) { +if (cur_mon->mon_cpu && +cur_mon->mon_cpu->cpu_index == UNASSIGNED_CPU_INDEX) { +object_unref((Object *) cur_mon->mon_cpu); +cur_mon->mon_cpu = NULL; +} if (!cur_mon->mon_cpu) { if (!first_cpu) { return NULL;
Re: [Qemu-devel] [PATCH 3/8] xen: defer call to xen_restrict until just before os_setup_post
On 10/09/2017 05:01 PM, Ian Jackson wrote: We need to restrict *all* the control fds that qemu opens. Looking in /proc/PID/fd shows there are many; their allocation seems scattered throughout Xen support code in qemu. We must postpone the restrict call until roughly the same time as qemu changes its uid, chroots (if applicable), and so on. There doesn't seem to be an appropriate hook already. The RunState change hook fires at different times depending on exactly what mode qemu is operating in. And it appears that no-one but the Xen code wants a hook at this phase of execution. So, introduce a bare call to a new function xen_setup_post, just before os_setup_post. Also provide the appropriate stub for when Xen compilation is disabled. We do the restriction before rather than after os_setup_post, because xen_restrict may need to open /dev/null, and os_setup_post might have called chroot. This works for normally starting a VM but doesn't seem to work when resuming/migrating. Here is the order of selected operations when starting a VM normally: VNC server running on 127.0.0.1:5901 xen_change_state_handler() xenstore_record_dm_state: running xen_setup_post() xentoolcore_restrict_all: rc = 0 os_setup_post() main_loop() Here is the order of selected operations when starting QEMU with -incoming fd:... : VNC server running on 127.0.0.1:5902 migration_fd_incoming() xen_setup_post() xentoolcore_restrict_all: rc = 0 os_setup_post() main_loop() migration_set_incoming_channel() migrate_set_state() xen_change_state_handler() xenstore_record_dm_state: running error recording dm state qemu exited with code 1 The issue is that QEMU needs xenstore access to write "running" but this is after it has already been restricted. Moving xen_setup_post() into xen_change_state_handler() works fine. The only issue is that in the migration case, it executes after os_setup_post() so QEMU might be chrooted in which case opening /dev/null to restrict fds doesn't work (unless its new root has a /dev/null). -- Ross Lagerwall
Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest
On Fri, 13 Oct 2017 15:53:26 +0800 Haozhong Zhang wrote: > On 10/12/17 17:45 +0200, Paolo Bonzini wrote: > > On 12/10/2017 14:45, Haozhong Zhang wrote: > > > Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and > > > /rom@etc/table-loader. The former is unstructured to guest, and > > > contains all data of guest ACPI. The latter is a BIOSLinkerLoader > > > organized as a set of commands, which direct the guest (e.g., SeaBIOS > > > on KVM/QEMU) to relocate data in the former file, recalculate checksum > > > of specified area, and fill guest address in specified ACPI field. > > > > > > One part of my patches is to implement a mechanism to tell Xen which > > > part of ACPI data is a table (NFIT), and which part defines a > > > namespace device and what the device name is. I can add two new loader > > > commands for them respectively. > > > > > > Because they just provide information and SeaBIOS in non-xen > > > environment ignores unrecognized commands, they will not break SeaBIOS > > > in non-xen environment. > > > > > > On QEMU side, most Xen-specific hacks in ACPI builder could be > > > dropped, and replaced by adding the new loader commands (though they > > > may be used only by Xen). > > > > > > On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor > > > are needed in, perhaps, hvmloader. > > > > If Xen has to parse BIOSLinkerLoader, it can use the existing commands > > to process a reduced set of ACPI tables. In other words, > > etc/acpi/tables would only include the NFIT, the SSDT with namespace > > devices, and the XSDT. etc/acpi/rsdp would include the RSDP table as usual. > > > > hvmloader can then: > > > > 1) allocate some memory for where the XSDT will go > > > > 2) process the BIOSLinkerLoader like SeaBIOS would do > > > > 3) find the RSDP in low memory, since the loader script must have placed > > it there. If it cannot find it, allocate some low memory, fill it with > > the RSDP header and revision, and and jump to step 6 > > > > 4) If it found QEMU's RSDP, use it to find QEMU's XSDT > > > > 5) Copy ACPI table pointers from QEMU to hvmloader's RSDT and/or XSDT. > > > > 6) build hvmloader tables and link them into the RSDT and/or XSDT as usual. > > > > 7) overwrite the RSDP in low memory with a pointer to hvmloader's own > > RSDT and/or XSDT, and updated the checksums > > > > QEMU's XSDT remains there somewhere in memory, unused but harmless. > > +1 to Paolo's suggestion, i.e. 1. add BIOSLinkerLoader into hvmloader 2. load/process QEMU's tables with #1 3. get pointers to QEMU generated NFIT and NVDIMM SSDT from QEMU's RSDT/XSDT and put them in hvmloader's RSDT > It can work for plan tables which do not contain AML. > > However, for a namespace device, Xen needs to know its name in order > to detect the potential name conflict with those used in Xen built > ACPI. Xen does not (and is not going to) introduce an AML parser, so > it cannot get those device names from QEMU built ACPI by its own. > > The idea of either this patch series or the new BIOSLinkerLoader > command is to let QEMU tell Xen where the definition body of a > namespace device (i.e. that part within the outmost "Device(NAME)") is > and what the device name is. Xen, after the name conflict check, can > re-package the definition body in a namespace device (w/ minimal AML > builder code added in Xen) and then in SSDT. I'd skip conflict check at runtime as hvmloader doesn't currently have "\\_SB\NVDR" device so instead of doing runtime check it might do primitive check at build time that ASL sources in hvmloader do not contain reserved for QEMU "NVDR" keyword to avoid its addition by accident in future. (it also might be reused in future if some other tables from QEMU will be reused). It's a bit hackinsh but at least it does the job and keeps BIOSLinkerLoader interface the same for all supported firmwares (I'd consider it as a temporary hack on the way to fully build by QEMU ACPI tables for Xen). Ideally it would be better for QEMU to build all ACPI tables for hvmloader to avoid split brain issues and need to invent extra interfaces every time a feature is added to pass configuration data from QEMU to firmware. But that's probably out of scope of this project, it could be done on top of this if Xen folks would like to do it. Adding BIOSLinkerLoader to hvmloader would be a good starting point for that future effort.
Re: [Qemu-devel] [PATCH 1/2] MAINTAINERS: Fix scsi path
On Fri, 10/13 10:30, Thomas Huth wrote: > On 13.10.2017 10:20, Fam Zheng wrote: > > On Fri, 10/13 10:04, Thomas Huth wrote: > >> On 13.10.2017 03:24, Fam Zheng wrote: > >>> Signed-off-by: Fam Zheng > >>> --- > >>> MAINTAINERS | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/MAINTAINERS b/MAINTAINERS > >>> index 772ac209e1..da3c78df47 100644 > >>> --- a/MAINTAINERS > >>> +++ b/MAINTAINERS > >>> @@ -982,7 +982,7 @@ S: Supported > >>> F: include/hw/scsi/* > >>> F: include/scsi/* > >>> F: hw/scsi/* > >>> -F: util/scsi* > >>> +F: scsi/* > >> > >> scsi/* is already covered by the new "Block SCSI subsystem" section, so > >> I think you can also simply remove that line instead. > > > > I didn't notice that section existing, should the two be merged into one? > > I don't think so. One section is about the emulated SCSI devices, and > the other one about the SCSI block backend code, so that's two different > parts. > > Some other ideas though: > > 1) I think "include/scsi/*" should be removed from the SCSI devices > section, it is already handled in the "Block SCSI subsystem" - and > headers that are related to devices should go into include/hw/scsi/ > anyway instead. > > 2) The orphan LSI53C895A section could maybe be removed from the > MAINTAINERS file? It's of no use in its current "Orphan" state, and the > file is already covered by the generic SCSI devices section. OK, that sounds good to me. Fam
Re: [Qemu-devel] [PATCH 0/2] monitor: increase the refcount of the current CPU
On Fri, 13 Oct 2017 10:35:06 +0200 Greg Kurz wrote: > If a CPU selected with the "cpu" command is hot-unplugged then "info cpus" > causes QEMU to exit: > > (qemu) device_del cpu1 > (qemu) info cpus > qemu:qemu_cpu_kick_thread: No such process > > I could verify that this happens with x86 and ppc, but I guess s390x is > also impacted. Not really, as s390x does not support cpu unplug. > > This series tries to fix the issue by using object_ref() and object_unref() > in the monitor code. For this to work on ppc, some preliminary work is > needed to let QOM handle the CPU object lifecycle. > > Please comment. > > -- > Greg > > --- > > Greg Kurz (2): > spapr_cpu_core: instantiate CPUs separately > monitor: add proper reference counting of the current CPU > > > hw/ppc/spapr.c | 10 +++--- > hw/ppc/spapr_cpu_core.c | 29 + > include/hw/ppc/spapr_cpu_core.h |2 +- > monitor.c | 12 > 4 files changed, 25 insertions(+), 28 deletions(-) >
[Qemu-devel] [PATCH v2 0/3] MAINTAINERS: Small fixes
v2: Adopt Thomas' suggestions. Fam Zheng (3): MAINTAINERS: Clean up SCSI device section MAINTAINERS: Fix Sun4v file MAINTAINERS: Track default-configs/pci.mak MAINTAINERS | 9 ++--- 1 file changed, 2 insertions(+), 7 deletions(-) -- 2.13.5
[Qemu-devel] [PATCH v2 2/3] MAINTAINERS: Fix Sun4v file
Suggested-by: Thomas Huth Signed-off-by: Fam Zheng --- MAINTAINERS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 18ddaf4070..3938ccbf8a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -778,7 +778,7 @@ F: pc-bios/openbios-sparc64 Sun4v M: Artyom Tarasenko S: Maintained -F: hw/sparc64/sun4v.c +F: hw/sparc64/niagara.c F: hw/timer/sun4v-rtc.c F: include/hw/timer/sun4v-rtc.h -- 2.13.5
[Qemu-devel] [PATCH v2 3/3] MAINTAINERS: Track default-configs/pci.mak
Suggested-by: Thomas Huth Signed-off-by: Fam Zheng --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 3938ccbf8a..7cb237babc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -934,6 +934,7 @@ F: hw/pci/* F: hw/pci-bridge/* F: docs/pci* F: docs/specs/*pci* +F: default-configs/pci.mak ACPI/SMBIOS M: Michael S. Tsirkin -- 2.13.5
[Qemu-devel] [PATCH v2 1/3] MAINTAINERS: Clean up SCSI device section
1. Remove nonexistent file util/scsi*. 2. Drop useless section for LSI53C895A. 3. Leave include/scsi to "Block SCSI subsystem" section. Suggested-by: Thomas Huth Signed-off-by: Fam Zheng --- MAINTAINERS | 6 -- 1 file changed, 6 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 772ac209e1..18ddaf4070 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -980,16 +980,10 @@ SCSI M: Paolo Bonzini S: Supported F: include/hw/scsi/* -F: include/scsi/* F: hw/scsi/* -F: util/scsi* F: tests/virtio-scsi-test.c T: git git://github.com/bonzini/qemu.git scsi-next -LSI53C895A -S: Orphan -F: hw/scsi/lsi53c895a.c - SSI M: Peter Crosthwaite M: Alistair Francis -- 2.13.5
Re: [Qemu-devel] [PATCH v2 2/5] bcm2836: Use the Cortex-A7 instead of Cortex-A15
On Thu, 12 Oct 2017 17:07:28 -0700 Alistair Francis wrote: > The BCM2836 uses a Cortex-A7 not a Cortex-A15. Update the device to use > the correct CPU. > https://www.raspberrypi.org/documentation/hardware/raspberrypi/bcm2836/QA7_rev3.4.pdf > > Signed-off-by: Alistair Francis > --- > V2: > - Fix the BCM2836 CPU > > hw/arm/bcm2836.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c > index 8c43291112..931795a878 100644 > --- a/hw/arm/bcm2836.c > +++ b/hw/arm/bcm2836.c > @@ -30,7 +30,7 @@ static void bcm2836_init(Object *obj) > > for (n = 0; n < BCM2836_NCPUS; n++) { > object_initialize(&s->cpus[n], sizeof(s->cpus[n]), > - "cortex-a15-" TYPE_ARM_CPU); > + "cortex-a7-" TYPE_ARM_CPU); ARM_CPU_TYPE_NAME("cortex-a7") > object_property_add_child(obj, "cpu[*]", OBJECT(&s->cpus[n]), >&error_abort); > }
Re: [Qemu-devel] [PATCH v2 1/3] MAINTAINERS: Clean up SCSI device section
On 13.10.2017 10:53, Fam Zheng wrote: > 1. Remove nonexistent file util/scsi*. > 2. Drop useless section for LSI53C895A. > 3. Leave include/scsi to "Block SCSI subsystem" section. > > Suggested-by: Thomas Huth > Signed-off-by: Fam Zheng > --- > MAINTAINERS | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 772ac209e1..18ddaf4070 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -980,16 +980,10 @@ SCSI > M: Paolo Bonzini > S: Supported > F: include/hw/scsi/* > -F: include/scsi/* > F: hw/scsi/* > -F: util/scsi* > F: tests/virtio-scsi-test.c > T: git git://github.com/bonzini/qemu.git scsi-next > > -LSI53C895A > -S: Orphan > -F: hw/scsi/lsi53c895a.c > - > SSI > M: Peter Crosthwaite > M: Alistair Francis Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v2 0/5] Add a valid_cpu_types property
On Thu, 12 Oct 2017 17:07:23 -0700 Alistair Francis wrote: > There are numorous QEMU machines that only have a single or a handful of > valid CPU options. To simplyfy the management of specificying which CPU > is/isn't valid let's create a property that can be set in the machine > init. We can then check to see if the user supplied CPU is in that list > or not. > > I have added the valid_cpu_types for some ARM machines only at the > moment. do you plan to complete work for other boards as well so that new interface would replace current opencoded checks across the tree? > Here is what specifying the CPUs looks like now: > > $ aarch64-softmmu/qemu-system-aarch64 -M netduino2 -kernel ./u-boot.elf > -nographic -cpu "cortex-m3" -S > QEMU 2.10.50 monitor - type 'help' for more information > (qemu) info cpus > * CPU #0: thread_id=24175 > (qemu) q > > $ aarch64-softmmu/qemu-system-aarch64 -M netduino2 -kernel ./u-boot.elf > -nographic -cpu "cortex-m4" -S > QEMU 2.10.50 monitor - type 'help' for more information > (qemu) q > > $ aarch64-softmmu/qemu-system-aarch64 -M netduino2 -kernel ./u-boot.elf > -nographic -cpu "cortex-m5" -S > qemu-system-aarch64: unable to find CPU model 'cortex-m5' > > $ aarch64-softmmu/qemu-system-aarch64 -M netduino2 -kernel ./u-boot.elf > -nographic -cpu "cortex-a9" -S > qemu-system-aarch64: Invalid CPU type: cortex-a9-arm-cpu > The valid types are: cortex-m3-arm-cpu, cortex-m4-arm-cpu > > V2: > - Rebase > - Reorder patches > - Add a Raspberry Pi 2 CPU fix > V1: > - Small fixes to prepare a series instead of RFC > - Add commit messages for the commits > - Expand the machine support to ARM machines > RFC v2: > - Rebase on Igor's work > - Use more QEMUisms inside the code > - List the supported machines in a NULL terminated array > > Alistair Francis (5): > netduino2: Specify the valid CPUs > bcm2836: Use the Cortex-A7 instead of Cortex-A15 > raspi: Specify the valid CPUs > xlnx-zcu102: Specify the valid CPUs > xilinx_zynq: : Specify the valid CPUs > > hw/arm/bcm2836.c | 2 +- > hw/arm/netduino2.c | 10 +- > hw/arm/raspi.c | 7 +++ > hw/arm/xilinx_zynq.c | 6 ++ > hw/arm/xlnx-zcu102.c | 17 + > 5 files changed, 40 insertions(+), 2 deletions(-) >
Re: [Qemu-devel] [PATCH v2 2/3] MAINTAINERS: Fix Sun4v file
On 13.10.2017 10:53, Fam Zheng wrote: > Suggested-by: Thomas Huth > Signed-off-by: Fam Zheng > --- > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 18ddaf4070..3938ccbf8a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -778,7 +778,7 @@ F: pc-bios/openbios-sparc64 > Sun4v > M: Artyom Tarasenko > S: Maintained > -F: hw/sparc64/sun4v.c > +F: hw/sparc64/niagara.c > F: hw/timer/sun4v-rtc.c > F: include/hw/timer/sun4v-rtc.h > > Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v2 1/5] netduino2: Specify the valid CPUs
On Thu, 12 Oct 2017 17:07:25 -0700 Alistair Francis wrote: > List all possible valid CPU options. > > Although the board only ever has a Cortex-M3 we mark the Cortex-M4 as > supported because the Netduino2 Plus supports the Cortex-M4 and the > Netduino2 Plus is similar to the Netduino2. > > Signed-off-by: Alistair Francis > Reviewed-by: Eduardo Habkost > Reviewed-by: Philippe Mathieu-Daudé > --- > > V2: > - Fixup allignment > RFC v2: > - Use a NULL terminated list > - Add the Cortex-M4 for testing > > > hw/arm/netduino2.c | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c > index f936017d4a..6427552a72 100644 > --- a/hw/arm/netduino2.c > +++ b/hw/arm/netduino2.c > @@ -34,18 +34,26 @@ static void netduino2_init(MachineState *machine) > DeviceState *dev; > > dev = qdev_create(NULL, TYPE_STM32F205_SOC); > -qdev_prop_set_string(dev, "cpu-type", ARM_CPU_TYPE_NAME("cortex-m3")); > +qdev_prop_set_string(dev, "cpu-type", machine->cpu_type); > object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal); > > armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename, > FLASH_SIZE); > } > > +const char *netduino_valid_cpus[] = { may be add 'static' here and ditto in other patches > +ARM_CPU_TYPE_NAME("cortex-m3"), > +ARM_CPU_TYPE_NAME("cortex-m4"), > +NULL > +}; > + > static void netduino2_machine_init(MachineClass *mc) > { > mc->desc = "Netduino 2 Machine"; > mc->init = netduino2_init; > mc->ignore_memory_transaction_failures = true; > +mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-m3"); > +mc->valid_cpu_types = netduino_valid_cpus; > } > > DEFINE_MACHINE("netduino2", netduino2_machine_init)
Re: [Qemu-devel] [Xen-devel] [PATCH 3/8] xen: defer call to xen_restrict until just before os_setup_post
On 13/10/2017 09:37, Ross Lagerwall wrote: > On 10/09/2017 05:01 PM, Ian Jackson wrote: >> We need to restrict *all* the control fds that qemu opens. Looking in >> /proc/PID/fd shows there are many; their allocation seems scattered >> throughout Xen support code in qemu. >> >> We must postpone the restrict call until roughly the same time as qemu >> changes its uid, chroots (if applicable), and so on. >> >> There doesn't seem to be an appropriate hook already. The RunState >> change hook fires at different times depending on exactly what mode >> qemu is operating in. >> >> And it appears that no-one but the Xen code wants a hook at this phase >> of execution. So, introduce a bare call to a new function >> xen_setup_post, just before os_setup_post. Also provide the >> appropriate stub for when Xen compilation is disabled. >> >> We do the restriction before rather than after os_setup_post, because >> xen_restrict may need to open /dev/null, and os_setup_post might have >> called chroot. >> > This works for normally starting a VM but doesn't seem to work when > resuming/migrating. > > Here is the order of selected operations when starting a VM normally: > VNC server running on 127.0.0.1:5901 > xen_change_state_handler() > xenstore_record_dm_state: running > xen_setup_post() > xentoolcore_restrict_all: rc = 0 > os_setup_post() > main_loop() > > Here is the order of selected operations when starting QEMU with > -incoming fd:... : > VNC server running on 127.0.0.1:5902 > migration_fd_incoming() > xen_setup_post() > xentoolcore_restrict_all: rc = 0 > os_setup_post() > main_loop() > migration_set_incoming_channel() > migrate_set_state() > xen_change_state_handler() > xenstore_record_dm_state: running > error recording dm state > qemu exited with code 1 > > The issue is that QEMU needs xenstore access to write "running" but > this is after it has already been restricted. Moving xen_setup_post() > into xen_change_state_handler() works fine. The only issue is that in > the migration case, it executes after os_setup_post() so QEMU might be > chrooted in which case opening /dev/null to restrict fds doesn't work > (unless its new root has a /dev/null). > Wasn't the agreement in the end to remove all use of xenstore from the device mode? This running notification can and should be QMP, at which point we break a causal dependency. For safety reasons, qemu needs to have restricted/dropped/etc all permissions before it looks at a single byte of incoming migration data, to protect against buggy or malicious alterations to the migration stream. ~Andrew
Re: [Qemu-devel] [PATCH v2 3/3] MAINTAINERS: Track default-configs/pci.mak
On 13.10.2017 10:53, Fam Zheng wrote: > Suggested-by: Thomas Huth > Signed-off-by: Fam Zheng > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3938ccbf8a..7cb237babc 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -934,6 +934,7 @@ F: hw/pci/* > F: hw/pci-bridge/* > F: docs/pci* > F: docs/specs/*pci* > +F: default-configs/pci.mak > > ACPI/SMBIOS > M: Michael S. Tsirkin > Reviewed-by: Thomas Huth
Re: [Qemu-devel] [Xen-devel] [PATCH 3/8] xen: defer call to xen_restrict until just before os_setup_post
> -Original Message- > From: Xen-devel [mailto:xen-devel-boun...@lists.xen.org] On Behalf Of > Andrew Cooper > Sent: 13 October 2017 10:00 > To: Ross Lagerwall ; Ian Jackson > ; qemu-devel@nongnu.org > Cc: Anthony Perard ; Juergen Gross > ; Stefano Stabellini ; xen- > de...@lists.xenproject.org > Subject: Re: [Xen-devel] [PATCH 3/8] xen: defer call to xen_restrict until > just > before os_setup_post > > On 13/10/2017 09:37, Ross Lagerwall wrote: > > On 10/09/2017 05:01 PM, Ian Jackson wrote: > >> We need to restrict *all* the control fds that qemu opens. Looking in > >> /proc/PID/fd shows there are many; their allocation seems scattered > >> throughout Xen support code in qemu. > >> > >> We must postpone the restrict call until roughly the same time as qemu > >> changes its uid, chroots (if applicable), and so on. > >> > >> There doesn't seem to be an appropriate hook already. The RunState > >> change hook fires at different times depending on exactly what mode > >> qemu is operating in. > >> > >> And it appears that no-one but the Xen code wants a hook at this phase > >> of execution. So, introduce a bare call to a new function > >> xen_setup_post, just before os_setup_post. Also provide the > >> appropriate stub for when Xen compilation is disabled. > >> > >> We do the restriction before rather than after os_setup_post, because > >> xen_restrict may need to open /dev/null, and os_setup_post might have > >> called chroot. > >> > > This works for normally starting a VM but doesn't seem to work when > > resuming/migrating. > > > > Here is the order of selected operations when starting a VM normally: > > VNC server running on 127.0.0.1:5901 > > xen_change_state_handler() > > xenstore_record_dm_state: running > > xen_setup_post() > > xentoolcore_restrict_all: rc = 0 > > os_setup_post() > > main_loop() > > > > Here is the order of selected operations when starting QEMU with > > -incoming fd:... : > > VNC server running on 127.0.0.1:5902 > > migration_fd_incoming() > > xen_setup_post() > > xentoolcore_restrict_all: rc = 0 > > os_setup_post() > > main_loop() > > migration_set_incoming_channel() > > migrate_set_state() > > xen_change_state_handler() > > xenstore_record_dm_state: running > > error recording dm state > > qemu exited with code 1 > > > > The issue is that QEMU needs xenstore access to write "running" but > > this is after it has already been restricted. Moving xen_setup_post() > > into xen_change_state_handler() works fine. The only issue is that in > > the migration case, it executes after os_setup_post() so QEMU might be > > chrooted in which case opening /dev/null to restrict fds doesn't work > > (unless its new root has a /dev/null). > > > > Wasn't the agreement in the end to remove all use of xenstore from the > device mode? This running notification can and should be QMP, at which > point we break a causal dependency. > Yes, that was the agreement. One problem is that there is not yet adequate separation in either QEMU's common and pv/hvm init routines to do this yet. Nor, I believe, is there support in libxl to spawn separate xenpv and xenfv instances of QEMU for the same guest. Paul > For safety reasons, qemu needs to have restricted/dropped/etc all > permissions before it looks at a single byte of incoming migration data, > to protect against buggy or malicious alterations to the migration stream. > > ~Andrew > > ___ > Xen-devel mailing list > xen-de...@lists.xen.org > https://lists.xen.org/xen-devel
[Qemu-devel] German BSI analysed security of KVM / QEMU
Hi, the German Bundesamt für Sicherheit in der Informationstechnik (Federal Office for Information Security) published a study on the security of KVM and QEMU: https://www.bsi.bund.de/DE/Publikationen/Studien/Sicherheitsanalyse_KVM/sicherheitsanalyse_kvm.html (article only available in German) Summary (Google translated to English): "In summary, it can be said that the investigated components - above all KVM, QEMU and libvirt - are suitable for realizing a technically mature and secure virtualization environment." Regards Stefan Weil
[Qemu-devel] [Bug 1723161] Re: Migration failing in qemu-2.10.1 but working qemu-2.9.1 and earlier with same options
Hi William, David's fine (I'm from Red Hat) OK, so it is the pflash; if you get some debug, please add it to this bug, I'll ask some colleagues on the block side if they have any ideas. Dave -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1723161 Title: Migration failing in qemu-2.10.1 but working qemu-2.9.1 and earlier with same options Status in QEMU: New Bug description: Qemu-2.10.1 migration failing with the following error: Receiving block device images qemu-system-x86_64: error while loading section id 2(block) qemu-system-x86_64: load of migration failed: Input/output error Migration is setup on the destination system of the migration using: -incoming tcp:0: Migration is initiated from the source using the following commands in its qemu monitor: migrate -b "tcp:localhost:" The command-line used in both the source and destination is: qemu-system-x86_64 \ -nodefaults \ -pidfile vm0.pid \ -enable-kvm \ -machine q35 \ -cpu host -smp 2 \ -m 4096M \ -drive if=pflash,format=raw,unit=0,file=OVMF_CODE.fd,readonly \ -drive if=pflash,format=raw,unit=1,file=OVMF_VARS.fd \ -drive file=${HDRIVE},format=qcow2 \ -drive media=cdrom \ -usb -device usb-tablet \ -vga std -vnc :0 \ -net nic,macaddr=${TAPMACADDR} -net user,net=192.168.2.0/24,dhcpstart=192.168.2.10 \ -serial stdio \ -monitor unix:${MONITORSOCKET},server,nowait To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1723161/+subscriptions
Re: [Qemu-devel] [PATCH v2 2/3] MAINTAINERS: Fix Sun4v file
On Fri, Oct 13, 2017 at 10:58 AM, Thomas Huth wrote: > On 13.10.2017 10:53, Fam Zheng wrote: >> Suggested-by: Thomas Huth >> Signed-off-by: Fam Zheng >> --- >> MAINTAINERS | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 18ddaf4070..3938ccbf8a 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -778,7 +778,7 @@ F: pc-bios/openbios-sparc64 >> Sun4v >> M: Artyom Tarasenko >> S: Maintained >> -F: hw/sparc64/sun4v.c >> +F: hw/sparc64/niagara.c >> F: hw/timer/sun4v-rtc.c >> F: include/hw/timer/sun4v-rtc.h >> >> > > Reviewed-by: Thomas Huth Acked-By: Artyom Tarasenko -- Regards, Artyom Tarasenko SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
Re: [Qemu-devel] [PATCH 0/2] monitor: increase the refcount of the current CPU
On Fri, 13 Oct 2017 10:46:22 +0200 Cornelia Huck wrote: > On Fri, 13 Oct 2017 10:35:06 +0200 > Greg Kurz wrote: > > > If a CPU selected with the "cpu" command is hot-unplugged then "info cpus" > > causes QEMU to exit: > > > > (qemu) device_del cpu1 > > (qemu) info cpus > > qemu:qemu_cpu_kick_thread: No such process > > > > I could verify that this happens with x86 and ppc, but I guess s390x is > > also impacted. > > Not really, as s390x does not support cpu unplug. > I must admit I didn't check. Thanks for telling me. :) > > > > This series tries to fix the issue by using object_ref() and object_unref() > > in the monitor code. For this to work on ppc, some preliminary work is > > needed to let QOM handle the CPU object lifecycle. > > > > Please comment. > > > > -- > > Greg > > > > --- > > > > Greg Kurz (2): > > spapr_cpu_core: instantiate CPUs separately > > monitor: add proper reference counting of the current CPU > > > > > > hw/ppc/spapr.c | 10 +++--- > > hw/ppc/spapr_cpu_core.c | 29 + > > include/hw/ppc/spapr_cpu_core.h |2 +- > > monitor.c | 12 > > 4 files changed, 25 insertions(+), 28 deletions(-) > > >
Re: [Qemu-devel] [PATCH 1/2] spapr_cpu_core: instantiate CPUs separately
On Fri, 13 Oct 2017 10:35:19 +0200 Greg Kurz wrote: > The current code assumes that only the CPU core object holds a > reference on each individual CPU object, and happily frees their > allocated memory when the core is unrealized. This is dangerous > as some other code can legitimely keep a pointer to a CPU if it > calls object_ref(), but it would end up with a dangling pointer. > > Let's allocate all CPUs with object_new() and let QOM frees them > when their reference count reaches zero. I agree with patch as it simplifies code and one doesn't have to fiddle wit size anymore. See some comments below. > > Signed-off-by: Greg Kurz > --- > hw/ppc/spapr.c | 10 +++--- > hw/ppc/spapr_cpu_core.c | 29 + > include/hw/ppc/spapr_cpu_core.h |2 +- > 3 files changed, 13 insertions(+), 28 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index fd9813bde82f..ba391c6ed5de 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -3153,12 +3153,10 @@ void spapr_core_release(DeviceState *dev) > > if (smc->pre_2_10_has_unused_icps) { > sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); > -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc)); > -size_t size = object_type_get_instance_size(scc->cpu_type); > int i; > > for (i = 0; i < cc->nr_threads; i++) { > -CPUState *cs = CPU(sc->threads + i * size); > +CPUState *cs = CPU(sc->threads[i]); > > pre_2_10_vmstate_register_dummy_icp(cs->cpu_index); > } > @@ -3204,7 +3202,7 @@ static void spapr_core_plug(HotplugHandler > *hotplug_dev, DeviceState *dev, > sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc); > sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev)); > CPUCore *cc = CPU_CORE(dev); > -CPUState *cs = CPU(core->threads); > +CPUState *cs = CPU(core->threads[0]); > sPAPRDRConnector *drc; > Error *local_err = NULL; > int smt = kvmppc_smt_threads(); > @@ -3249,13 +3247,11 @@ static void spapr_core_plug(HotplugHandler > *hotplug_dev, DeviceState *dev, > core_slot->cpu = OBJECT(dev); > > if (smc->pre_2_10_has_unused_icps) { > -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc)); > -size_t size = object_type_get_instance_size(scc->cpu_type); > int i; > > for (i = 0; i < cc->nr_threads; i++) { > sPAPRCPUCore *sc = SPAPR_CPU_CORE(dev); > -void *obj = sc->threads + i * size; > +void *obj = sc->threads[i]; > > cs = CPU(obj); > pre_2_10_vmstate_unregister_dummy_icp(cs->cpu_index); > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 3a4c17401226..47c408b52ca1 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -79,21 +79,18 @@ const char *spapr_get_cpu_core_type(const char *cpu_type) > static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp) > { > sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); > -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); > -size_t size = object_type_get_instance_size(scc->cpu_type); > CPUCore *cc = CPU_CORE(dev); > int i; > > for (i = 0; i < cc->nr_threads; i++) { > -void *obj = sc->threads + i * size; > -DeviceState *dev = DEVICE(obj); > +DeviceState *dev = DEVICE(sc->threads[i]); > CPUState *cs = CPU(dev); > PowerPCCPU *cpu = POWERPC_CPU(cs); > > spapr_cpu_destroy(cpu); > object_unparent(cpu->intc); > cpu_remove_sync(cs); > -object_unparent(obj); > +object_unparent(sc->threads[i]); > } > g_free(sc->threads); > } > @@ -146,9 +143,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, > Error **errp) > sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); > sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); > CPUCore *cc = CPU_CORE(OBJECT(dev)); > -size_t size; > Error *local_err = NULL; > -void *obj; > int i, j; > > if (!spapr) { > @@ -156,17 +151,14 @@ static void spapr_cpu_core_realize(DeviceState *dev, > Error **errp) > return; > } > > -size = object_type_get_instance_size(scc->cpu_type); > -sc->threads = g_malloc0(size * cc->nr_threads); > +sc->threads = g_new(void *, cc->nr_threads); ^^^ PowerPCCPU* and maybe g_new0() if incomplete cores are possible. > for (i = 0; i < cc->nr_threads; i++) { > char id[32]; > CPUState *cs; > PowerPCCPU *cpu; > > -obj = sc->threads + i * size; > - > -object_initialize(obj, size, scc->cpu_type); > -cs = CPU(obj); > +sc->threads[i] = object_new(scc->cpu_type); > +cs = CPU(sc->threads[i]); > cpu = POWERPC_CPU(cs); > cs->cpu_index = cc->core_id + i; > cpu->vcpu_id = (cc->cor
Re: [Qemu-devel] [PATCH 2/2] monitor: add proper reference counting of the current CPU
On Fri, 13 Oct 2017 10:35:31 +0200 Greg Kurz wrote: > If a CPU selected with the "cpu" command is hot-unplugged then "info cpus" > causes QEMU to exit: > > (qemu) device_del cpu1 > (qemu) info cpus > qemu:qemu_cpu_kick_thread: No such process > > This happens because "cpu" stores the pointer to the selected CPU into > the monitor structure. When the CPU is hot-unplugged, we end up with a > dangling pointer. The "info cpus" command then does: > > hmp_info_cpus() > monitor_get_cpu_index() > mon_get_cpu() >cpu_synchronize_state() <--- called with dangling pointer > > This could cause a QEMU crash as well. > > This patch switches the monitor to use object_ref() to ensure the > CPU object doesn't vanish unexpectedly. The reference is dropped > either when "cpu" is used to switch to another CPU, or when the > selected CPU is unrealized and cpu_list_remove() sets its cpu_index > back to UNASSIGNED_CPU_INDEX. I don't really like an idea of leaving dangling cpu around. Is it possible to store QOM path on set_cpu in monitor and resolving it each to instance each time it's needed? > Signed-off-by: Greg Kurz > --- > monitor.c | 12 > 1 file changed, 12 insertions(+) > > diff --git a/monitor.c b/monitor.c > index fe0d1bdbb461..1c0b9a2c3ad3 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -579,6 +579,9 @@ static void monitor_data_init(Monitor *mon) > > static void monitor_data_destroy(Monitor *mon) > { > +if (mon->mon_cpu) { > +object_unref((Object *) mon->mon_cpu); > +} > qemu_chr_fe_deinit(&mon->chr, false); > if (monitor_is_qmp(mon)) { > json_message_parser_destroy(&mon->qmp.parser); > @@ -1047,12 +1050,21 @@ int monitor_set_cpu(int cpu_index) > if (cpu == NULL) { > return -1; > } > +if (cur_mon->mon_cpu) { > +object_unref((Object *) cur_mon->mon_cpu); > +} > cur_mon->mon_cpu = cpu; > +object_ref((Object *) cpu); > return 0; > } > > CPUState *mon_get_cpu(void) > { > +if (cur_mon->mon_cpu && > +cur_mon->mon_cpu->cpu_index == UNASSIGNED_CPU_INDEX) { > +object_unref((Object *) cur_mon->mon_cpu); > +cur_mon->mon_cpu = NULL; > +} > if (!cur_mon->mon_cpu) { > if (!first_cpu) { > return NULL; > >
Re: [Qemu-devel] [PATCH] docker: Don't allocate tty unless DEBUG=1
Fam Zheng writes: > The existence of tty in the container seems to urge gcc into colorize > the output, but the escape chars will clutter the report once turned > into email replies on patchew. Move -t to debug mode. > > Reported-by: Eric Blake > Signed-off-by: Fam Zheng I certainly improves my Emacs compilation-mode buffer when applied: Reviewed-by: Alex Bennée > --- > tests/docker/Makefile.include | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include > index 6f9ea196a7..ab939f2bec 100644 > --- a/tests/docker/Makefile.include > +++ b/tests/docker/Makefile.include > @@ -134,10 +134,10 @@ docker-run: docker-qemu-src > " COPYING $(EXECUTABLE) to $(IMAGE)")) > $(call quiet-command, \ > $(SRC_PATH)/tests/docker/docker.py run \ > - $(if $(NOUSER),,-u $(shell id -u)) -t \ > + $(if $(NOUSER),,-u $(shell id -u)) \ > --security-opt seccomp=unconfined \ > $(if $V,,--rm) \ > - $(if $(DEBUG),-i,) \ > + $(if $(DEBUG),-ti,) \ > $(if $(NETWORK),$(if $(subst > $(NETWORK),,1),--net=$(NETWORK)),--net=none) \ > -e TARGET_LIST=$(TARGET_LIST) \ > -e EXTRA_CONFIGURE_OPTS="$(EXTRA_CONFIGURE_OPTS)" \ -- Alex Bennée
Re: [Qemu-devel] German BSI analysed security of KVM / QEMU
On Fri, 13 Oct 2017 11:10:05 +0200 Stefan Weil wrote: > Hi, > > the German Bundesamt für Sicherheit in der Informationstechnik > (Federal Office for Information Security) published a study on > the security of KVM and QEMU: > > https://www.bsi.bund.de/DE/Publikationen/Studien/Sicherheitsanalyse_KVM/sicherheitsanalyse_kvm.html > > (article only available in German) Thanks for posting this! I only looked at the conclusion for now. Some interesting points: - They state that QEMU's source code is well structured, readable and maintainable. I wonder what kind of source code they usually deal with ;) - Most problems noted seemed to be related to signed<->unsigned conversions, but none were found to be exploitable. - They liked hardening via stack protection, NX, and ASLR, as well as the mechanisms used by libvirt. - They generally seemed to be happy with QEMU being deployed via libvirt. - Restrictions imposed via KVM (guest access to some CPU registers) scored positive points. They did not like that Hyper-V and PMU were not deconfigurable. - Lack of support for encryption/signing of network-based images was criticized. They ended up using Ceph and GlusterFS, which they were reasonably happy with. That's just from a quick browse.
Re: [Qemu-devel] [PATCH 1/2] spapr_cpu_core: instantiate CPUs separately
On Fri, 13 Oct 2017 11:21:20 +0200 Igor Mammedov wrote: > On Fri, 13 Oct 2017 10:35:19 +0200 > Greg Kurz wrote: > > > The current code assumes that only the CPU core object holds a > > reference on each individual CPU object, and happily frees their > > allocated memory when the core is unrealized. This is dangerous > > as some other code can legitimely keep a pointer to a CPU if it > > calls object_ref(), but it would end up with a dangling pointer. > > > > Let's allocate all CPUs with object_new() and let QOM frees them > > when their reference count reaches zero. > I agree with patch as it simplifies code and one doesn't have to > fiddle wit size anymore. Yes, I'll mention that in the changelog as well. > See some comments below. > > > > > Signed-off-by: Greg Kurz > > --- > > hw/ppc/spapr.c | 10 +++--- > > hw/ppc/spapr_cpu_core.c | 29 + > > include/hw/ppc/spapr_cpu_core.h |2 +- > > 3 files changed, 13 insertions(+), 28 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index fd9813bde82f..ba391c6ed5de 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -3153,12 +3153,10 @@ void spapr_core_release(DeviceState *dev) > > > > if (smc->pre_2_10_has_unused_icps) { > > sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); > > -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc)); > > -size_t size = object_type_get_instance_size(scc->cpu_type); > > int i; > > > > for (i = 0; i < cc->nr_threads; i++) { > > -CPUState *cs = CPU(sc->threads + i * size); > > +CPUState *cs = CPU(sc->threads[i]); > > > > pre_2_10_vmstate_register_dummy_icp(cs->cpu_index); > > } > > @@ -3204,7 +3202,7 @@ static void spapr_core_plug(HotplugHandler > > *hotplug_dev, DeviceState *dev, > > sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc); > > sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev)); > > CPUCore *cc = CPU_CORE(dev); > > -CPUState *cs = CPU(core->threads); > > +CPUState *cs = CPU(core->threads[0]); > > sPAPRDRConnector *drc; > > Error *local_err = NULL; > > int smt = kvmppc_smt_threads(); > > @@ -3249,13 +3247,11 @@ static void spapr_core_plug(HotplugHandler > > *hotplug_dev, DeviceState *dev, > > core_slot->cpu = OBJECT(dev); > > > > if (smc->pre_2_10_has_unused_icps) { > > -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc)); > > -size_t size = object_type_get_instance_size(scc->cpu_type); > > int i; > > > > for (i = 0; i < cc->nr_threads; i++) { > > sPAPRCPUCore *sc = SPAPR_CPU_CORE(dev); > > -void *obj = sc->threads + i * size; > > +void *obj = sc->threads[i]; > > > > cs = CPU(obj); > > pre_2_10_vmstate_unregister_dummy_icp(cs->cpu_index); > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > > index 3a4c17401226..47c408b52ca1 100644 > > --- a/hw/ppc/spapr_cpu_core.c > > +++ b/hw/ppc/spapr_cpu_core.c > > @@ -79,21 +79,18 @@ const char *spapr_get_cpu_core_type(const char > > *cpu_type) > > static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp) > > { > > sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); > > -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); > > -size_t size = object_type_get_instance_size(scc->cpu_type); > > CPUCore *cc = CPU_CORE(dev); > > int i; > > > > for (i = 0; i < cc->nr_threads; i++) { > > -void *obj = sc->threads + i * size; > > -DeviceState *dev = DEVICE(obj); > > +DeviceState *dev = DEVICE(sc->threads[i]); > > CPUState *cs = CPU(dev); > > PowerPCCPU *cpu = POWERPC_CPU(cs); > > > > spapr_cpu_destroy(cpu); > > object_unparent(cpu->intc); > > cpu_remove_sync(cs); > > -object_unparent(obj); > > +object_unparent(sc->threads[i]); > > } > > g_free(sc->threads); > > } > > @@ -146,9 +143,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, > > Error **errp) > > sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); > > sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); > > CPUCore *cc = CPU_CORE(OBJECT(dev)); > > -size_t size; > > Error *local_err = NULL; > > -void *obj; > > int i, j; > > > > if (!spapr) { > > @@ -156,17 +151,14 @@ static void spapr_cpu_core_realize(DeviceState *dev, > > Error **errp) > > return; > > } > > > > -size = object_type_get_instance_size(scc->cpu_type); > > -sc->threads = g_malloc0(size * cc->nr_threads); > > +sc->threads = g_new(void *, cc->nr_threads); > >^^^ PowerPCCPU* > Right, I guess void * was appropriate to do the pointer arithmetics but we don't need that anymore. > and maybe g_new0() if incomplete cores are possible. >
Re: [Qemu-devel] German BSI analysed security of KVM / QEMU
On Fri, Oct 13, 2017 at 11:37:08AM +0200, Cornelia Huck wrote: > On Fri, 13 Oct 2017 11:10:05 +0200 > Stefan Weil wrote: > > > Hi, > > > > the German Bundesamt für Sicherheit in der Informationstechnik > > (Federal Office for Information Security) published a study on > > the security of KVM and QEMU: > > > > https://www.bsi.bund.de/DE/Publikationen/Studien/Sicherheitsanalyse_KVM/sicherheitsanalyse_kvm.html > > > > (article only available in German) > > Thanks for posting this! > > I only looked at the conclusion for now. Some interesting points: > > - They state that QEMU's source code is well structured, readable and > maintainable. I wonder what kind of source code they usually deal > with ;) Most closed source apps are worse than even badly structured open source code IME ;-) > - Most problems noted seemed to be related to signed<->unsigned > conversions, but none were found to be exploitable. > - They liked hardening via stack protection, NX, and ASLR, as well as > the mechanisms used by libvirt. > - They generally seemed to be happy with QEMU being deployed via > libvirt. > - Restrictions imposed via KVM (guest access to some CPU registers) > scored positive points. They did not like that Hyper-V and PMU were > not deconfigurable. > - Lack of support for encryption/signing of network-based images was > criticized. They ended up using Ceph and GlusterFS, which they were > reasonably happy with. Hopefully the 'luks' driver (which can be layered over any block backend including network ones), and the TLS support for NBD would be considered to address this last point to some degree. At least from the encryption side. Signing of disk images is impractical as it would imply having to download the entire image contents to validate signature, rather defeating the point of having a network based image. But perhaps this is lost in translation and they mean something else by "signing of images" ? Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [RFC 2/2] KVM: add virtio-pmem driver
On Thu, Oct 12, 2017 at 09:20:26PM +0530, Pankaj Gupta wrote: > This patch adds virtio-pmem driver for KVM guest. > Guest reads the persistent memory range information > over virtio bus from Qemu and reserves the range > as persistent memory. Guest also allocates a block > device corresponding to the pmem range which later > can be accessed with DAX compatible file systems. > Idea is to use the virtio channel between guest and > host to perform the block device flush for guest pmem > DAX device. > > There is work to do including DAX file system support > and other advanced features. > > Signed-off-by: Pankaj Gupta > --- > drivers/virtio/Kconfig | 10 ++ > drivers/virtio/Makefile | 1 + > drivers/virtio/virtio_pmem.c | 322 > +++ > include/uapi/linux/virtio_pmem.h | 55 +++ > 4 files changed, 388 insertions(+) > create mode 100644 drivers/virtio/virtio_pmem.c > create mode 100644 include/uapi/linux/virtio_pmem.h > > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > index cff773f15b7e..0192c4bda54b 100644 > --- a/drivers/virtio/Kconfig > +++ b/drivers/virtio/Kconfig > @@ -38,6 +38,16 @@ config VIRTIO_PCI_LEGACY > > If unsure, say Y. > > +config VIRTIO_PMEM > + tristate "Virtio pmem driver" > + depends on VIRTIO > + ---help--- > + This driver adds persistent memory range within a KVM guest. > + It also associates a block device corresponding to the pmem > + range. > + > + If unsure, say M. > + > config VIRTIO_BALLOON > tristate "Virtio balloon driver" > depends on VIRTIO > diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile > index 41e30e3dc842..032ade725cc2 100644 > --- a/drivers/virtio/Makefile > +++ b/drivers/virtio/Makefile > @@ -5,3 +5,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o > virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o > obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o > obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o > +obj-$(CONFIG_VIRTIO_PMEM) += virtio_pmem.o > diff --git a/drivers/virtio/virtio_pmem.c b/drivers/virtio/virtio_pmem.c > new file mode 100644 > index ..74e47cae0e24 > --- /dev/null > +++ b/drivers/virtio/virtio_pmem.c > @@ -0,0 +1,322 @@ > +/* > + * virtio-pmem driver > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +void devm_vpmem_disable(struct device *dev, struct resource *res, void *addr) > +{ > + devm_memunmap(dev, addr); > + devm_release_mem_region(dev, res->start, resource_size(res)); > +} > + > +static void pmem_flush_done(struct virtqueue *vq) > +{ > + return; > +}; > + > +static void virtio_pmem_release_queue(void *q) > +{ > + blk_cleanup_queue(q); > +} > + > +static void virtio_pmem_freeze_queue(void *q) > +{ > + blk_freeze_queue_start(q); > +} > + > +static void virtio_pmem_release_disk(void *__pmem) > +{ > + struct virtio_pmem *pmem = __pmem; > + > + del_gendisk(pmem->disk); > + put_disk(pmem->disk); > +} > + > +static int init_vq(struct virtio_pmem *vpmem) > +{ > + struct virtqueue *vq; > + > + /* single vq */ > + vq = virtio_find_single_vq(vpmem->vdev, pmem_flush_done, "flush_queue"); > + > + if (IS_ERR(vq)) > + return PTR_ERR(vq); > + > + return 0; > +} > + > +static struct vmem_altmap *setup_pmem_pfn(struct virtio_pmem *vpmem, > + struct resource *res, struct vmem_altmap *altmap) > +{ > + u32 start_pad = 0, end_trunc = 0; > + resource_size_t start, size; > + unsigned long npfns; > + phys_addr_t offset; > + > + size = resource_size(res); > + start = PHYS_SECTION_ALIGN_DOWN(res->start); > + > + if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM, > + IORES_DESC_NONE) == REGION_MIXED) { > + > + start = res->start; > + start_pad = PHYS_SECTION_ALIGN_UP(start) - start; > + } > + start = res->start; > + size = PHYS_SECTION_ALIGN_UP(start + size) - start; > + > + if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM, > + IORES_DESC_NONE) == REGION_MIXED) { > + > + size = resource_size(res); > + end_trunc = start + size - > + PHYS_SECTION_ALIGN_DOWN(start + size); > + } > + > + start += start_pad; > + size = resource_size(res); > + npfns = PFN_SECTION_ALIGN_UP((size - start_pad - end_trunc - SZ_8K) > + / PAGE_SIZE); > + > + /* > + * vmemmap_populate_hugepages() allocates the memmap array in > + * HPAGE_SIZE chunks. > + */ > + offset = ALIGN(start + SZ_8K + 64 * npfns, HPAGE_SIZE) - start; > + vpmem->data_offset = offset; > + > + struct vmem_altmap __altmap = { > + .base_pfn = init_altmap_ba
Re: [Qemu-devel] [PATCH] docker: Don't allocate tty unless DEBUG=1
On Fri, 10/13 10:35, Alex Bennée wrote: > > Fam Zheng writes: > > > The existence of tty in the container seems to urge gcc into colorize > > the output, but the escape chars will clutter the report once turned > > into email replies on patchew. Move -t to debug mode. > > > > Reported-by: Eric Blake > > Signed-off-by: Fam Zheng > > I certainly improves my Emacs compilation-mode buffer when applied: > > Reviewed-by: Alex Bennée Nice, thanks! I'll send a pull request soon. Fam
Re: [Qemu-devel] German BSI analysed security of KVM / QEMU
On Fri, 13 Oct 2017 10:44:00 +0100 "Daniel P. Berrange" wrote: > On Fri, Oct 13, 2017 at 11:37:08AM +0200, Cornelia Huck wrote: > > On Fri, 13 Oct 2017 11:10:05 +0200 > > Stefan Weil wrote: > > > > > Hi, > > > > > > the German Bundesamt für Sicherheit in der Informationstechnik > > > (Federal Office for Information Security) published a study on > > > the security of KVM and QEMU: > > > > > > https://www.bsi.bund.de/DE/Publikationen/Studien/Sicherheitsanalyse_KVM/sicherheitsanalyse_kvm.html > > > > > > (article only available in German) > > > > Thanks for posting this! > > > > I only looked at the conclusion for now. Some interesting points: > > > > - They state that QEMU's source code is well structured, readable and > > maintainable. I wonder what kind of source code they usually deal > > with ;) > > Most closed source apps are worse than even badly structured open > source code IME ;-) Ha, that's true from my experience as well ;) > > > - Most problems noted seemed to be related to signed<->unsigned > > conversions, but none were found to be exploitable. > > - They liked hardening via stack protection, NX, and ASLR, as well as > > the mechanisms used by libvirt. > > - They generally seemed to be happy with QEMU being deployed via > > libvirt. > > - Restrictions imposed via KVM (guest access to some CPU registers) > > scored positive points. They did not like that Hyper-V and PMU were > > not deconfigurable. > > - Lack of support for encryption/signing of network-based images was > > criticized. They ended up using Ceph and GlusterFS, which they were > > reasonably happy with. > > Hopefully the 'luks' driver (which can be layered over any block backend > including network ones), and the TLS support for NBD would be considered > to address this last point to some degree. At least from the encryption > side. > > Signing of disk images is impractical as it would imply having to download > the entire image contents to validate signature, rather defeating the point > of having a network based image. But perhaps this is lost in translation > and they mean something else by "signing of images" ? Could be my bad translation (they talk about "Verschlüsselung und Signierung"), but I haven't looked what they actually tried to accomplish.
Re: [Qemu-devel] [PULL 00/11] Ui 20171013 patches
On Fri, Oct 13, 2017 at 10:14:39AM +0200, Gerd Hoffmann wrote: > The following changes since commit bac960832015bf4c4c1b873011612e2675e4464c: > > Merge remote-tracking branch 'remotes/elmarco/tags/vus-pull-request' into > staging (2017-10-11 13:10:36 +0100) > > are available in the git repository at: > > git://git.kraxel.org/qemu tags/ui-20171013-pull-request > > for you to fetch changes up to 942a35335b2efa2f2997d51eb5142fc9903efe43: > > gtk: fix wrong id between texture and framebuffer (2017-10-13 10:10:36 > +0200) > > > ui: use keycodemapdb for key code mappings, part one (v2) > ui: add qemu-keymap, update reverse keymaps (for qemu -k $map) > ui: fix for vte 0.50 > ui: gtk texture fix > > I'm afraid this is quite likely to fail build smoke test again. I've just tried a build on OpenBSD with the bash -> sh fix in it, and I found that it still tried to build the keycodemap files in parallel with checking out the GIT submodules. It also tried to run the mkdir -p dtc/libfdt in parallel and this caused git to refuse to checkout the dtc module due to that empty dir existing. So there's still some deps problems in here I think that let make build in the wrong order :-( GEN config-host.h GEN module_block.h GEN ui/input-keymap-linux-to-qcode.c GEN ui/input-keymap-qcode-to-qnum.c GEN ui/input-keymap-qnum-to-qcode.c python: can't open file '/home/berrange/src/virt/qemu/ui/keycodemapdb/tools/keymap-gen': [Errno 2] No such file or directory python: can't open file '/home/berrange/src/virt/qemu/ui/keycodemapdb/tools/keymap-gen': [Errno 2] No such file or directory python: can't open file '/home/berrange/src/virt/qemu/ui/keycodemapdb/tools/keymap-gen': [Errno 2] No such file or directory mkdir -p dtc/libfdt GIT ui/keycodemapdb dtc mkdir -p dtc/tests This should be impossible because ui/input-keymap-* depends on the tools/keymap-gen, and that depends on the submodule status file, which in turn should trigger the checkout, but the build log showed that isn't working as we expect. In all my debugging the one thing I've seen work correctly is the re-running of configure (via config.status), which always happens earlier and is serialized wrt everything else. So I wonder if we should change direction slightly, and have configure checkout the submodules. Then just make sure that config.status is triggered when submodules are out of date. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 2/2] monitor: add proper reference counting of the current CPU
On Fri, 13 Oct 2017 11:24:59 +0200 Igor Mammedov wrote: > On Fri, 13 Oct 2017 10:35:31 +0200 > Greg Kurz wrote: > > > If a CPU selected with the "cpu" command is hot-unplugged then "info cpus" > > causes QEMU to exit: > > > > (qemu) device_del cpu1 > > (qemu) info cpus > > qemu:qemu_cpu_kick_thread: No such process > > > > This happens because "cpu" stores the pointer to the selected CPU into > > the monitor structure. When the CPU is hot-unplugged, we end up with a > > dangling pointer. The "info cpus" command then does: > > > > hmp_info_cpus() > > monitor_get_cpu_index() > > mon_get_cpu() > >cpu_synchronize_state() <--- called with dangling pointer > > > > This could cause a QEMU crash as well. > > > > This patch switches the monitor to use object_ref() to ensure the > > CPU object doesn't vanish unexpectedly. The reference is dropped > > either when "cpu" is used to switch to another CPU, or when the > > selected CPU is unrealized and cpu_list_remove() sets its cpu_index > > back to UNASSIGNED_CPU_INDEX. > I don't really like an idea of leaving dangling cpu around. > Is it possible to store QOM path on set_cpu in monitor and > resolving it each to instance each time it's needed? > It sounds workable. Also it would allow the fix to not depend on patch 1 for ppc. Thanks for the suggestion! > > > Signed-off-by: Greg Kurz > > --- > > monitor.c | 12 > > 1 file changed, 12 insertions(+) > > > > diff --git a/monitor.c b/monitor.c > > index fe0d1bdbb461..1c0b9a2c3ad3 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -579,6 +579,9 @@ static void monitor_data_init(Monitor *mon) > > > > static void monitor_data_destroy(Monitor *mon) > > { > > +if (mon->mon_cpu) { > > +object_unref((Object *) mon->mon_cpu); > > +} > > qemu_chr_fe_deinit(&mon->chr, false); > > if (monitor_is_qmp(mon)) { > > json_message_parser_destroy(&mon->qmp.parser); > > @@ -1047,12 +1050,21 @@ int monitor_set_cpu(int cpu_index) > > if (cpu == NULL) { > > return -1; > > } > > +if (cur_mon->mon_cpu) { > > +object_unref((Object *) cur_mon->mon_cpu); > > +} > > cur_mon->mon_cpu = cpu; > > +object_ref((Object *) cpu); > > return 0; > > } > > > > CPUState *mon_get_cpu(void) > > { > > +if (cur_mon->mon_cpu && > > +cur_mon->mon_cpu->cpu_index == UNASSIGNED_CPU_INDEX) { > > +object_unref((Object *) cur_mon->mon_cpu); > > +cur_mon->mon_cpu = NULL; > > +} > > if (!cur_mon->mon_cpu) { > > if (!first_cpu) { > > return NULL; > > > > >
[Qemu-devel] [Bug 1723161] Re: Migration failing in qemu-2.10.1 but working qemu-2.9.1 and earlier with same options
Hi William, Just to confirm: are both src and dest on the same host, using the same command line options? Then I'm confused why you need block migration because they share the same image files (for OVMF pflash and ${HDRIVE}). -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1723161 Title: Migration failing in qemu-2.10.1 but working qemu-2.9.1 and earlier with same options Status in QEMU: New Bug description: Qemu-2.10.1 migration failing with the following error: Receiving block device images qemu-system-x86_64: error while loading section id 2(block) qemu-system-x86_64: load of migration failed: Input/output error Migration is setup on the destination system of the migration using: -incoming tcp:0: Migration is initiated from the source using the following commands in its qemu monitor: migrate -b "tcp:localhost:" The command-line used in both the source and destination is: qemu-system-x86_64 \ -nodefaults \ -pidfile vm0.pid \ -enable-kvm \ -machine q35 \ -cpu host -smp 2 \ -m 4096M \ -drive if=pflash,format=raw,unit=0,file=OVMF_CODE.fd,readonly \ -drive if=pflash,format=raw,unit=1,file=OVMF_VARS.fd \ -drive file=${HDRIVE},format=qcow2 \ -drive media=cdrom \ -usb -device usb-tablet \ -vga std -vnc :0 \ -net nic,macaddr=${TAPMACADDR} -net user,net=192.168.2.0/24,dhcpstart=192.168.2.10 \ -serial stdio \ -monitor unix:${MONITORSOCKET},server,nowait To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1723161/+subscriptions
[Qemu-devel] [PATCH] configure: pick the right compiler for OpenBSD by default
The system compiler in OpenBSD is gcc 4.2.1 which is too old for our needs. If doing 'pkg_add gcc' you can get a much newer version (4.9.4 in OpenBSD 6.1) which works with QEMU. This installs binaries with two naming schemes: $ pkg_info -L gcc | grep bin /usr/local/bin/ecpp /usr/local/bin/egcc /usr/local/bin/egcc-ar /usr/local/bin/egcc-nm /usr/local/bin/egcc-ranlib /usr/local/bin/egcov /usr/local/bin/x86_64-unknown-openbsd6.0-egcc /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-ar /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-nm /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-ranlib /usr/local/bin/x86_64-unknown-openbsd6.0-gcc-4.9.3 We pick the short name this it won't change across OpenBSD releases. This means users don't need to manually pass custom --cc and --cxx args to configure to avoid immediate failure. Signed-off-by: Daniel P. Berrange --- configure | 18 -- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 06f18ea9af..fcb7523933 100755 --- a/configure +++ b/configure @@ -255,7 +255,21 @@ cross_prefix="" audio_drv_list="" block_drv_rw_whitelist="" block_drv_ro_whitelist="" -host_cc="cc" + +case `uname -s` in +OpenBSD) +# Default system cc in OpenBSD is unsufficient +# we need the 'gcc' pkg added, whch provides +# these modified binary names +host_cc="egcc" +host_cxx="eg++" +;; + *) +host_cc="cc" +host_cxx="c++" +;; +esac + libs_softmmu="" libs_tools="" audio_pt_int="" @@ -466,7 +480,7 @@ else fi if test -z "${CXX}${cross_prefix}"; then - cxx="c++" + cxx="$host_cxx" else cxx="${CXX-${cross_prefix}g++}" fi -- 2.13.5
Re: [Qemu-devel] German BSI analysed security of KVM / QEMU
Am 13.10.2017 um 11:37 schrieb Cornelia Huck: > On Fri, 13 Oct 2017 11:10:05 +0200 > Stefan Weil wrote: > >> Hi, >> >> the German Bundesamt für Sicherheit in der Informationstechnik >> (Federal Office for Information Security) published a study on >> the security of KVM and QEMU: >> >> https://www.bsi.bund.de/DE/Publikationen/Studien/Sicherheitsanalyse_KVM/sicherheitsanalyse_kvm.html >> >> (article only available in German) > Thanks for posting this! > > I only looked at the conclusion for now. Some interesting points: > > - They state that QEMU's source code is well structured, readable and > maintainable. I wonder what kind of source code they usually deal > with ;) > - Most problems noted seemed to be related to signed<->unsigned > conversions, but none were found to be exploitable. > - They liked hardening via stack protection, NX, and ASLR, as well as > the mechanisms used by libvirt. > - They generally seemed to be happy with QEMU being deployed via > libvirt. > - Restrictions imposed via KVM (guest access to some CPU registers) > scored positive points. They did not like that Hyper-V and PMU were > not deconfigurable. > - Lack of support for encryption/signing of network-based images was > criticized. They ended up using Ceph and GlusterFS, which they were > reasonably happy with. > > That's just from a quick browse. I already found some weaknesses in the study: * It makes the wrong assumption that all maintainers have write access to the git repository. (chapter 5.3) * It does not mention important quality measures like Coverity Scan, Continuous Integration and automated tests. So QEMU is even better than the BSI thinks. :-) Stefan
Re: [Qemu-devel] [PATCH 3/8] xen: defer call to xen_restrict until just before os_setup_post
Ross Lagerwall writes ("Re: [PATCH 3/8] xen: defer call to xen_restrict until just before os_setup_post"): > This works for normally starting a VM but doesn't seem to work when > resuming/migrating. > > Here is the order of selected operations when starting a VM normally: > VNC server running on 127.0.0.1:5901 > xen_change_state_handler() > xenstore_record_dm_state: running > xen_setup_post() > xentoolcore_restrict_all: rc = 0 > os_setup_post() > main_loop() > > Here is the order of selected operations when starting QEMU with > -incoming fd:... : > VNC server running on 127.0.0.1:5902 > migration_fd_incoming() > xen_setup_post() > xentoolcore_restrict_all: rc = 0 > os_setup_post() > main_loop() > migration_set_incoming_channel() > migrate_set_state() > xen_change_state_handler() > xenstore_record_dm_state: running > error recording dm state > qemu exited with code 1 > > The issue is that QEMU needs xenstore access to write "running" but this > is after it has already been restricted. Moving xen_setup_post() into > xen_change_state_handler() works fine. The only issue is that in the > migration case, it executes after os_setup_post() so QEMU might be > chrooted in which case opening /dev/null to restrict fds doesn't work > (unless its new root has a /dev/null). Thanks for the extensive diagnosis. We do in fact want the restriction to occur before the migration stream is read. This is because we are trying to defend against a guest which can exploit a bug in qemu. That means that the sending qemu must be assumed to be compromised. In this context I don't think qemu's migration stream receiver can be regarded as hardened against hostile input; it's far too complicated, even if efforts have been made in that direction (I haven't checked). So to avoid the receiving qemu being compromised while still unrestricted, we should restrict before starting to read the migration stream. The correct fix is to use a different technique to notify the toolstack that qemu is up and running. That obviously requires changes on the Xen tools side. I will look into that for the Xen 4.11 release cycle. Ian.
Re: [Qemu-devel] [RFC 2/2] KVM: add virtio-pmem driver
> > On Thu, Oct 12, 2017 at 09:20:26PM +0530, Pankaj Gupta wrote: > > This patch adds virtio-pmem driver for KVM guest. > > Guest reads the persistent memory range information > > over virtio bus from Qemu and reserves the range > > as persistent memory. Guest also allocates a block > > device corresponding to the pmem range which later > > can be accessed with DAX compatible file systems. > > Idea is to use the virtio channel between guest and > > host to perform the block device flush for guest pmem > > DAX device. > > > > There is work to do including DAX file system support > > and other advanced features. > > > > Signed-off-by: Pankaj Gupta > > --- > > drivers/virtio/Kconfig | 10 ++ > > drivers/virtio/Makefile | 1 + > > drivers/virtio/virtio_pmem.c | 322 > > +++ > > include/uapi/linux/virtio_pmem.h | 55 +++ > > 4 files changed, 388 insertions(+) > > create mode 100644 drivers/virtio/virtio_pmem.c > > create mode 100644 include/uapi/linux/virtio_pmem.h > > > > diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig > > index cff773f15b7e..0192c4bda54b 100644 > > --- a/drivers/virtio/Kconfig > > +++ b/drivers/virtio/Kconfig > > @@ -38,6 +38,16 @@ config VIRTIO_PCI_LEGACY > > > > If unsure, say Y. > > > > +config VIRTIO_PMEM > > + tristate "Virtio pmem driver" > > + depends on VIRTIO > > + ---help--- > > +This driver adds persistent memory range within a KVM guest. > > + It also associates a block device corresponding to the pmem > > +range. > > + > > +If unsure, say M. > > + > > config VIRTIO_BALLOON > > tristate "Virtio balloon driver" > > depends on VIRTIO > > diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile > > index 41e30e3dc842..032ade725cc2 100644 > > --- a/drivers/virtio/Makefile > > +++ b/drivers/virtio/Makefile > > @@ -5,3 +5,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o > > virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o > > obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o > > obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o > > +obj-$(CONFIG_VIRTIO_PMEM) += virtio_pmem.o > > diff --git a/drivers/virtio/virtio_pmem.c b/drivers/virtio/virtio_pmem.c > > new file mode 100644 > > index ..74e47cae0e24 > > --- /dev/null > > +++ b/drivers/virtio/virtio_pmem.c > > @@ -0,0 +1,322 @@ > > +/* > > + * virtio-pmem driver > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +void devm_vpmem_disable(struct device *dev, struct resource *res, void > > *addr) > > +{ > > + devm_memunmap(dev, addr); > > + devm_release_mem_region(dev, res->start, resource_size(res)); > > +} > > + > > +static void pmem_flush_done(struct virtqueue *vq) > > +{ > > + return; > > +}; > > + > > +static void virtio_pmem_release_queue(void *q) > > +{ > > + blk_cleanup_queue(q); > > +} > > + > > +static void virtio_pmem_freeze_queue(void *q) > > +{ > > + blk_freeze_queue_start(q); > > +} > > + > > +static void virtio_pmem_release_disk(void *__pmem) > > +{ > > + struct virtio_pmem *pmem = __pmem; > > + > > + del_gendisk(pmem->disk); > > + put_disk(pmem->disk); > > +} > > + > > +static int init_vq(struct virtio_pmem *vpmem) > > +{ > > + struct virtqueue *vq; > > + > > + /* single vq */ > > + vq = virtio_find_single_vq(vpmem->vdev, pmem_flush_done, "flush_queue"); > > + > > + if (IS_ERR(vq)) > > + return PTR_ERR(vq); > > + > > + return 0; > > +} > > + > > +static struct vmem_altmap *setup_pmem_pfn(struct virtio_pmem *vpmem, > > + struct resource *res, struct vmem_altmap *altmap) > > +{ > > + u32 start_pad = 0, end_trunc = 0; > > + resource_size_t start, size; > > + unsigned long npfns; > > + phys_addr_t offset; > > + > > + size = resource_size(res); > > + start = PHYS_SECTION_ALIGN_DOWN(res->start); > > + > > + if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM, > > + IORES_DESC_NONE) == REGION_MIXED) { > > + > > + start = res->start; > > + start_pad = PHYS_SECTION_ALIGN_UP(start) - start; > > + } > > + start = res->start; > > + size = PHYS_SECTION_ALIGN_UP(start + size) - start; > > + > > + if (region_intersects(start, size, IORESOURCE_SYSTEM_RAM, > > + IORES_DESC_NONE) == REGION_MIXED) { > > + > > + size = resource_size(res); > > + end_trunc = start + size - > > + PHYS_SECTION_ALIGN_DOWN(start + size); > > + } > > + > > + start += start_pad; > > + size = resource_size(res); > > + npfns = PFN_SECTION_ALIGN_UP((size - start_pad - end_trunc - SZ_8K) > > + / PAGE_SIZE); > > + > > + /* > > + * vmemmap_populate_hugepages() allocates the memmap array in > > + * HPA
[Qemu-devel] [Bug 1723161] Re: Migration failing in qemu-2.10.1 but working qemu-2.9.1 and earlier with same options
I can reproduce this: ./x86_64-softmmu/qemu-system-x86_64 -nographic -M q35 -drive id=d,file=/home/vmimages/dummy1,if=none,readonly -device virtio- blk,id=vb1,drive=d -drive if=pflash,format=raw,unit=0,file=/usr/share/edk2/ovmf/OVMF_CODE.secboot.fd,readonly -drive if=pflash,format=raw,unit=1,file=vars1.fd to: ./x86_64-softmmu/qemu-system-x86_64 -M q35 -nographic -drive id=d,file=/home/vmimages/dummy2,if=none,readonly -device virtio-blk,id=vb1,drive=d -drive if=pflash,format=raw,unit=0,file=romfile,readonly -drive if=pflash,format=raw,unit=1,file=vars2.fd -incoming tcp:: then migrate -d -b tcp:0: note I copied the vars file and rom file separately so it's not a locking issue -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1723161 Title: Migration failing in qemu-2.10.1 but working qemu-2.9.1 and earlier with same options Status in QEMU: New Bug description: Qemu-2.10.1 migration failing with the following error: Receiving block device images qemu-system-x86_64: error while loading section id 2(block) qemu-system-x86_64: load of migration failed: Input/output error Migration is setup on the destination system of the migration using: -incoming tcp:0: Migration is initiated from the source using the following commands in its qemu monitor: migrate -b "tcp:localhost:" The command-line used in both the source and destination is: qemu-system-x86_64 \ -nodefaults \ -pidfile vm0.pid \ -enable-kvm \ -machine q35 \ -cpu host -smp 2 \ -m 4096M \ -drive if=pflash,format=raw,unit=0,file=OVMF_CODE.fd,readonly \ -drive if=pflash,format=raw,unit=1,file=OVMF_VARS.fd \ -drive file=${HDRIVE},format=qcow2 \ -drive media=cdrom \ -usb -device usb-tablet \ -vga std -vnc :0 \ -net nic,macaddr=${TAPMACADDR} -net user,net=192.168.2.0/24,dhcpstart=192.168.2.10 \ -serial stdio \ -monitor unix:${MONITORSOCKET},server,nowait To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1723161/+subscriptions
Re: [Qemu-devel] [PATCH] configure: pick the right compiler for OpenBSD by default
On 13.10.2017 12:28, Daniel P. Berrange wrote: > The system compiler in OpenBSD is gcc 4.2.1 which is too > old for our needs. If doing 'pkg_add gcc' you can get a > much newer version (4.9.4 in OpenBSD 6.1) which works with > QEMU. This installs binaries with two naming schemes: > > $ pkg_info -L gcc | grep bin > /usr/local/bin/ecpp > /usr/local/bin/egcc > /usr/local/bin/egcc-ar > /usr/local/bin/egcc-nm > /usr/local/bin/egcc-ranlib > /usr/local/bin/egcov > /usr/local/bin/x86_64-unknown-openbsd6.0-egcc > /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-ar > /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-nm > /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-ranlib > /usr/local/bin/x86_64-unknown-openbsd6.0-gcc-4.9.3 > > We pick the short name this it won't change across OpenBSD > releases. > > This means users don't need to manually pass custom --cc > and --cxx args to configure to avoid immediate failure. > > Signed-off-by: Daniel P. Berrange > --- > configure | 18 -- > 1 file changed, 16 insertions(+), 2 deletions(-) > > diff --git a/configure b/configure > index 06f18ea9af..fcb7523933 100755 > --- a/configure > +++ b/configure > @@ -255,7 +255,21 @@ cross_prefix="" > audio_drv_list="" > block_drv_rw_whitelist="" > block_drv_ro_whitelist="" > -host_cc="cc" > + > +case `uname -s` in > +OpenBSD) > +# Default system cc in OpenBSD is unsufficient s/unsufficient/insufficient/ > +# we need the 'gcc' pkg added, whch provides s/whch/which/ > +# these modified binary names > +host_cc="egcc" > +host_cxx="eg++" > +;; > + *) > +host_cc="cc" > +host_cxx="c++" > +;; > +esac Do we really need such work-arounds in our configure script? GCC 4.2 is really vry old nowadays, so if the OpenBSD folks refuse to update the default in their distro, IMHO they should be punished by having to select the C compiler manually everywhere. Thomas
Re: [Qemu-devel] [PATCH] configure: pick the right compiler for OpenBSD by default
On 13.10.2017 12:52, Thomas Huth wrote: > On 13.10.2017 12:28, Daniel P. Berrange wrote: >> The system compiler in OpenBSD is gcc 4.2.1 which is too >> old for our needs. If doing 'pkg_add gcc' you can get a >> much newer version (4.9.4 in OpenBSD 6.1) which works with >> QEMU. This installs binaries with two naming schemes: >> >> $ pkg_info -L gcc | grep bin >> /usr/local/bin/ecpp >> /usr/local/bin/egcc >> /usr/local/bin/egcc-ar >> /usr/local/bin/egcc-nm >> /usr/local/bin/egcc-ranlib >> /usr/local/bin/egcov >> /usr/local/bin/x86_64-unknown-openbsd6.0-egcc >> /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-ar >> /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-nm >> /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-ranlib >> /usr/local/bin/x86_64-unknown-openbsd6.0-gcc-4.9.3 >> >> We pick the short name this it won't change across OpenBSD >> releases. >> >> This means users don't need to manually pass custom --cc >> and --cxx args to configure to avoid immediate failure. >> >> Signed-off-by: Daniel P. Berrange >> --- >> configure | 18 -- >> 1 file changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/configure b/configure >> index 06f18ea9af..fcb7523933 100755 >> --- a/configure >> +++ b/configure >> @@ -255,7 +255,21 @@ cross_prefix="" >> audio_drv_list="" >> block_drv_rw_whitelist="" >> block_drv_ro_whitelist="" >> -host_cc="cc" >> + >> +case `uname -s` in >> +OpenBSD) >> +# Default system cc in OpenBSD is unsufficient > > s/unsufficient/insufficient/ > >> +# we need the 'gcc' pkg added, whch provides > > s/whch/which/ > >> +# these modified binary names >> +host_cc="egcc" >> +host_cxx="eg++" >> +;; >> + *) >> +host_cc="cc" >> +host_cxx="c++" >> +;; >> +esac > > Do we really need such work-arounds in our configure script? GCC 4.2 is > really vry old nowadays, so if the OpenBSD folks refuse to update > the default in their distro, IMHO they should be punished by having to > select the C compiler manually everywhere. By the way, looks like OpenBSD is also switching to clang by default soon: https://www.phoronix.com/scan.php?page=news_item&px=OpenBSD-Default-Clang Thomas
Re: [Qemu-devel] [PATCH v3] tpm: Use EMSGSIZE instead of EBADMSG to compile on OpenBSD
On 10/11/2017 03:47 PM, Stefan Berger wrote: EBADMSG was only added to OpenBSD very recently. To make QEMU compilable on older OpenBSD versions use EMSGSIZE instead when a mismatch between number of received bytes and message size indicated in the header was found. Return -EMSGSIZE and convert all other errnos in the same functions to return the negative errno. Signed-off-by: Stefan Berger Can someone have a look at this, please? Stefan --- hw/tpm/tpm_util.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c index fb929f6..73d7796 100644 --- a/hw/tpm/tpm_util.c +++ b/hw/tpm/tpm_util.c @@ -68,10 +68,10 @@ static int tpm_util_test(int fd, n = write(fd, request, requestlen); if (n < 0) { -return errno; +return -errno; } if (n != requestlen) { -return EFAULT; +return -EFAULT; } FD_ZERO(&readfds); @@ -80,18 +80,18 @@ static int tpm_util_test(int fd, /* wait for a second */ n = select(fd + 1, &readfds, NULL, NULL, &tv); if (n != 1) { -return errno; +return -errno; } n = read(fd, &buf, sizeof(buf)); if (n < sizeof(struct tpm_resp_hdr)) { -return EFAULT; +return -EFAULT; } resp = (struct tpm_resp_hdr *)buf; /* check the header */ if (be32_to_cpu(resp->len) != n) { -return EBADMSG; +return -EMSGSIZE; } *return_tag = be16_to_cpu(resp->tag);
Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest
On 10/13/17 10:44 +0200, Igor Mammedov wrote: > On Fri, 13 Oct 2017 15:53:26 +0800 > Haozhong Zhang wrote: > > > On 10/12/17 17:45 +0200, Paolo Bonzini wrote: > > > On 12/10/2017 14:45, Haozhong Zhang wrote: > > > > Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and > > > > /rom@etc/table-loader. The former is unstructured to guest, and > > > > contains all data of guest ACPI. The latter is a BIOSLinkerLoader > > > > organized as a set of commands, which direct the guest (e.g., SeaBIOS > > > > on KVM/QEMU) to relocate data in the former file, recalculate checksum > > > > of specified area, and fill guest address in specified ACPI field. > > > > > > > > One part of my patches is to implement a mechanism to tell Xen which > > > > part of ACPI data is a table (NFIT), and which part defines a > > > > namespace device and what the device name is. I can add two new loader > > > > commands for them respectively. > > > > > > > > Because they just provide information and SeaBIOS in non-xen > > > > environment ignores unrecognized commands, they will not break SeaBIOS > > > > in non-xen environment. > > > > > > > > On QEMU side, most Xen-specific hacks in ACPI builder could be > > > > dropped, and replaced by adding the new loader commands (though they > > > > may be used only by Xen). > > > > > > > > On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor > > > > are needed in, perhaps, hvmloader. > > > > > > If Xen has to parse BIOSLinkerLoader, it can use the existing commands > > > to process a reduced set of ACPI tables. In other words, > > > etc/acpi/tables would only include the NFIT, the SSDT with namespace > > > devices, and the XSDT. etc/acpi/rsdp would include the RSDP table as > > > usual. > > > > > > hvmloader can then: > > > > > > 1) allocate some memory for where the XSDT will go > > > > > > 2) process the BIOSLinkerLoader like SeaBIOS would do > > > > > > 3) find the RSDP in low memory, since the loader script must have placed > > > it there. If it cannot find it, allocate some low memory, fill it with > > > the RSDP header and revision, and and jump to step 6 > > > > > > 4) If it found QEMU's RSDP, use it to find QEMU's XSDT > > > > > > 5) Copy ACPI table pointers from QEMU to hvmloader's RSDT and/or XSDT. > > > > > > 6) build hvmloader tables and link them into the RSDT and/or XSDT as > > > usual. > > > > > > 7) overwrite the RSDP in low memory with a pointer to hvmloader's own > > > RSDT and/or XSDT, and updated the checksums > > > > > > QEMU's XSDT remains there somewhere in memory, unused but harmless. > > > > +1 to Paolo's suggestion, i.e. > 1. add BIOSLinkerLoader into hvmloader > 2. load/process QEMU's tables with #1 > 3. get pointers to QEMU generated NFIT and NVDIMM SSDT from QEMU's RSDT/XSDT > and put them in hvmloader's RSDT > > > It can work for plan tables which do not contain AML. > > > > However, for a namespace device, Xen needs to know its name in order > > to detect the potential name conflict with those used in Xen built > > ACPI. Xen does not (and is not going to) introduce an AML parser, so > > it cannot get those device names from QEMU built ACPI by its own. > > > > The idea of either this patch series or the new BIOSLinkerLoader > > command is to let QEMU tell Xen where the definition body of a > > namespace device (i.e. that part within the outmost "Device(NAME)") is > > and what the device name is. Xen, after the name conflict check, can > > re-package the definition body in a namespace device (w/ minimal AML > > builder code added in Xen) and then in SSDT. > > I'd skip conflict check at runtime as hvmloader doesn't currently > have "\\_SB\NVDR" device so instead of doing runtime check it might > do primitive check at build time that ASL sources in hvmloader do > not contain reserved for QEMU "NVDR" keyword to avoid its addition > by accident in future. (it also might be reused in future if some > other tables from QEMU will be reused). > It's a bit hackinsh but at least it does the job and keeps > BIOSLinkerLoader interface the same for all supported firmwares > (I'd consider it as a temporary hack on the way to fully build > by QEMU ACPI tables for Xen). > > Ideally it would be better for QEMU to build all ACPI tables for > hvmloader to avoid split brain issues and need to invent extra > interfaces every time a feature is added to pass configuration > data from QEMU to firmware. > But that's probably out of scope of this project, it could be > done on top of this if Xen folks would like to do it. Adding > BIOSLinkerLoader to hvmloader would be a good starting point > for that future effort. If we can let QEMU build the entire guest ACPI, we may even not need to introduce fw_cfg and BIOSLinkerLoader code to hvmloader. SeaBIOS is currently loaded after hvmloader and can be used to load QEMU built ACPI. To Jan, Andrew, Stefano and Anthony, what do you think about allowing QEMU to build the entire guest A
Re: [Qemu-devel] [PATCH v3] tpm: Use EMSGSIZE instead of EBADMSG to compile on OpenBSD
Hi On Wed, Oct 11, 2017 at 9:47 PM, Stefan Berger wrote: > EBADMSG was only added to OpenBSD very recently. To make QEMU compilable > on older OpenBSD versions use EMSGSIZE instead when a mismatch between > number of received bytes and message size indicated in the header was > found. > > Return -EMSGSIZE and convert all other errnos in the same functions to > return the negative errno. > > Signed-off-by: Stefan Berger Reviewed-by: Marc-André Lureau Looks good to me, but given that the return value isn't used, perhaps you could have changed the function to return a bool success instead? > --- > hw/tpm/tpm_util.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c > index fb929f6..73d7796 100644 > --- a/hw/tpm/tpm_util.c > +++ b/hw/tpm/tpm_util.c > @@ -68,10 +68,10 @@ static int tpm_util_test(int fd, > > n = write(fd, request, requestlen); > if (n < 0) { > -return errno; > +return -errno; > } > if (n != requestlen) { > -return EFAULT; > +return -EFAULT; > } > > FD_ZERO(&readfds); > @@ -80,18 +80,18 @@ static int tpm_util_test(int fd, > /* wait for a second */ > n = select(fd + 1, &readfds, NULL, NULL, &tv); > if (n != 1) { > -return errno; > +return -errno; > } > > n = read(fd, &buf, sizeof(buf)); > if (n < sizeof(struct tpm_resp_hdr)) { > -return EFAULT; > +return -EFAULT; > } > > resp = (struct tpm_resp_hdr *)buf; > /* check the header */ > if (be32_to_cpu(resp->len) != n) { > -return EBADMSG; > +return -EMSGSIZE; > } > > *return_tag = be16_to_cpu(resp->tag); > -- > 2.5.5 > -- Marc-André Lureau
[Qemu-devel] [Bug 1723161] Re: Migration failing in qemu-2.10.1 but working qemu-2.9.1 and earlier with same options
i added some debug, and I'm seeing: blk_pwrite failed for pflash1 0 @ 0 cluster_size=1048576 ret=-5 so that's an EIO. The EIO is coming from blk_check_byte_request because it's trying to write a 1MB cluster size chunk, yet the var file is only ~128k -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1723161 Title: Migration failing in qemu-2.10.1 but working qemu-2.9.1 and earlier with same options Status in QEMU: New Bug description: Qemu-2.10.1 migration failing with the following error: Receiving block device images qemu-system-x86_64: error while loading section id 2(block) qemu-system-x86_64: load of migration failed: Input/output error Migration is setup on the destination system of the migration using: -incoming tcp:0: Migration is initiated from the source using the following commands in its qemu monitor: migrate -b "tcp:localhost:" The command-line used in both the source and destination is: qemu-system-x86_64 \ -nodefaults \ -pidfile vm0.pid \ -enable-kvm \ -machine q35 \ -cpu host -smp 2 \ -m 4096M \ -drive if=pflash,format=raw,unit=0,file=OVMF_CODE.fd,readonly \ -drive if=pflash,format=raw,unit=1,file=OVMF_VARS.fd \ -drive file=${HDRIVE},format=qcow2 \ -drive media=cdrom \ -usb -device usb-tablet \ -vga std -vnc :0 \ -net nic,macaddr=${TAPMACADDR} -net user,net=192.168.2.0/24,dhcpstart=192.168.2.10 \ -serial stdio \ -monitor unix:${MONITORSOCKET},server,nowait To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1723161/+subscriptions
[Qemu-devel] [PATCH] spapr_cpu_core: instantiate CPUs separately
The current code assumes that only the CPU core object holds a reference on each individual CPU object, and happily frees their allocated memory when the core is unrealized. This is dangerous as some other code can legitimely keep a pointer to a CPU if it calls object_ref(), but it would end up with a dangling pointer. Let's allocate all CPUs with object_new() and let QOM frees them when their reference count reaches zero. This greatly simplify the code as we don't have to fiddle with the instance size anymore. Signed-off-by: Greg Kurz --- v2: - mention code simplification in changelog - use PowerPCCPU * and Object * instead of void * --- hw/ppc/spapr.c | 11 +++ hw/ppc/spapr_cpu_core.c | 19 +++ include/hw/ppc/spapr_cpu_core.h |2 +- 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index fd9813bde82f..d9555a3677be 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -3153,12 +3153,10 @@ void spapr_core_release(DeviceState *dev) if (smc->pre_2_10_has_unused_icps) { sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc)); -size_t size = object_type_get_instance_size(scc->cpu_type); int i; for (i = 0; i < cc->nr_threads; i++) { -CPUState *cs = CPU(sc->threads + i * size); +CPUState *cs = CPU(sc->threads[i]); pre_2_10_vmstate_register_dummy_icp(cs->cpu_index); } @@ -3204,7 +3202,7 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev, sPAPRMachineClass *smc = SPAPR_MACHINE_CLASS(mc); sPAPRCPUCore *core = SPAPR_CPU_CORE(OBJECT(dev)); CPUCore *cc = CPU_CORE(dev); -CPUState *cs = CPU(core->threads); +CPUState *cs = CPU(core->threads[0]); sPAPRDRConnector *drc; Error *local_err = NULL; int smt = kvmppc_smt_threads(); @@ -3249,15 +3247,12 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, DeviceState *dev, core_slot->cpu = OBJECT(dev); if (smc->pre_2_10_has_unused_icps) { -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc)); -size_t size = object_type_get_instance_size(scc->cpu_type); int i; for (i = 0; i < cc->nr_threads; i++) { sPAPRCPUCore *sc = SPAPR_CPU_CORE(dev); -void *obj = sc->threads + i * size; -cs = CPU(obj); +cs = CPU(sc->threads[i]); pre_2_10_vmstate_unregister_dummy_icp(cs->cpu_index); } } diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index 3a4c17401226..588f9b45714a 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -79,13 +79,11 @@ const char *spapr_get_cpu_core_type(const char *cpu_type) static void spapr_cpu_core_unrealizefn(DeviceState *dev, Error **errp) { sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); -sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); -size_t size = object_type_get_instance_size(scc->cpu_type); CPUCore *cc = CPU_CORE(dev); int i; for (i = 0; i < cc->nr_threads; i++) { -void *obj = sc->threads + i * size; +Object *obj = OBJECT(sc->threads[i]); DeviceState *dev = DEVICE(obj); CPUState *cs = CPU(dev); PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -146,9 +144,8 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev)); sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev)); CPUCore *cc = CPU_CORE(OBJECT(dev)); -size_t size; Error *local_err = NULL; -void *obj; +Object *obj; int i, j; if (!spapr) { @@ -156,18 +153,16 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) return; } -size = object_type_get_instance_size(scc->cpu_type); -sc->threads = g_malloc0(size * cc->nr_threads); +sc->threads = g_new(PowerPCCPU *, cc->nr_threads); for (i = 0; i < cc->nr_threads; i++) { char id[32]; CPUState *cs; PowerPCCPU *cpu; -obj = sc->threads + i * size; +obj = object_new(scc->cpu_type); -object_initialize(obj, size, scc->cpu_type); cs = CPU(obj); -cpu = POWERPC_CPU(cs); +cpu = sc->threads[i] = POWERPC_CPU(obj); cs->cpu_index = cc->core_id + i; cpu->vcpu_id = (cc->core_id * spapr->vsmt / smp_threads) + i; if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) { @@ -192,7 +187,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) } for (j = 0; j < cc->nr_threads; j++) { -obj = sc->threads + j * size; +obj = OBJECT(sc->threads[j]); spapr_cpu_core_realize_child(obj, spapr, &local_err); if (local_err) { @@ -203,7 +198,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error
Re: [Qemu-devel] [PATCH] configure: pick the right compiler for OpenBSD by default
On Fri, Oct 13, 2017 at 12:52:26PM +0200, Thomas Huth wrote: > On 13.10.2017 12:28, Daniel P. Berrange wrote: > > The system compiler in OpenBSD is gcc 4.2.1 which is too > > old for our needs. If doing 'pkg_add gcc' you can get a > > much newer version (4.9.4 in OpenBSD 6.1) which works with > > QEMU. This installs binaries with two naming schemes: > > > > $ pkg_info -L gcc | grep bin > > /usr/local/bin/ecpp > > /usr/local/bin/egcc > > /usr/local/bin/egcc-ar > > /usr/local/bin/egcc-nm > > /usr/local/bin/egcc-ranlib > > /usr/local/bin/egcov > > /usr/local/bin/x86_64-unknown-openbsd6.0-egcc > > /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-ar > > /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-nm > > /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-ranlib > > /usr/local/bin/x86_64-unknown-openbsd6.0-gcc-4.9.3 > > > > We pick the short name this it won't change across OpenBSD > > releases. > > > > This means users don't need to manually pass custom --cc > > and --cxx args to configure to avoid immediate failure. > > > > Signed-off-by: Daniel P. Berrange > > --- > > configure | 18 -- > > 1 file changed, 16 insertions(+), 2 deletions(-) > > > > diff --git a/configure b/configure > > index 06f18ea9af..fcb7523933 100755 > > --- a/configure > > +++ b/configure > > @@ -255,7 +255,21 @@ cross_prefix="" > > audio_drv_list="" > > block_drv_rw_whitelist="" > > block_drv_ro_whitelist="" > > -host_cc="cc" > > + > > +case `uname -s` in > > +OpenBSD) > > +# Default system cc in OpenBSD is unsufficient > > s/unsufficient/insufficient/ > > > +# we need the 'gcc' pkg added, whch provides > > s/whch/which/ > > > +# these modified binary names > > +host_cc="egcc" > > +host_cxx="eg++" > > +;; > > + *) > > +host_cc="cc" > > +host_cxx="c++" > > +;; > > +esac > > Do we really need such work-arounds in our configure script? GCC 4.2 is > really vry old nowadays, so if the OpenBSD folks refuse to update > the default in their distro, IMHO they should be punished by having to > select the C compiler manually everywhere. I really object to such a user hostile POV. It is not beneficial to QEMU to punish our users for choices they make, and our punishment will do nothing to encourage OpenBSD distro maintainers to switch their compiler. This is a straightforward change to 'configure' that makes it 'just work' with no special args on current OpenBSD releases. Making it 'just work' is a goal we have for every aspect of the configure script, and there's no reason why we shouldn't try this for OpenBSD, as we do for other platforms, unless we're going to drop OpenBSD entirely. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH] configure: pick the right compiler for OpenBSD by default
On Fri, Oct 13, 2017 at 12:55:40PM +0200, Thomas Huth wrote: > On 13.10.2017 12:52, Thomas Huth wrote: > > On 13.10.2017 12:28, Daniel P. Berrange wrote: > >> The system compiler in OpenBSD is gcc 4.2.1 which is too > >> old for our needs. If doing 'pkg_add gcc' you can get a > >> much newer version (4.9.4 in OpenBSD 6.1) which works with > >> QEMU. This installs binaries with two naming schemes: > >> > >> $ pkg_info -L gcc | grep bin > >> /usr/local/bin/ecpp > >> /usr/local/bin/egcc > >> /usr/local/bin/egcc-ar > >> /usr/local/bin/egcc-nm > >> /usr/local/bin/egcc-ranlib > >> /usr/local/bin/egcov > >> /usr/local/bin/x86_64-unknown-openbsd6.0-egcc > >> /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-ar > >> /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-nm > >> /usr/local/bin/x86_64-unknown-openbsd6.0-egcc-ranlib > >> /usr/local/bin/x86_64-unknown-openbsd6.0-gcc-4.9.3 > >> > >> We pick the short name this it won't change across OpenBSD > >> releases. > >> > >> This means users don't need to manually pass custom --cc > >> and --cxx args to configure to avoid immediate failure. > >> > >> Signed-off-by: Daniel P. Berrange > >> --- > >> configure | 18 -- > >> 1 file changed, 16 insertions(+), 2 deletions(-) > >> > >> diff --git a/configure b/configure > >> index 06f18ea9af..fcb7523933 100755 > >> --- a/configure > >> +++ b/configure > >> @@ -255,7 +255,21 @@ cross_prefix="" > >> audio_drv_list="" > >> block_drv_rw_whitelist="" > >> block_drv_ro_whitelist="" > >> -host_cc="cc" > >> + > >> +case `uname -s` in > >> +OpenBSD) > >> +# Default system cc in OpenBSD is unsufficient > > > > s/unsufficient/insufficient/ > > > >> +# we need the 'gcc' pkg added, whch provides > > > > s/whch/which/ > > > >> +# these modified binary names > >> +host_cc="egcc" > >> +host_cxx="eg++" > >> +;; > >> + *) > >> +host_cc="cc" > >> +host_cxx="c++" > >> +;; > >> +esac > > > > Do we really need such work-arounds in our configure script? GCC 4.2 is > > really vry old nowadays, so if the OpenBSD folks refuse to update > > the default in their distro, IMHO they should be punished by having to > > select the C compiler manually everywhere. > > By the way, looks like OpenBSD is also switching to clang by default soon: > > https://www.phoronix.com/scan.php?page=news_item&px=OpenBSD-Default-Clang In a few years time we could potentially revert this patch, but in the meantime it is clearly beneficial for anyone using OpenBSD and has no significant maint burden for us to carry it while there are widely supported OpenBSD releases which need it. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
[Qemu-devel] [PULL v3 01/11] tpm: Use EMSGSIZE instead of EBADMSG to compile on OpenBSD
EBADMSG was only added to OpenBSD very recently. To make QEMU compilable on older OpenBSD versions use EMSGSIZE instead when a mismatch between number of received bytes and message size indicated in the header was found. Return -EMSGSIZE and convert all other errnos in the same functions to return the negative errno. Signed-off-by: Stefan Berger Reviewed-by: Marc-André Lureau --- hw/tpm/tpm_util.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c index 7b35429..4484207 100644 --- a/hw/tpm/tpm_util.c +++ b/hw/tpm/tpm_util.c @@ -43,10 +43,10 @@ static int tpm_util_test(int fd, n = write(fd, request, requestlen); if (n < 0) { -return errno; +return -errno; } if (n != requestlen) { -return EFAULT; +return -EFAULT; } FD_ZERO(&readfds); @@ -55,18 +55,18 @@ static int tpm_util_test(int fd, /* wait for a second */ n = select(fd + 1, &readfds, NULL, NULL, &tv); if (n != 1) { -return errno; +return -errno; } n = read(fd, &buf, sizeof(buf)); if (n < sizeof(struct tpm_resp_hdr)) { -return EFAULT; +return -EFAULT; } resp = (struct tpm_resp_hdr *)buf; /* check the header */ if (be32_to_cpu(resp->len) != n) { -return EBADMSG; +return -EMSGSIZE; } *return_tag = be16_to_cpu(resp->tag); -- 2.5.5
[Qemu-devel] [PULL v3 00/11] Merge tpm 2017/10/04
The following changes since commit f90ea7ba7c5ae7010ee0ce062207ae42530f57d6: Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20171012' into staging (2017-10-12 17:06:50 +0100) are available in the git repository at: git://github.com/stefanberger/qemu-tpm.git tags/pull-tpm-2017-10-04-3 for you to fetch changes up to 8dc67017220109fd5bc9d2bffa73674595f62e08: specs: Describe the TPM support in QEMU (2017-10-13 07:34:33 -0400) Merge tpm 2017/10/04 v3 Amarnath Valluri (9): tpm-backend: Remove unneeded member variable from backend class tpm-backend: Move thread handling inside TPMBackend tpm-backend: Initialize and free data members in it's own methods tpm-backend: Made few interface methods optional tpm-backend: Add new API to read backend TpmInfo tpm-backend: Move realloc_buffer() implementation to tpm-tis model tpm-passthrough: move reusable code to utils tpm: Added support for TPM emulator tpm: Move tpm_cleanup() to right place Stefan Berger (2): tpm: Use EMSGSIZE instead of EBADMSG to compile on OpenBSD specs: Describe the TPM support in QEMU backends/tpm.c | 115 -- configure| 13 +- docs/specs/tpm.txt | 123 +++ hmp.c| 5 + hw/tpm/Makefile.objs | 1 + hw/tpm/tpm_emulator.c| 587 hw/tpm/tpm_ioctl.h | 246 ++ hw/tpm/tpm_passthrough.c | 242 +++--- hw/tpm/tpm_tis.c | 14 +- hw/tpm/tpm_util.c| 35 - hw/tpm/tpm_util.h| 4 + include/sysemu/tpm_backend.h | 80 +- include/sysemu/tpm_backend_int.h | 41 - qapi/tpm.json| 21 ++- qemu-options.hx | 22 ++- tpm.c| 37 + vl.c | 1 + 17 files changed, 1216 insertions(+), 371 deletions(-) create mode 100644 docs/specs/tpm.txt create mode 100644 hw/tpm/tpm_emulator.c create mode 100644 hw/tpm/tpm_ioctl.h delete mode 100644 include/sysemu/tpm_backend_int.h -- 2.5.5
[Qemu-devel] [PULL v3 02/11] tpm-backend: Remove unneeded member variable from backend class
From: Amarnath Valluri TPMDriverOps inside TPMBackend is not required, as it is supposed to be a class member. The only possible reason for keeping in TPMBackend was, to get the backend type in tpm.c where dedicated backend api, tpm_backend_get_type() is present. Signed-off-by: Amarnath Valluri Reviewed-by: Marc-André Lureau Reviewed-by: Stefan Berger Signed-off-by: Stefan Berger --- hw/tpm/tpm_passthrough.c | 4 include/sysemu/tpm_backend.h | 1 - tpm.c| 2 +- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index 9234eb3..a0baf5f 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -46,8 +46,6 @@ #define TPM_PASSTHROUGH(obj) \ OBJECT_CHECK(TPMPassthruState, (obj), TYPE_TPM_PASSTHROUGH) -static const TPMDriverOps tpm_passthrough_driver; - /* data structures */ typedef struct TPMPassthruThreadParams { TPMState *tpm_state; @@ -462,8 +460,6 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id) /* let frontend set the fe_model to proper value */ tb->fe_model = -1; -tb->ops = &tpm_passthrough_driver; - if (tpm_passthrough_handle_device_opts(opts, tb)) { goto err_exit; } diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h index b0a9731..3708413 100644 --- a/include/sysemu/tpm_backend.h +++ b/include/sysemu/tpm_backend.h @@ -50,7 +50,6 @@ struct TPMBackend { enum TpmModel fe_model; char *path; char *cancel_path; -const TPMDriverOps *ops; QLIST_ENTRY(TPMBackend) list; }; diff --git a/tpm.c b/tpm.c index 2d830d0..abedf3f 100644 --- a/tpm.c +++ b/tpm.c @@ -211,7 +211,7 @@ static TPMInfo *qmp_query_tpm_inst(TPMBackend *drv) res->model = drv->fe_model; res->options = g_new0(TpmTypeOptions, 1); -switch (drv->ops->type) { +switch (tpm_backend_get_type(drv)) { case TPM_TYPE_PASSTHROUGH: res->options->type = TPM_TYPE_OPTIONS_KIND_PASSTHROUGH; tpo = g_new0(TPMPassthroughOptions, 1); -- 2.5.5
[Qemu-devel] [PULL v3 08/11] tpm-passthrough: move reusable code to utils
From: Amarnath Valluri Signed-off-by: Amarnath Valluri Reviewed-by: Stefan Berger Reviewed-by: Marc-André Lureau Signed-off-by: Stefan Berger --- hw/tpm/tpm_passthrough.c | 64 hw/tpm/tpm_util.c| 25 +++ hw/tpm/tpm_util.h| 4 +++ 3 files changed, 34 insertions(+), 59 deletions(-) diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index 22d3460..e6ace28 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -68,27 +68,6 @@ typedef struct TPMPassthruState TPMPassthruState; static void tpm_passthrough_cancel_cmd(TPMBackend *tb); -static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, uint32_t len) -{ -int ret, remain; - -remain = len; -while (remain > 0) { -ret = write(fd, buf, remain); -if (ret < 0) { -if (errno != EINTR && errno != EAGAIN) { -return -1; -} -} else if (ret == 0) { -break; -} else { -buf += ret; -remain -= ret; -} -} -return len - remain; -} - static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len) { int ret; @@ -102,45 +81,12 @@ static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len) } return ret; } - -static uint32_t tpm_passthrough_get_size_from_buffer(const uint8_t *buf) -{ -struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)buf; - -return be32_to_cpu(resp->len); -} - -/* - * Write an error message in the given output buffer. - */ -static void tpm_write_fatal_error_response(uint8_t *out, uint32_t out_len) -{ -if (out_len >= sizeof(struct tpm_resp_hdr)) { -struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out; - -resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND); -resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr)); -resp->errcode = cpu_to_be32(TPM_FAIL); -} -} - -static bool tpm_passthrough_is_selftest(const uint8_t *in, uint32_t in_len) -{ -struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in; - -if (in_len >= sizeof(*hdr)) { -return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest); -} - -return false; -} - static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt, const uint8_t *in, uint32_t in_len, uint8_t *out, uint32_t out_len, bool *selftest_done) { -int ret; +ssize_t ret; bool is_selftest; const struct tpm_resp_hdr *hdr; @@ -148,9 +94,9 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt, tpm_pt->tpm_executing = true; *selftest_done = false; -is_selftest = tpm_passthrough_is_selftest(in, in_len); +is_selftest = tpm_util_is_selftest(in, in_len); -ret = tpm_passthrough_unix_write(tpm_pt->tpm_fd, in, in_len); +ret = qemu_write_full(tpm_pt->tpm_fd, (const void *)in, (size_t)in_len); if (ret != in_len) { if (!tpm_pt->tpm_op_canceled || errno != ECANCELED) { error_report("tpm_passthrough: error while transmitting data " @@ -170,7 +116,7 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt, strerror(errno), errno); } } else if (ret < sizeof(struct tpm_resp_hdr) || - tpm_passthrough_get_size_from_buffer(out) != ret) { + be32_to_cpu(((struct tpm_resp_hdr *)out)->len) != ret) { ret = -1; error_report("tpm_passthrough: received invalid response " "packet from TPM"); @@ -183,7 +129,7 @@ static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt, err_exit: if (ret < 0) { -tpm_write_fatal_error_response(out, out_len); +tpm_util_write_fatal_error_response(out, out_len); } tpm_pt->tpm_executing = false; diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c index 4484207..73d7796 100644 --- a/hw/tpm/tpm_util.c +++ b/hw/tpm/tpm_util.c @@ -24,6 +24,31 @@ #include "tpm_int.h" /* + * Write an error message in the given output buffer. + */ +void tpm_util_write_fatal_error_response(uint8_t *out, uint32_t out_len) +{ +if (out_len >= sizeof(struct tpm_resp_hdr)) { +struct tpm_resp_hdr *resp = (struct tpm_resp_hdr *)out; + +resp->tag = cpu_to_be16(TPM_TAG_RSP_COMMAND); +resp->len = cpu_to_be32(sizeof(struct tpm_resp_hdr)); +resp->errcode = cpu_to_be32(TPM_FAIL); +} +} + +bool tpm_util_is_selftest(const uint8_t *in, uint32_t in_len) +{ +struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in; + +if (in_len >= sizeof(*hdr)) { +return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest); +} + +return false; +} + +/* * A basic test of a TPM device. We expect a well formatted response header * (error response is fine) within one second. */ diff --git a/hw/tpm/tpm_uti
[Qemu-devel] [PULL v3 09/11] tpm: Added support for TPM emulator
From: Amarnath Valluri This change introduces a new TPM backend driver that can communicate with swtpm(software TPM emulator) using unix domain socket interface. QEMU talks to the TPM emulator using QEMU's socket-based chardev backend device. Swtpm uses two Unix sockets for communications, one for plain TPM commands and responses, and one for out-of-band control messages. QEMU passes the data socket to be used over the control channel. The swtpm and associated tools can be found here: https://github.com/stefanberger/swtpm The swtpm's control channel protocol specification can be found here: https://github.com/stefanberger/swtpm/wiki/Control-Channel-Specification Usage: # setup TPM state directory mkdir /tmp/mytpm chown -R tss:root /tmp/mytpm /usr/bin/swtpm_setup --tpm-state /tmp/mytpm --createek # Ask qemu to use TPM emulator with given tpm state directory qemu-system-x86_64 \ [...] \ -chardev socket,id=chrtpm,path=/tmp/swtpm-sock \ -tpmdev emulator,id=tpm0,chardev=chrtpm \ -device tpm-tis,tpmdev=tpm0 \ [...] Signed-off-by: Amarnath Valluri Reviewed-by: Marc-André Lureau Tested-by: Stefan Berger Signed-off-by: Stefan Berger --- configure | 13 +- hmp.c | 5 + hw/tpm/Makefile.objs | 1 + hw/tpm/tpm_emulator.c | 587 ++ hw/tpm/tpm_ioctl.h| 246 + qapi/tpm.json | 21 +- qemu-options.hx | 22 +- 7 files changed, 888 insertions(+), 7 deletions(-) create mode 100644 hw/tpm/tpm_emulator.c create mode 100644 hw/tpm/tpm_ioctl.h diff --git a/configure b/configure index 6587e80..3c733f1 100755 --- a/configure +++ b/configure @@ -3495,6 +3495,12 @@ else tpm_passthrough=no fi +# TPM emulator is for all posix systems +if test "$mingw32" != "yes"; then + tpm_emulator=$tpm +else + tpm_emulator=no +fi ## # attr probe @@ -5412,6 +5418,7 @@ echo "gcov enabled $gcov" echo "TPM support $tpm" echo "libssh2 support $libssh2" echo "TPM passthrough $tpm_passthrough" +echo "TPM emulator $tpm_emulator" echo "QOM debugging $qom_cast_debug" echo "Live block migration $live_block_migration" echo "lzo support $lzo" @@ -6011,12 +6018,16 @@ if test "$live_block_migration" = "yes" ; then echo "CONFIG_LIVE_BLOCK_MIGRATION=y" >> $config_host_mak fi -# TPM passthrough support? if test "$tpm" = "yes"; then echo 'CONFIG_TPM=$(CONFIG_SOFTMMU)' >> $config_host_mak + # TPM passthrough support? if test "$tpm_passthrough" = "yes"; then echo "CONFIG_TPM_PASSTHROUGH=y" >> $config_host_mak fi + # TPM emulator support? + if test "$tpm_emulator" = "yes"; then +echo "CONFIG_TPM_EMULATOR=y" >> $config_host_mak + fi fi echo "TRACE_BACKENDS=$trace_backends" >> $config_host_mak diff --git a/hmp.c b/hmp.c index 739d330..ec61329 100644 --- a/hmp.c +++ b/hmp.c @@ -1000,6 +1000,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict) Error *err = NULL; unsigned int c = 0; TPMPassthroughOptions *tpo; +TPMEmulatorOptions *teo; info_list = qmp_query_tpm(&err); if (err) { @@ -1029,6 +1030,10 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict) tpo->has_cancel_path ? ",cancel-path=" : "", tpo->has_cancel_path ? tpo->cancel_path : ""); break; +case TPM_TYPE_OPTIONS_KIND_EMULATOR: +teo = ti->options->u.emulator.data; +monitor_printf(mon, ",chardev=%s", teo->chardev); +break; case TPM_TYPE_OPTIONS_KIND__MAX: break; } diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs index 64cecc3..41f0b7a 100644 --- a/hw/tpm/Makefile.objs +++ b/hw/tpm/Makefile.objs @@ -1,2 +1,3 @@ common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o tpm_util.o +common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o tpm_util.o diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c new file mode 100644 index 000..95e1e04 --- /dev/null +++ b/hw/tpm/tpm_emulator.c @@ -0,0 +1,587 @@ +/* + * Emulator TPM driver + * + * Copyright (c) 2017 Intel Corporation + * Author: Amarnath Valluri + * + * Copyright (c) 2010 - 2013 IBM Corporation + * Authors: + *Stefan Berger + * + * Copyright (C) 2011 IAIK, Graz University of Technology + *Author: Andreas Niederl + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General
[Qemu-devel] [PULL v3 10/11] tpm: Move tpm_cleanup() to right place
From: Amarnath Valluri As Emulator TPM backend uses chardev, tpm cleanup should happen before chardev similar to other vhost-users. Signed-off-by: Amarnath Valluri Reviewed-by: Stefan Berger Signed-off-by: Stefan Berger --- tpm.c | 1 - vl.c | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/tpm.c b/tpm.c index 3b8c7ed..317 100644 --- a/tpm.c +++ b/tpm.c @@ -172,7 +172,6 @@ int tpm_init(void) return -1; } -atexit(tpm_cleanup); return 0; } diff --git a/vl.c b/vl.c index d7c3492..0723835 100644 --- a/vl.c +++ b/vl.c @@ -4905,6 +4905,7 @@ int main(int argc, char **argv, char **envp) res_free(); /* vhost-user must be cleaned up before chardevs. */ +tpm_cleanup(); net_cleanup(); audio_cleanup(); monitor_cleanup(); -- 2.5.5
[Qemu-devel] [PULL v3 04/11] tpm-backend: Initialize and free data members in it's own methods
From: Amarnath Valluri Initialize and free TPMBackend data members in it's own instance_init() and instance_finalize methods. Took the opportunity to remove unneeded destroy() method from TpmDriverOps interface as TPMBackend is a Qemu Object, we can use object_unref() inplace of tpm_backend_destroy() to free the backend object, hence removed destroy() from TPMDriverOps interface. Signed-off-by: Amarnath Valluri Reviewed-by: Marc-André Lureau Reviewed-by: Stefan Berger Signed-off-by: Stefan Berger --- backends/tpm.c | 16 ++-- hw/tpm/tpm_passthrough.c | 31 --- include/sysemu/tpm_backend.h | 7 --- tpm.c| 2 +- 4 files changed, 19 insertions(+), 37 deletions(-) diff --git a/backends/tpm.c b/backends/tpm.c index ce56c3b..cf5abf1 100644 --- a/backends/tpm.c +++ b/backends/tpm.c @@ -51,15 +51,6 @@ const char *tpm_backend_get_desc(TPMBackend *s) return k->ops->desc(); } -void tpm_backend_destroy(TPMBackend *s) -{ -TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); - -k->ops->destroy(s); - -tpm_backend_thread_end(s); -} - int tpm_backend_init(TPMBackend *s, TPMState *state, TPMRecvDataCB *datacb) { @@ -182,17 +173,22 @@ static void tpm_backend_prop_set_opened(Object *obj, bool value, Error **errp) static void tpm_backend_instance_init(Object *obj) { +TPMBackend *s = TPM_BACKEND(obj); + object_property_add_bool(obj, "opened", tpm_backend_prop_get_opened, tpm_backend_prop_set_opened, NULL); - +s->fe_model = -1; } static void tpm_backend_instance_finalize(Object *obj) { TPMBackend *s = TPM_BACKEND(obj); +g_free(s->id); +g_free(s->path); +g_free(s->cancel_path); tpm_backend_thread_end(s); } diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index f50d9cf..815a72e 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -417,8 +417,6 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id) TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); tb->id = g_strdup(id); -/* let frontend set the fe_model to proper value */ -tb->fe_model = -1; if (tpm_passthrough_handle_device_opts(opts, tb)) { goto err_exit; @@ -432,26 +430,11 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id) return tb; err_exit: -g_free(tb->id); +object_unref(obj); return NULL; } -static void tpm_passthrough_destroy(TPMBackend *tb) -{ -TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); - -tpm_passthrough_cancel_cmd(tb); - -qemu_close(tpm_pt->tpm_fd); -qemu_close(tpm_pt->cancel_fd); - -g_free(tb->id); -g_free(tb->path); -g_free(tb->cancel_path); -g_free(tpm_pt->tpm_dev); -} - static const QemuOptDesc tpm_passthrough_cmdline_opts[] = { TPM_STANDARD_CMDLINE_OPTS, { @@ -472,7 +455,6 @@ static const TPMDriverOps tpm_passthrough_driver = { .opts = tpm_passthrough_cmdline_opts, .desc = tpm_passthrough_create_desc, .create = tpm_passthrough_create, -.destroy = tpm_passthrough_destroy, .init = tpm_passthrough_init, .startup_tpm = tpm_passthrough_startup_tpm, .realloc_buffer = tpm_passthrough_realloc_buffer, @@ -486,10 +468,21 @@ static const TPMDriverOps tpm_passthrough_driver = { static void tpm_passthrough_inst_init(Object *obj) { +TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj); + +tpm_pt->tpm_fd = -1; +tpm_pt->cancel_fd = -1; } static void tpm_passthrough_inst_finalize(Object *obj) { +TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(obj); + +tpm_passthrough_cancel_cmd(TPM_BACKEND(obj)); + +qemu_close(tpm_pt->tpm_fd); +qemu_close(tpm_pt->cancel_fd); +g_free(tpm_pt->tpm_dev); } static void tpm_passthrough_class_init(ObjectClass *klass, void *data) diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h index 58308b3..202ec8d 100644 --- a/include/sysemu/tpm_backend.h +++ b/include/sysemu/tpm_backend.h @@ -78,7 +78,6 @@ struct TPMDriverOps { const char *(*desc)(void); TPMBackend *(*create)(QemuOpts *opts, const char *id); -void (*destroy)(TPMBackend *t); /* initialize the backend */ int (*init)(TPMBackend *t); @@ -118,12 +117,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s); const char *tpm_backend_get_desc(TPMBackend *s); /** - * tpm_backend_destroy: - * @s: the backend to destroy - */ -void tpm_backend_destroy(TPMBackend *s); - -/** * tpm_backend_init: * @s: the backend to initialized * @state: TPMState diff --git a/tpm.c b/tpm.c index abedf3f..b19b1a3 100644 --- a/tpm.c +++ b/tpm.c @@ -157,7 +157,7 @@ void tpm_cleanup(void) QLIST_FOREACH_SAFE(drv, &tpm_backends, li
[Qemu-devel] [PULL v3 06/11] tpm-backend: Add new API to read backend TpmInfo
From: Amarnath Valluri TPM configuration options are backend implementation details and shall not be part of base TPMBackend object, and these shall not be accessed directly outside of the class, hence added a new interface method, get_tpm_options() to TPMDriverOps., which shall be implemented by the derived classes to return configured tpm options. A new tpm backend api - tpm_backend_query_tpm() which uses _get_tpm_options() to prepare TpmInfo. Signed-off-by: Amarnath Valluri Reviewed-by: Stefan Berger Reviewed-by: Marc-André Lureau Signed-off-by: Stefan Berger --- backends/tpm.c | 15 +++-- hw/tpm/tpm_passthrough.c | 51 +++- include/sysemu/tpm_backend.h | 15 +++-- tpm.c| 32 +-- 4 files changed, 59 insertions(+), 54 deletions(-) diff --git a/backends/tpm.c b/backends/tpm.c index 8911597..de313c9 100644 --- a/backends/tpm.c +++ b/backends/tpm.c @@ -142,6 +142,19 @@ TPMVersion tpm_backend_get_tpm_version(TPMBackend *s) return k->ops->get_tpm_version(s); } +TPMInfo *tpm_backend_query_tpm(TPMBackend *s) +{ +TPMInfo *info = g_new0(TPMInfo, 1); +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); + +info->id = g_strdup(s->id); +info->model = s->fe_model; +info->options = k->ops->get_tpm_options ? +k->ops->get_tpm_options(s) : NULL; + +return info; +} + static bool tpm_backend_prop_get_opened(Object *obj, Error **errp) { TPMBackend *s = TPM_BACKEND(obj); @@ -196,8 +209,6 @@ static void tpm_backend_instance_finalize(Object *obj) TPMBackend *s = TPM_BACKEND(obj); g_free(s->id); -g_free(s->path); -g_free(s->cancel_path); tpm_backend_thread_end(s); } diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index 4c21e52..84fc49a 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -30,6 +30,7 @@ #include "tpm_int.h" #include "hw/hw.h" #include "hw/i386/pc.h" +#include "qapi/clone-visitor.h" #include "tpm_tis.h" #include "tpm_util.h" @@ -49,7 +50,8 @@ struct TPMPassthruState { TPMBackend parent; -char *tpm_dev; +TPMPassthroughOptions *options; +const char *tpm_dev; int tpm_fd; bool tpm_executing; bool tpm_op_canceled; @@ -296,15 +298,14 @@ static TPMVersion tpm_passthrough_get_tpm_version(TPMBackend *tb) * in Documentation/ABI/stable/sysfs-class-tpm. * From /dev/tpm0 create /sys/class/misc/tpm0/device/cancel */ -static int tpm_passthrough_open_sysfs_cancel(TPMBackend *tb) +static int tpm_passthrough_open_sysfs_cancel(TPMPassthruState *tpm_pt) { -TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); int fd = -1; char *dev; char path[PATH_MAX]; -if (tb->cancel_path) { -fd = qemu_open(tb->cancel_path, O_WRONLY); +if (tpm_pt->options->cancel_path) { +fd = qemu_open(tpm_pt->options->cancel_path, O_WRONLY); if (fd < 0) { error_report("Could not open TPM cancel path : %s", strerror(errno)); @@ -319,7 +320,7 @@ static int tpm_passthrough_open_sysfs_cancel(TPMBackend *tb) dev) < sizeof(path)) { fd = qemu_open(path, O_WRONLY); if (fd >= 0) { -tb->cancel_path = g_strdup(path); +tpm_pt->options->cancel_path = g_strdup(path); } else { error_report("tpm_passthrough: Could not open TPM cancel " "path %s : %s", path, strerror(errno)); @@ -339,17 +340,18 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb) const char *value; value = qemu_opt_get(opts, "cancel-path"); -tb->cancel_path = g_strdup(value); +if (value) { +tpm_pt->options->cancel_path = g_strdup(value); +tpm_pt->options->has_cancel_path = true; +} value = qemu_opt_get(opts, "path"); -if (!value) { -value = TPM_PASSTHROUGH_DEFAULT_DEVICE; +if (value) { +tpm_pt->options->has_path = true; +tpm_pt->options->path = g_strdup(value); } -tpm_pt->tpm_dev = g_strdup(value); - -tb->path = g_strdup(tpm_pt->tpm_dev); - +tpm_pt->tpm_dev = value ? value : TPM_PASSTHROUGH_DEFAULT_DEVICE; tpm_pt->tpm_fd = qemu_open(tpm_pt->tpm_dev, O_RDWR); if (tpm_pt->tpm_fd < 0) { error_report("Cannot access TPM device using '%s': %s", @@ -370,10 +372,8 @@ static int tpm_passthrough_handle_device_opts(QemuOpts *opts, TPMBackend *tb) tpm_pt->tpm_fd = -1; err_free_parameters: -g_free(tb->path); -tb->path = NULL; - -g_free(tpm_pt->tpm_dev); +qapi_free_TPMPassthroughOptions(tpm_pt->options); +tpm_pt->options = NULL; tpm_pt->tpm_dev = NULL; return 1; @@ -391,7 +391,7 @@ static TPMBackend *tpm_passthrough_create(QemuOpts *opts, const char *id) goto err_exit; } -tpm_pt->cancel_fd = tpm_passthrou
[Qemu-devel] [PULL v3 07/11] tpm-backend: Move realloc_buffer() implementation to tpm-tis model
From: Amarnath Valluri buffer reallocation is very unlikely to be backend specific. Hence move inside the tis. Signed-off-by: Amarnath Valluri Reviewed-by: Stefan Berger Reviewed-by: Marc-André Lureau Signed-off-by: Stefan Berger --- backends/tpm.c | 9 - hw/tpm/tpm_passthrough.c | 12 hw/tpm/tpm_tis.c | 14 -- include/sysemu/tpm_backend.h | 12 4 files changed, 12 insertions(+), 35 deletions(-) diff --git a/backends/tpm.c b/backends/tpm.c index de313c9..37c84b7 100644 --- a/backends/tpm.c +++ b/backends/tpm.c @@ -80,15 +80,6 @@ bool tpm_backend_had_startup_error(TPMBackend *s) return s->had_startup_error; } -size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb) -{ -TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); - -assert(k->ops->realloc_buffer); - -return k->ops->realloc_buffer(sb); -} - void tpm_backend_deliver_request(TPMBackend *s) { g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_PROCESS_CMD, diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index 84fc49a..22d3460 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -247,17 +247,6 @@ static int tpm_passthrough_reset_tpm_established_flag(TPMBackend *tb, return 0; } -static size_t tpm_passthrough_realloc_buffer(TPMSizedBuffer *sb) -{ -size_t wanted_size = 4096; /* Linux tpm.c buffer size */ - -if (sb->size != wanted_size) { -sb->buffer = g_realloc(sb->buffer, wanted_size); -sb->size = wanted_size; -} -return sb->size; -} - static void tpm_passthrough_cancel_cmd(TPMBackend *tb) { TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); @@ -435,7 +424,6 @@ static const TPMDriverOps tpm_passthrough_driver = { .opts = tpm_passthrough_cmdline_opts, .desc = "Passthrough TPM backend driver", .create = tpm_passthrough_create, -.realloc_buffer = tpm_passthrough_realloc_buffer, .reset= tpm_passthrough_reset, .cancel_cmd = tpm_passthrough_cancel_cmd, .get_tpm_established_flag = tpm_passthrough_get_tpm_established_flag, diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c index a6440fe..d5118e7 100644 --- a/hw/tpm/tpm_tis.c +++ b/hw/tpm/tpm_tis.c @@ -963,6 +963,16 @@ static int tpm_tis_do_startup_tpm(TPMState *s) return tpm_backend_startup_tpm(s->be_driver); } +static void tpm_tis_realloc_buffer(TPMSizedBuffer *sb) +{ +size_t wanted_size = 4096; /* Linux tpm.c buffer size */ + +if (sb->size != wanted_size) { +sb->buffer = g_realloc(sb->buffer, wanted_size); +sb->size = wanted_size; +} +} + /* * Get the TPMVersion of the backend device being used */ @@ -1010,9 +1020,9 @@ static void tpm_tis_reset(DeviceState *dev) tis->loc[c].state = TPM_TIS_STATE_IDLE; tis->loc[c].w_offset = 0; -tpm_backend_realloc_buffer(s->be_driver, &tis->loc[c].w_buffer); +tpm_tis_realloc_buffer(&tis->loc[c].w_buffer); tis->loc[c].r_offset = 0; -tpm_backend_realloc_buffer(s->be_driver, &tis->loc[c].r_buffer); +tpm_tis_realloc_buffer(&tis->loc[c].r_buffer); } tpm_tis_do_startup_tpm(s); diff --git a/include/sysemu/tpm_backend.h b/include/sysemu/tpm_backend.h index e96c191..2c798a1 100644 --- a/include/sysemu/tpm_backend.h +++ b/include/sysemu/tpm_backend.h @@ -84,8 +84,6 @@ struct TPMDriverOps { /* start up the TPM on the backend */ int (*startup_tpm)(TPMBackend *t); -size_t (*realloc_buffer)(TPMSizedBuffer *sb); - void (*reset)(TPMBackend *t); void (*cancel_cmd)(TPMBackend *t); @@ -140,16 +138,6 @@ int tpm_backend_startup_tpm(TPMBackend *s); bool tpm_backend_had_startup_error(TPMBackend *s); /** - * tpm_backend_realloc_buffer: - * @s: the backend - * @sb: the TPMSizedBuffer to re-allocated to the size suitable for the - * backend. - * - * This function returns the size of the allocated buffer - */ -size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb); - -/** * tpm_backend_deliver_request: * @s: the backend to send the request to * -- 2.5.5
[Qemu-devel] [PULL v3 05/11] tpm-backend: Made few interface methods optional
From: Amarnath Valluri This allows backend implementations left optional interface methods. For mandatory methods assertion checks added. Took the opportunity to remove unused methods: - tpm_backend_get_desc() - TPMDriverOps->handle_startup_error Signed-off-by: Amarnath Valluri Reviewed-by: Marc-André Lureau Reviewed-by: Stefan Berger Signed-off-by: Stefan Berger --- backends/tpm.c | 39 --- hw/tpm/tpm_passthrough.c | 36 +--- include/sysemu/tpm_backend.h | 13 ++--- tpm.c| 2 +- 4 files changed, 28 insertions(+), 62 deletions(-) diff --git a/backends/tpm.c b/backends/tpm.c index cf5abf1..8911597 100644 --- a/backends/tpm.c +++ b/backends/tpm.c @@ -44,13 +44,6 @@ enum TpmType tpm_backend_get_type(TPMBackend *s) return k->ops->type; } -const char *tpm_backend_get_desc(TPMBackend *s) -{ -TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); - -return k->ops->desc(); -} - int tpm_backend_init(TPMBackend *s, TPMState *state, TPMRecvDataCB *datacb) { @@ -58,12 +51,14 @@ int tpm_backend_init(TPMBackend *s, TPMState *state, s->tpm_state = state; s->recv_data_callback = datacb; +s->had_startup_error = false; -return k->ops->init(s); +return k->ops->init ? k->ops->init(s) : 0; } int tpm_backend_startup_tpm(TPMBackend *s) { +int res = 0; TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); /* terminate a running TPM */ @@ -73,20 +68,24 @@ int tpm_backend_startup_tpm(TPMBackend *s) NULL); g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_INIT, NULL); -return k->ops->startup_tpm(s); +res = k->ops->startup_tpm ? k->ops->startup_tpm(s) : 0; + +s->had_startup_error = (res != 0); + +return res; } bool tpm_backend_had_startup_error(TPMBackend *s) { -TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); - -return k->ops->had_startup_error(s); +return s->had_startup_error; } size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb) { TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); +assert(k->ops->realloc_buffer); + return k->ops->realloc_buffer(sb); } @@ -100,15 +99,21 @@ void tpm_backend_reset(TPMBackend *s) { TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); -k->ops->reset(s); +if (k->ops->reset) { +k->ops->reset(s); +} tpm_backend_thread_end(s); + +s->had_startup_error = false; } void tpm_backend_cancel_cmd(TPMBackend *s) { TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); +assert(k->ops->cancel_cmd); + k->ops->cancel_cmd(s); } @@ -116,20 +121,24 @@ bool tpm_backend_get_tpm_established_flag(TPMBackend *s) { TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); -return k->ops->get_tpm_established_flag(s); +return k->ops->get_tpm_established_flag ? + k->ops->get_tpm_established_flag(s) : false; } int tpm_backend_reset_tpm_established_flag(TPMBackend *s, uint8_t locty) { TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); -return k->ops->reset_tpm_established_flag(s, locty); +return k->ops->reset_tpm_established_flag ? + k->ops->reset_tpm_established_flag(s, locty) : 0; } TPMVersion tpm_backend_get_tpm_version(TPMBackend *s) { TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); +assert(k->ops->get_tpm_version); + return k->ops->get_tpm_version(s); } diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index 815a72e..4c21e52 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -54,7 +54,6 @@ struct TPMPassthruState { bool tpm_executing; bool tpm_op_canceled; int cancel_fd; -bool had_startup_error; TPMVersion tpm_version; }; @@ -227,29 +226,11 @@ static void tpm_passthrough_handle_request(TPMBackend *tb, TPMBackendCmd cmd) } } -/* - * Start the TPM (thread). If it had been started before, then terminate - * and start it again. - */ -static int tpm_passthrough_startup_tpm(TPMBackend *tb) -{ -return 0; -} - static void tpm_passthrough_reset(TPMBackend *tb) { -TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); - DPRINTF("tpm_passthrough: CALL TO TPM_RESET!\n"); tpm_passthrough_cancel_cmd(tb); - -tpm_pt->had_startup_error = false; -} - -static int tpm_passthrough_init(TPMBackend *tb) -{ -return 0; } static bool tpm_passthrough_get_tpm_established_flag(TPMBackend *tb) @@ -264,13 +245,6 @@ static int tpm_passthrough_reset_tpm_established_flag(TPMBackend *tb, return 0; } -static bool tpm_passthrough_get_startup_error(TPMBackend *tb) -{ -TPMPassthruState *tpm_pt = TPM_PASSTHROUGH(tb); - -return tpm_pt->had_startup_error; -} - static size_t tpm_passthrough_realloc_buffer(TPMSizedBuffer *sb) { size_t wanted_size = 4096; /* Linux tpm.c buffer size */ @@ -309,11 +283,6 @@ static void t
[Qemu-devel] [PULL v3 11/11] specs: Describe the TPM support in QEMU
This patch adds a description of the current TPM support in QEMU to the specs. Several public specs are referenced via their landing page on the trustedcomputinggroup.org website. Signed-off-by: Stefan Berger Reviewed-by: Laszlo Ersek --- docs/specs/tpm.txt | 123 + 1 file changed, 123 insertions(+) create mode 100644 docs/specs/tpm.txt diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt new file mode 100644 index 000..914daac --- /dev/null +++ b/docs/specs/tpm.txt @@ -0,0 +1,123 @@ +QEMU TPM Device +=== + += Guest-side Hardware Interface = + +The QEMU TPM emulation implements a TPM TIS hardware interface following the +Trusted Computing Group's specification "TCG PC Client Specific TPM Interface +Specification (TIS)", Specification Version 1.3, 21 March 2013. This +specification, or a later version of it, can be accessed from the following +URL: + +https://trustedcomputinggroup.org/pc-client-work-group-pc-client-specific-tpm-interface-specification-tis/ + +The TIS interface makes a memory mapped IO region in the area 0xfed4 - +0xfed44fff available to the guest operating system. + + +QEMU files related to TPM TIS interface: + - hw/tpm/tpm_tis.c + - hw/tpm/tpm_tis.h + + += ACPI Interface = + +The TPM device is defined with ACPI ID "PNP0C31". QEMU builds a SSDT and passes +it into the guest through the fw_cfg device. The device description contains +the base address of the TIS interface 0xfed4 and the size of the MMIO area +(0x5000). In case a TPM2 is used by QEMU, a TPM2 ACPI table is also provided. +The device is described to be used in polling mode rather than interrupt mode +primarily because no unused IRQ could be found. + +To support measurement logs to be written by the firmware, e.g. SeaBIOS, a TCPA +table is implemented. This table provides a 64kb buffer where the firmware can +write its log into. For TPM 2 only a more recent version of the TPM2 table +provides support for measurements logs and a TCPA table does not need to be +created. + +The TCPA and TPM2 ACPI tables follow the Trusted Computing Group specification +"TCG ACPI Specification" Family "1.2" and "2.0", Level 00 Revision 00.37. This +specification, or a later version of it, can be accessed from the following +URL: + +https://trustedcomputinggroup.org/tcg-acpi-specification/ + + +QEMU files related to TPM ACPI tables: + - hw/i386/acpi-build.c + - include/hw/acpi/tpm.h + + += TPM backend devices = + +The TPM implementation is split into two parts, frontend and backend. The +frontend part is the hardware interface, such as the TPM TIS interface +described earlier, and the other part is the TPM backend interface. The backend +interfaces implement the interaction with a TPM device, which may be a physical +or an emulated device. The split between the front- and backend devices allows +a frontend to be connected with any available backend. This enables the TIS +interface to be used with the passthrough backend or the (future) swtpm backend. + + +QEMU files related to TPM backends: + - backends/tpm.c + - include/sysemu/tpm_backend.h + - include/sysemu/tpm_backend_int.h + + +== The QEMU TPM passthrough device == + +In case QEMU is run on Linux as the host operating system it is possible to +make the hardware TPM device available to a single QEMU guest. In this case the +user must make sure that no other program is using the device, e.g., /dev/tpm0, +before trying to start QEMU with it. + +The passthrough driver uses the host's TPM device for sending TPM commands +and receiving responses from. Besides that it accesses the TPM device's sysfs +entry for support of command cancellation. Since none of the state of a +hardware TPM can be migrated between hosts, virtual machine migration is +disabled when the TPM passthrough driver is used. + +Since the host's TPM device will already be initialized by the host's firmware, +certain commands, e.g. TPM_Startup(), sent by the virtual firmware for device +initialization, will fail. In this case the firmware should not use the TPM. + +Sharing the device with the host is generally not a recommended usage scenario +for a TPM device. The primary reason for this is that two operating systems can +then access the device's single set of resources, such as platform configuration +registers (PCRs). Applications or kernel security subsystems, such as the +Linux Integrity Measurement Architecture (IMA), are not expecting to share PCRs. + + +QEMU files related to the TPM passthrough device: + - hw/tpm/tpm_passthrough.c + - hw/tpm/tpm_util.c + - hw/tpm/tpm_util.h + + +Command line to start QEMU with the TPM passthrough device using the host's +hardware TPM /dev/tpm0: + +qemu-system-x86_64 -display sdl -enable-kvm \ + -m 1024 -boot d -bios bios-256k.bin -boot menu=on \ + -tpmdev passthrough,id=tpm0,path=/dev/tpm0 \ + -device tpm-tis,tpmdev=tpm0 test.img + +The following commands should result in similar output inside the VM with a
Re: [Qemu-devel] [PATCH] configure: pick the right compiler for OpenBSD by default
On 13.10.2017 13:38, Daniel P. Berrange wrote: > On Fri, Oct 13, 2017 at 12:55:40PM +0200, Thomas Huth wrote: >> On 13.10.2017 12:52, Thomas Huth wrote: [...] >> By the way, looks like OpenBSD is also switching to clang by default soon: >> >> https://www.phoronix.com/scan.php?page=news_item&px=OpenBSD-Default-Clang > > In a few years time we could potentially revert this patch, but in the > meantime it is clearly beneficial for anyone using OpenBSD and has no > significant maint burden for us to carry it while there are widely > supported OpenBSD releases which need it. I disagree. If the next OpenBSD release uses Clang by default, we're not building QEMU there with the *working default* C compiler anymore. You're then rather forcing the OpenBSD users then to install an additional (likely unliked, since GPLv3) GCC package on their systems. So IMHO, just drop this patch and wait for the next OpenBSD release, and the problem will be solved automatically. (and the few users who still use an older release of OpenBSD will likely use the QEMU from their ports system anyway) Thomas
[Qemu-devel] [PATCH] monitor: fix dangling CPU pointer
If a CPU selected with the "cpu" command is hot-unplugged then "info cpus" causes QEMU to exit: (qemu) device_del cpu1 (qemu) info cpus qemu:qemu_cpu_kick_thread: No such process This happens because "cpu" stores the pointer to the selected CPU into the monitor structure. When the CPU is hot-unplugged, we end up with a dangling pointer. The "info cpus" command then does: hmp_info_cpus() monitor_get_cpu_index() mon_get_cpu() cpu_synchronize_state() <--- called with dangling pointer This could cause a QEMU crash as well. This patch switches the monitor to store the QOM path instead of a pointer to the current CPU. The path is then resolved when needed. If the resolution fails, we assume that the CPU was removed and the path is resetted to the default (ie, path of first_cpu). Note that the resolution should really return a CPU object, otherwise we have a bug. This is achieved by relying on object_resolve_path() and CPU() instead of calling object_resolve_path_type(path, TYPE_CPU). Suggested-by: Igor Mammedov Signed-off-by: Greg Kurz --- monitor.c | 25 - 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/monitor.c b/monitor.c index fe0d1bdbb461..8489b2ad99c0 100644 --- a/monitor.c +++ b/monitor.c @@ -200,7 +200,7 @@ struct Monitor { ReadLineState *rs; MonitorQMP qmp; -CPUState *mon_cpu; +gchar *mon_cpu_path; BlockCompletionFunc *password_completion_cb; void *password_opaque; mon_cmd_t *cmd_table; @@ -579,6 +579,7 @@ static void monitor_data_init(Monitor *mon) static void monitor_data_destroy(Monitor *mon) { +g_free(mon->mon_cpu_path); qemu_chr_fe_deinit(&mon->chr, false); if (monitor_is_qmp(mon)) { json_message_parser_destroy(&mon->qmp.parser); @@ -1047,20 +1048,34 @@ int monitor_set_cpu(int cpu_index) if (cpu == NULL) { return -1; } -cur_mon->mon_cpu = cpu; +g_free(cur_mon->mon_cpu_path); +cur_mon->mon_cpu_path = object_get_canonical_path(OBJECT(cpu)); return 0; } CPUState *mon_get_cpu(void) { -if (!cur_mon->mon_cpu) { +CPUState *cpu; + +if (cur_mon->mon_cpu_path) { +Object *obj = object_resolve_path(cur_mon->mon_cpu_path, NULL); + +if (!obj) { +g_free(cur_mon->mon_cpu_path); +cur_mon->mon_cpu_path = NULL; +} else { +cpu = CPU(obj); +} +} +if (!cur_mon->mon_cpu_path) { if (!first_cpu) { return NULL; } monitor_set_cpu(first_cpu->cpu_index); +cpu = first_cpu; } -cpu_synchronize_state(cur_mon->mon_cpu); -return cur_mon->mon_cpu; +cpu_synchronize_state(cpu); +return cpu; } CPUArchState *mon_get_cpu_env(void)
[Qemu-devel] [PULL v3 03/11] tpm-backend: Move thread handling inside TPMBackend
From: Amarnath Valluri Move thread handling inside TPMBackend, this way backend implementations need not to maintain their own thread life cycle, instead they needs to implement 'handle_request()' class method that always been called from a thread. This change made tpm_backend_int.h kind of useless, hence removed it. Signed-off-by: Amarnath Valluri Reviewed-by: Marc-André Lureau Reviewed-by: Stefan Berger Signed-off-by: Stefan Berger --- backends/tpm.c | 62 +--- hw/tpm/tpm_passthrough.c | 58 ++--- include/sysemu/tpm_backend.h | 32 + include/sysemu/tpm_backend_int.h | 41 -- 4 files changed, 67 insertions(+), 126 deletions(-) delete mode 100644 include/sysemu/tpm_backend_int.h diff --git a/backends/tpm.c b/backends/tpm.c index 536f262..ce56c3b 100644 --- a/backends/tpm.c +++ b/backends/tpm.c @@ -18,7 +18,24 @@ #include "qapi/qmp/qerror.h" #include "sysemu/tpm.h" #include "qemu/thread.h" -#include "sysemu/tpm_backend_int.h" + +static void tpm_backend_worker_thread(gpointer data, gpointer user_data) +{ +TPMBackend *s = TPM_BACKEND(user_data); +TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); + +assert(k->handle_request != NULL); +k->handle_request(s, (TPMBackendCmd)data); +} + +static void tpm_backend_thread_end(TPMBackend *s) +{ +if (s->thread_pool) { +g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_END, NULL); +g_thread_pool_free(s->thread_pool, FALSE, TRUE); +s->thread_pool = NULL; +} +} enum TpmType tpm_backend_get_type(TPMBackend *s) { @@ -39,6 +56,8 @@ void tpm_backend_destroy(TPMBackend *s) TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); k->ops->destroy(s); + +tpm_backend_thread_end(s); } int tpm_backend_init(TPMBackend *s, TPMState *state, @@ -46,13 +65,23 @@ int tpm_backend_init(TPMBackend *s, TPMState *state, { TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); -return k->ops->init(s, state, datacb); +s->tpm_state = state; +s->recv_data_callback = datacb; + +return k->ops->init(s); } int tpm_backend_startup_tpm(TPMBackend *s) { TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); +/* terminate a running TPM */ +tpm_backend_thread_end(s); + +s->thread_pool = g_thread_pool_new(tpm_backend_worker_thread, s, 1, TRUE, + NULL); +g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_INIT, NULL); + return k->ops->startup_tpm(s); } @@ -72,9 +101,8 @@ size_t tpm_backend_realloc_buffer(TPMBackend *s, TPMSizedBuffer *sb) void tpm_backend_deliver_request(TPMBackend *s) { -TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); - -k->ops->deliver_request(s); +g_thread_pool_push(s->thread_pool, (gpointer)TPM_BACKEND_CMD_PROCESS_CMD, + NULL); } void tpm_backend_reset(TPMBackend *s) @@ -82,6 +110,8 @@ void tpm_backend_reset(TPMBackend *s) TPMBackendClass *k = TPM_BACKEND_GET_CLASS(s); k->ops->reset(s); + +tpm_backend_thread_end(s); } void tpm_backend_cancel_cmd(TPMBackend *s) @@ -156,29 +186,14 @@ static void tpm_backend_instance_init(Object *obj) tpm_backend_prop_get_opened, tpm_backend_prop_set_opened, NULL); -} -void tpm_backend_thread_deliver_request(TPMBackendThread *tbt) -{ - g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_PROCESS_CMD, NULL); } -void tpm_backend_thread_create(TPMBackendThread *tbt, - GFunc func, gpointer user_data) +static void tpm_backend_instance_finalize(Object *obj) { -if (!tbt->pool) { -tbt->pool = g_thread_pool_new(func, user_data, 1, TRUE, NULL); -g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_INIT, NULL); -} -} +TPMBackend *s = TPM_BACKEND(obj); -void tpm_backend_thread_end(TPMBackendThread *tbt) -{ -if (tbt->pool) { -g_thread_pool_push(tbt->pool, (gpointer)TPM_BACKEND_CMD_END, NULL); -g_thread_pool_free(tbt->pool, FALSE, TRUE); -tbt->pool = NULL; -} +tpm_backend_thread_end(s); } static const TypeInfo tpm_backend_info = { @@ -186,6 +201,7 @@ static const TypeInfo tpm_backend_info = { .parent = TYPE_OBJECT, .instance_size = sizeof(TPMBackend), .instance_init = tpm_backend_instance_init, +.instance_finalize = tpm_backend_instance_finalize, .class_size = sizeof(TPMBackendClass), .abstract = true, }; diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index a0baf5f..f50d9cf 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -30,7 +30,6 @@ #include "tpm_int.h" #include "hw/hw.h" #include "hw/i386/pc.h" -#include "sysemu/tpm_backend_int.h" #include "tpm_tis.h" #include "tpm_util.h" @@ -47,20 +46,9 @@ OBJECT_CHECK(TPMPasst
Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest
>>> On 13.10.17 at 13:13, wrote: > To Jan, Andrew, Stefano and Anthony, > > what do you think about allowing QEMU to build the entire guest ACPI > and letting SeaBIOS to load it? ACPI builder code in hvmloader is > still there and just bypassed in this case. Well, if that can be made work in a non-quirky way and without loss of functionality, I'd probably be fine. I do think, however, that there's a reason this is being handled in hvmloader right now. Jan
Re: [Qemu-devel] [Qemu-block] [PATCH v3 0/3] add bdrv_co_drain_begin/end BlockDriver callbacks
On Sat, Sep 23, 2017 at 02:14:08PM +0300, Manos Pitsidianakis wrote: > This patch series renames bdrv_co_drain to bdrv_co_drain_begin and adds a new > bdrv_co_drain_end callback to match bdrv_drained_begin/end and > drained_begin/end of BdrvChild. This is needed because the throttle driver > (block/throttle.c) needs a way to mark the end of the drain in order to > toggle > io_limits_disabled correctly. > > Based-on: <20170918202529.28379-1-el13...@mail.ntua.gr> > "block/throttle-groups.c: allocate RestartData on the heap" > Which fixes a coroutine crash in block/throttle-groups.c > > v3: > fixed commit message typo in first patch [Fam] > rephrased doc comment based on mailing discussion > v2: > add doc for callbacks and change order of request polling for completion > [Stefan] > > Manos Pitsidianakis (3): > block: add bdrv_co_drain_end callback > block: rename bdrv_co_drain to bdrv_co_drain_begin > block/throttle.c: add bdrv_co_drain_begin/end callbacks > > include/block/block_int.h | 13 ++--- > block/io.c| 48 > +-- > block/qed.c | 6 +++--- > block/throttle.c | 18 ++ > 4 files changed, 65 insertions(+), 20 deletions(-) > > -- > 2.11.0 > > Thanks, applied to my block tree: https://github.com/stefanha/qemu/commits/block Stefan
[Qemu-devel] Qemu - tracing
Hello, I'm student at Slovak University of Technology in Bratislava and I'm working on tracing process's memory access. I found out Qemu is good tool for task such as mine. I would like to generate trace file from process with two informations: type of access to memory and memory address accessed. I can also parse some file generated by tracing tool from Qemu. Although, I have big trouble with configuring Qemu, lack of guides to tracing in Qemu and basically with tracing. So I would like to ask and it would be really helpful, If you could send me some manual for how to generate such file (with needed configurations). Also if there is some other way to get such information with other tool or something you can recommend. I'm using Ubuntu, but windows is no problem. Question: Can Qemu get physical memory address or is it some "qemu represented" address? Thank you for your answer. Maros
[Qemu-devel] dynamic DRAM base for ArmVirtQemu
Hi Ard, Leif, the current physical memory map of the "virt" machine type doesn't leave much room for ECAM / MMCONFIG, which limits the number of PCI Express root ports and downstream ports (each port takes a separate bus number, and each bus number eats up a chunk of the ECAM area). Also, each port can only accommodate a single PCI Express device. In practice this limits the number of (hot-pluggable) PCIe devices to approx. 16, which is deemed by some "not scaleable enough". (For devices that only need to be cold-plugged, they can be placed directly on the root complex, as integrated devices, possibly grouping them into multifunction devices even; so those don't need bus numbers.) In order to grow the MMCONFIG area (and for some other reasons possibly), the phys memmap of "virt" should be shuffled around a bit. This affects the "system" DRAM too. One idea is to keep the current system DRAM base at 1GB, but limit its size to 1GB. And, if there's more DRAM, use another, disjoint address range for that, above 4GB. This would be easy to support for ArmVirtQemu (basically nothing new would be necessary), as the high area would be handled transparently by "ArmVirtPkg/HighMemDxe". However, this appears to present complications for QEMU. (I don't exactly know what complications -- I would be very happy to hear them, in detail.) Another idea is to move *the* system DRAM base to a different guest-phys address. (Likely using a different version of the "virt" machine type, or even a different machine type entirely.) This would not be compatible with current ArmVirtQemu, which hard-codes the system DRAM base in several, quite brittle / sensitive, locations. (More on this later -- that's going to be the larger part of my email anyway.) In order to handle the new base in ArmVirtQemu, two approaches are possible: change the hard-coded address(es), or cope with the address dynamically. Changing the hard-coded addresses is easy for edk2 contributors (just add a new build flag like -D DRAM_BASE_AT_XXX_GB, and dependent on it, set a number of fixed-at-build PCDs to new values). For RHEL downstream, this is not an un-attractive option, as we are free to break compatibility at this time. For upstream users and other distros however, it likely wouldn't be convenient, because "old" ArmVirtQemu firmware wouldn't boot on the "new" machine type, and vice versa. (If we can agree that the above "boundary" in firmwares and machine types is widely tolerable, then we need not discuss the rest of this email.) Finally, coping with "any" system DRAM base address in ArmVirtQemu is both the most flexible for users, and the most difficult to implement. When QEMU launches the guest, the base of the system DRAM (which equals the location of the DTB too) is exposed in the x0 register. The challenge is to make everything in the earliest phases of ArmVirtQemu to adapt to this value dynamically, and to propagate the value to further parts of the firmware. I've been looking into this for a few days now and would like to pick your minds on what I've found. ( As a side note, I know (superficially) of Ard's ArmVirtXen and ArmVirtQemuKernel work. If I understand correctly, Ard has turned some of the PCDs I'm about to discuss into "patchable" ones, from "fixed-at-build". The difference in storage is that these constants are now placed in the final firmware image such that they are externally patchable, just before "deployment". Because the ArmVirtXen and ArmVirtQemuKernel firmware binaries are loaded into DRAM immediately, this self-patching -- based on the initial value of x0 -- is feasible, in the earliest part of the firmware. (I'm not saying "easy" -- to the contrary; but it's feasible.) However, ArmVirtQemu is launched from pflash; its SEC and PEI phases execute-in-place from pflash (until MemoryInitPeim installs the permanent PEI RAM, and the PEI_CORE relocates the HOB list, itself, and the PEIMs into DRAM). Therefore the SEC and PEI phases of ArmVirtQemu cannot be patched like this, i.e., through patchable PCDs. (Unless, of course, the patching is implemented in QEMU itself -- but I don't think that's probable). ) Now, exactly because SEC and PEI execute in place from pflash, their execution (= instruction fetches) are not affected by different initial x0 values. However, the following are affected: - the initial stack, - the base address of the initial DTB, - the location of the permanent PEI RAM. In ArmVirtQemu, these are represented by the following PCDs (all fixed-at-build currently): - PcdCPUCoresStackBase - PcdDeviceTreeInitialBaseAddress - PcdSystemMemoryBase I've attempted to audit each of these PCDs. (I should note in advance that I focused on their use *only* in the ArmVirtQemu firmware platform, and only on AARCH64. I ignored ARM32, and ArmVirtXen and ArmVirtQemuKernel. We obviously must not regress those other arches / platforms by messing with these PCDs, but this is only a "fea
Re: [Qemu-devel] [PATCH 18/31] qcow2: Update qcow2_get_cluster_offset() to support L2 slices
On Thu, Oct 12, 2017 at 04:05:32PM +0300, Alberto Garcia wrote: > @@ -522,8 +522,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, > uint64_t offset, > { > BDRVQcow2State *s = bs->opaque; > unsigned int l2_index; > -uint64_t l1_index, l2_offset, *l2_table; > -int l1_bits, c; > +uint64_t l1_index, l2_offset, *l2_slice; > +int c; > unsigned int offset_in_cluster; > uint64_t bytes_available, bytes_needed, nb_clusters; > QCow2ClusterType type; > @@ -532,12 +532,11 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, > uint64_t offset, > offset_in_cluster = offset_into_cluster(s, offset); > bytes_needed = (uint64_t) *bytes + offset_in_cluster; > > -l1_bits = s->l2_bits + s->cluster_bits; > - > /* compute how many bytes there are between the start of the cluster > - * containing offset and the end of the l1 entry */ > -bytes_available = (1ULL << l1_bits) - (offset & ((1ULL << l1_bits) - 1)) > -+ offset_in_cluster; > + * containing offset and the end of the l2 slice that contains > + * the entry pointing to it */ > +bytes_available = (s->l2_slice_size - offset_to_l2_slice_index(s, > offset)) > +<< s->cluster_bits; There's a mistake here, this should be cast to uint64_t before doing the shift, else it overflows if the cluster size is large: bytes_available = ((uint64_t) (s->l2_slice_size - offset_to_l2_slice_index(s, offset))) << s->cluster_bits; The next revision will have this correction, but I'm not sending it now, so if you want to test/review this one please go ahead. Berto
[Qemu-devel] [PATCH v2] usb-ccid: remove needless migration state code
This code appears to be unused since its introduction. We need to keep the state_vmstate field byte in VMState for compatibility reasons. Signed-off-by: Marc-André Lureau --- hw/usb/dev-smartcard-reader.c | 23 +-- 1 file changed, 1 insertion(+), 22 deletions(-) diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c index 0c77d2a41d..e334d3be11 100644 --- a/hw/usb/dev-smartcard-reader.c +++ b/hw/usb/dev-smartcard-reader.c @@ -270,11 +270,6 @@ typedef struct BulkIn { uint32_t pos; } BulkIn; -enum { -MIGRATION_NONE, -MIGRATION_MIGRATED, -}; - typedef struct CCIDBus { BusState qbus; } CCIDBus; @@ -306,9 +301,6 @@ typedef struct USBCCIDState { CCID_ProtocolDataStructure abProtocolDataStructure; uint32_t ulProtocolDataStructureSize; uint32_t state_vmstate; -uint32_t migration_target_ip; -uint16_t migration_target_port; -uint8_t migration_state; uint8_t bmSlotICCState; uint8_t powered; uint8_t notify_slot_change; @@ -1243,9 +1235,6 @@ int ccid_card_ccid_attach(CCIDCardState *card) USBCCIDState *s = USB_CCID_DEV(dev); DPRINTF(s, 1, "CCID Attach\n"); -if (s->migration_state == MIGRATION_MIGRATED) { -s->migration_state = MIGRATION_NONE; -} return 0; } @@ -1341,9 +1330,6 @@ static void ccid_realize(USBDevice *dev, Error **errp) s->intr = usb_ep_get(dev, USB_TOKEN_IN, CCID_INT_IN_EP); s->bulk = usb_ep_get(dev, USB_TOKEN_IN, CCID_BULK_IN_EP); s->card = NULL; -s->migration_state = MIGRATION_NONE; -s->migration_target_ip = 0; -s->migration_target_port = 0; s->dev.speed = USB_SPEED_FULL; s->dev.speedmask = USB_SPEED_MASK_FULL; s->notify_slot_change = false; @@ -1379,13 +1365,6 @@ static int ccid_pre_save(void *opaque) USBCCIDState *s = opaque; s->state_vmstate = s->dev.state; -if (s->dev.attached) { -/* - * Migrating an open device, ignore reconnection CHR_EVENT to avoid an - * erroneous detach. - */ -s->migration_state = MIGRATION_MIGRATED; -} return 0; } @@ -1452,7 +1431,7 @@ static VMStateDescription ccid_vmstate = { VMSTATE_STRUCT_ARRAY(pending_answers, USBCCIDState, PENDING_ANSWERS_NUM, 1, answer_vmstate, Answer), VMSTATE_UINT32(pending_answers_num, USBCCIDState), -VMSTATE_UINT8(migration_state, USBCCIDState), +VMSTATE_UNUSED(1), /* was migration_state */ VMSTATE_UINT32(state_vmstate, USBCCIDState), VMSTATE_END_OF_LIST() } -- 2.15.0.rc0.40.gaefcc5f6f
[Qemu-devel] iotest 77 failure [was: [PATCH v2 0/5] backup improvements part 1]
[adding qemu-block] On 10/13/2017 02:37 AM, Vladimir Sementsov-Ogievskiy wrote: > 077 is unrelated to backup, it's already broken test: > > i=1; while check -nbd 077; do echo $((i++)) - OK; done; echo $i - FAIL > > failed on 3rd try for me on master > >> 077 - output mismatch (see 077.out.bad) >> --- /tmp/qemu-test/src/tests/qemu-iotests/077.out 2017-10-12 >> 14:02:13.0 + >> +++ /tmp/qemu-test/build/tests/qemu-iotests/077.out.bad 2017-10-12 >> 14:29:25.909887675 + >> @@ -40,9 +40,9 @@ >> wrote XXX/XXX bytes at offset XXX >> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> blkdebug: Resuming request 'B' >> +blkdebug: Resuming request 'A' >> wrote XXX/XXX bytes at offset XXX >> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> -blkdebug: Resuming request 'A' >> wrote XXX/XXX bytes at offset XXX >> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) >> wrote XXX/XXX bytes at offset XXX Looks like we have a data race. Kevin, you wrote this test, any ideas on how we can make it more robust? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] dynamic DRAM base for ArmVirtQemu
On 13 October 2017 at 13:51, Laszlo Ersek wrote: > Hi Ard, Leif, > > the current physical memory map of the "virt" machine type doesn't leave > much room for ECAM / MMCONFIG, which limits the number of PCI Express > root ports and downstream ports (each port takes a separate bus number, > and each bus number eats up a chunk of the ECAM area). Also, each port > can only accommodate a single PCI Express device. In practice this > limits the number of (hot-pluggable) PCIe devices to approx. 16, which > is deemed by some "not scaleable enough". (For devices that only need to > be cold-plugged, they can be placed directly on the root complex, as > integrated devices, possibly grouping them into multifunction devices > even; so those don't need bus numbers.) > > In order to grow the MMCONFIG area (and for some other reasons > possibly), the phys memmap of "virt" should be shuffled around a bit. > This affects the "system" DRAM too. > Is it really necessary to put the ECAM area below 4 GB? For ARM, I understand this would be an issue, but does the spec actually mandate anything like this? > > One idea is to keep the current system DRAM base at 1GB, but limit its > size to 1GB. And, if there's more DRAM, use another, disjoint address > range for that, above 4GB. This would be easy to support for ArmVirtQemu > (basically nothing new would be necessary), as the high area would be > handled transparently by "ArmVirtPkg/HighMemDxe". However, this appears > to present complications for QEMU. (I don't exactly know what > complications -- I would be very happy to hear them, in detail.) > > > Another idea is to move *the* system DRAM base to a different guest-phys > address. (Likely using a different version of the "virt" machine type, > or even a different machine type entirely.) This would not be compatible > with current ArmVirtQemu, which hard-codes the system DRAM base in > several, quite brittle / sensitive, locations. (More on this later -- > that's going to be the larger part of my email anyway.) In order to > handle the new base in ArmVirtQemu, two approaches are possible: change > the hard-coded address(es), or cope with the address dynamically. > > Changing the hard-coded addresses is easy for edk2 contributors (just > add a new build flag like -D DRAM_BASE_AT_XXX_GB, and dependent on it, > set a number of fixed-at-build PCDs to new values). For RHEL downstream, > this is not an un-attractive option, as we are free to break > compatibility at this time. For upstream users and other distros > however, it likely wouldn't be convenient, because "old" ArmVirtQemu > firmware wouldn't boot on the "new" machine type, and vice versa. > > (If we can agree that the above "boundary" in firmwares and machine > types is widely tolerable, then we need not discuss the rest of this > email.) > > > Finally, coping with "any" system DRAM base address in ArmVirtQemu is > both the most flexible for users, and the most difficult to implement. > When QEMU launches the guest, the base of the system DRAM (which equals > the location of the DTB too) is exposed in the x0 register. The > challenge is to make everything in the earliest phases of ArmVirtQemu to > adapt to this value dynamically, and to propagate the value to further > parts of the firmware. > > I've been looking into this for a few days now and would like to pick > your minds on what I've found. > > ( > > As a side note, I know (superficially) of Ard's ArmVirtXen and > ArmVirtQemuKernel work. If I understand correctly, Ard has turned some > of the PCDs I'm about to discuss into "patchable" ones, from > "fixed-at-build". The difference in storage is that these constants > are now placed in the final firmware image such that they are > externally patchable, just before "deployment". > > Because the ArmVirtXen and ArmVirtQemuKernel firmware binaries are > loaded into DRAM immediately, this self-patching -- based on the > initial value of x0 -- is feasible, in the earliest part of the > firmware. (I'm not saying "easy" -- to the contrary; but it's > feasible.) > > However, ArmVirtQemu is launched from pflash; its SEC and PEI phases > execute-in-place from pflash (until MemoryInitPeim installs the > permanent PEI RAM, and the PEI_CORE relocates the HOB list, itself, > and the PEIMs into DRAM). Therefore the SEC and PEI phases of > ArmVirtQemu cannot be patched like this, i.e., through patchable PCDs. > (Unless, of course, the patching is implemented in QEMU itself -- but > I don't think that's probable). > > ) > > Now, exactly because SEC and PEI execute in place from pflash, their > execution (= instruction fetches) are not affected by different initial > x0 values. However, the following are affected: > > - the initial stack, > - the base address of the initial DTB, > - the location of the permanent PEI RAM. > > In ArmVirtQemu, these are represented by the following PCDs (all > fixed-at-build currently): > > - PcdCPUCoresStackBase > - P
[Qemu-devel] QEMU CII Best Practices record
Many projects these days are recording progress wrt CII best practices for FLOOS projects. I filled out a record for QEMU: https://bestpractices.coreinfrastructure.org/projects/1309 I only looked at the 'Passing' criteria, not considered the 'Silver' and 'Gold' criteria. So if anyone else wants to contribute, register an account there and tell me the username whereupon I can add you as a collaborator. Two items I don't think QEMU achieves for the basic "Passing" criteria - The release notes MUST identify every publicly known vulnerability that is fixed in each new release. I don't see a list of CVEs mentioned in our release Changelogs or indeed a historic list of CVEs anywhere even outside the release notes ? - It is SUGGESTED that if the software produced by the project includes software written using a memory-unsafe language (e.g., C or C++), then at least one dynamic tool (e.g., a fuzzer or web application scanner) be routinely used in combination with a mechanism to detect memory safety problems such as buffer overwrites. NB this is not 'coverity' which falls under the 'static anlaysis' group. I'm unclear if anyone in the community does regular fuzzing or analysis with ASAN & equiv ? If i'm wrong just say There's many questions under Silver/Gold level we likely don't meet and some of them start to get quiet opinionated about the way a project should be run, so IMHO its not unreasonable to say we're not going to aim for perfection in this respect. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH] monitor: fix dangling CPU pointer
On Fri, 13 Oct 2017 13:47:51 +0200 Greg Kurz wrote: > If a CPU selected with the "cpu" command is hot-unplugged then "info cpus" > causes QEMU to exit: > > (qemu) device_del cpu1 > (qemu) info cpus > qemu:qemu_cpu_kick_thread: No such process > > This happens because "cpu" stores the pointer to the selected CPU into > the monitor structure. When the CPU is hot-unplugged, we end up with a > dangling pointer. The "info cpus" command then does: > > hmp_info_cpus() > monitor_get_cpu_index() > mon_get_cpu() >cpu_synchronize_state() <--- called with dangling pointer > > This could cause a QEMU crash as well. > > This patch switches the monitor to store the QOM path instead of a > pointer to the current CPU. The path is then resolved when needed. > If the resolution fails, we assume that the CPU was removed and the > path is resetted to the default (ie, path of first_cpu). > > Note that the resolution should really return a CPU object, otherwise > we have a bug. This is achieved by relying on object_resolve_path() > and CPU() instead of calling object_resolve_path_type(path, TYPE_CPU). > > Suggested-by: Igor Mammedov > Signed-off-by: Greg Kurz > --- Whoever merges this patch, please add: Reported-by: Satheesh Rajendran Thanks! > monitor.c | 25 - > 1 file changed, 20 insertions(+), 5 deletions(-) > > diff --git a/monitor.c b/monitor.c > index fe0d1bdbb461..8489b2ad99c0 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -200,7 +200,7 @@ struct Monitor { > > ReadLineState *rs; > MonitorQMP qmp; > -CPUState *mon_cpu; > +gchar *mon_cpu_path; > BlockCompletionFunc *password_completion_cb; > void *password_opaque; > mon_cmd_t *cmd_table; > @@ -579,6 +579,7 @@ static void monitor_data_init(Monitor *mon) > > static void monitor_data_destroy(Monitor *mon) > { > +g_free(mon->mon_cpu_path); > qemu_chr_fe_deinit(&mon->chr, false); > if (monitor_is_qmp(mon)) { > json_message_parser_destroy(&mon->qmp.parser); > @@ -1047,20 +1048,34 @@ int monitor_set_cpu(int cpu_index) > if (cpu == NULL) { > return -1; > } > -cur_mon->mon_cpu = cpu; > +g_free(cur_mon->mon_cpu_path); > +cur_mon->mon_cpu_path = object_get_canonical_path(OBJECT(cpu)); > return 0; > } > > CPUState *mon_get_cpu(void) > { > -if (!cur_mon->mon_cpu) { > +CPUState *cpu; > + > +if (cur_mon->mon_cpu_path) { > +Object *obj = object_resolve_path(cur_mon->mon_cpu_path, NULL); > + > +if (!obj) { > +g_free(cur_mon->mon_cpu_path); > +cur_mon->mon_cpu_path = NULL; > +} else { > +cpu = CPU(obj); > +} > +} > +if (!cur_mon->mon_cpu_path) { > if (!first_cpu) { > return NULL; > } > monitor_set_cpu(first_cpu->cpu_index); > +cpu = first_cpu; > } > -cpu_synchronize_state(cur_mon->mon_cpu); > -return cur_mon->mon_cpu; > +cpu_synchronize_state(cpu); > +return cpu; > } > > CPUArchState *mon_get_cpu_env(void) > >
Re: [Qemu-devel] [PATCH v16 5/5] virtio-balloon: VIRTIO_BALLOON_F_CTRL_VQ
On Thu, Oct 12, 2017 at 11:54:56AM +0800, Wei Wang wrote: > > But I think flushing is very fragile. You will easily run into races > > if one of the actors gets out of sync and keeps adding data. > > I think adding an ID in the free vq stream is a more robust > > approach. > > > > Adding ID to the free vq would need the device to distinguish whether it > receives an ID or a free page hint, Not really. It's pretty simple: a 64 bit buffer is an ID. A 4K and bigger one is a page. > so an extra protocol is needed for the two sides to talk. Currently, we > directly assign the free page > address to desc->addr. With ID support, we would need to first allocate > buffer for the protocol header, > and add the free page address to the header, then desc->addr = &header. I do not think you should add ID on each page. What would be the point? Add it each time you detect a new start command. > How about putting the ID to the command path? This would avoid the above > trouble. > > For example, using the 32-bit config registers: > first 16-bit: Command field > send 16-bit: ID field > > Then, the working flow would look like this: > > 1) Host writes "Start, 1" to the Host2Guest register and notify; > > 2) Guest reads Host2Guest register, and ACKs by writing "Start, 1" to > Guest2Host register; > > 3) Guest starts report free pages; > > 4) Each time when the host receives a free page hint from the free_page_vq, > it compares the ID fields of > the Host2Guest and Guest2Host register. If matching, then filter out the > free page from the migration dirty bitmap, > otherwise, simply push back without doing the filtering. > > > Best, > Wei All fine but config and vq ops are asynchronous. Host has no idea when were entries added to vq. So the ID sent to host needs to be through vq. And I would make it a 64 or at least 32 bit ID, not a 16 bit one, to avoid wrap-around. -- MST