Re: [Qemu-devel] [PATCH v9 03/11] vfio: add aer support for vfio device
Hi Alex, I am Dou. I am testing these patches. At 09/01/2016 03:44 AM, Alex Williamson wrote: [...] + +pcie_cap_deverr_init(pdev); +return pcie_aer_init(pdev, pos, size); pcie_aer_init() adds a v2 AER capability regardless of the version of the AER capability on the device. Is this expected? I think it is not good. v2 defines a lot more bits in various registers than v1, so are we simply hoping that devices have reserved bits as zero like they're supposed to? I read the Capability Version in PCIe Spec. it says: Capability Version – This field is a PCI-SIG defined version number that indicates the version of the Capability structure present. OR This field must be 2h if the End-End TLP Prefix Supported bit (see Section 7.8.15) is Set and must be 1h or 2h otherwise. In my option, I guess that the spec may mean that v1/v2 is used for the End-End TLP Prefix Supported bit supported. And I find that Kernel and QEmu don't do some special work for it. this feature may not affect the registers in AER. can you give me some AER bits that are defined in v2 ,but not in v1? It's a bit strange that for an Intel 82576 NIC I get a v1 AER capability w/o aer=on and a v2 with. Thanks, Yes, I do the test, and get the same result for me. I use the following device to test: 06:00.0 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection (rev 01) Firstly, In host, I use the command "lspci -vvv -s 06:00.0": [...] Capabilities: [100 v1] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq+ ACSViol- UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr- CEMsk: RxErr- BadTLP+ BadDLLP- Rollover- Timeout- NonFatalErr+ AERCap: First Error Pointer: 12, GenCap- CGenEn- ChkCap- ChkEn- [...] Secondly, I pass through the device to the guest. In the guest: [...] Capabilities: [100 v2] Advanced Error Reporting UESta: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq- ACSViol- UEMsk: DLP- SDES- TLP- FCP- CmpltTO- CmpltAbrt- UnxCmplt- RxOF- MalfTLP- ECRC- UnsupReq+ ACSViol- UESvrt: DLP+ SDES- TLP- FCP+ CmpltTO- CmpltAbrt- UnxCmplt- RxOF+ MalfTLP+ ECRC- UnsupReq- ACSViol- CESta: RxErr- BadTLP- BadDLLP- Rollover- Timeout- NonFatalErr- CEMsk: RxErr- BadTLP+ BadDLLP- Rollover- Timeout- NonFatalErr+ AERCap: First Error Pointer: 12, GenCap- CGenEn- ChkCap- ChkEn- [...] They look the same, except the capabilities version. Thirdly, I drop the patches and do the test. After I launch QEmu, In guest: [...] Capabilities: [100 v1] Advanced Error Reporting [...] Fourthly, I modify the PCI_ERR_VER: #define PCI_ERR_VER 1. In guest: [...] Capabilities: [100 v1] Advanced Error Reporting [...] They also have the same features. At last, I think the v1/v2 is not influence on our AER function. But, it is actually strange that we can get a v1 AER capability w/o aer=on and a v2 with. I suggest that we add a capability version parameter to extend the pcie_aer_init(),or add a new pcie_aer_v1_init() for the devices with v1 AER cap. [...] Thanks, Dou.
Re: [Qemu-devel] [PATCH v4 0/4] Introduce error_report_{fatal|abort}
Eric Blake writes: > On 09/09/2016 12:05 PM, Markus Armbruster wrote: >> >> You effectively propose to revise this coding rule from error.h: >> >> * Please don't error_setg(&error_fatal, ...), use error_report() and >> * exit(), because that's more obvious. >> * Likewise, don't error_setg(&error_abort, ...), use assert(). >> >> If we accept your proposal, you get to add a patch to update the rule :) >> >> We've discussed the preferred way to report fatal errors to the human >> user before. With actual patches, we can see how a change of rules >> changes the code. Do we like the change shown by this patch set? > > That includes diffstats, to help gauge the extent of the change (not as > easy is gauging the ratio of the changed code to the rest of the code > that did not need to change - if we are touching 40% of all callers, it > is invasive because the remaining 60% is not that much more dominant; if > we are touching 2% of all callers it is a great patch for keeping us > consistent with the remaining 98%). > > error_report_fatal() had a lot of hits: > > 76 files changed, 557 insertions(+), 799 deletions(-) > create mode 100644 scripts/coccinelle/error_report_fatal.cocci > > while error_report_abort() was not as common: > > 12 files changed, 41 insertions(+), 32 deletions(-) > create mode 100644 scripts/coccinelle/error_report_abort.cocci I count ~1350 occurences of error_report() in master, a smilar number of exit(), and ~650 abort(). The series fuses 329 of them into error_report_fatal(), and 16 into error_report_abort(). Touches roughly one in four error_report(). Additionally, it normalizes 13 uses of error_setg(&error_fatal, ...) and 2 of error_setg(&error_abort, ...). >> I believe there are a number of separate issues to discuss here: >> >> * Shall we get rid of error_setg(&error_fatal, ...)? >> >> This is a no-brainer for me. Such a simple thing should be done in >> one way, not two ways. I count 14 instances of >> error_setg(&error_fatal, ...), but more than 300 of error_report(...); >> exit(1). > > So this adds some of the percentages that I allude to above: 14/300 is > definitely a case where the outliers are worth making common (so getting > rid of error_setg(&error_fatal) makes sense). Now, whether we get rid > of the differences by making it all error_setg()/exit() (to match the > 300), or whether we change ALL of these to a new error_report_fatal (for > 314 changes) is up for a bit more debate: > >> >> * Shall we fuse error_report() and exit() into error_report_fatal()? >> >> Saves ~200 lines, not counting the Coccinelle semantic patch. >> >> I think the real question is what's easier to read and to write. Do >> you prefer something like >> >> error_report("ISA bus not available for %s", c->name); >> exit(1); >> >> or something like >> >> error_report_fatal("ISA bus not available for %s", >>c->name); >> >> The second form saves a tiny bit of instruction space, I guess. > > I can live with error_report_fatal(). It makes indentation a bit more > awkward to think about, and hides the fact that it is exit()ing, but if > done commonly enough (and 314 instances in the code base seems > relatively common) along with a Coccinelle script to enforce it, it > seems like it would be a usable idiom. Roughly 1000 exit() left. Terminating the program when you shouldn't is a fairly common issue. To finding places that do that, you need to grep for functions that do it. The more we create, the more cumbersome that gets. This isn't a particularly strong argument against having error_report_fatal(). However, we need arguments *for* having it :) >> * Shall we get rid of error_setg(&error_abort, ...)? >> >> Getting rid of it is again a no-brainer, but what to replace it with >> isn't. >> >> In my personal opinion, abort() is a perfectly fine way to handle >> "this cannot happen" conditions, and printing pretty messages right >> before abort() is a waste of time. If the abort() happens, the >> program is broken, and all the end user needs to know is that he needs >> to find someone to debug and fix it. If the end user really needs to >> know more, use of abort() is usually wrong. >> >> But others have different opinions. If you want to print pretty >> messages before abort(), you get to print them. >> >> The question is whether to provide a fused error_report_abort(). I'd >> be willing to provide it just for symmetry with error_report_fatal(), >> if we decide we want error_report_fatal(). > > I'm leaning towards error_report_abort() as useless, agreeing with the > argument that reporting a nice message before abort()ing is a waste of > time (the ideal program never aborts, so the nice message is either dead > code, or the error is reachable in situations such that you should not > have been trying to abort). But if we don't
Re: [Qemu-devel] [PATCH v3 06/34] tcg: Add EXCP_ATOMIC
Richard Henderson writes: > On 09/12/2016 07:16 AM, Alex Bennée wrote: >>> +void cpu_exec_step(CPUState *cpu) >>> +{ >>> +CPUArchState *env = (CPUArchState *)cpu->env_ptr; >>> +TranslationBlock *tb; >>> +target_ulong cs_base, pc; >>> +uint32_t flags; >>> +bool old_tb_flushed; >>> + >>> +old_tb_flushed = cpu->tb_flushed; >>> +cpu->tb_flushed = false; >> >> Advanced warning, these disappear soon when the async safe work (plus >> safe tb flush) patches get merged. > > Fair enough. > > Having thought about this more, I think it may be better to handle this > without > flushing the tb. To have parallel_cpus included in the TB flags or somesuch > and keep that TB around. > > My thinking is that there are certain things, like alignment, that could > result > in repeated single-stepping. So better to keep the TB around than keep having > to regenerate it. > >>> +/* ??? When we begin running cpus in parallel, we should >>> + stop all cpus, clear parallel_cpus, and interpret a >>> + single insn with cpu_exec_step. In the meantime, >>> + we should never get here. */ >>> +abort(); >> >> Possibly more correct would be: >> >> g_assert(parallel_cpus == false); >> abort(); > > No, since it is here that we would *set* parallel_cpus to false. Or did you > mean assert parallel_cpus true? Not that that helps for the moment... For SoftMMU this particular loop should never hit because paralell_cpus should be false, hence we never generate any code that might EXCP_ATOMIC. It only fails at the moment because of the "hack" for testing which makes parallel_cpus true. The MTTCG adds a new thread function for MTTCG mode which will have to handle EXCP_ATOMIC as discussed. > >>> +static void step_atomic(CPUState *cpu) >>> +{ >>> +start_exclusive(); >>> + >>> +/* Since we got here, we know that parallel_cpus must be true. */ >>> +parallel_cpus = false; >>> +cpu_exec_step(cpu); >>> +parallel_cpus = true; >>> + >>> +end_exclusive(); >>> +} >>> + >> >> Paolo's safe work patches bring the start/end_exclusive functions into >> cpu-exec-common so I think after that has been merged this function >> can also be moved and called directly from the MTTCG loop on an >> EXCP_ATOMIC exit. > > Excellent. Perhaps I should rebase this upon that right away. Have you got a > pointer to a tree handy? Paolo posted a new version recently but I haven't built a tree with it yet. I was hoping some of the hot-path and maybe barrier stuff would get merged while I finish reviewing this ;-) See: Subject: [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Date: Mon, 12 Sep 2016 13:12:25 +0200 Message-Id: <1473678761-8885-1-git-send-email-pbonz...@redhat.com> > >>> +bool parallel_cpus; >> >> So lets add some words to this to distinguish between this and the mttcg >> enabled flags and its relation to -smp. Something like: >> >> parallel_cpus indicates to the front-ends if code needs to be >> generated taking into account multiple threads of execution. It will >> be true for linux-user after the first thread clone and if system mode >> if MTTCG is enabled. On the transition from false->true any code >> generated while false needs to be invalidated. It may be temporally >> set to false when generating non-cached code in an exclusive context. > > Sure. > > > r~ -- Alex Bennée
Re: [Qemu-devel] [PATCH] virtio-bus: Plug devices after features are negotiated
On 09/12/2016 08:41 PM, Eric Blake wrote: On 09/10/2016 03:23 AM, Maxime Coquelin wrote: Currently, devices are plugged before features are negotiated. If the backend doesn't support VIRTIO_F_VERSION_1, the transport need to rewind some settings. This is the case for CCW, for which a post_plugged callback had been introduced, where max_rev field is just updated if VIRTIO_F_VERSION_1 is not supported by the backend. For PCI, implementing the post_plugged would be much more complicated, so it needs to know whether the backend supports VIRTIO_F_VERSION_1 at plug time. Currently, nothing is done for PCI. Modern capabilitities get s/capabilitities/capabilities/ exposed to the guest even if VIRTIO_F_VERSION_1 is not supported by the backend, which confuses the guest. This patch proposes to replace existing post_plugged solution with an approach that fits with both transports. Features negociation is performed before ->device_plugged() call. s/negociation/negotiation/ Thanks Eric, it will be fixed in next version. Maxime
Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize
Alex Williamson writes: > On Mon, 12 Sep 2016 16:00:18 +0200 > Auger Eric wrote: > >> Hi Markus, >> >> On 12/09/2016 14:45, Markus Armbruster wrote: >> > Eric Auger writes: >> > >> >> This patch converts VFIO PCI to realize function. >> >> >> >> Also original initfn errors now are propagated using QEMU >> >> error objects. All errors are formatted with the same pattern: >> >> "vfio: %s: the error description" >> > >> > Example: >> > >> > $ upstream-qemu -device vfio-pci >> > upstream-qemu: -device vfio-pci: vfio: :00:00.0: no iommu_group found: >> > Unknown error -2 >> > >> > Two remarks: >> > >> > * "Unknown error -2" should be "No such file or directory". See below. >> Hum. I noticed that but I didn't have the presence of mind to get it was >> due to -errno! >> > >> > * Five colons, not counting the ones in the PCI address. Do we need the >> > "vfio: :00:00.0:" part? If yes, could we find a nicer way to >> > print it? Up to you. >> Well we have quite a lot of traces with such format, hence my choice. >> Alex do you want to change this? > > Well, we need to identify the component with the error, it's not > uncommon to have multiple assigned devices. The PCI address is just > the basename in vfio code, it might also be the name of a device node > in sysfs, such as a uuid of an mdev devices. AFAIK we cannot count on > a id and even if we could libvirt assigns them based on order in the > xml, making them a bit magic. Maybe I'm not understanding the > question. Regarding trace vs error message, I expect that it's going > to be a more advanced user/developer enabling tracing, error reports > should try a little harder to be comprehensible to an average user. Yes, the error message needs to identify the part that's wrong. However, we need to consider the *complete* error message to judge it. Consider: $ upstream-qemu -device vfio-pci upstream-qemu: -device vfio-pci: vfio: :00:00.0: no iommu_group found: No such file or directory Which parts are actually useful for the user? "-device vfio-pci" identifies the part that's wrong. "vfio: :00:00.0" is gobbledygook unless you're somewhat familiar with vfio, and then it's redundant. The "vfio: :00:00.0" *is* useful in messages outside realize() context, because then the system can't prefix the "-device vfio-pci" part. >> > Always, always, always test your error messages :) Because what you think you see in the code may differ from what the user will see. Anyway, your choice, I'm just giving feedback on the error messages I observe in testing. [...]
Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error
Cc: Alex for device assignment expertise. Cao jin writes: > On 09/12/2016 09:29 PM, Markus Armbruster wrote: >> Cao jin writes: >> >>> The input parameters is used for creating the msix capable device, so >>> they must obey the PCI spec, or else, it should be programming error. >> >> True when the the parameters come from a device model attempting to >> define a PCI device violating the spec. But what if the parameters come >> from an actual PCI device violating the spec, via device assignment? > > Before the patch, on invalid param, the vfio behaviour is: > error_report("vfio: msix_init failed"); > then, device create fail. > > After the patch, its behaviour is: > asserted. > > Do you mean we should still report some useful info to user on invalid > params? In the normal case, asking msix_init() to create MSI-X that are out of spec is a programming error: the code that does it is broken and needs fixing. Device assignment might be the exception: there, the parameters for msix_init() come from the assigned device, not the program. If they violate the spec, the device is broken. This wouldn't be a programming error. Alex, can this happen? If yes, we may want to handle it by failing device assignment. > Cao jin >> >> For what it's worth, the new behavior seems consistent with msi_init(), >> which is good. Whatever behavior on out-of-spec parameters we choose, msi_init() and msix_init() should behave the same.
Re: [Qemu-devel] [PATCH v2 3/5] pci: Convert msix_init() to Error and fix callers to check it
On 09/12/2016 09:47 PM, Markus Armbruster wrote: Cao jin writes: static void vmxnet3_cleanup_msix(VMXNET3State *s) { @@ -2315,9 +2289,19 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp) * is a programming error. Fall back to INTx silently on -ENOTSUP */ assert(!ret || ret == -ENOTSUP); -if (!vmxnet3_init_msix(s)) { -VMW_WRPRN("Failed to initialize MSI-X, configuration is inconsistent."); -} +ret = msix_init(pci_dev, VMXNET3_MAX_INTRS, +&s->msix_bar, +VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_TABLE, +&s->msix_bar, +VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA(s), +VMXNET3_MSIX_OFFSET(s), NULL); +/* Any error other than -ENOTSUP(board's MSI support is broken) + * is a programming error. Fall back to INTx silently on -ENOTSUP */ +assert(!ret || ret == -ENOTSUP); +s->msix_used = !ret; +/* VMXNET3_MAX_INTRS is passed, so it will never fail when mark vector. + * For simplicity, no need to check return value. */ +vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS); vmxnet3_net_init(s); Uh, this is more than just a conversion to Error. Before, the code falls back to not using MSI-X on any error, with a warning. After, it falls back on ENOTSUP only, silently, and crashes on any other error. Such a change needs to be documented in the commit message, or be in a separate patch. I prefer separate patch. Dmitry has option that we should check the return value of msix_vector_use and prefer to keep init function, so I will withdraw this modification. diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 188f954..4280c5d 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -3594,25 +3594,6 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp) dev->config[PCI_CACHE_LINE_SIZE] = 0x10; dev->config[0x60] = 0x30; /* release number */ -usb_xhci_init(xhci); - -if (xhci->msi != ON_OFF_AUTO_OFF) { -ret = msi_init(dev, 0x70, xhci->numintrs, true, false, &err); -/* Any error other than -ENOTSUP(board's MSI support is broken) - * is a programming error */ -assert(!ret || ret == -ENOTSUP); -if (ret && xhci->msi == ON_OFF_AUTO_ON) { -/* Can't satisfy user's explicit msi=on request, fail */ -error_append_hint(&err, "You have to use msi=auto (default) or " -"msi=off with this machine type.\n"); -error_propagate(errp, err); -return; -} -assert(!err || xhci->msi == ON_OFF_AUTO_AUTO); -/* With msi=auto, we fall back to MSI off silently */ -error_free(err); -} - if (xhci->numintrs > MAXINTRS) { xhci->numintrs = MAXINTRS; } @@ -3622,21 +3603,60 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp) if (xhci->numintrs < 1) { xhci->numintrs = 1; } + if (xhci->numslots > MAXSLOTS) { xhci->numslots = MAXSLOTS; } if (xhci->numslots < 1) { xhci->numslots = 1; } + if (xhci_get_flag(xhci, XHCI_FLAG_ENABLE_STREAMS)) { xhci->max_pstreams_mask = 7; /* == 256 primary streams */ } else { xhci->max_pstreams_mask = 0; } -xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer, xhci); +if (xhci->msi != ON_OFF_AUTO_OFF) { +ret = msi_init(dev, 0x70, xhci->numintrs, true, false, &err); +/* Any error other than -ENOTSUP(board's MSI support is broken) + * is a programming error */ +assert(!ret || ret == -ENOTSUP); +if (ret && xhci->msi == ON_OFF_AUTO_ON) { +/* Can't satisfy user's explicit msi=on request, fail */ +error_append_hint(&err, "You have to use msi=auto (default) or " +"msi=off with this machine type.\n"); +error_propagate(errp, err); +return; +} +assert(!err || xhci->msi == ON_OFF_AUTO_AUTO); +/* With msi=auto, we fall back to MSI off silently */ +error_free(err); +} Can you explain why you're moving this code? Sorry I forget to mention this: msi_init() uses xhci->numintrs, but there is value checking/correcting on xhci->numintrs, it should be done before using. -- Yours Sincerely, Cao jin
Re: [Qemu-devel] [PATCH v2 2/2] e1000: fix buliding complaint
> On 9 Sep 2016, at 10:15 AM, Gonglei (Arei) wrote: > > Who can pick up this patch? Dmitry or Jason? Thanks! Jason, would you please? > > > Regards, > -Gonglei > > >> -Original Message- >> From: Dmitry Fleytman [mailto:dmi...@daynix.com] >> Sent: Wednesday, August 31, 2016 8:59 PM >> To: Gonglei (Arei) >> Cc: qemu-devel@nongnu.org; berra...@redhat.com >> Subject: Re: [PATCH v2 2/2] e1000: fix buliding complaint >> >> Reviewed-by: Dmitry Fleytman >> >>> On 30 Aug 2016, at 07:10 AM, Gonglei wrote: >>> >>> hw/net/e1000e_core.c:56: warning: e1000e_set_interrupt_cause declared >> inline after being called >>> hw/net/e1000e_core.c:56: warning: previous declaration of >> e1000e_set_interrupt_cause was here >>> >>> Signed-off-by: Gonglei >>> --- >>> hw/net/e1000e_core.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c >>> index badb1fe..825e169 100644 >>> --- a/hw/net/e1000e_core.c >>> +++ b/hw/net/e1000e_core.c >>> @@ -2168,7 +2168,7 @@ e1000e_update_interrupt_state(E1000ECore >> *core) >>>} >>> } >>> >>> -static inline void >>> +static void >>> e1000e_set_interrupt_cause(E1000ECore *core, uint32_t val) >>> { >>>trace_e1000e_irq_set_cause_entry(val, core->mac[ICR]); >>> -- >>> 1.7.12.4 >>> >>> >
[Qemu-devel] [PULL 2/2] qapi/block-core: add doc describing GlusterServer vs. SocketAddress
From: Prasanna Kumar Kalever Added documentation describing relation between GlusterServer and SocketAddress qapi schemas. Thanks to Markus Armbruster Reviewed-by: Markus Armbruster Reviewed-by: Jeff Cody Signed-off-by: Prasanna Kumar Kalever Message-id: 1471715924-3642-1-git-send-email-prasanna.kale...@redhat.com Signed-off-by: Jeff Cody --- qapi/block-core.json | 12 1 file changed, 12 insertions(+) diff --git a/qapi/block-core.json b/qapi/block-core.json index 59128f7..173fb08 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2133,6 +2133,18 @@ # # @tcp:host address and port number # +# This is similar to SocketAddress, only distinction: +# +# 1. GlusterServer is a flat union, SocketAddress is a simple union. +#A flat union is nicer than simple because it avoids nesting +#(i.e. more {}) on the wire. +# +# 2. GlusterServer lacks case 'fd', since gluster doesn't let you +#pass in a file descriptor. +# +# GlusterServer is actually not Gluster-specific, its a +# compatibility evolved into an alternate for SocketAddress. +# # Since: 2.7 ## { 'union': 'GlusterServer', -- 2.7.4
[Qemu-devel] [PULL 0/2] Block patches
The following changes since commit 7263da78045dc91cc207f350911efe4259e99b3c: Merge remote-tracking branch 'remotes/mcayland/tags/qemu-openbios-signed' into staging (2016-09-12 15:09:47 +0100) are available in the git repository at: g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request for you to fetch changes up to c76d7aab81c264e3452e778f030fb3760e5edbb9: qapi/block-core: add doc describing GlusterServer vs. SocketAddress (2016-09-13 01:34:55 -0400) Block patches Prasanna Kumar Kalever (2): block/gluster: add support to choose libgfapi logfile qapi/block-core: add doc describing GlusterServer vs. SocketAddress block/gluster.c | 42 ++ qapi/block-core.json | 17 - 2 files changed, 54 insertions(+), 5 deletions(-) -- 2.7.4
[Qemu-devel] [PULL 1/2] block/gluster: add support to choose libgfapi logfile
From: Prasanna Kumar Kalever currently all the libgfapi logs defaults to '/dev/stderr' as it was hardcoded in a call to glfs logging api. When the debug level is chosen to DEBUG/TRACE, gfapi logs will be huge and fill/overflow the console view. This patch provides a commandline option to mention log file path which helps in logging to the specified file and also help in persisting the gfapi logs. Usage: - *URI Style: - -drive file=gluster://hostname/volname/image.qcow2,file.debug=9,\ file.logfile=/var/log/qemu/qemu-gfapi.log *JSON Style: -- 'json:{ "driver":"qcow2", "file":{ "driver":"gluster", "volume":"volname", "path":"image.qcow2", "debug":"9", "logfile":"/var/log/qemu/qemu-gfapi.log", "server":[ { "type":"tcp", "host":"1.2.3.4", "port":24007 }, { "type":"unix", "socket":"/var/run/glusterd.socket" } ] } }' Reviewed-by: Jeff Cody Reviewed-by: Markus Armbruster Signed-off-by: Prasanna Kumar Kalever Signed-off-by: Jeff Cody --- block/gluster.c | 42 ++ qapi/block-core.json | 5 - 2 files changed, 42 insertions(+), 5 deletions(-) diff --git a/block/gluster.c b/block/gluster.c index 01b479f..e7bd13c 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -30,6 +30,8 @@ #define GLUSTER_DEFAULT_PORT24007 #define GLUSTER_DEBUG_DEFAULT 4 #define GLUSTER_DEBUG_MAX 9 +#define GLUSTER_OPT_LOGFILE "logfile" +#define GLUSTER_LOGFILE_DEFAULT "-" /* handled in libgfapi as /dev/stderr */ #define GERR_INDEX_HINT "hint: check in 'server' array index '%d'\n" @@ -44,6 +46,7 @@ typedef struct GlusterAIOCB { typedef struct BDRVGlusterState { struct glfs *glfs; struct glfs_fd *fd; +char *logfile; bool supports_seek_data; int debug_level; } BDRVGlusterState; @@ -73,6 +76,11 @@ static QemuOptsList qemu_gluster_create_opts = { .type = QEMU_OPT_NUMBER, .help = "Gluster log level, valid range is 0-9", }, +{ +.name = GLUSTER_OPT_LOGFILE, +.type = QEMU_OPT_STRING, +.help = "Logfile path of libgfapi", +}, { /* end of list */ } } }; @@ -91,6 +99,11 @@ static QemuOptsList runtime_opts = { .type = QEMU_OPT_NUMBER, .help = "Gluster log level, valid range is 0-9", }, +{ +.name = GLUSTER_OPT_LOGFILE, +.type = QEMU_OPT_STRING, +.help = "Logfile path of libgfapi", +}, { /* end of list */ } }, }; @@ -341,7 +354,7 @@ static struct glfs *qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf, } } -ret = glfs_set_logging(glfs, "-", gconf->debug_level); +ret = glfs_set_logging(glfs, gconf->logfile, gconf->debug_level); if (ret < 0) { goto out; } @@ -576,7 +589,9 @@ static struct glfs *qemu_gluster_init(BlockdevOptionsGluster *gconf, if (ret < 0) { error_setg(errp, "invalid URI"); error_append_hint(errp, "Usage: file=gluster[+transport]://" - "[host[:port]]/volume/path[?socket=...]\n"); +"[host[:port]]volume/path[?socket=...]" +"[,file.debug=N]" +"[,file.logfile=/path/filename.log]\n"); errno = -ret; return NULL; } @@ -586,7 +601,9 @@ static struct glfs *qemu_gluster_init(BlockdevOptionsGluster *gconf, error_append_hint(errp, "Usage: " "-drive driver=qcow2,file.driver=gluster," "file.volume=testvol,file.path=/path/a.qcow2" - "[,file.debug=9],file.server.0.type=tcp," + "[,file.debug=9]" + "[,file.logfile=/path/filename.log]," + "file.server.0.type=tcp," "file.server.0.host=1.2.3.4," "file.server.0.port=24007," "file.server.1.transport=unix," @@ -677,7 +694,7 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options, BlockdevOptionsGluster *gconf = NULL; QemuOpts *opts; Error *local_err = NULL; -const char *filename; +const char *filename, *logfile; opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); qemu_opts_absorb_qdict(opts, options, &local_err); @@ -700,6 +717,13 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict *options, gconf = g_new0(BlockdevOptionsGluster, 1);
Re: [Qemu-devel] [PATCH v6 3/3] tests: add RTAS command in the protocol
On Mon, Sep 12, 2016 at 07:09:37PM +0200, Laurent Vivier wrote: > > > On 12/09/2016 06:17, David Gibson wrote: > > On Thu, Sep 08, 2016 at 09:00:07PM +0200, Laurent Vivier wrote: > >> Add a first test to validate the protocol: > >> > >> - rtas/get-time-of-day compares the time > >> from the guest with the time from the host. > >> > >> Signed-off-by: Laurent Vivier > >> --- > >> v6: > >> - rebase > >> > >> v5: > >> - use qtest_spapr_boot() instead of machine_alloc_init() > >> > >> v4: > >> - use qemu_strtoXXX() instead strtoXX() > >> > >> v3: > >> - use mktimegm() instead of timegm() > >> > >> v2: > >> - add a missing space in qrtas_call() prototype > >> > >> hw/ppc/spapr_rtas.c | 19 > >> include/hw/ppc/spapr_rtas.h | 10 +++ > >> qtest.c | 17 +++ > >> tests/Makefile.include | 3 ++ > >> tests/libqos/rtas.c | 71 > >> + > >> tests/libqos/rtas.h | 11 +++ > >> tests/libqtest.c| 10 +++ > >> tests/libqtest.h| 15 ++ > >> tests/rtas-test.c | 40 + > >> 9 files changed, 196 insertions(+) > >> create mode 100644 include/hw/ppc/spapr_rtas.h > >> create mode 100644 tests/libqos/rtas.c > >> create mode 100644 tests/libqos/rtas.h > >> create mode 100644 tests/rtas-test.c > >> > >> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c > >> index 27b5ad4..b80c1db 100644 > >> --- a/hw/ppc/spapr_rtas.c > >> +++ b/hw/ppc/spapr_rtas.c > >> @@ -37,6 +37,7 @@ > >> > >> #include "hw/ppc/spapr.h" > >> #include "hw/ppc/spapr_vio.h" > >> +#include "hw/ppc/spapr_rtas.h" > >> #include "hw/ppc/ppc.h" > >> #include "qapi-event.h" > >> #include "hw/boards.h" > >> @@ -692,6 +693,24 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, > >> sPAPRMachineState *spapr, > >> return H_PARAMETER; > >> } > >> > >> +uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args, > >> + uint32_t nret, uint64_t rets) > >> +{ > >> +int token; > >> + > >> +for (token = 0; token < RTAS_TOKEN_MAX - RTAS_TOKEN_BASE; token++) { > >> +if (strcmp(cmd, rtas_table[token].name) == 0) { > >> +sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine()); > >> +PowerPCCPU *cpu = POWERPC_CPU(first_cpu); > >> + > >> +rtas_table[token].fn(cpu, spapr, token + RTAS_TOKEN_BASE, > >> + nargs, args, nret, rets); > >> +return H_SUCCESS; > >> +} > >> +} > >> +return H_PARAMETER; > >> +} > > > > I may be misunderstanding how qtest works here. But wouldn't it test > > >From the point of view of the machine, qtest is an accelerator: so the > CPU is stopped, and the qtest accelerator open a socket allowing another > process to read/write/in/out/... the memory of the machine. > This allows to test the machine hardware or the hypervisor, not the CPU > or the firmware. kvm-unit-tests is better for this kind of test. Ah! Yes, I was misunderstanding. So the qtest script is basically replacing the guest's cpu. > The qtest protocol is human readable: the second process sends strings > like "read 0x1000" and the qtest accelerator answers something like "OK > 0" or "ERROR". This is why, from my point of view, using the name of the > service rather than the token number seems better. I see your point. > > a bit more of the common code paths if rather than doing your own > > token lookup here, you actually generated a full guest style rtas > > parameter buffer (token + args + rets in one structure), then dispatch > > We can't do the token lookup at the level of the second process as the > token is extracted from the OF device tree and for the moment we don't > have the functions in this process to scan the OF device tree. It is > possible as we can read the guest memory, but the functions are not > here. I plan to add this kind of feature, for instance to read the > memory size, but for the moment nothing is done. Right. Possible, but fiddly. Leaving it until later seems fair enough. > > through h_rtas, or spapr_rtas_call? > > > >> + > >> void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn) > >> { > >> assert((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX)); > >> diff --git a/include/hw/ppc/spapr_rtas.h b/include/hw/ppc/spapr_rtas.h > >> new file mode 100644 > >> index 000..383611f > >> --- /dev/null > >> +++ b/include/hw/ppc/spapr_rtas.h > >> @@ -0,0 +1,10 @@ > >> +#ifndef HW_SPAPR_RTAS_H > >> +#define HW_SPAPR_RTAS_H > >> +/* > >> + * This work is licensed under the terms of the GNU GPL, version 2 or > >> later. > >> + * See the COPYING file in the top-level directory. > >> + */ > >> + > >> +uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args, > >> + uint32_t nret, uint64_t rets); > >> +#endif /* HW_SPAPR_RTAS_H */ > >> diff --git a/qtest.c b
[Qemu-devel] [PATCH] usb:xhci:fix memory leak in usb_xhci_exit
If the xhci uses msix, it doesn't free the corresponding memory, thus leading a memory leak issue. This patch avoid this. Signed-off-by: Li Qiang --- hw/usb/hcd-xhci.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 188f954..281a2a5 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -3709,8 +3709,7 @@ static void usb_xhci_exit(PCIDevice *dev) /* destroy msix memory region */ if (dev->msix_table && dev->msix_pba && dev->msix_entry_used) { -memory_region_del_subregion(&xhci->mem, &dev->msix_table_mmio); -memory_region_del_subregion(&xhci->mem, &dev->msix_pba_mmio); +msix_uninit(dev, &xhci->mem, &xhci->mem); } usb_bus_release(&xhci->bus); -- 1.8.3.1
Re: [Qemu-devel] [PATCH v2 1/1] qapi/block-core: add doc describing GlusterServer vs. SocketAddress
On Sat, Aug 20, 2016 at 11:28:44PM +0530, Prasanna Kumar Kalever wrote: > Added documentation describing relation between GlusterServer and > SocketAddress qapi schemas. > > Thanks to Markus Armbruster > > Signed-off-by: Prasanna Kumar Kalever > --- > v2: apply suggestions from Markus on v1 > v1: initial doc changes > --- > qapi/block-core.json | 12 > 1 file changed, 12 insertions(+) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 5e2d7d7..4bd513f 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -2121,6 +2121,18 @@ > # > # @tcp:host address and port number > # > +# This is similar to SocketAddress, only distinction: > +# > +# 1. GlusterServer is a flat union, SocketAddress is a simple union. > +#A flat union is nicer than simple because it avoids nesting > +#(i.e. more {}) on the wire. > +# > +# 2. GlusterServer lacks case 'fd', since gluster doesn't let you > +#pass in a file descriptor. > +# > +# GlusterServer is actually not Gluster-specific, its a > +# compatibility evolved into an alternate for SocketAddress. > +# > # Since: 2.7 > ## > { 'union': 'GlusterServer', > -- > 2.7.4 > Thanks, Applied to my block branch: git://github.com/codyprime/qemu-kvm-jtc.git block -Jeff
Re: [Qemu-devel] [PATCH 2/2] mirror: fix improperly filled copy_bitmap for mirror block job
On Fri, Sep 09, 2016 at 03:31:48PM +0300, Denis V. Lunev wrote: > bdrv_is_allocated_above() returns true in the case even for completel > zeroed areas as BDRV_BLOCK_ALLOCATED flag is set in both cases. > > The patch stops using bdrv_is_allocated_above() wrapper and switches to > bdrv_get_block_status_above() to distinguish zeroed areas and areas with > data to avoid extra IO operations if possible. > > Signed-off-by: Denis V. Lunev > CC: Stefan Hajnoczi > CC: Fam Zheng > CC: Kevin Wolf > CC: Max Reitz > CC: Jeff Cody > --- > block/mirror.c | 18 -- > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/block/mirror.c b/block/mirror.c > index e0b3f41..da55375 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -548,14 +548,15 @@ static void mirror_throttle(MirrorBlockJob *s) > > static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) > { > -int64_t sector_num, end; > +int64_t sector_num, end, alloc_mask; > BlockDriverState *base = s->base; > BlockDriverState *bs = blk_bs(s->common.blk); > BlockDriverState *target_bs = blk_bs(s->target); > -int ret, n; > +int n; > > end = s->bdev_length / BDRV_SECTOR_SIZE; > > +alloc_mask = BDRV_BLOCK_ALLOCATED; > if (base == NULL && !bdrv_has_zero_init(target_bs)) { > if (!bdrv_can_write_zeroes_with_unmap(target_bs)) { > bdrv_set_dirty_bitmap(s->dirty_bitmap, 0, end); > @@ -583,6 +584,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob > *s) > } > > mirror_drain(s); > + > +alloc_mask = BDRV_BLOCK_DATA; What about when base == NULL, and bdrv_has_zero_init(target_bs) == true? In that case we also know the target image is zeroed, but this does not take advantage of that. > } > > /* First part, loop on the sectors and initialize the dirty bitmap. */ > @@ -590,6 +593,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob > *s) > /* Just to make sure we are not exceeding int limit. */ > int nb_sectors = MIN(INT_MAX >> BDRV_SECTOR_BITS, > end - sector_num); > +int64_t status; > +BlockDriverState *file; > > mirror_throttle(s); > > @@ -597,13 +602,14 @@ static int coroutine_fn > mirror_dirty_init(MirrorBlockJob *s) > return 0; > } > > -ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n); > -if (ret < 0) { > -return ret; > +status = bdrv_get_block_status_above(bs, base, sector_num, > + nb_sectors, &n, &file); > +if (status < 0) { > +return status; > } > > assert(n > 0); > -if (ret == 1) { > +if (status & alloc_mask) { > bdrv_set_dirty_bitmap(s->dirty_bitmap, sector_num, n); > } > sector_num += n; -Jeff
[Qemu-devel] [PATCH v2 14/15] virtio-crypto: add data virtqueue processing handler
At present, we only support cipher and algorithm chainning. Signed-off-by: Gonglei --- hw/virtio/virtio-crypto.c | 364 ++ 1 file changed, 364 insertions(+) diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c index 96c5a2a..16b1a2b 100644 --- a/hw/virtio/virtio-crypto.c +++ b/hw/virtio/virtio-crypto.c @@ -22,6 +22,8 @@ #include "hw/virtio/virtio-crypto.h" #include "hw/virtio/virtio-access.h" +static int32_t virtio_crypto_flush_dataq(VirtIOCryptoQueue *q); + static void virtio_crypto_process(VirtIOCrypto *vcrypto) { } @@ -31,6 +33,14 @@ static inline int virtio_crypto_vq2q(int queue_index) return queue_index; } +static VirtIOCryptoQueue * +virtio_crypto_get_subqueue(CryptoClientState *cc) +{ +VirtIOCrypto *vcrypto = qemu_get_crypto_legacy_hw_opaque(cc); + +return &vcrypto->vqs[cc->queue_index]; +} + static void virtio_crypto_cipher_session_helper(VirtIODevice *vdev, CryptoSymSessionInfo *info, @@ -255,12 +265,366 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) } } +static CryptoSymOpInfo * +virtio_crypto_cipher_op_helper(VirtIODevice *vdev, + struct virtio_crypto_cipher_para *para, + struct virtio_crypto_cipher_output *out, + uint32_t aad_len, + uint64_t aad_data_addr) +{ +CryptoSymOpInfo *op_info; +uint32_t src_len, dst_len; +uint32_t iv_len; +size_t max_len, curr_size = 0; +hwaddr iv_gpa, src_gpa; +void *iv_hva, *src_hva, *aad_hva; +hwaddr len; + +iv_len = para->iv_len; +src_len = para->src_data_len; +dst_len = para->dst_data_len; + +max_len = iv_len + aad_len + src_len + dst_len; +op_info = g_malloc0(sizeof(CryptoSymOpInfo) + max_len); +op_info->iv_len = iv_len; +op_info->src_len = src_len; +op_info->dst_len = dst_len; +op_info->aad_len = aad_len; +/* handle the initilization vector */ +if (op_info->iv_len > 0) { +len = op_info->iv_len; +DPRINTF("iv_len=%" PRIu32 "\n", len); +op_info->iv = op_info->data + curr_size; + +iv_gpa = out->iv_addr; +iv_hva = cpu_physical_memory_map(iv_gpa, &len, false); +memcpy(op_info->iv, iv_hva, len); +cpu_physical_memory_unmap(iv_hva, len, false, len); +curr_size += len; +} + +/* handle additional authentication data if exist */ +if (op_info->aad_len > 0) { +len = op_info->aad_len; +DPRINTF("aad_len=%" PRIu32 "\n", len); +op_info->aad_data = op_info->data + curr_size; + +aad_hva = cpu_physical_memory_map(aad_data_addr, &len, false); +memcpy(op_info->aad_data, aad_hva, len); +cpu_physical_memory_unmap(aad_hva, len, false, len); +curr_size += len; +} + +/* handle the source data */ +if (op_info->src_len > 0) { +len = op_info->src_len; +DPRINTF("src_len=%" PRIu32 "\n", len); +op_info->src = op_info->data + curr_size; + +src_gpa = out->src_data_addr; +src_hva = cpu_physical_memory_map(src_gpa, &len, false); +memcpy(op_info->src, src_hva, len); +cpu_physical_memory_unmap(src_hva, len, false, len); +curr_size += len; +} +op_info->dst = op_info->data + curr_size; +DPRINTF("dst_len=%" PRIu32 "\n", op_info->dst_len); + +return op_info; +} + +static void +virtio_crypto_sym_input_data_helper(VirtIODevice *vdev, +void *idata_hva, +uint32_t status, +CryptoSymOpInfo *sym_op_info) +{ +struct virtio_crypto_sym_input *idata = idata_hva; +hwaddr dst_gpa, len; +void *dst_hva; + +idata->status = status; +if (status != VIRTIO_CRYPTO_OP_OK) { +return; +} + +/* save the cipher result */ +dst_gpa = idata->dst_data_addr; +/* Note: length of dest_data is equal to length of src_data for cipher */ +len = sym_op_info->src_len; +dst_hva = cpu_physical_memory_map(dst_gpa, &len, true); +memcpy(dst_hva, sym_op_info->dst, len); +cpu_physical_memory_unmap(dst_hva, len, true, len); + +if (sym_op_info->op_type == + VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) { +hwaddr digest_gpa; +void *digest_hva; + +/* save the digest result */ +digest_gpa = idata->digest_result_addr; +len = sym_op_info->dst_len - sym_op_info->src_len; +digest_hva = cpu_physical_memory_map(digest_gpa, &len, true); +memcpy(digest_hva, sym_op_info->dst + sym_op_info->src_len, len); +cpu_physical_memory_unmap(digest_hva, len, true, len); +} +} + +static void virtio_crypto_tx_complete(CryptoClientState *cc, + int ret) +{ +VirtIOCrypto *vcrypto = qemu_get_crypto_legacy_hw_opaque(cc); +VirtIOCryptoQueue *q = virtio_crypto_get_subqueue(cc); +VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto); +uint32_t flags = q->async_tx.flags; + +if (flags
[Qemu-devel] [PATCH v2 07/15] virtio-crypto: introduce virtio-crypto.h
This patch introduces the header of virtio crypto emulation. Signed-off-by: Gonglei --- include/hw/virtio/virtio-crypto.h | 84 +++ 1 file changed, 84 insertions(+) create mode 100644 include/hw/virtio/virtio-crypto.h diff --git a/include/hw/virtio/virtio-crypto.h b/include/hw/virtio/virtio-crypto.h new file mode 100644 index 000..243ed71 --- /dev/null +++ b/include/hw/virtio/virtio-crypto.h @@ -0,0 +1,84 @@ +/* + * Virtio crypto Support + * + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. + * + * Authors: + *Gonglei + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * (at your option) any later version. See the COPYING file in the + * top-level directory. + */ + +#ifndef _QEMU_VIRTIO_CRYPTO_H +#define _QEMU_VIRTIO_CRYPTO_H + +#include "standard-headers/linux/virtio_crypto.h" +#include "hw/virtio/virtio.h" +#include "sysemu/iothread.h" +#include "crypto/crypto.h" + +#define VIRTIO_ID_CRYPTO 20 + +/* #define DEBUG_VIRTIO_CRYPTO */ + +#ifdef DEBUG_VIRTIO_CRYPTO +#define DPRINTF(fmt, ...) \ +do { printf("virtio_crypto: " fmt , ## __VA_ARGS__); } while (0) +#else +#define DPRINTF(fmt, ...) do { } while (0) +#endif + +#define TYPE_VIRTIO_CRYPTO "virtio-crypto-device" +#define VIRTIO_CRYPTO(obj) \ +OBJECT_CHECK(VirtIOCrypto, (obj), TYPE_VIRTIO_CRYPTO) +#define VIRTIO_CRYPTO_GET_PARENT_CLASS(obj) \ +OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_CRYPTO) + + +/* Limit the number of packets that can be sent via a single flush + * of the TX queue. This gives us a guaranteed exit condition and + * ensures fairness in the io path. 256 conveniently matches the + * length of the TX queue and shows a good balance of performance + * and latency. */ +#define VIRTIO_CRYPTO_TX_BURST 256 + +typedef struct VirtIOCryptoConf { +int32_t txburst; +} VirtIOCryptoConf; + +struct VirtIOCrypto; + +typedef struct VirtIOCryptoQueue { +VirtQueue *dataq; +QEMUBH *tx_bh; +int tx_waiting; +struct { +VirtQueueElement *elem; +uint32_t flags; +CryptoSymOpInfo *op_info; +void *idata_hva; +} async_tx; +struct VirtIOCrypto *vcrypto; +} VirtIOCryptoQueue; + +typedef struct VirtIOCrypto { +VirtIODevice parent_obj; + +VirtIOCryptoQueue *vqs; +VirtQueue *ctrl_vq; +CryptoLegacyHWState *crypto; +CryptoLegacyHWConf legacy_conf; + +VirtIOCryptoConf conf; +int32_t tx_burst; +uint32_t max_queues; +uint32_t status; + +int multiqueue; +uint32_t curr_queues; +size_t config_size; +} VirtIOCrypto; + +#endif /* _QEMU_VIRTIO_CRYPTO_H */ -- 1.7.12.4
[Qemu-devel] [PATCH v2 15/15] virtio-crypto: support scatter gather list
Now, we can support scatter gather list on source data, destination data, and associated anthentication data. Signed-off-by: Gonglei --- hw/virtio/virtio-crypto.c | 224 + include/hw/virtio/virtio-crypto.h | 12 ++ include/standard-headers/linux/virtio_crypto.h | 46 +++-- 3 files changed, 235 insertions(+), 47 deletions(-) diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c index 16b1a2b..661223b 100644 --- a/hw/virtio/virtio-crypto.c +++ b/hw/virtio/virtio-crypto.c @@ -265,25 +265,177 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) } } +static void virtio_crypto_map_iovec(unsigned int *p_num_sg, hwaddr *addr, + struct iovec *iov, + unsigned int max_num_sg, + hwaddr pa, size_t sz, + bool is_write) +{ +unsigned num_sg = *p_num_sg; +assert(num_sg <= max_num_sg); + +if (!sz) { +error_report("virtio-crypto: zero sized buffers are not allowed"); +exit(1); +} + +while (sz) { +hwaddr len = sz; + +if (num_sg == max_num_sg) { +error_report("virtio-crypto: too many entries " +"in the scatter gather list"); +exit(1); +} + +iov[num_sg].iov_base = cpu_physical_memory_map(pa, &len, is_write); +iov[num_sg].iov_len = len; +addr[num_sg] = pa; + +sz -= len; +pa += len; +num_sg++; +} +*p_num_sg = num_sg; +} + +static void virtio_crypto_unmap_iovec(VirtIOCryptoBuffer *buf, + unsigned int len, + bool is_write) +{ +unsigned int offset; +int i; + +if (is_write) { +offset = 0; +for (i = 0; i < buf->num; i++) { +size_t size = MIN(len - offset, buf->sg[i].iov_len); + +cpu_physical_memory_unmap(buf->sg[i].iov_base, + buf->sg[i].iov_len, + 1, size); + +offset += size; +} +} else { +for (i = 0; i < buf->num; i++) { +cpu_physical_memory_unmap(buf->sg[i].iov_base, + buf->sg[i].iov_len, + 0, buf->sg[i].iov_len); +} +} +} + +static void *virtio_crypto_read_next_iovec(VirtIODevice *vdev, +struct virtio_crypto_iovec *iovec, +bool is_write, +struct iovec *iov, +unsigned int *num) +{ +struct virtio_crypto_iovec *iovec_hva; +hwaddr pa; +hwaddr len; + +/* If this descriptor says it doesn't chain, we're done. */ +if (!(iovec->flags & VIRTIO_CRYPTO_IOVEC_F_NEXT)) { +return NULL; +} + +pa = iovec->next_iovec; +len = sizeof(*iovec_hva); +iovec_hva = cpu_physical_memory_map(pa, &len, is_write); +assert(len == sizeof(*iovec_hva)); + +iov[*num].iov_base = iovec_hva; +iov[*num].iov_len = len; +(*num)++; + +return iovec_hva; +} + +static void *virtio_crypto_alloc_buf(unsigned num) +{ +VirtIOCryptoBuffer *buf; +size_t addr_ofs = QEMU_ALIGN_UP(sizeof(*buf), __alignof__(buf->addr[0])); +size_t addr_end = addr_ofs + num * sizeof(buf->addr[0]); +size_t sg_ofs = QEMU_ALIGN_UP(addr_end, __alignof__(buf->sg[0])); +size_t sg_end = sg_ofs + num * sizeof(buf->sg[0]); + +buf = g_malloc(sg_end); +buf->num = num; + +buf->addr = (void *)buf + addr_ofs; +buf->sg = (void *)buf + sg_ofs; +return buf; +} + +static void *virtio_crypto_iovec_read(VirtIODevice *vdev, + struct virtio_crypto_iovec *iovec, + bool is_write) +{ + +VirtIOCryptoBuffer *buf; +hwaddr addr[VIRTIO_CRYPTO_SG_MAX]; +struct iovec iov[VIRTIO_CRYPTO_SG_MAX]; +unsigned int num = 0; +/* Save virtio_crypto_iov structure's hva information in sg_list */ +struct iovec vc_iov[VIRTIO_CRYPTO_SG_MAX]; +unsigned int vc_num = 0; +unsigned int i; + +struct virtio_crypto_iovec *p_iovec = iovec; + +/* Collect all the sgs */ +do { +virtio_crypto_map_iovec(&num, addr, iov, + VIRTIO_CRYPTO_SG_MAX, + p_iovec->addr, p_iovec->len, + is_write); +} while ((p_iovec = virtio_crypto_read_next_iovec(vdev, +p_iovec, false, vc_iov, &vc_num)) +!= NULL); + +/* Now copy what we have collected and mapped */ +buf = virtio_crypto_alloc_buf(num); +for (i = 0; i < num; i++) { +buf->addr[i] = addr[i]; +buf->sg[i] = iov[i]; +} +/* Unmap all virtio_crypto_iov structure if exists */ +for (i = 0; i < vc_num; i++) { +cpu_physical_memory_unmap(vc_iov[i].iov_base, +
[Qemu-devel] [PATCH v2 13/15] virtio-crypto: get correct input data address for each request
We MUST use the original hva for input data, but not the copyed address, otherwise the guest can't get the results. Fix a non-initial problem in an exception case as well. Signed-off-by: Gonglei --- hw/virtio/virtio-crypto.c | 37 ++--- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c index 0d7ab63..96c5a2a 100644 --- a/hw/virtio/virtio-crypto.c +++ b/hw/virtio/virtio-crypto.c @@ -62,7 +62,8 @@ static int64_t virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto, struct virtio_crypto_sym_create_session_req *sess_req, uint32_t queue_id, - uint64_t *session_id) + uint64_t *session_id, + VirtQueueElement *elem) { VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto); CryptoSymSessionInfo info; @@ -74,6 +75,8 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto, void *auth_key_hva; struct virtio_crypto_session_input *input; hwaddr len; +size_t input_offset; +struct iovec *iov = elem->in_sg; memset(&info, 0, sizeof(info)); op_type = sess_req->op_type; @@ -83,13 +86,19 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto, virtio_crypto_cipher_session_helper(vdev, &info, &sess_req->u.cipher.para, &sess_req->u.cipher.out); -input = &sess_req->u.cipher.input; +/* calculate the offset of input data */ +input_offset = offsetof(struct virtio_crypto_op_ctrl_req, + u.sym_create_session.u.cipher.input); +input = (void *)iov[0].iov_base + input_offset; } else if (op_type == VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) { /* cipher part */ virtio_crypto_cipher_session_helper(vdev, &info, &sess_req->u.chain.para.cipher_param, &sess_req->u.chain.out.cipher); -input = &sess_req->u.chain.input; +/* calculate the offset of input data */ +input_offset = offsetof(struct virtio_crypto_op_ctrl_req, +u.sym_create_session.u.chain.input); +input = (void *)iov[0].iov_base + input_offset; /* hash part */ info.alg_chain_order = sess_req->u.chain.para.alg_chain_order; info.add_len = sess_req->u.chain.para.aad_len; @@ -120,6 +129,10 @@ virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto, goto err; } } else { +/* calculate the offset of input data */ +input_offset = offsetof(struct virtio_crypto_op_ctrl_req, +u.sym_create_session.u.cipher.input); +input = (void *)iov[0].iov_base + input_offset; /* VIRTIO_CRYPTO_SYM_OP_NONE */ error_report("unsupported cipher type"); goto err; @@ -149,13 +162,17 @@ err: static void virtio_crypto_handle_close_session(VirtIOCrypto *vcrypto, struct virtio_crypto_destroy_session_req *close_sess_req, - uint32_t queue_id) + uint32_t queue_id, + VirtQueueElement *elem) { int ret; CryptoClientState *cc = vcrypto->crypto->ccs; uint64_t session_id; uint32_t status; int queue_index = virtio_crypto_vq2q(queue_id); +struct iovec *iov = elem->in_sg; +size_t status_offset; +void *in_status_ptr; session_id = close_sess_req->session_id; DPRINTF("close session, id=%" PRIu64 "\n", session_id); @@ -168,8 +185,12 @@ virtio_crypto_handle_close_session(VirtIOCrypto *vcrypto, status = VIRTIO_CRYPTO_OP_ERR; } +/* calculate the offset of status bits */ +status_offset = offsetof(struct virtio_crypto_op_ctrl_req, + u.destroy_session.status); +in_status_ptr = (void *)iov[0].iov_base + status_offset; /* Set the result, notify the frontend driver soon */ -close_sess_req->status = status; +memcpy(in_status_ptr, &status, sizeof(status)); } static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) @@ -207,7 +228,8 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) virtio_crypto_create_sym_session(vcrypto, &ctrl.u.sym_create_session, queue_id, - &session_id); + &session_id, + elem); break; case VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION: @@ -215,7 +237,8 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) case VIRTIO_CRYPTO_MAC_DESTROY_SESSION: case VIRTIO_CRYPTO_AEAD_DESTROY_SESSION: virtio_crypto_handle_close_session(vcrypto, - &ctrl.u.destroy_session, queue_id); + &ctrl.u.destroy_session, queue_id, + elem); break; case VIRTIO_CRY
[Qemu-devel] [PATCH v2 04/15] crypto: add symetric algorithms support
This patch include three parts, the first is define the session structure and the second is the opertion structure, whose properties are needed to finish the symetric algorithms. The third part defines some function pointers. Signed-off-by: Gonglei --- crypto/crypto.c | 16 +- include/crypto/crypto.h | 85 + 2 files changed, 100 insertions(+), 1 deletion(-) diff --git a/crypto/crypto.c b/crypto/crypto.c index 3f760fd..958a959 100644 --- a/crypto/crypto.c +++ b/crypto/crypto.c @@ -177,7 +177,21 @@ int qemu_deliver_crypto_packet(CryptoClientState *sender, void *header_opqaue, void *opaque) { -return 0; +CryptoClientState *cc = opaque; +int ret = -1; + +if (!cc->ready) { +return 1; +} + +if (flags == QEMU_CRYPTO_PACKET_FLAG_SYM) { +CryptoSymOpInfo *op_info = header_opqaue; +if (cc->info->do_sym_op) { +ret = cc->info->do_sym_op(cc, op_info); +} +} + +return ret; } int qemu_send_crypto_packet_async(CryptoClientState *sender, diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h index 4f0efb7..95cca23 100644 --- a/include/crypto/crypto.h +++ b/include/crypto/crypto.h @@ -33,10 +33,50 @@ #define MAX_CRYPTO_QUEUE_NUM 64 +#define QEMU_CRYPTO_PACKET_FLAG_NONE (0) +#define QEMU_CRYPTO_PACKET_FLAG_SYM (1 << 0) + +typedef struct CryptoSymSessionInfo { +uint8_t op_code; +uint8_t op_type; +uint8_t direction; +uint32_t cipher_alg; +uint32_t key_len; +uint8_t *cipher_key; + +uint32_t hash_alg; +uint8_t hash_mode; +uint32_t hash_result_len; +uint8_t alg_chain_order; +uint32_t auth_key_len; +uint32_t add_len; +uint8_t *auth_key; +} CryptoSymSessionInfo; + +typedef struct CryptoSymOpInfo { +uint64_t session_id; +uint8_t op_type; /* cipher or algo chainning */ +uint8_t *src; +uint8_t *dst; +uint8_t *iv; +uint8_t *aad_data; /* additional auth data */ +uint32_t aad_len; +uint32_t iv_len; +uint32_t src_len; +/* the dst_len is equal to src_len + hash_result_len + * if hash alg configured */ +uint32_t dst_len; +uint8_t data[0]; +} CryptoSymOpInfo; + typedef void (CryptoPoll)(CryptoClientState *, bool); typedef void (CryptoCleanup) (CryptoClientState *); typedef void (CryptoClientDestructor)(CryptoClientState *); typedef void (CryptoHWStatusChanged)(CryptoClientState *); +typedef int (CryptoCreateSymSession)(CryptoClientState *, + CryptoSymSessionInfo *, uint64_t *); +typedef int (CryptoCloseSession)(CryptoClientState *, uint64_t); +typedef int (CryptoDoSymOp)(CryptoClientState *, CryptoSymOpInfo *); typedef struct CryptoClientInfo { CryptoClientOptionsKind type; @@ -45,6 +85,9 @@ typedef struct CryptoClientInfo { CryptoCleanup *cleanup; CryptoPoll *poll; CryptoHWStatusChanged *hw_status_changed; +CryptoCreateSymSession *create_session; +CryptoCloseSession *close_session; +CryptoDoSymOp *do_sym_op; } CryptoClientInfo; struct CryptoClientState { @@ -57,6 +100,21 @@ struct CryptoClientState { char info_str[256]; CryptoQueue *incoming_queue; unsigned int queue_index; + +/* Supported service mask */ +uint32_t crypto_services; + +/* Detailed algorithms mask */ +uint32_t cipher_algo_l; +uint32_t cipher_algo_h; +uint32_t hash_algo; +uint32_t mac_algo_l; +uint32_t mac_algo_h; +uint32_t asym_algo; +uint32_t kdf_algo; +uint32_t aead_algo; +uint32_t primitive_algo; + CryptoClientDestructor *destructor; }; @@ -69,6 +127,20 @@ typedef struct CryptoLegacyHWPeers { typedef struct CryptoLegacyHWConf { CryptoLegacyHWPeers peers; + +/* Supported service mask */ +uint32_t crypto_services; + +/* Detailed algorithms mask */ +uint32_t cipher_algo_l; +uint32_t cipher_algo_h; +uint32_t hash_algo; +uint32_t mac_algo_l; +uint32_t mac_algo_h; +uint32_t asym_algo; +uint32_t kdf_algo; +uint32_t aead_algo; +uint32_t primitive_algo; } CryptoLegacyHWConf; typedef struct CryptoLegacyHWState { @@ -104,4 +176,17 @@ void qemu_del_crypto_legacy_hw(CryptoLegacyHWState *crypto); CryptoClientState * qemu_get_crypto_subqueue(CryptoLegacyHWState *crypto, int queue_index); +CryptoLegacyHWState *qemu_get_crypto_legacy_hw(CryptoClientState *cc); + +void *qemu_get_crypto_legacy_hw_opaque(CryptoClientState *cc); + +int qemu_find_crypto_clients_except(const char *id, CryptoClientState **ccs, + CryptoClientOptionsKind type, int max); + +int qemu_crypto_create_session(CryptoClientState *cc, + CryptoSymSessionInfo *info, + uint64_t *session_id); +int qemu_crypto_close_session(CryptoClientState *cc, + uint64_t session_id); + #endif /
[Qemu-devel] [PATCH v2 02/15] crypto: introduce crypto queue handler
crypto queue is a gallery used for executing crypto operation, which supports both synchronization and asynchronization. The thoughts stolen from net/queue.c Signed-off-by: Gonglei --- crypto/Makefile.objs | 1 + crypto/crypto-queue.c | 206 ++ crypto/crypto.c | 28 ++ include/crypto/crypto-queue.h | 69 ++ include/crypto/crypto.h | 12 +++ 5 files changed, 316 insertions(+) create mode 100644 crypto/crypto-queue.c create mode 100644 include/crypto/crypto-queue.h diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs index 2a63cb8..652b429 100644 --- a/crypto/Makefile.objs +++ b/crypto/Makefile.objs @@ -27,6 +27,7 @@ crypto-obj-y += block.o crypto-obj-y += block-qcow.o crypto-obj-y += block-luks.o crypto-obj-y += crypto.o +crypto-obj-y += crypto-queue.o # Let the userspace emulators avoid linking gnutls/etc crypto-aes-obj-y = aes.o diff --git a/crypto/crypto-queue.c b/crypto/crypto-queue.c new file mode 100644 index 000..3e91be9 --- /dev/null +++ b/crypto/crypto-queue.c @@ -0,0 +1,206 @@ +/* + * Queue management for crypto device (based on net/qeueu.c) + * + * Copyright (c) 2003-2008 Fabrice Bellard + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. + * + * Authors: + *Gonglei + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "qemu/osdep.h" +#include "crypto/crypto-queue.h" +#include "crypto/crypto.h" +#include "qemu/queue.h" + + +/* The delivery handler may only return zero if it will call + * qemu_crypto_queue_flush() when it determines that it is once again able + * to deliver packets. It must also call qemu_crypto_queue_purge() in its + * cleanup path. + * + * If a sent callback is provided to send(), the caller must handle a + * zero return from the delivery handler by not sending any more packets + * until we have invoked the callback. Only in that case will we queue + * the packet. + * + * If a sent callback isn't provided, we just drop the packet to avoid + * unbounded queueing. + */ + +struct CryptoPacket { +QTAILQ_ENTRY(CryptoPacket) entry; +CryptoClientState *sender; +unsigned flags; /* algorithms' type etc. */ +CryptoPacketSent *sent_cb; /* callback after packet sent */ +void *opaque; /* header struct pointer of operation */ +uint8_t data[0]; +}; + +struct CryptoQueue { +void *opaque; +uint32_t nq_maxlen; +uint32_t nq_count; +CryptoQueueDeliverFunc *deliver; + +QTAILQ_HEAD(packets, CryptoPacket) packets; + +unsigned delivering:1; +}; + +CryptoQueue * +qemu_new_crypto_queue(CryptoQueueDeliverFunc *deliver, void *opaque) +{ +CryptoQueue *queue; + +queue = g_new0(CryptoQueue, 1); + +queue->opaque = opaque; +queue->nq_maxlen = 1; +queue->nq_count = 0; +queue->deliver = deliver; + +QTAILQ_INIT(&queue->packets); + +queue->delivering = 0; + +return queue; +} + +void qemu_del_crypto_queue(CryptoQueue *queue) +{ +CryptoPacket *packet, *next; + +QTAILQ_FOREACH_SAFE(packet, &queue->packets, entry, next) { +QTAILQ_REMOVE(&queue->packets, packet, entry); +g_free(packet->opaque); +g_free(packet); +} + +g_free(queue); +} + +void qemu_crypto_queue_cache(CryptoQueue *queue, + unsigned flags, + CryptoClientState *sender, + void *opaque, + CryptoPacketSent *sent_cb) +{ +CryptoPacket *packet; + +if (queue->nq_count >= queue->nq_maxlen && !sent_cb) { +return; /* drop if queue full and no callback */ +} + +packet = g_malloc(sizeof(CryptoPacket)); +packet->sender = sender; +packet->sent_cb = sent_cb; +packet->flags = flags, +packet->opaque = opaque; + +queue->nq_count++; +QTAILQ_INSERT_TAIL(&queue->packets, packet, entry); +} + +static ssize_t qemu_crypto_queue_de
[Qemu-devel] [PATCH v2 11/15] virtio-crypto: add control queue handler
Realize the Symmetric algos session creation handler, including plain cipher and chainning algorithms. Signed-off-by: Gonglei --- hw/virtio/virtio-crypto.c | 175 +- 1 file changed, 174 insertions(+), 1 deletion(-) diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c index b5a108f..1090946 100644 --- a/hw/virtio/virtio-crypto.c +++ b/hw/virtio/virtio-crypto.c @@ -26,8 +26,181 @@ static void virtio_crypto_process(VirtIOCrypto *vcrypto) { } +static inline int virtio_crypto_vq2q(int queue_index) +{ +return queue_index; +} + +static void +virtio_crypto_cipher_session_helper(VirtIODevice *vdev, + CryptoSymSessionInfo *info, + struct virtio_crypto_cipher_session_para *cipher_para, + struct virtio_crypto_cipher_session_output *cipher_out) +{ +hwaddr key_gpa; +void *key_hva; +hwaddr len; + +info->cipher_alg = cipher_para->algo; +info->key_len = cipher_para->keylen; +info->direction = cipher_para->op; +len = info->key_len; +/* get cipher key */ +if (len > 0) { +DPRINTF("keylen=%" PRIu32 "\n", info->key_len); +key_gpa = cipher_out->key_addr; + +key_hva = cpu_physical_memory_map(key_gpa, &len, 0); + +info->cipher_key = g_malloc(info->key_len); +memcpy(info->cipher_key, key_hva, info->key_len); +cpu_physical_memory_unmap(key_hva, len, 0, len); +} +} + +static int64_t +virtio_crypto_create_sym_session(VirtIOCrypto *vcrypto, + struct virtio_crypto_sym_create_session_req *sess_req, + uint32_t queue_id, + uint64_t *session_id) +{ +VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto); +CryptoSymSessionInfo info; +int ret; +CryptoClientState *cc; +int queue_index;; +uint32_t op_type; +hwaddr auth_key_gpa; +void *auth_key_hva; +struct virtio_crypto_session_input *input; +hwaddr len; + +memset(&info, 0, sizeof(info)); +op_type = sess_req->op_type; +info.op_type = op_type; + +if (op_type == VIRTIO_CRYPTO_SYM_OP_CIPHER) { +virtio_crypto_cipher_session_helper(vdev, &info, + &sess_req->u.cipher.para, + &sess_req->u.cipher.out); +input = &sess_req->u.cipher.input; +} else if (op_type == VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING) { +/* cipher part */ +virtio_crypto_cipher_session_helper(vdev, &info, + &sess_req->u.chain.para.cipher_param, + &sess_req->u.chain.out.cipher); +input = &sess_req->u.chain.input; +/* hash part */ +info.alg_chain_order = sess_req->u.chain.para.alg_chain_order; +info.add_len = sess_req->u.chain.para.aad_len; +info.hash_mode = sess_req->u.chain.para.hash_mode; +if (info.hash_mode == VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH) { +info.hash_alg = sess_req->u.chain.para.u.mac_param.algo; +len = info.auth_key_len = + sess_req->u.chain.para.u.mac_param.auth_key_len; +info.hash_result_len = +sess_req->u.chain.para.u.mac_param.hash_result_len; +/* get auth key */ +if (len > 0) { +DPRINTF("keylen=%" PRIu32 "\n", info.auth_key_len); +auth_key_gpa = sess_req->u.chain.out.mac.auth_key_addr; +auth_key_hva = cpu_physical_memory_map(auth_key_gpa, + &len, false); +info.auth_key = g_malloc(len); +memcpy(info.auth_key, auth_key_hva, len); +cpu_physical_memory_unmap(auth_key_hva, len, false, len); +} +} else if (info.hash_mode == VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN) { +info.hash_alg = sess_req->u.chain.para.u.hash_param.algo; +info.hash_result_len = + sess_req->u.chain.para.u.hash_param.hash_result_len; +} else { +/* VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED */ +error_report("unsupported hash mode"); +goto err; +} +} else { +/* VIRTIO_CRYPTO_SYM_OP_NONE */ +error_report("unsupported cipher type"); +goto err; +} + +queue_index = virtio_crypto_vq2q(queue_id); +cc = qemu_get_crypto_subqueue(vcrypto->crypto, queue_index); +ret = qemu_crypto_create_session(cc, &info, session_id); +if (ret == 0) { +DPRINTF("create session_id=%" PRIu64 "\n", *session_id); +/* Set the result, notify the frontend driver soon */ +input->status = VIRTIO_CRYPTO_OP_OK; +input->session_id = *session_id; + +g_free(info.cipher_key); +g_free(info.auth_key); +return 0; +} + +err: +g_free(info.cipher_key); +g_free(info.auth_key); +input->status = VIRTIO_CRYPTO_OP_ERR; +return -1; +} + static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) { +Vir
[Qemu-devel] [PATCH v2 06/15] crypto: add internal handle logic layer
This patch add some transfering layer or functions, such as session creation, closing etc, which will be used by subsequent virtio crypto device patches. Signed-off-by: Gonglei --- crypto/crypto.c | 68 + 1 file changed, 68 insertions(+) diff --git a/crypto/crypto.c b/crypto/crypto.c index 1d3d1d3..184f837 100644 --- a/crypto/crypto.c +++ b/crypto/crypto.c @@ -291,3 +291,71 @@ void qemu_del_crypto_legacy_hw(CryptoLegacyHWState *crypto) g_free(crypto); } + +CryptoLegacyHWState *qemu_get_crypto_legacy_hw(CryptoClientState *cc) +{ +CryptoClientState *cc0 = cc - cc->queue_index; + +return (CryptoLegacyHWState *)((void *)cc0 - cc->info->size); +} + +void *qemu_get_crypto_legacy_hw_opaque(CryptoClientState *cc) +{ +CryptoLegacyHWState *crypto = qemu_get_crypto_legacy_hw(cc); + +return crypto->opaque; +} + +int qemu_find_crypto_clients_except(const char *id, CryptoClientState **ccs, + CryptoClientOptionsKind type, int max) +{ +CryptoClientState *cc; +int ret = 0; + +QTAILQ_FOREACH(cc, &crypto_clients, next) { +if (cc->info->type == type) { +continue; +} +if (!id || !strcmp(cc->name, id)) { +if (ret < max) { +ccs[ret] = cc; +} +ret++; +} +} + +return ret; +} + +int qemu_crypto_create_session(CryptoClientState *cc, + CryptoSymSessionInfo *info, + uint64_t *session_id) +{ +int ret = -1; +CryptoClientState *peer = cc->peer; + +if (!peer || !peer->ready) { +return ret; +} + +if (peer->info->create_session) { +ret = peer->info->create_session(peer, info, session_id); +} +return ret; +} + +int qemu_crypto_close_session(CryptoClientState *cc, + uint64_t session_id) +{ +int ret = -1; +CryptoClientState *peer = cc->peer; + +if (!peer || !peer->ready) { +return ret; +} + +if (peer->info->close_session) { +ret = peer->info->close_session(peer, session_id); +} +return ret; +} -- 1.7.12.4
[Qemu-devel] [PATCH v2 12/15] virtio-crypto: add destroy session logic
Signed-off-by: Gonglei --- hw/virtio/virtio-crypto.c | 35 --- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c index 1090946..0d7ab63 100644 --- a/hw/virtio/virtio-crypto.c +++ b/hw/virtio/virtio-crypto.c @@ -146,6 +146,32 @@ err: return -1; } +static void +virtio_crypto_handle_close_session(VirtIOCrypto *vcrypto, + struct virtio_crypto_destroy_session_req *close_sess_req, + uint32_t queue_id) +{ +int ret; +CryptoClientState *cc = vcrypto->crypto->ccs; +uint64_t session_id; +uint32_t status; +int queue_index = virtio_crypto_vq2q(queue_id); + +session_id = close_sess_req->session_id; +DPRINTF("close session, id=%" PRIu64 "\n", session_id); +cc = qemu_get_crypto_subqueue(vcrypto->crypto, queue_index); +ret = qemu_crypto_close_session(cc, session_id); +if (ret == 0) { +status = VIRTIO_CRYPTO_OP_OK; +} else { +error_report("destroy session failed"); +status = VIRTIO_CRYPTO_OP_ERR; +} + +/* Set the result, notify the frontend driver soon */ +close_sess_req->status = status; +} + static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) { VirtIOCrypto *vcrypto = VIRTIO_CRYPTO(vdev); @@ -185,12 +211,15 @@ static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) break; case VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION: -case VIRTIO_CRYPTO_HASH_CREATE_SESSION: case VIRTIO_CRYPTO_HASH_DESTROY_SESSION: -case VIRTIO_CRYPTO_MAC_CREATE_SESSION: case VIRTIO_CRYPTO_MAC_DESTROY_SESSION: -case VIRTIO_CRYPTO_AEAD_CREATE_SESSION: case VIRTIO_CRYPTO_AEAD_DESTROY_SESSION: +virtio_crypto_handle_close_session(vcrypto, + &ctrl.u.destroy_session, queue_id); +break; +case VIRTIO_CRYPTO_HASH_CREATE_SESSION: +case VIRTIO_CRYPTO_MAC_CREATE_SESSION: +case VIRTIO_CRYPTO_AEAD_CREATE_SESSION: default: error_report("virtio-crypto unsupported ctrl opcode: %u", opcode); -- 1.7.12.4
[Qemu-devel] [PATCH v2 08/15] virtio-crypto-pci: add virtio crypto pci support
This patch adds virtio-crypto-pci, which is the pci proxy for the virtio crypto device. Signed-off-by: Gonglei --- hw/virtio/Makefile.objs | 2 +- hw/virtio/virtio-crypto-pci.c | 71 +++ hw/virtio/virtio-pci.h| 15 + 3 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 hw/virtio/virtio-crypto-pci.c diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs index e716308..d29966e 100644 --- a/hw/virtio/Makefile.objs +++ b/hw/virtio/Makefile.objs @@ -5,5 +5,5 @@ common-obj-y += virtio-mmio.o obj-y += virtio.o virtio-balloon.o obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o - obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o +obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o diff --git a/hw/virtio/virtio-crypto-pci.c b/hw/virtio/virtio-crypto-pci.c new file mode 100644 index 000..4436955 --- /dev/null +++ b/hw/virtio/virtio-crypto-pci.c @@ -0,0 +1,71 @@ +/* + * Virtio crypto device + * + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. + * + * Authors: + *Gonglei + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * (at your option) any later version. See the COPYING file in the + * top-level directory. + * + */ +#include "qemu/osdep.h" +#include "hw/pci/pci.h" +#include "hw/virtio/virtio.h" +#include "hw/virtio/virtio-bus.h" +#include "hw/virtio/virtio-pci.h" +#include "hw/virtio/virtio-crypto.h" + +static Property virtio_crypto_pci_properties[] = { +DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, +VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), +DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), +DEFINE_PROP_END_OF_LIST(), +}; + +static void virtio_crypto_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp) +{ +VirtIOCryptoPCI *vcrypto = VIRTIO_CRYPTO_PCI(vpci_dev); +DeviceState *vdev = DEVICE(&vcrypto->vdev); + +qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus)); +virtio_pci_force_virtio_1(vpci_dev); +object_property_set_bool(OBJECT(vdev), true, "realized", errp); +} + +static void virtio_crypto_pci_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass); +PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass); + +k->realize = virtio_crypto_pci_realize; +set_bit(DEVICE_CATEGORY_MISC, dc->categories); +dc->props = virtio_crypto_pci_properties; + +pcidev_k->class_id = PCI_CLASS_OTHERS; +} + +static void virtio_crypto_initfn(Object *obj) +{ +VirtIOCryptoPCI *dev = VIRTIO_CRYPTO_PCI(obj); + +virtio_instance_init_common(obj, &dev->vdev, sizeof(dev->vdev), +TYPE_VIRTIO_CRYPTO); +} + +static const TypeInfo virtio_crypto_pci_info = { +.name = TYPE_VIRTIO_CRYPTO_PCI, +.parent= TYPE_VIRTIO_PCI, +.instance_size = sizeof(VirtIOCryptoPCI), +.instance_init = virtio_crypto_initfn, +.class_init= virtio_crypto_pci_class_init, +}; + +static void virtio_crypto_pci_register_types(void) +{ +type_register_static(&virtio_crypto_pci_info); +} +type_init(virtio_crypto_pci_register_types) diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index 0698157..6a31fde 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -25,6 +25,8 @@ #include "hw/virtio/virtio-bus.h" #include "hw/virtio/virtio-input.h" #include "hw/virtio/virtio-gpu.h" +#include "hw/virtio/virtio-crypto.h" + #ifdef CONFIG_VIRTFS #include "hw/9pfs/virtio-9p.h" #endif @@ -48,6 +50,7 @@ typedef struct VirtIOInputHIDPCI VirtIOInputHIDPCI; typedef struct VirtIOInputHostPCI VirtIOInputHostPCI; typedef struct VirtIOGPUPCI VirtIOGPUPCI; typedef struct VHostVSockPCI VHostVSockPCI; +typedef struct VirtIOCryptoPCI VirtIOCryptoPCI; /* virtio-pci-bus */ @@ -347,6 +350,18 @@ struct VHostVSockPCI { }; #endif +/* + * virtio-crypto-pci: This extends VirtioPCIProxy. + */ +#define TYPE_VIRTIO_CRYPTO_PCI "virtio-crypto-pci" +#define VIRTIO_CRYPTO_PCI(obj) \ +OBJECT_CHECK(VirtIOCryptoPCI, (obj), TYPE_VIRTIO_CRYPTO_PCI) + +struct VirtIOCryptoPCI { +VirtIOPCIProxy parent_obj; +VirtIOCrypto vdev; +}; + /* Virtio ABI version, if we increment this, we break the guest driver. */ #define VIRTIO_PCI_ABI_VERSION 0 -- 1.7.12.4
[Qemu-devel] [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto legacy hardware
cryptodev backend is used to realize the active work for virtual crypto device. CryptoLegacyHW device is a cryptographic hardware device seen by the virtual machine. The relationship between cryptodev backend and legacy hadware as follow: crypto_legacy_hw device (1)--->(n) cryptodev client backend Signed-off-by: Gonglei --- crypto/Makefile.objs| 1 + crypto/crypto.c | 171 include/crypto/crypto.h | 66 +++ include/qemu/typedefs.h | 1 + include/sysemu/sysemu.h | 1 + qapi-schema.json| 46 + qemu-options.hx | 3 + vl.c| 13 8 files changed, 302 insertions(+) create mode 100644 crypto/crypto.c create mode 100644 include/crypto/crypto.h diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs index a36d2d9..2a63cb8 100644 --- a/crypto/Makefile.objs +++ b/crypto/Makefile.objs @@ -26,6 +26,7 @@ crypto-obj-y += xts.o crypto-obj-y += block.o crypto-obj-y += block-qcow.o crypto-obj-y += block-luks.o +crypto-obj-y += crypto.o # Let the userspace emulators avoid linking gnutls/etc crypto-aes-obj-y = aes.o diff --git a/crypto/crypto.c b/crypto/crypto.c new file mode 100644 index 000..fbc6497 --- /dev/null +++ b/crypto/crypto.c @@ -0,0 +1,171 @@ +/* + * QEMU Crypto Device Implement + * + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. + * + * Authors: + *Gonglei + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#include "qemu/osdep.h" +#include "sysemu/sysemu.h" +#include "qapi/error.h" +#include "qemu/iov.h" +#include "qapi-visit.h" +#include "qapi/opts-visitor.h" + +#include "crypto/crypto.h" +#include "qemu/config-file.h" +#include "monitor/monitor.h" + + +static QTAILQ_HEAD(, CryptoClientState) crypto_clients; + +QemuOptsList qemu_cryptodev_opts = { +.name = "cryptodev", +.implied_opt_name = "type", +.head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head), +.desc = { +/* + * no elements => accept any params + * validation will happen later + */ +{ /* end of list */ } +}, +}; + +static int +crypto_init_cryptodev(void *dummy, QemuOpts *opts, Error **errp) +{ +Error *local_err = NULL; +int ret; + +ret = crypto_client_init(opts, &local_err); +if (local_err) { +error_report_err(local_err); +return -1; +} + +return ret; +} + +int crypto_init_clients(void) +{ +QTAILQ_INIT(&crypto_clients); + +if (qemu_opts_foreach(qemu_find_opts("cryptodev"), + crypto_init_cryptodev, NULL, NULL)) { +return -1; +} + +return 0; +} + +static int (* const crypto_client_init_fun[CRYPTO_CLIENT_OPTIONS_KIND__MAX])( +const CryptoClientOptions *opts, +const char *name, +CryptoClientState *peer, Error **errp); + +static int crypto_client_init1(const void *object, Error **errp) +{ +const CryptoClientOptions *opts; +const char *name; + +const Cryptodev *cryptodev = object; +opts = cryptodev->opts; +name = cryptodev->id; + +if (opts->type == CRYPTO_CLIENT_OPTIONS_KIND_LEGACY_HW || +!crypto_client_init_fun[opts->type]) { +error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type", + "a cryptodev backend type"); +return -1; +} + +if (crypto_client_init_fun[opts->type](opts, name, NULL, errp) < 0) { +if (errp && !*errp) { +error_setg(errp, QERR_DEVICE_INIT_FAILED, + CryptoClientOptionsKind_lookup[opts->type]); +} +return -1; +} +return 0; +} + +int crypto_client_init(QemuOpts *opts, Error **errp) +{ +void *object = NULL; +Error *err = NULL; +int ret = -1; +Visitor *v = opts_visitor_new(opts); + +visit_type_Cryptodev(v, NULL, (Cryptodev **)&object, &err); +if (!err) { +ret = crypto_client_init1(object, &err); +} + +qapi_free_Cry
[Qemu-devel] [PATCH v2 09/15] virtio-crypto: add virtio crypto realization
Introduce the virtio crypto realization, I'll finish the core code in the following patches. The thoughts came from virtio net realization. For more information see: http://qemu-project.org/Features/VirtioCrypto Signed-off-by: Gonglei --- hw/core/qdev-properties-system.c | 86 ++ hw/virtio/Makefile.objs | 1 + hw/virtio/virtio-crypto.c| 250 +++ include/hw/qdev-properties.h | 3 + 4 files changed, 340 insertions(+) create mode 100644 hw/virtio/virtio-crypto.c diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index e55afe6..93afd64 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -22,6 +22,8 @@ #include "qapi/visitor.h" #include "sysemu/char.h" #include "sysemu/iothread.h" +#include "crypto/crypto.h" + static void get_pointer(Object *obj, Visitor *v, Property *prop, char *(*print)(void *ptr), @@ -430,3 +432,87 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd) } nd->instantiated = 1; } + +/* --- cryptodev device --- */ +static void get_cryptodev(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ +DeviceState *dev = DEVICE(obj); +Property *prop = opaque; +CryptoLegacyHWPeers *peers_ptr = qdev_get_prop_ptr(dev, prop); +char *p = g_strdup(peers_ptr->ccs[0] ? peers_ptr->ccs[0]->name : ""); +visit_type_str(v, name, &p, errp); +g_free(p); +} + +static void set_cryptodev(Object *obj, Visitor *v, const char *name, + void *opaque, Error **errp) +{ +DeviceState *dev = DEVICE(obj); +Property *prop = opaque; +CryptoLegacyHWPeers *peers_ptr = qdev_get_prop_ptr(dev, prop); +CryptoClientState **ccs = peers_ptr->ccs; +CryptoClientState *peers[MAX_CRYPTO_QUEUE_NUM]; +Error *local_err = NULL; +int queues, err = 0, i = 0; +char *str; + +if (dev->realized) { +qdev_prop_set_after_realize(dev, name, errp); +return; +} + +visit_type_str(v, name, &str, &local_err); +if (local_err) { +error_propagate(errp, local_err); +return; +} + +queues = qemu_find_crypto_clients_except(str, peers, + CRYPTO_CLIENT_OPTIONS_KIND_LEGACY_HW, + MAX_CRYPTO_QUEUE_NUM); +if (queues == 0) { +err = -ENOENT; +goto out; +} + +if (queues > MAX_CRYPTO_QUEUE_NUM) { +error_setg(errp, + "queues of backend '%s'(%d) exceeds QEMU limitation(%d)", + str, queues, MAX_CRYPTO_QUEUE_NUM); +goto out; +} + +for (i = 0; i < queues; i++) { +if (peers[i] == NULL) { +err = -ENOENT; +goto out; +} + +if (peers[i]->peer) { +err = -EEXIST; +goto out; +} + +if (ccs[i]) { +err = -EINVAL; +goto out; +} + +ccs[i] = peers[i]; +ccs[i]->queue_index = i; +} + +peers_ptr->queues = queues; + +out: +error_set_from_qdev_prop_error(errp, err, dev, prop, str); +g_free(str); +} + +PropertyInfo qdev_prop_cryptodev = { +.name = "str", +.description = "ID of a cryptodev to use as a backend", +.get = get_cryptodev, +.set = set_cryptodev, +}; diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs index d29966e..00731f6 100644 --- a/hw/virtio/Makefile.objs +++ b/hw/virtio/Makefile.objs @@ -7,3 +7,4 @@ obj-y += virtio.o virtio-balloon.o obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o obj-$(CONFIG_VIRTIO_PCI) += virtio-crypto-pci.o +obj-y += virtio-crypto.o diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c new file mode 100644 index 000..23c5041 --- /dev/null +++ b/hw/virtio/virtio-crypto.c @@ -0,0 +1,250 @@ +/* + * Virtio crypto Support + * + * Based on virtio-net.c + * + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. + * + * Authors: + *Gonglei + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * (at your option) any later version. See the COPYING file in the + * top-level directory. + */ +#include "qemu/osdep.h" +#include "qemu/iov.h" +#include "hw/qdev.h" +#include "qapi/error.h" +#include "qemu/error-report.h" + +#include "hw/virtio/virtio.h" +#include "hw/virtio/virtio-crypto.h" +#include "hw/virtio/virtio-access.h" + +static void virtio_crypto_process(VirtIOCrypto *vcrypto) +{ +} + +static void virtio_crypto_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) +{ +} + +static void virtio_crypto_handle_dataq_bh(VirtIODevice *vdev, VirtQueue *vq) +{ +} + +static void virtio_crypto_dataq_bh(void *opaque) +{ +} + +static void virtio_crypto_add_queue(VirtIOCrypto *vcrypto, int index) +{ +VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto); + +vcrypto->vqs[index].dataq = virt
[Qemu-devel] [PATCH v2 05/15] crypto: add cryptodev-linux as a cryptodev backend
Cryptodev-linux is a device that allows access to Linux kernel cryptographic drivers; thus allowing of userspace applications to take advantage of hardware accelerators. Cryptodev-linux is implemented as a standalone module that requires no dependencies other than a stock linux kernel. The Cryptodev-linux project website is: http://cryptodev-linux.org/ Meanwile, I introdue the virtio_crypto.h which follows virtio-crypto specification bacause cryptodev-linux.c include it. Signed-off-by: Gonglei --- configure | 16 + crypto/Makefile.objs | 1 + crypto/crypto.c| 8 +- crypto/cryptodev-linux.c | 419 +++ include/crypto/crypto-clients.h| 39 +++ include/standard-headers/linux/virtio_crypto.h | 448 + qapi-schema.json | 17 +- qemu-options.hx| 18 +- 8 files changed, 963 insertions(+), 3 deletions(-) create mode 100644 crypto/cryptodev-linux.c create mode 100644 include/crypto/crypto-clients.h create mode 100644 include/standard-headers/linux/virtio_crypto.h diff --git a/configure b/configure index 331c36f..5982ff8 100755 --- a/configure +++ b/configure @@ -321,6 +321,7 @@ vhdx="" numa="" tcmalloc="no" jemalloc="no" +cryptodev_linux="no" # parse CC options first for opt do @@ -3622,6 +3623,16 @@ EOF fi ## +# cryptodev-linux header probe +cat > $TMPC << EOF +#include +int main(void) { return 0; } +EOF +if compile_prog "" "" ; then +cryptodev_linux="yes" +fi + +## # signalfd probe signalfd="no" cat > $TMPC << EOF @@ -4927,6 +4938,7 @@ echo "NUMA host support $numa" echo "tcmalloc support $tcmalloc" echo "jemalloc support $jemalloc" echo "avx2 optimization $avx2_opt" +echo "cryptodev-linux support $cryptodev_linux" if test "$sdl_too_old" = "yes"; then echo "-> Your SDL version is too old - please upgrade to have SDL support" @@ -5331,6 +5343,10 @@ if test "$avx2_opt" = "yes" ; then echo "CONFIG_AVX2_OPT=y" >> $config_host_mak fi +if test "$cryptodev_linux" = "yes" ; then + echo "CONFIG_CRYPTODEV_LINUX=y" >> $config_host_mak +fi + if test "$lzo" = "yes" ; then echo "CONFIG_LZO=y" >> $config_host_mak fi diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs index 652b429..1f2ec24 100644 --- a/crypto/Makefile.objs +++ b/crypto/Makefile.objs @@ -28,6 +28,7 @@ crypto-obj-y += block-qcow.o crypto-obj-y += block-luks.o crypto-obj-y += crypto.o crypto-obj-y += crypto-queue.o +crypto-obj-$(CONFIG_CRYPTODEV_LINUX) += cryptodev-linux.o # Let the userspace emulators avoid linking gnutls/etc crypto-aes-obj-y = aes.o diff --git a/crypto/crypto.c b/crypto/crypto.c index 958a959..1d3d1d3 100644 --- a/crypto/crypto.c +++ b/crypto/crypto.c @@ -34,6 +34,7 @@ #include "crypto/crypto.h" #include "qemu/config-file.h" #include "monitor/monitor.h" +#include "crypto/crypto-clients.h" static QTAILQ_HEAD(, CryptoClientState) crypto_clients; @@ -81,7 +82,12 @@ int crypto_init_clients(void) static int (* const crypto_client_init_fun[CRYPTO_CLIENT_OPTIONS_KIND__MAX])( const CryptoClientOptions *opts, const char *name, -CryptoClientState *peer, Error **errp); +CryptoClientState *peer, Error **errp) = { +#ifdef CONFIG_CRYPTODEV_LINUX +[CRYPTO_CLIENT_OPTIONS_KIND_CRYPTODEV_LINUX] = + crypto_init_cryptodev_linux, +#endif +}; static int crypto_client_init1(const void *object, Error **errp) { diff --git a/crypto/cryptodev-linux.c b/crypto/cryptodev-linux.c new file mode 100644 index 000..ae2dcdb --- /dev/null +++ b/crypto/cryptodev-linux.c @@ -0,0 +1,419 @@ +/* + * cryptodev-linux backend support + * + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. + * + * Authors: + *Gonglei + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * (at your option) any later version. See the COPYING file in the + * top-level directory. + */ + +#include +#include +#include + +#include "qemu/osdep.h" +#include "sysemu/sysemu.h" +#include "qapi/error.h" +#include "qemu/iov.h" +#include "qapi-visit.h" +#include "qapi/opts-visitor.h" +#include "qemu/error-report.h" + +#include "crypto/crypto.h" +#include "crypto/crypto-clients.h" +#include "standard-headers/linux/virtio_crypto.h" + + +#define CRYPTO_CHARDEV_PATH "/dev/crypto" + + +typedef struct CryptodevLinuxSession { +struct session_op *sess; +uint8_t direction; /* encryption or decryption */ +uint8_t type; /* cipher? hash? aead? */ +QTAILQ_ENTRY(CryptodevLinuxSession) next; +} CryptodevLinuxSession; + +typedef struct CryptodevLinuxState { +CryptoClientState cc; +int fd; +bool read_poll; +bool write_poll; +bool enabled; +QTAILQ_HEAD(, CryptodevLinuxSession) sessions; +} CryptodevLi
[Qemu-devel] [PATCH v2 00/15] virtio-crypto: introduce framework and device emulation
Changes since v1: - rmmove mixed endian-ness handler for virtio-crypto device, just use little-endian. [mst] - add sg list support according virtio-crypto spec v10 (will be posted soon). - fix a memory leak in session handler. - add a feature page link in qemu.org (http://qemu-project.org/Features/VirtioCrypto) - fix some trivial problems, sush as 's/Since 2.7/Since 2.8/g' in qapi-schema.json - rebase the latest qemu master tree. Please review, thanks! This patch series realize the framework and emulation of a new virtio crypto device, which is similar with virtio net device. - I introduce the cryptodev backend as the client of virtio crypto device which can be realized by different methods, such as cryptodev-linux in my series, vhost-crypto kernel module, vhost-user etc. - The patch set abides by the virtio crypto speccification. - The virtio crypto support symmetric algorithms (including CIPHER and algorithm chainning) at present, except HASH, MAC and AEAD services. - unsupport hot plug/unplug cryptodev client at this moment. Cryptodev-linux is a device that allows access to Linux kernel cryptographic drivers; thus allowing of userspace applications to take advantage of hardware accelerators. It can be found here: http://cryptodev-linux.org/ (please use the latest version) To use the cryptodev-linux as the client, the cryptodev.ko should be insert on the host. # enter cryptodev-linux module root directory, then make;make install then configuring QEMU shows: [...] jemalloc support no avx2 optimization no cryptodev-linux support yes QEMU can then be started using the following parameters: qemu-system-x86_64 \ [...] \ -cryptodev type=cryptodev-linux,id=cryptodev0 \ -device virtio-crypto-pci,id=crypto0,cryptodev=cryptodev0 \ [...] The front-end linux kernel driver (Experimental at present) is publicly accessible from: https://github.com/gongleiarei/virtio-crypto-linux-driver.git After insmod virtio-crypto.ko, you also can use cryptodev-linux test the crypto function in the guest. For example: linux-guest:/home/gonglei/cryptodev-linux/tests # ./cipher - requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc AES Test passed requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc requested cipher CRYPTO_AES_CBC, got cbc(aes) with driver virtio_crypto_aes_cbc Test passed QEMU code also can be accessible from: https://github.com/gongleiarei/qemu.git branch virtio-crypto For more information, please see: http://qemu-project.org/Features/VirtioCrypto Gonglei (15): crypto: introduce cryptodev backend and crypto legacy hardware crypto: introduce crypto queue handler crypto: add cryptoLegacyHW stuff crypto: add symetric algorithms support crypto: add cryptodev-linux as a cryptodev backend crypto: add internal handle logic layer virtio-crypto: introduce virtio-crypto.h virtio-crypto-pci: add virtio crypto pci support virtio-crypto: add virtio crypto realization virtio-crypto: set capacity of crypto legacy hardware virtio-crypto: add control queue handler virtio-crypto: add destroy session logic virtio-crypto: get correct input data address for each request virtio-crypto: add data virtqueue processing handler virtio-crypto: support scatter gather list configure | 16 + crypto/Makefile.objs |3 + crypto/crypto-queue.c | 206 + crypto/crypto.c| 378 + crypto/cryptodev-linux.c | 419 ++ hw/core/qdev-properties-system.c | 86 ++ hw/virtio/Makefile.objs|3 +- hw/virtio/virtio-crypto-pci.c | 71 ++ hw/virtio/virtio-crypto.c | 1013 hw/virtio/virtio-pci.h | 15 + include/crypto/crypto-clients.h| 39 + include/crypto/crypto-queue.h | 69 ++ include/crypto/crypto.h| 192 + include/hw/qdev-properties.h |3 + include/hw/virtio/virtio-crypto.h | 96 +++ include/qemu/typedefs.h|1 + include/standard-headers/linux/virtio_crypto.h | 466 +++ include/sysemu/sysemu.h|1 + qapi-schema.json | 61 ++ qemu-options.hx| 19 + vl.c | 13 + 21 files changed, 3169 insertions(+), 1 deletion(-) create mode 100644 crypto/crypto-queue.c create mode 100644 crypto/crypto.c create mode 100644 crypto/cryptodev-linux.c create mode 100644 hw/virtio/virtio-crypto-pci.c create mode 100644 hw/virtio/virtio-crypto.c create mode 100644 include/crypto/crypto-clients.h create mode 100644 include/crypto/crypto-q
[Qemu-devel] [PATCH v2 03/15] crypto: add cryptoLegacyHW stuff
In previous patch, we define CryptoLegacyHWOptions in qapi-schema.json. we introduce the new/delete funciton about crypto legacy hardware device. Note: legacy cryptographic device support muliple queue. Signed-off-by: Gonglei --- crypto/crypto.c | 74 + include/crypto/crypto.h | 29 +++ 2 files changed, 103 insertions(+) diff --git a/crypto/crypto.c b/crypto/crypto.c index a0e4a34..3f760fd 100644 --- a/crypto/crypto.c +++ b/crypto/crypto.c @@ -197,3 +197,77 @@ int qemu_send_crypto_packet_async(CryptoClientState *sender, return qemu_crypto_queue_send(queue, flags, sender, opaque, sent_cb); } + +CryptoLegacyHWState * +qemu_new_crypto_legacy_hw(CryptoClientInfo *info, + CryptoLegacyHWConf *conf, + const char *model, + const char *name, + void *opaque) +{ +CryptoLegacyHWState *crypto; +CryptoClientState **peers = conf->peers.ccs; +int i, queues = MAX(1, conf->peers.queues); + +assert(info->type == CRYPTO_CLIENT_OPTIONS_KIND_LEGACY_HW); +assert(info->size >= sizeof(CryptoLegacyHWState)); + +crypto = g_malloc0(info->size + sizeof(CryptoClientState) * queues); +crypto->ccs = (void *)crypto + info->size; +crypto->opaque = opaque; +crypto->conf = conf; + +for (i = 0; i < queues; i++) { +crypto_client_setup(&crypto->ccs[i], info, peers[i], model, name, + NULL); +crypto->ccs[i].queue_index = i; +crypto->ccs[i].ready = true; +} + +return crypto; +} + +static void qemu_cleanup_crypto_client(CryptoClientState *cc) +{ +QTAILQ_REMOVE(&crypto_clients, cc, next); + +if (cc->info->cleanup) { +cc->info->cleanup(cc); +} +} + +static void qemu_free_crypto_client(CryptoClientState *cc) +{ +if (cc->incoming_queue) { +qemu_del_crypto_queue(cc->incoming_queue); +} +if (cc->peer) { +cc->peer->peer = NULL; +} +g_free(cc->model); +g_free(cc->name); + +if (cc->destructor) { +cc->destructor(cc); +} +} + +CryptoClientState * +qemu_get_crypto_subqueue(CryptoLegacyHWState *crypto, int queue_index) +{ +return crypto->ccs + queue_index; +} + +void qemu_del_crypto_legacy_hw(CryptoLegacyHWState *crypto) +{ +int i, queues = MAX(crypto->conf->peers.queues, 1); + +for (i = queues - 1; i >= 0; i--) { +CryptoClientState *cc = qemu_get_crypto_subqueue(crypto, i); + +qemu_cleanup_crypto_client(cc); +qemu_free_crypto_client(cc); +} + +g_free(crypto); +} diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h index 46b3b9e..4f0efb7 100644 --- a/include/crypto/crypto.h +++ b/include/crypto/crypto.h @@ -31,6 +31,7 @@ #include "qapi-types.h" #include "crypto/crypto-queue.h" +#define MAX_CRYPTO_QUEUE_NUM 64 typedef void (CryptoPoll)(CryptoClientState *, bool); typedef void (CryptoCleanup) (CryptoClientState *); @@ -59,6 +60,24 @@ struct CryptoClientState { CryptoClientDestructor *destructor; }; +/* qdev crypto legacy hardware properties */ + +typedef struct CryptoLegacyHWPeers { +CryptoClientState *ccs[MAX_CRYPTO_QUEUE_NUM]; +int32_t queues; +} CryptoLegacyHWPeers; + +typedef struct CryptoLegacyHWConf { +CryptoLegacyHWPeers peers; +} CryptoLegacyHWConf; + +typedef struct CryptoLegacyHWState { +CryptoClientState *ccs; +void *opaque; +CryptoLegacyHWConf *conf; +bool peer_deleted; +} CryptoLegacyHWState; + int crypto_client_init(QemuOpts *opts, Error **errp); int crypto_init_clients(void); @@ -74,5 +93,15 @@ int qemu_send_crypto_packet_async(CryptoClientState *sender, unsigned flags, void *opaque, CryptoPacketSent *sent_cb); +CryptoLegacyHWState * +qemu_new_crypto_legacy_hw(CryptoClientInfo *info, + CryptoLegacyHWConf *conf, + const char *model, + const char *name, + void *opaque); +void qemu_del_crypto_legacy_hw(CryptoLegacyHWState *crypto); + +CryptoClientState * +qemu_get_crypto_subqueue(CryptoLegacyHWState *crypto, int queue_index); #endif /* QCRYPTO_CRYPTO_H__ */ -- 1.7.12.4
[Qemu-devel] [PATCH v2 10/15] virtio-crypto: set capacity of crypto legacy hardware
Set the crypto legacy hardware's capacity according to the backend peer cryptodev's capacity. We only support only one queue at present. Virtio crypto device is a kind of crypto legacy hardware. Signed-off-by: Gonglei --- crypto/crypto.c | 17 + hw/virtio/virtio-crypto.c | 18 +- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/crypto/crypto.c b/crypto/crypto.c index 184f837..ba99d7c 100644 --- a/crypto/crypto.c +++ b/crypto/crypto.c @@ -228,6 +228,7 @@ qemu_new_crypto_legacy_hw(CryptoClientInfo *info, CryptoLegacyHWState *crypto; CryptoClientState **peers = conf->peers.ccs; int i, queues = MAX(1, conf->peers.queues); +int has_set = 0; assert(info->type == CRYPTO_CLIENT_OPTIONS_KIND_LEGACY_HW); assert(info->size >= sizeof(CryptoLegacyHWState)); @@ -242,6 +243,22 @@ qemu_new_crypto_legacy_hw(CryptoClientInfo *info, NULL); crypto->ccs[i].queue_index = i; crypto->ccs[i].ready = true; +/* The mask bits of crypto_services and algos in + CryptoLegacyHWConf is set only once */ +if (has_set == 0 && peers[i]) { +conf->crypto_services = peers[i]->crypto_services; +conf->cipher_algo_l = peers[i]->cipher_algo_l; +conf->cipher_algo_h = peers[i]->cipher_algo_h; +conf->hash_algo = peers[i]->hash_algo; +conf->mac_algo_l = peers[i]->mac_algo_l; +conf->mac_algo_h = peers[i]->mac_algo_h; +conf->asym_algo = peers[i]->asym_algo; +conf->kdf_algo = peers[i]->kdf_algo; +conf->aead_algo = peers[i]->aead_algo; +conf->primitive_algo = peers[i]->primitive_algo; + +has_set = 1; +} } return crypto; diff --git a/hw/virtio/virtio-crypto.c b/hw/virtio/virtio-crypto.c index 23c5041..b5a108f 100644 --- a/hw/virtio/virtio-crypto.c +++ b/hw/virtio/virtio-crypto.c @@ -197,7 +197,23 @@ static Property virtio_crypto_properties[] = { static void virtio_crypto_get_config(VirtIODevice *vdev, uint8_t *config) { - +VirtIOCrypto *c = VIRTIO_CRYPTO(vdev); +struct virtio_crypto_config crypto_cfg; + +crypto_cfg.status = c->status; +crypto_cfg.max_dataqueues = c->max_queues; +crypto_cfg.crypto_services = c->legacy_conf.crypto_services; +crypto_cfg.cipher_algo_l = c->legacy_conf.cipher_algo_l; +crypto_cfg.cipher_algo_h = c->legacy_conf.cipher_algo_h; +crypto_cfg.hash_algo = c->legacy_conf.hash_algo; +crypto_cfg.mac_algo_l = c->legacy_conf.mac_algo_l; +crypto_cfg.mac_algo_h = c->legacy_conf.mac_algo_h; +crypto_cfg.asym_algo = c->legacy_conf.asym_algo; +crypto_cfg.kdf_algo = c->legacy_conf.kdf_algo; +crypto_cfg.aead_algo = c->legacy_conf.aead_algo; +crypto_cfg.primitive_algo = c->legacy_conf.primitive_algo; + +memcpy(config, &crypto_cfg, c->config_size; } static void virtio_crypto_set_config(VirtIODevice *vdev, const uint8_t *config) -- 1.7.12.4
[Qemu-devel] question
When I send mail to subscribe the mailing list(qemu-devel@nongnu.org), it reply to me that I have already subscribed to this mailing list. But I can not receive any mail form qemu-devel@nongnu.org, How to solve this problem?
Re: [Qemu-devel] [PATCH v5 14/14] aspeed: allocate RAM after the memory controller has checked the size
On Fri, 2016-09-09 at 18:22 +0200, Cédric Le Goater wrote: > If the RAM size is invalid, the memory controller will use a default > value. > > Signed-off-by: Cédric Le Goater Reviewed-by: Andrew Jeffery > --- > hw/arm/aspeed.c | 18 +- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 562bbb253391..6b18c7f1727c 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -108,11 +108,6 @@ static void aspeed_board_init(MachineState *machine, > > sc = ASPEED_SOC_GET_CLASS(&bmc->soc); > > -memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size); > -memory_region_add_subregion(get_system_memory(), sc->info->sdram_base, > -&bmc->ram); > -object_property_add_const_link(OBJECT(&bmc->soc), "ram", > OBJECT(&bmc->ram), > - &error_abort); > object_property_set_int(OBJECT(&bmc->soc), ram_size, "ram-size", > &error_abort); > object_property_set_int(OBJECT(&bmc->soc), cfg->hw_strap1, "hw-strap1", > @@ -120,6 +115,19 @@ static void aspeed_board_init(MachineState *machine, > object_property_set_bool(OBJECT(&bmc->soc), true, "realized", > &error_abort); > > +/* > + * Allocate RAM after the memory controller has checked the size > + * was valid. If not, a default value is used. > + */ > +ram_size = object_property_get_int(OBJECT(&bmc->soc), "ram-size", > + &error_abort); > + > +memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size); > +memory_region_add_subregion(get_system_memory(), sc->info->sdram_base, > +&bmc->ram); > +object_property_add_const_link(OBJECT(&bmc->soc), "ram", > OBJECT(&bmc->ram), > + &error_abort); > + > aspeed_board_init_flashes(&bmc->soc.smc, "n25q256a", &error_abort); > aspeed_board_init_flashes(&bmc->soc.spi, "mx25l25635e", &error_abort); > signature.asc Description: This is a digitally signed message part
Re: [Qemu-devel] [PATCH v5 13/14] aspeed: add a ram_size property to the memory controller
On Fri, 2016-09-09 at 18:22 +0200, Cédric Le Goater wrote: > Configure the size of the RAM of the SOC using a property to propagate > the value down to the memory controller from the board level. > > Signed-off-by: Cédric Le Goater Reviewed-by: Andrew Jeffery > --- > hw/arm/aspeed.c | 2 ++ > hw/arm/aspeed_soc.c | 2 ++ > hw/misc/aspeed_sdmc.c | 23 +-- > include/hw/misc/aspeed_sdmc.h | 1 + > 4 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c > index 9013d35a674c..562bbb253391 100644 > --- a/hw/arm/aspeed.c > +++ b/hw/arm/aspeed.c > @@ -113,6 +113,8 @@ static void aspeed_board_init(MachineState *machine, > &bmc->ram); > object_property_add_const_link(OBJECT(&bmc->soc), "ram", > OBJECT(&bmc->ram), > &error_abort); > +object_property_set_int(OBJECT(&bmc->soc), ram_size, "ram-size", > + &error_abort); > object_property_set_int(OBJECT(&bmc->soc), cfg->hw_strap1, "hw-strap1", > &error_abort); > object_property_set_bool(OBJECT(&bmc->soc), true, "realized", > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c > index 93bc7bb66e4b..c0a310205842 100644 > --- a/hw/arm/aspeed_soc.c > +++ b/hw/arm/aspeed_soc.c > @@ -113,6 +113,8 @@ static void aspeed_soc_init(Object *obj) > qdev_set_parent_bus(DEVICE(&s->sdmc), sysbus_get_default()); > qdev_prop_set_uint32(DEVICE(&s->sdmc), "silicon-rev", > sc->info->silicon_rev); > +object_property_add_alias(obj, "ram-size", OBJECT(&s->sdmc), > + "ram-size", &error_abort); > } > > static void aspeed_soc_realize(DeviceState *dev, Error **errp) > diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c > index 20bcdb52c4df..8830dc084c38 100644 > --- a/hw/misc/aspeed_sdmc.c > +++ b/hw/misc/aspeed_sdmc.c > @@ -140,9 +140,9 @@ static const MemoryRegionOps aspeed_sdmc_ops = { > .valid.max_access_size = 4, > }; > > -static int ast2400_rambits(void) > +static int ast2400_rambits(AspeedSDMCState *s) > { > -switch (ram_size >> 20) { > +switch (s->ram_size >> 20) { > case 64: > return ASPEED_SDMC_DRAM_64MB; > case 128: > @@ -156,14 +156,15 @@ static int ast2400_rambits(void) > } > > /* use a common default */ > -error_report("warning: Invalid RAM size 0x" RAM_ADDR_FMT > - ". Using default 256M", ram_size); > +error_report("warning: Invalid RAM size 0x%" PRIx64 > + ". Using default 256M", s->ram_size); > +s->ram_size = 256 << 20; > return ASPEED_SDMC_DRAM_256MB; > } > > -static int ast2500_rambits(void) > +static int ast2500_rambits(AspeedSDMCState *s) > { > -switch (ram_size >> 20) { > +switch (s->ram_size >> 20) { > case 128: > return ASPEED_SDMC_AST2500_128MB; > case 256: > @@ -177,8 +178,9 @@ static int ast2500_rambits(void) > } > > /* use a common default */ > -error_report("warning: Invalid RAM size 0x" RAM_ADDR_FMT > - ". Using default 512M", ram_size); > +error_report("warning: Invalid RAM size 0x%" PRIx64 > + ". Using default 512M", s->ram_size); > +s->ram_size = 512 << 20; > return ASPEED_SDMC_AST2500_512MB; > } > > @@ -222,11 +224,11 @@ static void aspeed_sdmc_realize(DeviceState *dev, Error > **errp) > > switch (s->silicon_rev) { > case AST2400_A0_SILICON_REV: > -s->ram_bits = ast2400_rambits(); > +s->ram_bits = ast2400_rambits(s); > break; > case AST2500_A0_SILICON_REV: > case AST2500_A1_SILICON_REV: > -s->ram_bits = ast2500_rambits(); > +s->ram_bits = ast2500_rambits(s); > break; > default: > g_assert_not_reached(); > @@ -249,6 +251,7 @@ static const VMStateDescription vmstate_aspeed_sdmc = { > > static Property aspeed_sdmc_properties[] = { > DEFINE_PROP_UINT32("silicon-rev", AspeedSDMCState, silicon_rev, 0), > +DEFINE_PROP_UINT64("ram-size", AspeedSDMCState, ram_size, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/hw/misc/aspeed_sdmc.h b/include/hw/misc/aspeed_sdmc.h > index df7dce0edddf..551c8afdf4be 100644 > --- a/include/hw/misc/aspeed_sdmc.h > +++ b/include/hw/misc/aspeed_sdmc.h > @@ -26,6 +26,7 @@ typedef struct AspeedSDMCState { > uint32_t regs[ASPEED_SDMC_NR_REGS]; > uint32_t silicon_rev; > uint32_t ram_bits; > +uint64_t ram_size; > > } AspeedSDMCState; > signature.asc Description: This is a digitally signed message part
Re: [Qemu-devel] [PATCH v2 1/5] msix_init: assert programming error
On 09/12/2016 09:29 PM, Markus Armbruster wrote: Cao jin writes: The input parameters is used for creating the msix capable device, so they must obey the PCI spec, or else, it should be programming error. True when the the parameters come from a device model attempting to define a PCI device violating the spec. But what if the parameters come from an actual PCI device violating the spec, via device assignment? Before the patch, on invalid param, the vfio behaviour is: error_report("vfio: msix_init failed"); then, device create fail. After the patch, its behaviour is: asserted. Do you mean we should still report some useful info to user on invalid params? Cao jin For what it's worth, the new behavior seems consistent with msi_init(), which is good. CC: Markus Armbruster CC: Marcel Apfelbaum CC: Michael S. Tsirkin Signed-off-by: Cao jin --- hw/pci/msix.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/pci/msix.c b/hw/pci/msix.c index 0ec1cb1..384a29d 100644 --- a/hw/pci/msix.c +++ b/hw/pci/msix.c @@ -253,9 +253,7 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, return -ENOTSUP; } -if (nentries < 1 || nentries > PCI_MSIX_FLAGS_QSIZE + 1) { -return -EINVAL; -} +assert(nentries >= 1 && nentries <= PCI_MSIX_FLAGS_QSIZE + 1); table_size = nentries * PCI_MSIX_ENTRY_SIZE; pba_size = QEMU_ALIGN_UP(nentries, 64) / 8; @@ -266,7 +264,7 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, /* Sanity test: table & pba don't overlap, fit within BARs, min aligned */ if ((table_bar_nr == pba_bar_nr && ranges_overlap(table_offset, table_size, pba_offset, pba_size)) || table_offset + table_size > memory_region_size(table_bar) || pba_offset + pba_size > memory_region_size(pba_bar) || (table_offset | pba_offset) & PCI_MSIX_FLAGS_BIRMASK) { -return -EINVAL; +assert(0); } Instead of if (... complicated condition ...) { assert(0); } let's write assert(... negation of the complicated condition ...); cap = pci_add_capability(dev, PCI_CAP_ID_MSIX, cap_pos, MSIX_CAP_LENGTH); .
Re: [Qemu-devel] [PATCH v5 12/14] aspeed: use error_report instead of LOG_GUEST_ERROR
On Fri, 2016-09-09 at 18:22 +0200, Cédric Le Goater wrote: > Also change the default value used in case of an error. The minimum > size is a bit severe, so let's just use an average RAM size. I'm not sure we should be switching the default value in this patch, but I'm not sure it's worth any effort to split it out either... Otherwise, both the changes seem sensible enough. Andrew > > Signed-off-by: Cédric Le Goater > --- > hw/misc/aspeed_sdmc.c | 17 + > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c > index 1d2825237728..20bcdb52c4df 100644 > --- a/hw/misc/aspeed_sdmc.c > +++ b/hw/misc/aspeed_sdmc.c > @@ -9,6 +9,7 @@ > > #include "qemu/osdep.h" > #include "qemu/log.h" > +#include "qemu/error-report.h" > #include "hw/misc/aspeed_sdmc.h" > #include "hw/misc/aspeed_scu.h" > #include "hw/qdev-properties.h" > @@ -151,13 +152,13 @@ static int ast2400_rambits(void) > case 512: > return ASPEED_SDMC_DRAM_512MB; > default: > -qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid RAM size: 0x" > - RAM_ADDR_FMT "\n", __func__, ram_size); > break; > } > > -/* set a minimum default */ > -return ASPEED_SDMC_DRAM_64MB; > +/* use a common default */ > +error_report("warning: Invalid RAM size 0x" RAM_ADDR_FMT > + ". Using default 256M", ram_size); > +return ASPEED_SDMC_DRAM_256MB; > } > > static int ast2500_rambits(void) > @@ -172,13 +173,13 @@ static int ast2500_rambits(void) > case 1024: > return ASPEED_SDMC_AST2500_1024MB; > default: > -qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid RAM size: 0x" > - RAM_ADDR_FMT "\n", __func__, ram_size); > break; > } > > -/* set a minimum default */ > -return ASPEED_SDMC_AST2500_128MB; > +/* use a common default */ > +error_report("warning: Invalid RAM size 0x" RAM_ADDR_FMT > + ". Using default 512M", ram_size); > +return ASPEED_SDMC_AST2500_512MB; > } > > static void aspeed_sdmc_reset(DeviceState *dev) signature.asc Description: This is a digitally signed message part
Re: [Qemu-devel] [PATCH v5 11/14] aspeed: calculate the RAM size bits at realize time
On Fri, 2016-09-09 at 18:22 +0200, Cédric Le Goater wrote: > There is no need to do this at each reset as the RAM size will not > change. > > Signed-off-by: Cédric Le Goater Reviewed-by: Andrew Jeffery > --- > hw/misc/aspeed_sdmc.c | 16 ++-- > include/hw/misc/aspeed_sdmc.h | 1 + > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c > index 244e5c0dc5ef..1d2825237728 100644 > --- a/hw/misc/aspeed_sdmc.c > +++ b/hw/misc/aspeed_sdmc.c > @@ -192,7 +192,7 @@ static void aspeed_sdmc_reset(DeviceState *dev) > case AST2400_A0_SILICON_REV: > s->regs[R_CONF] |= > ASPEED_SDMC_VGA_COMPAT | > -ASPEED_SDMC_DRAM_SIZE(ast2400_rambits()); > +ASPEED_SDMC_DRAM_SIZE(s->ram_bits); > break; > > case AST2500_A0_SILICON_REV: > @@ -200,7 +200,7 @@ static void aspeed_sdmc_reset(DeviceState *dev) > s->regs[R_CONF] |= > ASPEED_SDMC_HW_VERSION(1) | > ASPEED_SDMC_VGA_APERTURE(ASPEED_SDMC_VGA_64MB) | > -ASPEED_SDMC_DRAM_SIZE(ast2500_rambits()); > +ASPEED_SDMC_DRAM_SIZE(s->ram_bits); > break; > > default: > @@ -219,6 +219,18 @@ static void aspeed_sdmc_realize(DeviceState *dev, Error > **errp) > return; > } > > +switch (s->silicon_rev) { > +case AST2400_A0_SILICON_REV: > +s->ram_bits = ast2400_rambits(); > +break; > +case AST2500_A0_SILICON_REV: > +case AST2500_A1_SILICON_REV: > +s->ram_bits = ast2500_rambits(); > +break; > +default: > +g_assert_not_reached(); > +} > + > memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_sdmc_ops, s, > TYPE_ASPEED_SDMC, 0x1000); > sysbus_init_mmio(sbd, &s->iomem); > diff --git a/include/hw/misc/aspeed_sdmc.h b/include/hw/misc/aspeed_sdmc.h > index 7e081f6d2b86..df7dce0edddf 100644 > --- a/include/hw/misc/aspeed_sdmc.h > +++ b/include/hw/misc/aspeed_sdmc.h > @@ -25,6 +25,7 @@ typedef struct AspeedSDMCState { > > uint32_t regs[ASPEED_SDMC_NR_REGS]; > uint32_t silicon_rev; > +uint32_t ram_bits; > > } AspeedSDMCState; > signature.asc Description: This is a digitally signed message part
Re: [Qemu-devel] [PATCH v7 2/4] vfio: VFIO driver for mediated devices
On 09/08/2016 10:45 AM, Jike Song wrote: > On 08/25/2016 05:22 PM, Dong Jia wrote: >> On Thu, 25 Aug 2016 09:23:53 +0530 >> Kirti Wankhede wrote: >> >> [...] >> >> Dear Kirti, >> >> I just rebased my vfio-ccw patches to this series. >> With a little fix, which was pointed it out in my reply to the #3 >> patch, it works fine. >> > > Hi Jia, > > Sorry I didn't follow a lot in previous discussion, but since > vfio-mdev in v7 patchset is at least PCI-agnostic, would you share > with us why you still need a vfio-ccw? Kind ping :) Hi Dong Jia, Since Kirti has confirmed that in v7 it is designed to have only one vfio-mdev driver for all mdev devices, would you please tell us the reason of your vfio-ccw? It could possibly be an architectural gap and the earlier we discuss it the better :) -- Thanks, Jike >>> +static long vfio_mdev_unlocked_ioctl(void *device_data, >>> +unsigned int cmd, unsigned long arg) >>> +{ >>> + int ret = 0; >>> + struct vfio_mdev *vmdev = device_data; >>> + struct parent_device *parent = vmdev->mdev->parent; >>> + unsigned long minsz; >>> + >>> + switch (cmd) { >>> + case VFIO_DEVICE_GET_INFO: >>> + { >>> + struct vfio_device_info info; >>> + >>> + minsz = offsetofend(struct vfio_device_info, num_irqs); >>> + >>> + if (copy_from_user(&info, (void __user *)arg, minsz)) >>> + return -EFAULT; >>> + >>> + if (info.argsz < minsz) >>> + return -EINVAL; >>> + >>> + if (parent->ops->get_device_info) >>> + ret = parent->ops->get_device_info(vmdev->mdev, &info); >>> + else >>> + return -EINVAL; >>> + >>> + if (ret) >>> + return ret; >>> + >>> + if (parent->ops->reset) >>> + info.flags |= VFIO_DEVICE_FLAGS_RESET; >> Shouldn't this be done inside the get_device_info callback? >> >>> + >>> + memcpy(&vmdev->dev_info, &info, sizeof(info)); >>> + >>> + return copy_to_user((void __user *)arg, &info, minsz); >>> + } >> [...] >> >>> + >>> +static ssize_t vfio_mdev_read(void *device_data, char __user *buf, >>> + size_t count, loff_t *ppos) >>> +{ >>> + struct vfio_mdev *vmdev = device_data; >>> + struct mdev_device *mdev = vmdev->mdev; >>> + struct parent_device *parent = mdev->parent; >>> + unsigned int done = 0; >>> + int ret; >>> + >>> + if (!parent->ops->read) >>> + return -EINVAL; >>> + >>> + while (count) { >> Here, I have to say sorry to you guys for that I didn't notice the >> bad impact of this change to my patches during the v6 discussion. >> >> For vfio-ccw, I introduced an I/O region to input/output I/O >> instruction parameters and results for Qemu. The @count of these data >> currently is 140. So supporting arbitrary lengths in one shot here, and >> also in vfio_mdev_write, seems the better option for this case. >> >> I believe that if the pci drivers want to iterate in a 4 bytes step, you >> can do that in the parent read/write callbacks instead. >> >> What do you think? >> >>> + size_t filled; >>> + >>> + if (count >= 4 && !(*ppos % 4)) { >>> + u32 val; >>> + >>> + ret = parent->ops->read(mdev, (char *)&val, sizeof(val), >>> + *ppos); >>> + if (ret <= 0) >>> + goto read_err; >>> + >>> + if (copy_to_user(buf, &val, sizeof(val))) >>> + goto read_err; >>> + >>> + filled = 4; >>> + } else if (count >= 2 && !(*ppos % 2)) { >>> + u16 val; >>> + >>> + ret = parent->ops->read(mdev, (char *)&val, sizeof(val), >>> + *ppos); >>> + if (ret <= 0) >>> + goto read_err; >>> + >>> + if (copy_to_user(buf, &val, sizeof(val))) >>> + goto read_err; >>> + >>> + filled = 2; >>> + } else { >>> + u8 val; >>> + >>> + ret = parent->ops->read(mdev, &val, sizeof(val), *ppos); >>> + if (ret <= 0) >>> + goto read_err; >>> + >>> + if (copy_to_user(buf, &val, sizeof(val))) >>> + goto read_err; >>> + >>> + filled = 1; >>> + } >>> + >>> + count -= filled; >>> + done += filled; >>> + *ppos += filled; >>> + buf += filled; >>> + } >>> + >>> + return done; >>> + >>> +read_err: >>> + return -EFAULT; >>> +} >> [...] >> >> >> Dong Jia >>
Re: [Qemu-devel] (no subject)
On Mon, 09/12 16:23, Stephen Bates wrote: > Hi Hi Stephen, > > I sent this to qemu-discuss with no success so resending to qemu-devel. > > I am doing some very low level OS design work and wanted to be able to > alter some values in the physical memory of my QEMU guest. I can see quite > a few ways to print/dump both physical and virtual addresses but nothing > that can alter arbitrary physical/virtual addresses? > > Does such a feature exist in Qemu and if it does are there pointers to > documentation for it? Have you tried the builtin gdbstub in QEMU? You can add "-s" to the QEMU command line and then connect to it from gdb with "target remote :1234". There you can inspect or change memory more easily. Fam > > I do see that we can use file backing for very large memory regions via > the memory-backing-file option but I am not really trying to alter massive > regions of memory in this case. > > Cheers > > Stephen Bates >
[Qemu-devel] (no subject)
Hi I sent this to qemu-discuss with no success so resending to qemu-devel. I am doing some very low level OS design work and wanted to be able to alter some values in the physical memory of my QEMU guest. I can see quite a few ways to print/dump both physical and virtual addresses but nothing that can alter arbitrary physical/virtual addresses? Does such a feature exist in Qemu and if it does are there pointers to documentation for it? I do see that we can use file backing for very large memory regions via the memory-backing-file option but I am not really trying to alter massive regions of memory in this case. Cheers Stephen Bates
Re: [Qemu-devel] [PULL 00/17] Block patches
On Mon, 09/12 16:56, Peter Maydell wrote: > On 12 September 2016 at 16:12, Peter Maydell wrote: > > On 12 September 2016 at 15:08, Stefan Hajnoczi wrote: > >> The following changes since commit > >> c2a57aae9a1c3dd7de77daf5478df10379aeeebf: > >> > >> Merge remote-tracking branch 'remotes/famz/tags/docker-pull-request' > >> into staging (2016-09-09 12:49:41 +0100) > >> > >> are available in the git repository at: > >> > >> git://github.com/stefanha/qemu.git tags/block-pull-request > >> > >> for you to fetch changes up to 8f1096787517c66b67cc29bab65edc0188a86326: > >> > >> tests: fix qvirtqueue_kick (2016-09-12 15:06:29 +0100) > > > > > > /replication/primary/get_error: OK > > /replication/secondary/get_error:OK > > > > Please can you rename these tests? They create false positives in > > scripts that look in the build logs for errors by grepping for > > "error:" or "warning:". > > Also, two new sanitizer errors: > > /home/petmay01/linaro/qemu-for-merges/block/qcow2.c:1807:41: runtime > error: null pointer passed as argument 2, which is declared to never > be null > /home/petmay01/linaro/qemu-for-merges/block/qcow2-cluster.c:86:26: > runtime error: null pointer passed as argument 2, which is declared to > never be null > > both attempts to memcpy() from a NULL source pointer while running > the tests/test-replication test. > Stefan, if you are going to do another PULL, do you mind including https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg01717.html as well? :) Fam
Re: [Qemu-devel] [PATCH COLO-Frame v19 00/22] COarse-grain LOck-stepping(COLO) Virtual Machines for Non-stop Service (FT)
On 2016/9/10 16:21, Amit Shah wrote: Hi, On (Fri) 09 Sep 2016 [14:45:36], Hailiang Zhang wrote: ping ? Juan will start reviewing and picking patches soon. Great, and the pull request of block replication series has been sent by Stefan, so there is no obstacle to impede this series to be merged into upstream. Thanks very much ;) .
[Qemu-devel] [PULL v3 17/18] target-i386: Generate fences for x86
From: Pranith Kumar Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-15-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- target-i386/translate.c | 8 1 file changed, 8 insertions(+) diff --git a/target-i386/translate.c b/target-i386/translate.c index fa2ac48..9447557 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -8012,13 +8012,21 @@ static target_ulong disas_insn(CPUX86State *env, DisasContext *s, || (prefixes & PREFIX_LOCK)) { goto illegal_op; } +tcg_gen_mb(TCG_MO_ST_ST | TCG_BAR_SC); break; case 0xe8 ... 0xef: /* lfence */ +if (!(s->cpuid_features & CPUID_SSE) +|| (prefixes & PREFIX_LOCK)) { +goto illegal_op; +} +tcg_gen_mb(TCG_MO_LD_LD | TCG_BAR_SC); +break; case 0xf0 ... 0xf7: /* mfence */ if (!(s->cpuid_features & CPUID_SSE2) || (prefixes & PREFIX_LOCK)) { goto illegal_op; } +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); break; default: -- 2.7.4
[Qemu-devel] [PULL v3 18/18] tcg: Optimize fence instructions
From: Pranith Kumar This commit optimizes fence instructions. Two optimizations are currently implemented: (1) unnecessary duplicate fence instructions, and (2) merging weaker fences into a stronger fence. [rth: Merge tcg_optimize_mb back into tcg_optimize, so that we only loop over the opcode stream once. Merge "unrelated" weaker barriers into one stronger barrier.] Signed-off-by: Pranith Kumar Message-Id: <20160823134825.32578-1-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/optimize.c | 54 ++ 1 file changed, 54 insertions(+) diff --git a/tcg/optimize.c b/tcg/optimize.c index cffe89b..0455285 100644 --- a/tcg/optimize.c +++ b/tcg/optimize.c @@ -542,6 +542,7 @@ static bool swap_commutative2(TCGArg *p1, TCGArg *p2) void tcg_optimize(TCGContext *s) { int oi, oi_next, nb_temps, nb_globals; +TCGArg *prev_mb_args = NULL; /* Array VALS has an element for each temp. If this temp holds a constant then its value is kept in VALS' element. @@ -1295,5 +1296,58 @@ void tcg_optimize(TCGContext *s) } break; } + +/* Eliminate duplicate and redundant fence instructions. */ +if (prev_mb_args) { +TCGArg pop, cop; +TCGBar pty, cty; + +switch (opc) { +case INDEX_op_mb: +pop = prev_mb_args[0]; +cop = args[0]; +pty = pop & 0xF0; +cty = cop & 0xF0; + +if (cty == pty) { +/* Two barriers of the same type. Merge the set of + * memories to which this applies. */ +pop |= cop & 0x0F; +} else { +/* Merge a weaker barrier into a stronger one, + * or two weaker barriers into a stronger one. + * mb; strl => mb; st + * ldaq; mb => ld; mb + * ldaq; strl => ld; mb; st + * Other combinations are also merged into a strong + * barrier. This is stricter than specified but for + * the purposes of TCG is better than not optimizing. + */ +pop = TCG_BAR_SC | ((cop | pop) & 0x0F); +} +/* Change the previous barrier to the merged state. + * Then we can remove the current barrier. */ +prev_mb_args[0] = pop; +tcg_op_remove(s, op); +break; + +default: +/* Opcodes that end the block stop the optimization. */ +if ((def->flags & TCG_OPF_BB_END) == 0) { +break; +} +/* fallthru */ +case INDEX_op_qemu_ld_i32: +case INDEX_op_qemu_ld_i64: +case INDEX_op_qemu_st_i32: +case INDEX_op_qemu_st_i64: +case INDEX_op_call: +/* Opcodes that touch guest memory stop the optimization. */ +prev_mb_args = NULL; +break; +} +} else if (opc == INDEX_op_mb) { +prev_mb_args = args; +} } } -- 2.7.4
[Qemu-devel] [PULL v3 15/18] target-alpha: Generate fence op
From: Pranith Kumar Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-13-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- target-alpha/translate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target-alpha/translate.c b/target-alpha/translate.c index 0ea0e6e..c27c7b9 100644 --- a/target-alpha/translate.c +++ b/target-alpha/translate.c @@ -2338,11 +2338,11 @@ static ExitStatus translate_one(DisasContext *ctx, uint32_t insn) break; case 0x4000: /* MB */ -/* No-op */ +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); break; case 0x4400: /* WMB */ -/* No-op */ +tcg_gen_mb(TCG_MO_ST_ST | TCG_BAR_SC); break; case 0x8000: /* FETCH */ -- 2.7.4
[Qemu-devel] [PULL v3 12/18] tcg/sparc: Add support for fence
From: Pranith Kumar Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-10-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/sparc/tcg-target.inc.c | 13 + 1 file changed, 13 insertions(+) diff --git a/tcg/sparc/tcg-target.inc.c b/tcg/sparc/tcg-target.inc.c index 10e4126..bca25e2 100644 --- a/tcg/sparc/tcg-target.inc.c +++ b/tcg/sparc/tcg-target.inc.c @@ -249,6 +249,8 @@ static const int tcg_target_call_oarg_regs[] = { #define STWA (INSN_OP(3) | INSN_OP3(0x14)) #define STXA (INSN_OP(3) | INSN_OP3(0x1e)) +#define MEMBAR (INSN_OP(2) | INSN_OP3(0x28) | INSN_RS1(15) | (1 << 13)) + #ifndef ASI_PRIMARY_LITTLE #define ASI_PRIMARY_LITTLE 0x88 #endif @@ -835,6 +837,12 @@ static void tcg_out_call(TCGContext *s, tcg_insn_unit *dest) tcg_out_nop(s); } +static void tcg_out_mb(TCGContext *s, TCGArg a0) +{ +/* Note that the TCG memory order constants mirror the Sparc MEMBAR. */ +tcg_out32(s, MEMBAR | (a0 & TCG_MO_ALL)); +} + #ifdef CONFIG_SOFTMMU static tcg_insn_unit *qemu_ld_trampoline[16]; static tcg_insn_unit *qemu_st_trampoline[16]; @@ -1465,6 +1473,10 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, tcg_out_arithc(s, a0, TCG_REG_G0, a1, const_args[1], c); break; +case INDEX_op_mb: +tcg_out_mb(s, a0); +break; + case INDEX_op_mov_i32: /* Always emitted via tcg_out_mov. */ case INDEX_op_mov_i64: case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi. */ @@ -1566,6 +1578,7 @@ static const TCGTargetOpDef sparc_op_defs[] = { { INDEX_op_qemu_st_i32, { "sZ", "A" } }, { INDEX_op_qemu_st_i64, { "SZ", "A" } }, +{ INDEX_op_mb, { } }, { -1 }, }; -- 2.7.4
[Qemu-devel] [PULL v3 16/18] target-aarch64: Generate fences for aarch64
From: Pranith Kumar Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-14-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- target-arm/translate-a64.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index f5e29d2..09877bc 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -1305,7 +1305,7 @@ static void handle_sync(DisasContext *s, uint32_t insn, return; case 4: /* DSB */ case 5: /* DMB */ -/* We don't emulate caches so barriers are no-ops */ +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); return; case 6: /* ISB */ /* We need to break the TB after this insn to execute @@ -1934,7 +1934,13 @@ static void disas_ldst_excl(DisasContext *s, uint32_t insn) if (!is_store) { s->is_ldex = true; gen_load_exclusive(s, rt, rt2, tcg_addr, size, is_pair); +if (is_lasr) { +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ); +} } else { +if (is_lasr) { +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); +} gen_store_exclusive(s, rs, rt, rt2, tcg_addr, size, is_pair); } } else { @@ -1943,11 +1949,17 @@ static void disas_ldst_excl(DisasContext *s, uint32_t insn) /* Generate ISS for non-exclusive accesses including LASR. */ if (is_store) { +if (is_lasr) { +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_STRL); +} do_gpr_st(s, tcg_rt, tcg_addr, size, true, rt, iss_sf, is_lasr); } else { do_gpr_ld(s, tcg_rt, tcg_addr, size, false, false, true, rt, iss_sf, is_lasr); +if (is_lasr) { +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ); +} } } } -- 2.7.4
[Qemu-devel] [PULL v3 14/18] target-arm: Generate fences in ARMv7 frontend
From: Pranith Kumar Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-12-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- target-arm/translate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target-arm/translate.c b/target-arm/translate.c index bd5d5cb..693d4bc 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -8083,7 +8083,7 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn) case 4: /* dsb */ case 5: /* dmb */ ARCH(7); -/* We don't emulate caches so these are a no-op. */ +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); return; case 6: /* isb */ /* We need to break the TB after this insn to execute @@ -10432,7 +10432,7 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw break; case 4: /* dsb */ case 5: /* dmb */ -/* These execute as NOPs. */ +tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC); break; case 6: /* isb */ /* We need to break the TB after this insn -- 2.7.4
[Qemu-devel] [PULL v3 09/18] tcg/mips: Add support for fence
From: Pranith Kumar Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-7-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/mips/tcg-target.inc.c | 30 ++ 1 file changed, 30 insertions(+) diff --git a/tcg/mips/tcg-target.inc.c b/tcg/mips/tcg-target.inc.c index 8614ff8..8b9fb73 100644 --- a/tcg/mips/tcg-target.inc.c +++ b/tcg/mips/tcg-target.inc.c @@ -292,6 +292,7 @@ typedef enum { OPC_JALR = OPC_SPECIAL | 0x09, OPC_MOVZ = OPC_SPECIAL | 0x0A, OPC_MOVN = OPC_SPECIAL | 0x0B, +OPC_SYNC = OPC_SPECIAL | 0x0F, OPC_MFHI = OPC_SPECIAL | 0x10, OPC_MFLO = OPC_SPECIAL | 0x12, OPC_MULT = OPC_SPECIAL | 0x18, @@ -339,6 +340,14 @@ typedef enum { * backwards-compatible at the assembly level. */ OPC_MUL = use_mips32r6_instructions ? OPC_MUL_R6 : OPC_MUL_R5, + +/* MIPS r6 introduced names for weaker variants of SYNC. These are + backward compatible to previous architecture revisions. */ +OPC_SYNC_WMB = OPC_SYNC | 0x04 << 5, +OPC_SYNC_MB = OPC_SYNC | 0x10 << 5, +OPC_SYNC_ACQUIRE = OPC_SYNC | 0x11 << 5, +OPC_SYNC_RELEASE = OPC_SYNC | 0x12 << 5, +OPC_SYNC_RMB = OPC_SYNC | 0x13 << 5, } MIPSInsn; /* @@ -1383,6 +1392,22 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is_64) #endif } +static void tcg_out_mb(TCGContext *s, TCGArg a0) +{ +static const MIPSInsn sync[] = { +/* Note that SYNC_MB is a slightly weaker than SYNC 0, + as the former is an ordering barrier and the latter + is a completion barrier. */ +[0 ... TCG_MO_ALL]= OPC_SYNC_MB, +[TCG_MO_LD_LD]= OPC_SYNC_RMB, +[TCG_MO_ST_ST]= OPC_SYNC_WMB, +[TCG_MO_LD_ST]= OPC_SYNC_RELEASE, +[TCG_MO_LD_ST | TCG_MO_ST_ST] = OPC_SYNC_RELEASE, +[TCG_MO_LD_ST | TCG_MO_LD_LD] = OPC_SYNC_ACQUIRE, +}; +tcg_out32(s, sync[a0 & TCG_MO_ALL]); +} + static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args, const int *const_args) { @@ -1652,6 +1677,9 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, const_args[4], const_args[5], true); break; +case INDEX_op_mb: +tcg_out_mb(s, a0); +break; case INDEX_op_mov_i32: /* Always emitted via tcg_out_mov. */ case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi. */ case INDEX_op_call: /* Always emitted via tcg_out_call. */ @@ -1732,6 +1760,8 @@ static const TCGTargetOpDef mips_op_defs[] = { { INDEX_op_qemu_ld_i64, { "L", "L", "lZ", "lZ" } }, { INDEX_op_qemu_st_i64, { "SZ", "SZ", "SZ", "SZ" } }, #endif + +{ INDEX_op_mb, { } }, { -1 }, }; -- 2.7.4
[Qemu-devel] [PULL v3 10/18] tcg/ppc: Add support for fence
From: Pranith Kumar Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-8-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/ppc/tcg-target.inc.c | 21 + 1 file changed, 21 insertions(+) diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c index 82ac4b3..4aee8ea 100644 --- a/tcg/ppc/tcg-target.inc.c +++ b/tcg/ppc/tcg-target.inc.c @@ -469,6 +469,10 @@ static int tcg_target_const_match(tcg_target_long val, TCGType type, #define STHX XO31(407) #define STWX XO31(151) +#define EIEIO XO31(854) +#define HWSYNC XO31(598) +#define LWSYNC (HWSYNC | (1u << 21)) + #define SPR(a, b) a)<<5)|(b))<<11) #define LR SPR(8, 0) #define CTRSPR(9, 0) @@ -1243,6 +1247,18 @@ static void tcg_out_brcond2 (TCGContext *s, const TCGArg *args, tcg_out_bc(s, BC | BI(7, CR_EQ) | BO_COND_TRUE, arg_label(args[5])); } +static void tcg_out_mb(TCGContext *s, TCGArg a0) +{ +uint32_t insn = HWSYNC; +a0 &= TCG_MO_ALL; +if (a0 == TCG_MO_LD_LD) { +insn = LWSYNC; +} else if (a0 == TCG_MO_ST_ST) { +insn = EIEIO; +} +tcg_out32(s, insn); +} + #ifdef __powerpc64__ void ppc_tb_set_jmp_target(uintptr_t jmp_addr, uintptr_t addr) { @@ -2452,6 +2468,10 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args, tcg_out32(s, MULHD | TAB(args[0], args[1], args[2])); break; +case INDEX_op_mb: +tcg_out_mb(s, args[0]); +break; + case INDEX_op_mov_i32: /* Always emitted via tcg_out_mov. */ case INDEX_op_mov_i64: case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi. */ @@ -2599,6 +2619,7 @@ static const TCGTargetOpDef ppc_op_defs[] = { { INDEX_op_qemu_st_i64, { "S", "S", "S", "S" } }, #endif +{ INDEX_op_mb, { } }, { -1 }, }; -- 2.7.4
[Qemu-devel] [PULL v3 07/18] tcg/arm: Add support for fence
From: Pranith Kumar Cc: Andrzej Zaborowski Cc: Peter Maydell Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-5-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/arm/tcg-target.inc.c | 18 ++ 1 file changed, 18 insertions(+) diff --git a/tcg/arm/tcg-target.inc.c b/tcg/arm/tcg-target.inc.c index 58ffc0d..f3ff6f2 100644 --- a/tcg/arm/tcg-target.inc.c +++ b/tcg/arm/tcg-target.inc.c @@ -313,6 +313,10 @@ typedef enum { INSN_LDRD_REG = 0x00d0, INSN_STRD_IMM = 0x004000f0, INSN_STRD_REG = 0x00f0, + +INSN_DMB_ISH = 0x5bf07ff5, +INSN_DMB_MCR = 0xba0f07ee, + } ARMInsn; #define SHIFT_IMM_LSL(im) (((im) << 7) | 0x00) @@ -1066,6 +1070,15 @@ static inline void tcg_out_goto_label(TCGContext *s, int cond, TCGLabel *l) } } +static inline void tcg_out_mb(TCGContext *s, TCGArg a0) +{ +if (use_armv7_instructions) { +tcg_out32(s, INSN_DMB_ISH); +} else if (use_armv6_instructions) { +tcg_out32(s, INSN_DMB_MCR); +} +} + #ifdef CONFIG_SOFTMMU /* helper signature: helper_ret_ld_mmu(CPUState *env, target_ulong addr, * int mmu_idx, uintptr_t ra) @@ -1928,6 +1941,10 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, tcg_out_udiv(s, COND_AL, args[0], args[1], args[2]); break; +case INDEX_op_mb: +tcg_out_mb(s, args[0]); +break; + case INDEX_op_mov_i32: /* Always emitted via tcg_out_mov. */ case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi. */ case INDEX_op_call: /* Always emitted via tcg_out_call. */ @@ -2002,6 +2019,7 @@ static const TCGTargetOpDef arm_op_defs[] = { { INDEX_op_div_i32, { "r", "r", "r" } }, { INDEX_op_divu_i32, { "r", "r", "r" } }, +{ INDEX_op_mb, { } }, { -1 }, }; -- 2.7.4
[Qemu-devel] [PULL v3 13/18] tcg/tci: Add support for fence
From: Pranith Kumar Cc: Stefan Weil Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-11-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/tci/tcg-target.inc.c | 3 +++ tci.c| 4 2 files changed, 7 insertions(+) diff --git a/tcg/tci/tcg-target.inc.c b/tcg/tci/tcg-target.inc.c index 3c47ea7..9dbf4d5 100644 --- a/tcg/tci/tcg-target.inc.c +++ b/tcg/tci/tcg-target.inc.c @@ -255,6 +255,7 @@ static const TCGTargetOpDef tcg_target_op_defs[] = { { INDEX_op_bswap32_i32, { R, R } }, #endif +{ INDEX_op_mb, { } }, { -1 }, }; @@ -800,6 +801,8 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, const TCGArg *args, } tcg_out_i(s, *args++); break; +case INDEX_op_mb: +break; case INDEX_op_mov_i32: /* Always emitted via tcg_out_mov. */ case INDEX_op_mov_i64: case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi. */ diff --git a/tci.c b/tci.c index b488c0d..4bdc645 100644 --- a/tci.c +++ b/tci.c @@ -1236,6 +1236,10 @@ uintptr_t tcg_qemu_tb_exec(CPUArchState *env, uint8_t *tb_ptr) tcg_abort(); } break; +case INDEX_op_mb: +/* Ensure ordering for all kinds */ +smp_mb(); +break; default: TODO(); break; -- 2.7.4
[Qemu-devel] [PULL v3 11/18] tcg/s390: Add support for fence
From: Pranith Kumar Cc: Alexander Graf Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-9-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/s390/tcg-target.inc.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/tcg/s390/tcg-target.inc.c b/tcg/s390/tcg-target.inc.c index c30a7ef..ada607f 100644 --- a/tcg/s390/tcg-target.inc.c +++ b/tcg/s390/tcg-target.inc.c @@ -343,6 +343,7 @@ static tcg_insn_unit *tb_ret_addr; #define FACILITY_EXT_IMM (1ULL << (63 - 21)) #define FACILITY_GEN_INST_EXT (1ULL << (63 - 34)) #define FACILITY_LOAD_ON_COND (1ULL << (63 - 45)) +#define FACILITY_FAST_BCR_SER FACILITY_LOAD_ON_COND static uint64_t facilities; @@ -2167,6 +2168,15 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, tgen_deposit(s, args[0], args[2], args[3], args[4]); break; +case INDEX_op_mb: +/* The host memory model is quite strong, we simply need to + serialize the instruction stream. */ +if (args[0] & TCG_MO_ST_LD) { +tcg_out_insn(s, RR, BCR, + facilities & FACILITY_FAST_BCR_SER ? 14 : 15, 0); +} +break; + case INDEX_op_mov_i32: /* Always emitted via tcg_out_mov. */ case INDEX_op_mov_i64: case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi. */ @@ -2288,6 +2298,7 @@ static const TCGTargetOpDef s390_op_defs[] = { { INDEX_op_movcond_i64, { "r", "r", "rC", "r", "0" } }, { INDEX_op_deposit_i64, { "r", "0", "r" } }, +{ INDEX_op_mb, { } }, { -1 }, }; -- 2.7.4
[Qemu-devel] [PULL v3 05/18] tcg/i386: Add support for fence
From: Pranith Kumar Generate a 'lock orl $0,0(%esp)' instruction for ordering instead of mfence which has similar ordering semantics. Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-3-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/i386/tcg-target.inc.c | 17 + 1 file changed, 17 insertions(+) diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c index 1573e69..b4f3223 100644 --- a/tcg/i386/tcg-target.inc.c +++ b/tcg/i386/tcg-target.inc.c @@ -686,6 +686,18 @@ static inline void tcg_out_pushi(TCGContext *s, tcg_target_long val) } } +static inline void tcg_out_mb(TCGContext *s, TCGArg a0) +{ +/* Given the strength of x86 memory ordering, we only need care for + store-load ordering. Experimentally, "lock orl $0,0(%esp)" is + faster than "mfence", so don't bother with the sse insn. */ +if (a0 & TCG_MO_ST_LD) { +tcg_out8(s, 0xf0); +tcg_out_modrm_offset(s, OPC_ARITH_EvIb, ARITH_OR, TCG_REG_ESP, 0); +tcg_out8(s, 0); +} +} + static inline void tcg_out_push(TCGContext *s, int reg) { tcg_out_opc(s, OPC_PUSH_r32 + LOWREGMASK(reg), 0, reg, 0); @@ -2130,6 +2142,9 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, } break; +case INDEX_op_mb: +tcg_out_mb(s, args[0]); +break; case INDEX_op_mov_i32: /* Always emitted via tcg_out_mov. */ case INDEX_op_mov_i64: case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi. */ @@ -2195,6 +2210,8 @@ static const TCGTargetOpDef x86_op_defs[] = { { INDEX_op_add2_i32, { "r", "r", "0", "1", "ri", "ri" } }, { INDEX_op_sub2_i32, { "r", "r", "0", "1", "ri", "ri" } }, +{ INDEX_op_mb, { } }, + #if TCG_TARGET_REG_BITS == 32 { INDEX_op_brcond2_i32, { "r", "r", "ri", "ri" } }, { INDEX_op_setcond2_i32, { "r", "r", "r", "ri", "ri" } }, -- 2.7.4
[Qemu-devel] [PULL v3 04/18] Introduce TCGOpcode for memory barrier
From: Pranith Kumar This commit introduces the TCGOpcode for memory barrier instruction. This opcode takes an argument which is the type of memory barrier which should be generated. Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-2-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/README| 17 + tcg/tcg-op.c | 17 + tcg/tcg-op.h | 2 ++ tcg/tcg-opc.h | 2 ++ tcg/tcg.h | 19 +++ 5 files changed, 57 insertions(+) diff --git a/tcg/README b/tcg/README index ce8beba..1d48aa9 100644 --- a/tcg/README +++ b/tcg/README @@ -402,6 +402,23 @@ double-word product T0. The later is returned in two single-word outputs. Similar to mulu2, except the two inputs T1 and T2 are signed. +* Memory Barrier support + +* mb <$arg> + +Generate a target memory barrier instruction to ensure memory ordering as being +enforced by a corresponding guest memory barrier instruction. The ordering +enforced by the backend may be stricter than the ordering required by the guest. +It cannot be weaker. This opcode takes a constant argument which is required to +generate the appropriate barrier instruction. The backend should take care to +emit the target barrier instruction only when necessary i.e., for SMP guests and +when MTTCG is enabled. + +The guest translators should generate this opcode for all guest instructions +which have ordering side effects. + +Please see docs/atomics.txt for more information on memory barriers. + * 64-bit guest on 32-bit host support The following opcodes are internal to TCG. Thus they are to be implemented by diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c index 0243c99..e3af4dd 100644 --- a/tcg/tcg-op.c +++ b/tcg/tcg-op.c @@ -148,6 +148,23 @@ void tcg_gen_op6(TCGContext *ctx, TCGOpcode opc, TCGArg a1, TCGArg a2, tcg_emit_op(ctx, opc, pi); } +void tcg_gen_mb(TCGArg mb_type) +{ +bool emit_barriers = true; + +#ifndef CONFIG_USER_ONLY +/* TODO: When MTTCG is available for system mode, we will check + * the following condition and enable emit_barriers + * (qemu_tcg_mttcg_enabled() && smp_cpus > 1) + */ +emit_barriers = false; +#endif + +if (emit_barriers) { +tcg_gen_op1(&tcg_ctx, INDEX_op_mb, mb_type); +} +} + /* 32 bit ops */ void tcg_gen_addi_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2) diff --git a/tcg/tcg-op.h b/tcg/tcg-op.h index f217e80..41890cc 100644 --- a/tcg/tcg-op.h +++ b/tcg/tcg-op.h @@ -261,6 +261,8 @@ static inline void tcg_gen_br(TCGLabel *l) tcg_gen_op1(&tcg_ctx, INDEX_op_br, label_arg(l)); } +void tcg_gen_mb(TCGArg a); + /* Helper calls. */ /* 32 bit ops */ diff --git a/tcg/tcg-opc.h b/tcg/tcg-opc.h index 6d0410c..45528d2 100644 --- a/tcg/tcg-opc.h +++ b/tcg/tcg-opc.h @@ -42,6 +42,8 @@ DEF(br, 0, 0, 1, TCG_OPF_BB_END) # define IMPL64 TCG_OPF_64BIT #endif +DEF(mb, 0, 0, 1, 0) + DEF(mov_i32, 1, 1, 0, TCG_OPF_NOT_PRESENT) DEF(movi_i32, 1, 0, 1, TCG_OPF_NOT_PRESENT) DEF(setcond_i32, 1, 2, 1, 0) diff --git a/tcg/tcg.h b/tcg/tcg.h index 8856f02..f8dfe4c 100644 --- a/tcg/tcg.h +++ b/tcg/tcg.h @@ -476,6 +476,25 @@ static inline intptr_t QEMU_ARTIFICIAL GET_TCGV_PTR(TCGv_ptr t) #define TCG_CALL_DUMMY_TCGV MAKE_TCGV_I32(-1) #define TCG_CALL_DUMMY_ARG ((TCGArg)(-1)) +/* used to indicate the type of accesses on which ordering is to be + ensured. Modeled after SPARC barriers */ +typedef enum { +TCG_MO_LD_LD= 1, +TCG_MO_ST_LD= 2, +TCG_MO_LD_ST= 4, +TCG_MO_ST_ST= 8, +TCG_MO_ALL = 0xF, /* OR of all above */ +} TCGOrder; + +/* used to indicate the kind of ordering which is to be ensured by the + instruction. These types are derived from x86/aarch64 instructions. + It should be noted that these are different from C11 semantics */ +typedef enum { +TCG_BAR_LDAQ = 0x10, /* generated for aarch64 load-acquire inst. */ +TCG_BAR_STRL = 0x20, /* generated for aarch64 store-rel inst. */ +TCG_BAR_SC = 0x40, /* generated for all other ordering inst. */ +} TCGBar; + /* Conditions. Note that these are laid out for easy manipulation by the functions below: bit 0 is used for inverting; -- 2.7.4
[Qemu-devel] [PULL v3 06/18] tcg/aarch64: Add support for fence
From: Pranith Kumar Cc: Claudio Fontana Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-4-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/aarch64/tcg-target.inc.c | 24 1 file changed, 24 insertions(+) diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c index 2f5629e..6caa9a4 100644 --- a/tcg/aarch64/tcg-target.inc.c +++ b/tcg/aarch64/tcg-target.inc.c @@ -372,6 +372,11 @@ typedef enum { I3510_EOR = 0x4a00, I3510_EON = 0x4a20, I3510_ANDS = 0x6a00, + +/* System instructions. */ +DMB_ISH = 0xd50338bf, +DMB_LD = 0x0100, +DMB_ST = 0x0200, } AArch64Insn; static inline uint32_t tcg_in32(TCGContext *s) @@ -981,6 +986,20 @@ static inline void tcg_out_addsub2(TCGContext *s, int ext, TCGReg rl, tcg_out_mov(s, ext, orig_rl, rl); } +static inline void tcg_out_mb(TCGContext *s, TCGArg a0) +{ +uint32_t dmb_type = DMB_ISH; +a0 &= TCG_MO_ALL; +if (a0 == TCG_MO_LD_LD) { +dmb_type |= DMB_LD; +} else if (a0 == TCG_MO_ST_ST) { +dmb_type |= DMB_ST; +} else { +dmb_type |= DMB_LD | DMB_ST; +} +tcg_out32(s, dmb_type); +} + #ifdef CONFIG_SOFTMMU /* helper signature: helper_ret_ld_mmu(CPUState *env, target_ulong addr, * TCGMemOpIdx oi, uintptr_t ra) @@ -1647,6 +1666,10 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc, tcg_out_insn(s, 3508, SMULH, TCG_TYPE_I64, a0, a1, a2); break; +case INDEX_op_mb: +tcg_out_mb(s, a0); +break; + case INDEX_op_mov_i32: /* Always emitted via tcg_out_mov. */ case INDEX_op_mov_i64: case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi. */ @@ -1771,6 +1794,7 @@ static const TCGTargetOpDef aarch64_op_defs[] = { { INDEX_op_muluh_i64, { "r", "r", "r" } }, { INDEX_op_mulsh_i64, { "r", "r", "r" } }, +{ INDEX_op_mb, { } }, { -1 }, }; -- 2.7.4
[Qemu-devel] [PULL v3 08/18] tcg/ia64: Add support for fence
From: Pranith Kumar Cc: Aurelien Jarno Signed-off-by: Pranith Kumar Message-Id: <20160714202026.9727-6-bobby.pr...@gmail.com> Signed-off-by: Richard Henderson --- tcg/ia64/tcg-target.inc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/tcg/ia64/tcg-target.inc.c b/tcg/ia64/tcg-target.inc.c index 7642390..b04d716 100644 --- a/tcg/ia64/tcg-target.inc.c +++ b/tcg/ia64/tcg-target.inc.c @@ -247,6 +247,7 @@ enum { OPC_LD4_M3= 0x0a08000ull, OPC_LD8_M1= 0x080c000ull, OPC_LD8_M3= 0x0a0c000ull, +OPC_MF_M24= 0x0011000ull, OPC_MUX1_I3 = 0x0eca000ull, OPC_NOP_B9= 0x0400800ull, OPC_NOP_F16 = 0x800ull, @@ -2231,6 +2232,9 @@ static inline void tcg_out_op(TCGContext *s, TCGOpcode opc, tcg_out_qemu_st(s, args); break; +case INDEX_op_mb: +tcg_out_bundle(s, mmI, OPC_MF_M24, INSN_NOP_M, INSN_NOP_I); +break; case INDEX_op_mov_i32: /* Always emitted via tcg_out_mov. */ case INDEX_op_mov_i64: case INDEX_op_movi_i32: /* Always emitted via tcg_out_movi. */ @@ -2344,6 +2348,7 @@ static const TCGTargetOpDef ia64_op_defs[] = { { INDEX_op_qemu_st_i32, { "SZ", "r" } }, { INDEX_op_qemu_st_i64, { "SZ", "r" } }, +{ INDEX_op_mb, { } }, { -1 }, }; -- 2.7.4
[Qemu-devel] [PULL v3 01/18] tcg: Support arbitrary size + alignment
Previously we allowed fully unaligned operations, but not operations that are aligned but with less alignment than the operation size. In addition, arm32, ia64, mips, and sparc had been omitted from the previous overalignment patch, which would have led to that alignment being enforced. Signed-off-by: Richard Henderson --- softmmu_template.h | 16 ++-- tcg/aarch64/tcg-target.inc.c | 11 tcg/arm/tcg-target.inc.c | 19 +- tcg/i386/tcg-target.inc.c| 16 ++-- tcg/ia64/tcg-target.inc.c| 22 +++- tcg/mips/tcg-target.inc.c| 12 ++--- tcg/ppc/tcg-target.inc.c | 57 +--- tcg/s390/tcg-target.inc.c| 13 +++--- tcg/sparc/tcg-target.inc.c | 17 +++- tcg/tcg.h| 62 +--- 10 files changed, 132 insertions(+), 113 deletions(-) diff --git a/softmmu_template.h b/softmmu_template.h index 284ab2c..7ea0a41 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -146,14 +146,14 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong addr, unsigned mmu_idx = get_mmuidx(oi); int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); target_ulong tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ; -int a_bits = get_alignment_bits(get_memop(oi)); +unsigned a_bits = get_alignment_bits(get_memop(oi)); uintptr_t haddr; DATA_TYPE res; /* Adjust the given return address. */ retaddr -= GETPC_ADJ; -if (a_bits > 0 && (addr & ((1 << a_bits) - 1)) != 0) { +if (addr & nbits(a_bits)) { cpu_unaligned_access(ENV_GET_CPU(env), addr, READ_ACCESS_TYPE, mmu_idx, retaddr); } @@ -220,14 +220,14 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, target_ulong addr, unsigned mmu_idx = get_mmuidx(oi); int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); target_ulong tlb_addr = env->tlb_table[mmu_idx][index].ADDR_READ; -int a_bits = get_alignment_bits(get_memop(oi)); +unsigned a_bits = get_alignment_bits(get_memop(oi)); uintptr_t haddr; DATA_TYPE res; /* Adjust the given return address. */ retaddr -= GETPC_ADJ; -if (a_bits > 0 && (addr & ((1 << a_bits) - 1)) != 0) { +if (addr & nbits(a_bits)) { cpu_unaligned_access(ENV_GET_CPU(env), addr, READ_ACCESS_TYPE, mmu_idx, retaddr); } @@ -331,13 +331,13 @@ void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, unsigned mmu_idx = get_mmuidx(oi); int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; -int a_bits = get_alignment_bits(get_memop(oi)); +unsigned a_bits = get_alignment_bits(get_memop(oi)); uintptr_t haddr; /* Adjust the given return address. */ retaddr -= GETPC_ADJ; -if (a_bits > 0 && (addr & ((1 << a_bits) - 1)) != 0) { +if (addr & nbits(a_bits)) { cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); } @@ -414,13 +414,13 @@ void helper_be_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, unsigned mmu_idx = get_mmuidx(oi); int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; -int a_bits = get_alignment_bits(get_memop(oi)); +unsigned a_bits = get_alignment_bits(get_memop(oi)); uintptr_t haddr; /* Adjust the given return address. */ retaddr -= GETPC_ADJ; -if (a_bits > 0 && (addr & ((1 << a_bits) - 1)) != 0) { +if (addr & nbits(a_bits)) { cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); } diff --git a/tcg/aarch64/tcg-target.inc.c b/tcg/aarch64/tcg-target.inc.c index 08b2d03..2f5629e 100644 --- a/tcg/aarch64/tcg-target.inc.c +++ b/tcg/aarch64/tcg-target.inc.c @@ -1081,23 +1081,22 @@ static void tcg_out_tlb_read(TCGContext *s, TCGReg addr_reg, TCGMemOp opc, int tlb_offset = is_read ? offsetof(CPUArchState, tlb_table[mem_index][0].addr_read) : offsetof(CPUArchState, tlb_table[mem_index][0].addr_write); -int a_bits = get_alignment_bits(opc); +unsigned a_bits = get_alignment_bits(opc); +unsigned s_bits = opc & MO_SIZE; TCGReg base = TCG_AREG0, x3; uint64_t tlb_mask; /* For aligned accesses, we check the first byte and include the alignment bits within the address. For unaligned access, we check that we don't cross pages using the address of the last byte of the access. */ -if (a_bits >= 0) { -/* A byte access or an alignment check required */ -tlb_mask = TARGET_PAGE_MASK | ((1 << a_bits) - 1); +if (a_bits >= s_bits) { x3 = addr_reg; } else { tcg_out_insn(s, 34
[Qemu-devel] [PULL v3 03/18] cpu-exec: Check -dfilter for -d cpu
Signed-off-by: Richard Henderson --- cpu-exec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpu-exec.c b/cpu-exec.c index 5d9710a..e7f851c 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -147,7 +147,8 @@ static inline tcg_target_ulong cpu_tb_exec(CPUState *cpu, TranslationBlock *itb) itb->tc_ptr, itb->pc, lookup_symbol(itb->pc)); #if defined(DEBUG_DISAS) -if (qemu_loglevel_mask(CPU_LOG_TB_CPU)) { +if (qemu_loglevel_mask(CPU_LOG_TB_CPU) +&& qemu_log_in_addr_range(itb->pc)) { #if defined(TARGET_I386) log_cpu_state(cpu, CPU_DUMP_CCOP); #elif defined(TARGET_M68K) -- 2.7.4
[Qemu-devel] [PULL v3 02/18] tcg: Merge GETPC and GETRA
The return address argument to the softmmu template helpers was confused. In the legacy case, we wanted to indicate that there is no return address, and so passed in NULL. However, we then immediately subtracted GETPC_ADJ from NULL, resulting in a non-zero value, indicating the presence of an (invalid) return address. Push the GETPC_ADJ subtraction down to the only point it's required: immediately before use within cpu_restore_state, after all NULL pointer checks have been completed. This makes GETPC and GETRA identical. Remove GETRA as the lesser used macro, replacing all uses with GETPC. Signed-off-by: Richard Henderson --- cputlb.c| 6 ++ include/exec/exec-all.h | 9 +++-- softmmu_template.h | 32 ++-- target-arm/helper.c | 6 +++--- target-mips/op_helper.c | 18 +- translate-all.c | 1 + user-exec.c | 7 +-- 7 files changed, 29 insertions(+), 50 deletions(-) diff --git a/cputlb.c b/cputlb.c index d068ee5..3c99c34 100644 --- a/cputlb.c +++ b/cputlb.c @@ -543,10 +543,8 @@ static bool victim_tlb_hit(CPUArchState *env, size_t mmu_idx, size_t index, #undef MMUSUFFIX #define MMUSUFFIX _cmmu -#undef GETPC_ADJ -#define GETPC_ADJ 0 -#undef GETRA -#define GETRA() ((uintptr_t)0) +#undef GETPC +#define GETPC() ((uintptr_t)0) #define SOFTMMU_CODE_ACCESS #define SHIFT 0 diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index d008296..8b557d8 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -347,13 +347,12 @@ static inline void tb_add_jump(TranslationBlock *tb, int n, tb_next->jmp_list_first = (uintptr_t)tb | n; } -/* GETRA is the true target of the return instruction that we'll execute, - defined here for simplicity of defining the follow-up macros. */ +/* GETPC is the true target of the return instruction that we'll execute. */ #if defined(CONFIG_TCG_INTERPRETER) extern uintptr_t tci_tb_ptr; -# define GETRA() tci_tb_ptr +# define GETPC() tci_tb_ptr #else -# define GETRA() \ +# define GETPC() \ ((uintptr_t)__builtin_extract_return_addr(__builtin_return_address(0))) #endif @@ -366,8 +365,6 @@ extern uintptr_t tci_tb_ptr; smaller than 4 bytes, so we don't worry about special-casing this. */ #define GETPC_ADJ 2 -#define GETPC() (GETRA() - GETPC_ADJ) - #if !defined(CONFIG_USER_ONLY) struct MemoryRegion *iotlb_to_region(CPUState *cpu, diff --git a/softmmu_template.h b/softmmu_template.h index 7ea0a41..b3eb821 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -150,9 +150,6 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong addr, uintptr_t haddr; DATA_TYPE res; -/* Adjust the given return address. */ -retaddr -= GETPC_ADJ; - if (addr & nbits(a_bits)) { cpu_unaligned_access(ENV_GET_CPU(env), addr, READ_ACCESS_TYPE, mmu_idx, retaddr); @@ -193,10 +190,8 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong addr, do_unaligned_access: addr1 = addr & ~(DATA_SIZE - 1); addr2 = addr1 + DATA_SIZE; -/* Note the adjustment at the beginning of the function. - Undo that for the recursion. */ -res1 = helper_le_ld_name(env, addr1, oi, retaddr + GETPC_ADJ); -res2 = helper_le_ld_name(env, addr2, oi, retaddr + GETPC_ADJ); +res1 = helper_le_ld_name(env, addr1, oi, retaddr); +res2 = helper_le_ld_name(env, addr2, oi, retaddr); shift = (addr & (DATA_SIZE - 1)) * 8; /* Little-endian combine. */ @@ -224,9 +219,6 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, target_ulong addr, uintptr_t haddr; DATA_TYPE res; -/* Adjust the given return address. */ -retaddr -= GETPC_ADJ; - if (addr & nbits(a_bits)) { cpu_unaligned_access(ENV_GET_CPU(env), addr, READ_ACCESS_TYPE, mmu_idx, retaddr); @@ -267,10 +259,8 @@ WORD_TYPE helper_be_ld_name(CPUArchState *env, target_ulong addr, do_unaligned_access: addr1 = addr & ~(DATA_SIZE - 1); addr2 = addr1 + DATA_SIZE; -/* Note the adjustment at the beginning of the function. - Undo that for the recursion. */ -res1 = helper_be_ld_name(env, addr1, oi, retaddr + GETPC_ADJ); -res2 = helper_be_ld_name(env, addr2, oi, retaddr + GETPC_ADJ); +res1 = helper_be_ld_name(env, addr1, oi, retaddr); +res2 = helper_be_ld_name(env, addr2, oi, retaddr); shift = (addr & (DATA_SIZE - 1)) * 8; /* Big-endian combine. */ @@ -334,9 +324,6 @@ void helper_le_st_name(CPUArchState *env, target_ulong addr, DATA_TYPE val, unsigned a_bits = get_alignment_bits(get_memop(oi)); uintptr_t haddr; -/* Adjust the given return address. */ -retaddr -= GETPC_ADJ; - if (addr & nbits(a_bits)) { cpu_unaligned_access(ENV_GET_CPU(env), addr, MMU_DATA_STORE,
[Qemu-devel] [PULL v3 00/18] tcg queued patches
Mostly the same as v2, except rebased and the tcg/mips patch adjusted for the mips32r6 discussion with Leon. r~ The following changes since commit c2a57aae9a1c3dd7de77daf5478df10379aeeebf: Merge remote-tracking branch 'remotes/famz/tags/docker-pull-request' into staging (2016-09-09 12:49:41 +0100) are available in the git repository at: git://github.com/rth7680/qemu.git tags/pull-tcg-20160912 for you to fetch changes up to 3c8951649d996facd9581752b87b6c09d2dbab06: tcg: Optimize fence instructions (2016-09-12 16:29:50 -0700) queued tcg patches Pranith Kumar (15): Introduce TCGOpcode for memory barrier tcg/i386: Add support for fence tcg/aarch64: Add support for fence tcg/arm: Add support for fence tcg/ia64: Add support for fence tcg/mips: Add support for fence tcg/ppc: Add support for fence tcg/s390: Add support for fence tcg/sparc: Add support for fence tcg/tci: Add support for fence target-arm: Generate fences in ARMv7 frontend target-alpha: Generate fence op target-aarch64: Generate fences for aarch64 target-i386: Generate fences for x86 tcg: Optimize fence instructions Richard Henderson (3): tcg: Support arbitrary size + alignment tcg: Merge GETPC and GETRA cpu-exec: Check -dfilter for -d cpu cpu-exec.c | 3 +- cputlb.c | 6 ++-- include/exec/exec-all.h | 9 ++--- softmmu_template.h | 48 -- target-alpha/translate.c | 4 +-- target-arm/helper.c | 6 ++-- target-arm/translate-a64.c | 14 +++- target-arm/translate.c | 4 +-- target-i386/translate.c | 8 + target-mips/op_helper.c | 18 +- tcg/README | 17 ++ tcg/aarch64/tcg-target.inc.c | 35 +++ tcg/arm/tcg-target.inc.c | 37 tcg/i386/tcg-target.inc.c| 33 +- tcg/ia64/tcg-target.inc.c| 27 +++ tcg/mips/tcg-target.inc.c| 42 +-- tcg/optimize.c | 54 + tcg/ppc/tcg-target.inc.c | 78 +++--- tcg/s390/tcg-target.inc.c| 24 - tcg/sparc/tcg-target.inc.c | 30 tcg/tcg-op.c | 17 ++ tcg/tcg-op.h | 2 ++ tcg/tcg-opc.h| 2 ++ tcg/tcg.h| 81 +++- tcg/tci/tcg-target.inc.c | 3 ++ tci.c| 4 +++ translate-all.c | 1 + user-exec.c | 7 ++-- 28 files changed, 445 insertions(+), 169 deletions(-)
Re: [Qemu-devel] [PATCH v2] target-i386: Use struct X86XSaveArea in fpu_helper.c
On Wed, Sep 07, 2016 at 09:47:07AM -0700, Richard Henderson wrote: > On 07/06/2016 01:35 PM, Richard Henderson wrote: > > This avoids a double hand-full of magic numbers in the > > xsave and xrstor helper functions. > > > > Signed-off-by: Richard Henderson > > --- > > target-i386/cpu.c| 7 ++- > > target-i386/cpu.h| 10 + > > target-i386/fpu_helper.c | 108 > > ++- > > 3 files changed, 67 insertions(+), 58 deletions(-) > > --- > > > > V2: Adjust struct X86XSaveHeader so as to retain the test for 16 bytes of > > zeros within xrstor. Comment on the slight inconsistency in two sections > > of the current Intel documentation. > > Ping. Applied to x86-next, thanks! -- Eduardo
[Qemu-devel] [Bug 672934] Re: FPU incorrect on Mac OS X
On Sep 12, 2016, at 5:03 PM, qemu-devel-requ...@nongnu.org wrote: Looks like the ISO from comment #4 (thanks for attaching that one!) shows the correct behavior with up to date QEMU 2.7. Also, the affected softfloat code has been completely reworked in between (e.g. with commit cf67c6bad56d43e6d60), so I assume this has been fixed sometimes in the past years. ** Changed in: qemu Status: New => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/672934 Title: FPU incorrect on Mac OS X Status in QEMU: Fix Released Bug description: I am using the 0.13.0 release version of QEMU on Mac OS X 10.6.4. I work for a university and the affected guest OS is our own research OS. I believe I found a bug in QEMU's FPU emulation, which only triggers on the Mac. You can reproduce the problem by booting the attached ISO image. Investigating the problem, I found that the lua interpreter in our loader component (called "ned") internally uses doubles to represent all lua-numbers. These doubles are showing completely wrong values on QEMU/Mac, resulting in the lua code not processing properly. I also attached a patch which fixes the problem for me. The attached ZIP-file also contains "before" and "after" screenshots. Note that booting the ISO on a real machine or on a Linux-QEMU always shows the correct "after" behavior. Only QEMU on the Mac exhibits the wrong "before" behavior without my patch. The patch might break other systems setting the CONFIG_BSD flag, so maybe the preprocessor should check for __APPLE__ instead to make the fix Mac-only. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/672934/+subscriptions I have always suspected a FPU bug with qemu-system-ppc. Apple's audio processing code uses floating point code a lot. As a possible result the playback of audio on a Mac OS guest is very poor. Is this a problem with certain floating point instructions? Also could you send me the patch. I would like to test it. Thanks.
Re: [Qemu-devel] [PATCH 9/9] tests: cris: add v17 ADDC test
On Mon, Sep 05, 2016 at 01:54:12PM +0200, Rabin Vincent wrote: > From: Rabin Vincent > > Add a test for the newly implemented ADDC instruction in the v17 CRIS > CPU. > > Signed-off-by: Rabin Vincent Acked-by: Edgar E. Iglesias > --- > tests/tcg/cris/Makefile| 19 ++-- > tests/tcg/cris/check_addcv17.s | 65 > ++ > 2 files changed, 82 insertions(+), 2 deletions(-) > create mode 100644 tests/tcg/cris/check_addcv17.s > > diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile > index 14a9eb5..6b3dba4 100644 > --- a/tests/tcg/cris/Makefile > +++ b/tests/tcg/cris/Makefile > @@ -23,6 +23,7 @@ SYS= sys.o > TESTCASES += check_abs.tst > TESTCASES += check_addc.tst > TESTCASES += check_addcm.tst > +TESTCASES += check_addcv17.tst > TESTCASES += check_addo.tst > TESTCASES += check_addoq.tst > TESTCASES += check_addi.tst > @@ -134,13 +135,27 @@ all: build > %.ctst: %.o > $(CC) $(CFLAGS) $(LDLIBS) $< -o $@ > > + > +sysv10.o: sys.c > + $(CC) $(CFLAGS) -mcpu=v10 -c $< -o $@ > + > +crtv10.o: crt.s > + $(AS) $(ASFLAGS) -mcpu=v10 -c $< -o $@ > + > +check_addcv17.tst: ASFLAGS += -mcpu=v10 > +check_addcv17.tst: CRT := crtv10.o > +check_addcv17.tst: SYS := sysv10.o > +check_addcv17.tst: crtv10.o sysv10.o > + > build: $(CRT) $(SYS) $(TESTCASES) > > check: $(CRT) $(SYS) $(TESTCASES) > @echo -e "\nQEMU simulator." > for case in $(TESTCASES); do \ > echo -n "$$case "; \ > - $(SIM) ./$$case; \ > + SIMARGS=; \ > + case $$case in *v17*) SIMARGS="-cpu crisv17";; esac; \ > + $(SIM) $$SIMARGS ./$$case; \ > done > check-g: $(CRT) $(SYS) $(TESTCASES) > @echo -e "\nGDB simulator." > @@ -150,4 +165,4 @@ check-g: $(CRT) $(SYS) $(TESTCASES) > done > > clean: > - $(RM) -fr $(TESTCASES) $(CRT) $(SYS) > + $(RM) -fr $(TESTCASES) *.o > diff --git a/tests/tcg/cris/check_addcv17.s b/tests/tcg/cris/check_addcv17.s > new file mode 100644 > index 000..52ef7a9 > --- /dev/null > +++ b/tests/tcg/cris/check_addcv17.s > @@ -0,0 +1,65 @@ > +# mach: crisv17 > + > + .include "testutils.inc" > + > + .macro addc Rs Rd inc=0 > +# Create the instruction manually since there is no assembler support yet > + .word (\Rd << 12) | \Rs | (\inc << 10) | 0x09a0 > + .endm > + > + start > + > + .data > +mem1: > + .dword 0x0 > +mem2: > + .dword 0x12345678 > + > + .text > + move.d mem1,r4 > + clearf nzvc > + addc 4 3 > + test_cc 0 1 0 0 > + checkr3 0 > + > + move.d mem1,r4 > + clearf nzvc > + ax > + addc 4 3 > + test_cc 0 0 0 0 > + checkr3 0 > + > + move.d mem1,r4 > + clearf nzvc > + setf c > + addc 4 3 > + test_cc 0 0 0 0 > + checkr3 1 > + > + move.d mem2,r4 > + moveq 2, r3 > + clearf nzvc > + setf c > + addc 4 3 > + test_cc 0 0 0 0 > + checkr3 1234567b > + > + move.d mem2,r5 > + clearf nzvc > + cmp.d r4,r5 > + test_cc 0 1 0 0 > + > + move.d mem2,r4 > + moveq 2, r3 > + clearf nzvc > + addc 4 3 inc=1 > + test_cc 0 0 0 0 > + checkr3 1234567a > + > + move.d mem2,r5 > + clearf nzvc > + addq 4,r5 > + cmp.d r4,r5 > + test_cc 0 1 0 0 > + > + quit > -- > 2.1.4 >
Re: [Qemu-devel] [PATCH 4/9] tests: cris: remove check_time1
On Mon, Sep 05, 2016 at 01:54:07PM +0200, Rabin Vincent wrote: > From: Rabin Vincent > > This test, borrowed from the GDB simulator test suite, checks that every > syscall increments the time returned by gettimeofday() by exactly 1 ms. > This is not guaranteed or even desirable on QEMU so remove this test. > > Signed-off-by: Rabin Vincent Reviewed-by: Edgar E. Iglesias > --- > tests/tcg/cris/Makefile | 1 - > tests/tcg/cris/check_time1.c | 46 > > 2 files changed, 47 deletions(-) > delete mode 100644 tests/tcg/cris/check_time1.c > > diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile > index f5230fc..14a9eb5 100644 > --- a/tests/tcg/cris/Makefile > +++ b/tests/tcg/cris/Makefile > @@ -114,7 +114,6 @@ TESTCASES += check_mmap1.ctst > TESTCASES += check_mmap2.ctst > TESTCASES += check_mmap3.ctst > TESTCASES += check_sigalrm.ctst > -TESTCASES += check_time1.ctst > TESTCASES += check_time2.ctst > TESTCASES += check_settls1.ctst > > diff --git a/tests/tcg/cris/check_time1.c b/tests/tcg/cris/check_time1.c > deleted file mode 100644 > index 3fcf0e1..000 > --- a/tests/tcg/cris/check_time1.c > +++ /dev/null > @@ -1,46 +0,0 @@ > -/* Basic time functionality test: check that milliseconds are > - incremented for each syscall (does not work on host). */ > -#include > -#include > -#include > -#include > -#include > - > -void err (const char *s) > -{ > - perror (s); > - abort (); > -} > - > -int > -main (void) > -{ > - struct timeval t_m = {0, 0}; > - struct timezone t_z = {0, 0}; > - struct timeval t_m1 = {0, 0}; > - int i; > - > - if (gettimeofday (&t_m, &t_z) != 0) > -err ("gettimeofday"); > - > - for (i = 1; i < 1; i++) > -if (gettimeofday (&t_m1, NULL) != 0) > - err ("gettimeofday 1"); > -else > - if (t_m1.tv_sec * 100 + t_m1.tv_usec > - != (t_m.tv_sec * 100 + t_m.tv_usec + i * 1000)) > - { > - fprintf (stderr, "t0 (%ld, %ld), i %d, t1 (%ld, %ld)\n", > -t_m.tv_sec, t_m.tv_usec, i, t_m1.tv_sec, t_m1.tv_usec); > - abort (); > - } > - > - if (time (NULL) != t_m1.tv_sec) > -{ > - fprintf (stderr, "time != gettod\n"); > - abort (); > -} > - > - printf ("pass\n"); > - exit (0); > -} > -- > 2.1.4 >
Re: [Qemu-devel] [PATCH 6/9] target-cris: reduce v32isms from v10 log dumps
On Mon, Sep 05, 2016 at 01:54:09PM +0200, Rabin Vincent wrote: > From: Hans-Peter Nilsson > > Use the correct register names for v10 and don't dump support function > registers for pre-v32. > > Signed-off-by: Hans-Peter Nilsson > Signed-off-by: Rabin Vincent > --- > target-cris/translate.c | 36 +++- > 1 file changed, 23 insertions(+), 13 deletions(-) > > diff --git a/target-cris/translate.c b/target-cris/translate.c > index c280e24..a4512b5 100644 > --- a/target-cris/translate.c > +++ b/target-cris/translate.c > @@ -140,14 +140,14 @@ static void gen_BUG(DisasContext *dc, const char *file, > int line) > cpu_abort(CPU(dc->cpu), "%s:%d\n", file, line); > } > > -static const char *regnames[] = > +static const char *regnames_v32[] = > { > "$r0", "$r1", "$r2", "$r3", > "$r4", "$r5", "$r6", "$r7", > "$r8", "$r9", "$r10", "$r11", > "$r12", "$r13", "$sp", "$acr", > }; > -static const char *pregnames[] = > +static const char *pregnames_v32[] = > { > "$bz", "$vr", "$pid", "$srs", > "$wz", "$exs", "$eda", "$mof", > @@ -3336,12 +3336,20 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, > fprintf_function cpu_fprintf, > { > CRISCPU *cpu = CRIS_CPU(cs); > CPUCRISState *env = &cpu->env; > +const char **regnames; > +const char **pregnames; > int i; > -uint32_t srs; > > if (!env || !f) { > return; > } > +if (env->pregs[PR_VR] < 32) { > +pregnames = pregnames_v10; > +regnames = regnames_v10; > +} else { > +pregnames = pregnames_v32; > +regnames = regnames_v32; > +} > > cpu_fprintf(f, "PC=%x CCS=%x btaken=%d btarget=%x\n" > "cc_op=%d cc_src=%d cc_dest=%d cc_result=%x cc_mask=%x\n", > @@ -3363,14 +3371,16 @@ void cris_cpu_dump_state(CPUState *cs, FILE *f, > fprintf_function cpu_fprintf, > cpu_fprintf(f, "\n"); > } > } > -srs = env->pregs[PR_SRS]; > -cpu_fprintf(f, "\nsupport function regs bank %x:\n", srs); > -if (srs < ARRAY_SIZE(env->sregs)) { > -for (i = 0; i < 16; i++) { > -cpu_fprintf(f, "s%2.2d=%8.8x ", > -i, env->sregs[srs][i]); > -if ((i + 1) % 4 == 0) { > -cpu_fprintf(f, "\n"); > +if (env->pregs[PR_SRS] >= 32) { did you mean env->pregs[PR_VR] >= 32 here? > +uint32_t srs = env->pregs[PR_SRS]; > +cpu_fprintf(f, "\nsupport function regs bank %x:\n", srs); > +if (srs < ARRAY_SIZE(env->sregs)) { > +for (i = 0; i < 16; i++) { > +cpu_fprintf(f, "s%2.2d=%8.8x ", > +i, env->sregs[srs][i]); > +if ((i + 1) % 4 == 0) { > +cpu_fprintf(f, "\n"); > +} > } > } > } > @@ -3415,12 +3425,12 @@ void cris_initialize_tcg(void) > for (i = 0; i < 16; i++) { > cpu_R[i] = tcg_global_mem_new(cpu_env, >offsetof(CPUCRISState, regs[i]), > - regnames[i]); > + regnames_v32[i]); > } > for (i = 0; i < 16; i++) { > cpu_PR[i] = tcg_global_mem_new(cpu_env, > offsetof(CPUCRISState, pregs[i]), > - pregnames[i]); > + pregnames_v32[i]); > } > } > > -- > 2.1.4 >
Re: [Qemu-devel] [PATCH 3/9] tests: cris: remove openpf4 test
On Mon, Sep 05, 2016 at 01:54:06PM +0200, Rabin Vincent wrote: > From: Rabin Vincent > > This test, borrowed from the GDB simulator test suite, is meant to test > the GDB simulator's --sysroot feature and always fails in QEMU. Remove > it. openpf3 tests the same sequence of system calls (without assuming > the precence of --sysroot). > > Signed-off-by: Rabin Vincent Reviewed-by: Edgar E. Iglesias > --- > tests/tcg/cris/Makefile| 1 - > tests/tcg/cris/check_openpf4.c | 5 - > 2 files changed, 6 deletions(-) > delete mode 100644 tests/tcg/cris/check_openpf4.c > > diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile > index d34bfd8..f5230fc 100644 > --- a/tests/tcg/cris/Makefile > +++ b/tests/tcg/cris/Makefile > @@ -108,7 +108,6 @@ TESTCASES += check_stat4.ctst > TESTCASES += check_openpf1.ctst > TESTCASES += check_openpf2.ctst > TESTCASES += check_openpf3.ctst > -TESTCASES += check_openpf4.ctst > TESTCASES += check_openpf5.ctst > TESTCASES += check_mapbrk.ctst > TESTCASES += check_mmap1.ctst > diff --git a/tests/tcg/cris/check_openpf4.c b/tests/tcg/cris/check_openpf4.c > deleted file mode 100644 > index 8bbee41..000 > --- a/tests/tcg/cris/check_openpf4.c > +++ /dev/null > @@ -1,5 +0,0 @@ > -/* Basic file operations, now *with* sysroot. > -#sim: --sysroot=@exedir@ > -*/ > -#define PREFIX "/" > -#include "check_openpf3.c" > -- > 2.1.4 >
Re: [Qemu-devel] [PATCH 7/9] target-cris: ignore prefix insns in singlestep
On Mon, Sep 05, 2016 at 01:54:10PM +0200, Rabin Vincent wrote: > From: Hans-Peter Nilsson > > Don't count prefix instructions as separate when singlestepping. > > For example, for this following instruction > > 1ad8: a011 e00b move.d r0,[r1-96] > > before this patch, we get two register dumps: > > PC=1ad8 CCS=0 btaken=1 btarget=1ac6 > cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0 > $r0= $r1=4360 $r2=4308 $r3=026c > $r4=2076 $r5=2022 $r6= $r7= > $r8= $r9=0ea2 $r10=0002 $r11=4308 > $r12=1080 $r13=0ec0 $sp=bfd8 $pc=1ad4 > > PC=1ada CCS=800 btaken=1 btarget=1ac6 > cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0 > $r0= $r1=4360 $r2=4308 $r3=026c > $r4=2076 $r5=2022 $r6= $r7= > $r8= $r9=0ea2 $r10=0002 $r11=4308 > $r12=1080 $r13=0ec0 $sp=bfd8 $pc=1ad4 > > With the patch, we get only one: > > PC=1ad8 CCS=0 btaken=1 btarget=1ac6 > cc_op=1 cc_src=3746 cc_dest=1 cc_result=ea2 cc_mask=0 > $r0= $r1=4360 $r2=4308 $r3=026c > $r4=2076 $r5=2022 $r6= $r7= > $r8= $r9=0ea2 $r10=0002 $r11=4308 > $r12=1080 $r13=0ec0 $sp=bfd8 $pc=1ad4 Hi, A concern I have is that we can't guard against all split prefix sequences (e.g at page boundaries or with icount). So it may be more confusing to see the prefix insns sometimes than every time. Perhaps we should more clearly be showing prefix state in the logs? BTW, are you guys doing post-processing on this or is it only for human inspection? Cheers, Edgar > > Signed-off-by: Hans-Peter Nilsson > Signed-off-by: Rabin Vincent > --- > target-cris/translate.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/target-cris/translate.c b/target-cris/translate.c > index a4512b5..c9b1e65 100644 > --- a/target-cris/translate.c > +++ b/target-cris/translate.c > @@ -3262,7 +3262,8 @@ void gen_intermediate_code(CPUCRISState *env, struct > TranslationBlock *tb) > } > } while (!dc->is_jmp && !dc->cpustate_changed > && !tcg_op_buf_full() > -&& !singlestep > +/* We don't count prefix insns as separate wrt. singlestep. */ > +&& (!singlestep || (dc->tb_flags & PFIX_FLAG)) > && (dc->pc < next_page_start) > && num_insns < max_insns); > > -- > 2.1.4 >
Re: [Qemu-devel] [PATCH v6 18/18] Replace qmp-commands.hx by docs/qmp-commands.txt
Hi On Tue, Sep 13, 2016 at 1:54 AM Peter Maydell wrote: > On 12 September 2016 at 10:19, Marc-André Lureau > wrote: > > The only remaining function of qmp-commands.hx is to let us generate > > qmp-commands.txt from it. Replace qmp-commands.hx by qmp-commands.txt. > > > > We intend to move the documentation into the QAPI schema and generate > > qapi-commands.txt from it, but not right now. > > This is a bit sad, though I see that there were not all that > many actual docstrings to autogenerate from. > > What's the plan for reintroducing autogeneration of the > protocol docs? > > I'll rebase my qapi-doc branch, do some cleanups, and submit (last update: https://github.com/elmarco/qemu/commits/qapi-doc). Trouble is the series got pretty large after I splitted all the documentation move. It's over 150 patches iirc, but it helps a lot with reviewing. Should I send it by chunks, or should I send a single patch and points to the github branch for the details? other options? -- Marc-André Lureau
Re: [Qemu-devel] [PATCH 8/9] target-cris: add v17 CPU
On Mon, Sep 05, 2016 at 01:54:11PM +0200, Rabin Vincent wrote: > From: Rabin Vincent > > In the CRIS v17 CPU an ADDC (add with carry) instruction has been added > compared to the v10 instruction set. > > Assembler syntax: > > ADDC [Rs],Rd > ADDC [Rs+],Rd > > Size: Dword > > Description: > > The source data is added together with the carry flag to the > destination register. The size of the operation is dword. > > Operation: > > Rd += s + C-flag; > > Flags affected: > > S R P U I X N Z V C > - - - - - 0 * * * * > > Instruction format: ADDC [Rs],Rd > > +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ > |Destination(Rd)| 1 0 0 1 1 0 1 0 | Source(Rs) | > +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ > > Instruction format: ADDC [Rs+],Rd > > +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ > |Destination(Rd)| 1 1 0 1 1 0 1 0 | Source(Rs) | > +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+ > > Signed-off-by: Rabin Vincent > --- > target-cris/cpu.c| 14 ++ > target-cris/crisv10-decode.h | 1 + > target-cris/translate_v10.c | 8 > 3 files changed, 23 insertions(+) > > diff --git a/target-cris/cpu.c b/target-cris/cpu.c > index c5a656b..d680cfb 100644 > --- a/target-cris/cpu.c > +++ b/target-cris/cpu.c > @@ -246,6 +246,16 @@ static void crisv11_cpu_class_init(ObjectClass *oc, void > *data) > cc->gdb_read_register = crisv10_cpu_gdb_read_register; > } > > +static void crisv17_cpu_class_init(ObjectClass *oc, void *data) > +{ > +CPUClass *cc = CPU_CLASS(oc); > +CRISCPUClass *ccc = CRIS_CPU_CLASS(oc); > + > +ccc->vr = 17; > +cc->do_interrupt = crisv10_cpu_do_interrupt; > +cc->gdb_read_register = crisv10_cpu_gdb_read_register; > +} > + > static void crisv32_cpu_class_init(ObjectClass *oc, void *data) > { > CRISCPUClass *ccc = CRIS_CPU_CLASS(oc); > @@ -273,6 +283,10 @@ static const TypeInfo cris_cpu_model_type_infos[] = { > .parent = TYPE_CRIS_CPU, > .class_init = crisv11_cpu_class_init, > }, { > +.name = TYPE("crisv17"), > +.parent = TYPE_CRIS_CPU, > +.class_init = crisv17_cpu_class_init, > +}, { > .name = TYPE("crisv32"), > .parent = TYPE_CRIS_CPU, > .class_init = crisv32_cpu_class_init, > diff --git a/target-cris/crisv10-decode.h b/target-cris/crisv10-decode.h > index 587fbdd..bdb4b6d 100644 > --- a/target-cris/crisv10-decode.h > +++ b/target-cris/crisv10-decode.h > @@ -92,6 +92,7 @@ > #define CRISV10_IND_JUMP_M 4 > #define CRISV10_IND_DIP 5 > #define CRISV10_IND_JUMP_R 6 > +#define CRISV17_IND_ADDC 6 > #define CRISV10_IND_BOUND7 > #define CRISV10_IND_BCC_M7 > #define CRISV10_IND_MOVE_M_SPR 8 > diff --git a/target-cris/translate_v10.c b/target-cris/translate_v10.c > index a3da425..33d86eb 100644 > --- a/target-cris/translate_v10.c > +++ b/target-cris/translate_v10.c > @@ -1097,6 +1097,14 @@ static unsigned int dec10_ind(CPUCRISState *env, > DisasContext *dc) > insn_len = dec10_bdap_m(env, dc, size); > break; > default: > +if (dc->opcode == CRISV17_IND_ADDC && dc->size == 2 && > +env->pregs[PR_VR] == 17) { Hi Rabin, Could you please add some comments on the insn encoding? Put the stuff from the commit msg in here. IIRC, ADDC and v17 are modifications made to the CRISv10 family of cores that never made it into the public manuals. Or am I wrong? Cheers, Edgar > +LOG_DIS("addc op=%d %d\n", dc->src, dc->dst); > +cris_cc_mask(dc, CC_MASK_NZVC); > +insn_len += dec10_ind_alu(env, dc, CC_OP_ADDC, size); > +break; > +} > + > LOG_DIS("pc=%x var-ind.%d %d r%d r%d\n", >dc->pc, size, dc->opcode, dc->src, dc->dst); > cpu_abort(CPU(dc->cpu), "Unhandled opcode"); > -- > 2.1.4 >
Re: [Qemu-devel] [PATCH v6 18/18] Replace qmp-commands.hx by docs/qmp-commands.txt
On 12 September 2016 at 10:19, Marc-André Lureau wrote: > The only remaining function of qmp-commands.hx is to let us generate > qmp-commands.txt from it. Replace qmp-commands.hx by qmp-commands.txt. > > We intend to move the documentation into the QAPI schema and generate > qapi-commands.txt from it, but not right now. This is a bit sad, though I see that there were not all that many actual docstrings to autogenerate from. What's the plan for reintroducing autogeneration of the protocol docs? thanks -- PMM
Re: [Qemu-devel] [PATCH 2/2] mirror: fix improperly filled copy_bitmap for mirror block job
On 09/09/2016 07:31 AM, Denis V. Lunev wrote: > bdrv_is_allocated_above() returns true in the case even for completel s/completel/completely/ > zeroed areas as BDRV_BLOCK_ALLOCATED flag is set in both cases. > > The patch stops using bdrv_is_allocated_above() wrapper and switches to > bdrv_get_block_status_above() to distinguish zeroed areas and areas with > data to avoid extra IO operations if possible. > > Signed-off-by: Denis V. Lunev > CC: Stefan Hajnoczi > CC: Fam Zheng > CC: Kevin Wolf > CC: Max Reitz > CC: Jeff Cody > --- > block/mirror.c | 18 -- > 1 file changed, 12 insertions(+), 6 deletions(-) > > @@ -597,13 +602,14 @@ static int coroutine_fn > mirror_dirty_init(MirrorBlockJob *s) > return 0; > } > > -ret = bdrv_is_allocated_above(bs, base, sector_num, nb_sectors, &n); > -if (ret < 0) { > -return ret; > +status = bdrv_get_block_status_above(bs, base, sector_num, > + nb_sectors, &n, &file); Eventually, we should probably fix bdrv_get_block_status_above() to be byte-based, but that's not a problem with this patch. Looks okay to me, but I haven't thought closely enough about potential corner cases to feel comfortable with giving R-b yet... -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v6 18/18] Replace qmp-commands.hx by docs/qmp-commands.txt
On 09/12/2016 04:19 AM, Marc-André Lureau wrote: > The only remaining function of qmp-commands.hx is to let us generate > qmp-commands.txt from it. Replace qmp-commands.hx by qmp-commands.txt. > > We intend to move the documentation into the QAPI schema and generate > qapi-commands.txt from it, but not right now. > > Signed-off-by: Marc-André Lureau > --- > Makefile |6 +- > .gitignore |1 - > MAINTAINERS |1 - > docs/qapi-code-gen.txt |6 +- > qmp-commands.hx => docs/qmp-commands.txt | 1127 > -- > docs/writing-qmp-commands.txt| 38 - > 6 files changed, 4 insertions(+), 1175 deletions(-) > rename qmp-commands.hx => docs/qmp-commands.txt (87%) > > +++ b/MAINTAINERS > @@ -1237,7 +1237,6 @@ M: Markus Armbruster > S: Supported > F: qmp.c > F: monitor.c > -F: qmp-commands.hx > F: docs/*qmp-* > F: scripts/qmp/ > T: git git://repo.or.cz/qemu/armbru.git qapi-next Shouldn't we be adding F: qmp-commands.txt somewhere to the file in the interim, at least until the promised later patch lands that once again generates documentation (this time from .json files)? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [Bug 672934] Re: FPU incorrect on Mac OS X
I can confirm that recent QEMU works fine. Sorry, I forgot about this bug and did not update it. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/672934 Title: FPU incorrect on Mac OS X Status in QEMU: Fix Released Bug description: I am using the 0.13.0 release version of QEMU on Mac OS X 10.6.4. I work for a university and the affected guest OS is our own research OS. I believe I found a bug in QEMU's FPU emulation, which only triggers on the Mac. You can reproduce the problem by booting the attached ISO image. Investigating the problem, I found that the lua interpreter in our loader component (called "ned") internally uses doubles to represent all lua-numbers. These doubles are showing completely wrong values on QEMU/Mac, resulting in the lua code not processing properly. I also attached a patch which fixes the problem for me. The attached ZIP-file also contains "before" and "after" screenshots. Note that booting the ISO on a real machine or on a Linux-QEMU always shows the correct "after" behavior. Only QEMU on the Mac exhibits the wrong "before" behavior without my patch. The patch might break other systems setting the CONFIG_BSD flag, so maybe the preprocessor should check for __APPLE__ instead to make the fix Mac-only. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/672934/+subscriptions
[Qemu-devel] [PATCH] ipmi: Add graceful shutdown handling to the external BMC
From: Corey Minyard I misunderstood the workings of the power settings, the power off is a force off operation and there needs to be a separate graceful shutdown operation. So replace the force off operation with a graceful shutdown. Signed-off-by: Corey Minyard --- hw/ipmi/ipmi_bmc_extern.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) This is the final patch to implement proper shutdown for the external interface. OpenIPMI is changed to match this. diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c index 032720a..a141f0a 100644 --- a/hw/ipmi/ipmi_bmc_extern.c +++ b/hw/ipmi/ipmi_bmc_extern.c @@ -54,7 +54,8 @@ #define VM_CAPABILITIES_IRQ 0x04 #define VM_CAPABILITIES_NMI 0x08 #define VM_CAPABILITIES_ATTN 0x10 -#define VM_CMD_FORCEOFF0x09 +#define VM_CAPABILITIES_GRACEFUL_SHUTDOWN 0x20 +#define VM_CMD_GRACEFUL_SHUTDOWN 0x09 #define TYPE_IPMI_BMC_EXTERN "ipmi-bmc-extern" #define IPMI_BMC_EXTERN(obj) OBJECT_CHECK(IPMIBmcExtern, (obj), \ @@ -272,8 +273,8 @@ static void handle_hw_op(IPMIBmcExtern *ibe, unsigned char hw_op) k->do_hw_op(s, IPMI_SEND_NMI, 0); break; -case VM_CMD_FORCEOFF: -qemu_system_shutdown_request(); +case VM_CMD_GRACEFUL_SHUTDOWN: +k->do_hw_op(s, IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 0); break; } } @@ -397,6 +398,10 @@ static void chr_event(void *opaque, int event) if (k->do_hw_op(ibe->parent.intf, IPMI_POWEROFF_CHASSIS, 1) == 0) { v |= VM_CAPABILITIES_POWER; } +if (k->do_hw_op(ibe->parent.intf, IPMI_SHUTDOWN_VIA_ACPI_OVERTEMP, 1) +== 0) { +v |= VM_CAPABILITIES_GRACEFUL_SHUTDOWN; +} if (k->do_hw_op(ibe->parent.intf, IPMI_RESET_CHASSIS, 1) == 0) { v |= VM_CAPABILITIES_RESET; } -- 2.7.4
Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize
Hi, On 12/09/2016 22:05, Alex Williamson wrote: > On Mon, 12 Sep 2016 21:39:22 +0200 > Auger Eric wrote: > >> Hi, >> On 12/09/2016 18:17, Alex Williamson wrote: >>> On Mon, 12 Sep 2016 16:00:18 +0200 >>> Auger Eric wrote: >>> Hi Markus, On 12/09/2016 14:45, Markus Armbruster wrote: > Eric Auger writes: > >> This patch converts VFIO PCI to realize function. >> >> Also original initfn errors now are propagated using QEMU >> error objects. All errors are formatted with the same pattern: >> "vfio: %s: the error description" > > Example: > > $ upstream-qemu -device vfio-pci > upstream-qemu: -device vfio-pci: vfio: :00:00.0: no iommu_group > found: Unknown error -2 > > Two remarks: > > * "Unknown error -2" should be "No such file or directory". See below. > Hum. I noticed that but I didn't have the presence of mind to get it was due to -errno! > > * Five colons, not counting the ones in the PCI address. Do we need the > "vfio: :00:00.0:" part? If yes, could we find a nicer way to > print it? Up to you. Well we have quite a lot of traces with such format, hence my choice. Alex do you want to change this? >>> >>> Well, we need to identify the component with the error, it's not >>> uncommon to have multiple assigned devices. The PCI address is just >>> the basename in vfio code, it might also be the name of a device node >>> in sysfs, such as a uuid of an mdev devices. AFAIK we cannot count on >>> a id and even if we could libvirt assigns them based on order in the >>> xml, making them a bit magic. Maybe I'm not understanding the >>> question. Regarding trace vs error message, I expect that it's going >>> to be a more advanced user/developer enabling tracing, error reports >>> should try a little harder to be comprehensible to an average user. >> On my side I would be inclined to keep the "vfio: BDF" prefix. Maybe the >> PCI domain may be omitted? > > I don't really see the point of making the device name smaller. If a > user happens to have multiple domains, they're going to care about that > component of the address. Is QEMU going to probe the host system to > see if multiple domains are available and update if a new PCI chassis > handled as a separate domain is hot attached? Even an approach like > only printing the domain if it's non-zero devolves into needing logic > to know that basename is a PCI path and not a random sysfs device > path. And then if we only print the domain when non-zero, what about > the bus number or slot number. It's a lot of logic for a problem that > I'm not even convinced is a problem. Thanks, I tend to agree. So I will keep the prefix as is and take into account Markus' other comments. Thanks! Eric > > Alex >
Re: [Qemu-devel] [PATCH v2 9/9] [optional] arm: smmu-v3: ACPI IORT initial support
On Fri, Sep 9, 2016 at 8:54 PM, Auger Eric wrote: > Hi Prem, > > On 22/08/2016 18:17, Prem Mallappa wrote: > > Added ACPI IORT tables, was needed for internal project purpose, but > > posting here for anyone looking for testing ACPI on ARM platforms. > > (P.S: Linux side IORT patches are WIP) > I am also interested in IORT ITS group and currently prototyping > something, hence my few comments/questions. > > > > Signed-off-by: Prem Mallappa > > --- > > hw/arm/virt-acpi-build.c| 43 +++ > > include/hw/acpi/acpi-defs.h | 84 ++ > +++ > > 2 files changed, 127 insertions(+) > > > > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > > index 1fa0581..d5fb69e 100644 > > --- a/hw/arm/virt-acpi-build.c > > +++ b/hw/arm/virt-acpi-build.c > > @@ -382,6 +382,45 @@ build_rsdp(GArray *rsdp_table, BIOSLinker *linker, > unsigned rsdt_tbl_offset) > > return rsdp_table; > > } > > > > +/* > > + * TODO: Simple IORT for now, will add ID mappings as we go > > + * basic idea is to instantiate SMMU from ACPI > > + */ > > +static void > > +build_iort(GArray *table_data, BIOSLinker *linker, VirtGuestInfo > *guest_info) > > +{ > > +int iort_start = table_data->len; > > +AcpiIortTable *iort; > > +AcpiIortNode *iort_node; > > +AcpiIortSmmu3 *smmu; > > +AcpiIortRC *rc; > > +const MemMapEntry *memmap = guest_info->memmap; > > + > > +iort = acpi_data_push(table_data, sizeof(*iort)); > > + > > +iort->length = sizeof(*iort); > Isn't is supposed to be the length of the whole IORT (including the node > cumulated sizes?) > > +iort->node_offset = table_data->len - iort_start; > > +iort->num_nodes++; > > + > > +smmu = acpi_data_push(table_data, sizeof(*smmu)); > > +iort_node = &smmu->iort_node; > > +iort_node->type = 0x04; /* SMMUv3 */ > To match existing code (include/hw/acpi/acpi-defs.h), maybe enum values > can be created (ACPI_IORT_NODE_SMMU_V3). This also matches kernel enum. > > I have made these changes, will send out ASAP. > More generally Shannon advised to use the same field names as the ones > used in the kernel header: acpi_iort_node_type in include/acpi/actbl2.h > Will change this accordingly > > +iort_node->length = sizeof(*smmu); > > +smmu->base_addr = cpu_to_le64(memmap[VIRT_SMMU].base); > > + > > +iort->num_nodes++; > > + > > +rc = acpi_data_push(table_data, sizeof(*rc)); > > +iort_node = &rc->iort_node; > > +iort_node->type = 0x02; /* RC */ > > +iort_node->length = sizeof(*rc); > I think the mem_access_prop field should be set to 1 now the host > controller is assumed to be cache coherent. > > +rc->ats_attr = 1; > no ATS support instead? > > +rc->pci_seg_num = 0; > ID mappings are mandated for me to support MSIs with ITS. > These changes are made as I write, > Shannon told me we should match the kernel datatypes & fields > > for instance in include/acpi/actbl2.h we have: > > struct acpi_iort_id_mapping { > u32 input_base; /* Lowest value in input range */ > u32 id_count; /* Number of IDs */ > u32 output_base;/* Lowest value in output range */ > u32 output_reference; /* A reference to the output node */ > u32 flags; > }; > > This also holds for other struct definitions. > > Sure will change this accordingly. -- Cheers, /Prem
[Qemu-devel] FreeBSD guest and Linux host with EVENT_IDX feature
Hello, I've been having trouble when enabling the EVENT_IDX feature in the virtio-net guest FreeBSD driver. The feature enables fine, but traffic through the virtual interface (perceived from the guest side) stops relatively quickly after starting the KVM instance on the host side. I'm using multi-queue on the virtual NIC as well with vhost-net. If I disable EVENT_IDX then everything works fine, but I need the performance benefit of EVENT_IDX. I've heavily instrumented the guest driver, but I've been unable to find the problem. One thing I've noticed is the value of "vq->vq_ring.used->ring[vq->vq_ring.num].id" in the guest driver always increases. I've started instrumenting the notification code on the host side as well, but I'm afraid I'm heading down a very confusing rabbit hole. I'm hoping someone here may be able to give me some useful information where best to be looking if I'm not already looking in the correct spot :) Thanks, Vince
[Qemu-devel] [Bug 974958] Re: It dumps when following this tutorial on hello world os
The referenced website is no longer available, thus setting this ticket to INVALID. Please feel free to re-open it if the information is available somewhere else and you can reproduce it with the latest version of QEMU. ** Changed in: qemu Status: New => Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/974958 Title: It dumps when following this tutorial on hello world os Status in QEMU: Invalid Bug description: http://mikeos.berlios.de/write-your-own-os.html Following the steps, it works on ubuntu, but on osx, it ALWAYS dumps. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/974958/+subscriptions
Re: [Qemu-devel] [PATCH v2 1/9] log: Add new IOMMU type
On Fri, Sep 9, 2016 at 9:06 PM, Auger Eric wrote: > Hi Prem, > > Missing commit message > > > Signed-off-by: Prem Mallappa > > --- > > include/qemu/log.h | 1 + > > util/log.c | 2 ++ > > 2 files changed, 3 insertions(+) > > > > diff --git a/include/qemu/log.h b/include/qemu/log.h > > index 234fa81..3dd2131 100644 > > --- a/include/qemu/log.h > > +++ b/include/qemu/log.h > > @@ -42,6 +42,7 @@ static inline bool qemu_log_separate(void) > > #define CPU_LOG_TB_NOCHAIN (1 << 13) > > #define CPU_LOG_PAGE (1 << 14) > > #define LOG_TRACE (1 << 15) > > +#define CPU_LOG_IOMMU (1 << 16) > why is it prefixed with CPU_ ? > besides all arm gic devices seem to use LOG_GUEST_ERROR. what is the > rationale behind introducing a new enum? > Will change this to LOG_GUEST_ERROR, if others on the list are okay. -- Cheers, /Prem
Re: [Qemu-devel] [PATCH v2 2/9] devicetree: Added new APIs to make use of more fdt functions
On Fri, Sep 9, 2016 at 9:32 PM, Auger Eric wrote: > Hi Prem, > > > SMMUv3 needs device tree entry like below > To me the commit message should be more explicit and mention appendprop > functionality > > > > interrupt-names = "gerror", "priq", "eventq", "cmdq-sync"; > > > > This patch introduces helper function to add entries like above > > > > Signed-off-by: Prem Mallappa > > --- > > device_tree.c| 35 +++ > > include/sysemu/device_tree.h | 18 ++ > > 2 files changed, 53 insertions(+) > > > > diff --git a/device_tree.c b/device_tree.c > > index 6e06320..5d5966e 100644 > > --- a/device_tree.c > > +++ b/device_tree.c > > @@ -297,6 +297,24 @@ int qemu_fdt_setprop(void *fdt, const char > *node_path, > > return r; > > } > > > > +int qemu_fdt_appendprop(void *fdt, const char *node_path, > > + const char *property, const void *val, int size) > > +{ > > +int r; > > + > > +r = fdt_appendprop(fdt, findnode_nofail(fdt, node_path), property, > > + val, size); > > +if (r < 0) { > > +error_report("%s: Couldn't set %s/%s: %s", __func__, node_path, > > + property, fdt_strerror(r)); > > +exit(1); > > +} > > + > > +return r; > > +} > > + > > + > spare void lines > > + > > int qemu_fdt_setprop_cell(void *fdt, const char *node_path, > >const char *property, uint32_t val) > > { > > @@ -319,6 +337,23 @@ int qemu_fdt_setprop_u64(void *fdt, const char > *node_path, > > return qemu_fdt_setprop(fdt, node_path, property, &val, > sizeof(val)); > > } > > > > +int qemu_fdt_appendprop_string(void *fdt, const char *node_path, > > +const char *property, const char *string) > > +{ > > +int r; > > + > > +r = fdt_appendprop_string(fdt, findnode_nofail(fdt, node_path), > > + property, string); > > +if (r < 0) { > > +error_report("%s: Couldn't set %s/%s = %s: %s", __func__, > > + node_path, property, string, fdt_strerror(r)); > > +exit(1); > > +} > > + > > +return r; > > +} > > + > same > > + > > int qemu_fdt_setprop_string(void *fdt, const char *node_path, > > const char *property, const char *string) > > { > > diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h > > index 705650a..5a0a297 100644 > > --- a/include/sysemu/device_tree.h > > +++ b/include/sysemu/device_tree.h > > @@ -45,12 +45,16 @@ char **qemu_fdt_node_path(void *fdt, const char > *name, char *compat, > > > > int qemu_fdt_setprop(void *fdt, const char *node_path, > > const char *property, const void *val, int size); > > +int qemu_fdt_appendprop(void *fdt, const char *node_path, > > + const char *property, const void *val, int size); > > int qemu_fdt_setprop_cell(void *fdt, const char *node_path, > >const char *property, uint32_t val); > > int qemu_fdt_setprop_u64(void *fdt, const char *node_path, > > const char *property, uint64_t val); > > int qemu_fdt_setprop_string(void *fdt, const char *node_path, > > const char *property, const char *string); > > +int qemu_fdt_appendprop_string(void *fdt, const char *node_path, > > + const char *property, const char > *string); > > int qemu_fdt_setprop_phandle(void *fdt, const char *node_path, > > const char *property, > > const char *target_node_path); > > @@ -98,6 +102,20 @@ int qemu_fdt_add_subnode(void *fdt, const char > *name); > > sizeof(qdt_tmp)); > \ > > } while (0) > > > > + > same > > +#define qemu_fdt_appendprop_cells(fdt, node_path, property, ...) > \ > > +do { > \ > > +uint32_t qdt_tmp[] = { __VA_ARGS__ }; >\ > > +int i; > \ > > + > \ > > +for (i = 0; i < ARRAY_SIZE(qdt_tmp); i++) { >\ > > +qdt_tmp[i] = cpu_to_be32(qdt_tmp[i]); >\ > > +} >\ > > +qemu_fdt_appendprop(fdt, node_path, property, qdt_tmp, > \ > > + sizeof(qdt_tmp)); > \ > > +} while (0) > > + > > + > same here. > > Will take care of the extra blank lines. > While I understand the benefits I think we could manage without this new > API by using qemu_fdt_setprop and populating the uint32_t array separately. > > I'll give this a try and see how it goes. > I understood the QEMU API for manipulating flattened trees was not > supposed to grow too much but I don't have a strong option here. You > should CC David Gibson I think. > > Will sure do in the next drop. -- Cheers, /Prem
Re: [Qemu-devel] [PATCH v2 5/9] hw: arm: Add SMMUv3 to virt platform, create DTS accordingly
On Fri, Sep 9, 2016 at 10:01 PM, Auger Eric wrote: > Hi Prem, > > > Default virt platform now creates SMMU device. > > Default config to build SMMU device along is in previous patches. > > > > Signed-off-by: Prem Mallappa > > --- > > hw/arm/virt.c | 62 ++ > + > > include/hw/arm/smmu.h | 33 +++ > > include/hw/arm/virt.h | 2 ++ > > 3 files changed, 97 insertions(+) > > create mode 100644 include/hw/arm/smmu.h > > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > index c5c125e..f3c7891 100644 > > --- a/hw/arm/virt.c > > +++ b/hw/arm/virt.c > > @@ -57,6 +57,7 @@ > > #include "hw/smbios/smbios.h" > > #include "qapi/visitor.h" > > #include "standard-headers/linux/input.h" > > +#include "hw/arm/smmu.h" > > > > /* Number of external interrupt lines to configure the GIC with */ > > #define NUM_IRQS 256 > > @@ -77,6 +78,7 @@ typedef struct VirtBoardInfo { > > uint32_t gic_phandle; > > uint32_t v2m_phandle; > > bool using_psci; > > +uint32_t smmu_phandle; > nit: would group the phandles together > > } VirtBoardInfo; > > > > typedef struct { > > @@ -175,6 +177,7 @@ static const MemMapEntry a15memmap[] = { > > [VIRT_FW_CFG] = { 0x0902, 0x0018 }, > > [VIRT_GPIO] = { 0x0903, 0x1000 }, > > [VIRT_SECURE_UART] ={ 0x0904, 0x1000 }, > > +[VIRT_SMMU] = { 0x0905, 0x0002 }, /* 128K, > needed */ > > [VIRT_MMIO] = { 0x0a00, 0x0200 }, > > /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that > size */ > > [VIRT_PLATFORM_BUS] = { 0x0c00, 0x0200 }, > > @@ -195,9 +198,19 @@ static const int a15irqmap[] = { > > [VIRT_SECURE_UART] = 8, > > [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */ > > [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */ > Currently don' we have NUM_GICV2M_SPIS = 64? > > +[VIRT_SMMU] = 74,/* ...to 74 + NUM_SMMU_IRQS - 1 */ > > [VIRT_PLATFORM_BUS] = 112, /* ...to 112 + PLATFORM_BUS_NUM_IRQS -1 > */ > > }; > > > > +static const struct smmuirq { > > +const char *name; > > +} smmuirqmap[NUM_SMMU_IRQS] = { > > +[SMMU_IRQ_EVTQ] = {"eventq"}, > > +[SMMU_IRQ_PRIQ] = {"priq"}, > > +[SMMU_IRQ_CMD_SYNC] = {"cmdq-sync"}, > > +[SMMU_IRQ_GERROR] = {"gerror"}, > > +}; > > + > > static VirtBoardInfo machines[] = { > > { > > .cpu_model = "cortex-a15", > > @@ -938,6 +951,50 @@ static void create_pcie_irq_map(const VirtBoardInfo > *vbi, uint32_t gic_phandle, > > 0x7 /* PCI irq */); > > } > > > > +static void alloc_smmu_phandle(VirtBoardInfo *vbi) > > +{ > I would rather put that code in create_smmu as it is done in create_gic > for instance > > +if (!vbi->smmu_phandle) > > +vbi->smmu_phandle = qemu_fdt_alloc_phandle(vbi->fdt); > > +} > > + > > +static void create_smmu(VirtBoardInfo *vbi, qemu_irq *pic) > > +{ > > +int i; > > +char *smmu; > > +const char compat[] = "arm,smmu-v3"; > > +int irq = vbi->irqmap[VIRT_SMMU]; > > +hwaddr base = vbi->memmap[VIRT_SMMU].base; > > +hwaddr size = vbi->memmap[VIRT_SMMU].size; > > +int type = GIC_FDT_IRQ_TYPE_SPI; > > + > > +sysbus_create_varargs("smmuv3", base, > > + pic[irq], > > + pic[irq + 1], > > + pic[irq + 2], > > + pic[irq + 3], > > + NULL); > > + > > +smmu = g_strdup_printf("/smmuv3@%" PRIx64, base); > > +qemu_fdt_add_subnode(vbi->fdt, smmu); > > +qemu_fdt_setprop(vbi->fdt, smmu, "compatible", compat, > sizeof(compat)); > > +qemu_fdt_setprop_sized_cells(vbi->fdt, smmu, "reg", 2, base, 2, > size); > > + > > +for (i = 0; i < NUM_SMMU_IRQS; i++) { > > +qemu_fdt_appendprop_cells(vbi->fdt, smmu, "interrupts", > > + type, irq + i, > > + GIC_FDT_IRQ_FLAGS_LEVEL_HI); > > +qemu_fdt_appendprop_string(vbi->fdt, smmu, "interrupt-names", > > + smmuirqmap[i].name); > so as I mentionned before I think we can manage with appendprop helpers > but let's see ... > > +} > > + > > +qemu_fdt_setprop_cell(vbi->fdt, smmu, "clocks", > vbi->clock_phandle); > > +qemu_fdt_setprop_cell(vbi->fdt, smmu, "#iommu-cells", 0); > > +qemu_fdt_setprop_string(vbi->fdt, smmu, "clock-names", "apb_pclk"); > > + > > +qemu_fdt_setprop_cell(vbi->fdt, smmu, "phandle", > vbi->smmu_phandle); > > +g_free(smmu); > > +} > > + > > static void create_pcie(const VirtBoardInfo *vbi, qemu_irq *pic, > > bool use_highmem) > > { > > @@ -1048,6 +1105,7 @@ static void create_pcie(const VirtBoardInfo *vbi, > qemu_irq *pic, > > } > > > Hi Eric, > > qemu_fdt_setprop_cell(vbi->fdt, nodename, "#inter
Re: [Qemu-devel] [PATCH v3 06/34] tcg: Add EXCP_ATOMIC
On 09/12/2016 07:16 AM, Alex Bennée wrote: +void cpu_exec_step(CPUState *cpu) +{ +CPUArchState *env = (CPUArchState *)cpu->env_ptr; +TranslationBlock *tb; +target_ulong cs_base, pc; +uint32_t flags; +bool old_tb_flushed; + +old_tb_flushed = cpu->tb_flushed; +cpu->tb_flushed = false; Advanced warning, these disappear soon when the async safe work (plus safe tb flush) patches get merged. Fair enough. Having thought about this more, I think it may be better to handle this without flushing the tb. To have parallel_cpus included in the TB flags or somesuch and keep that TB around. My thinking is that there are certain things, like alignment, that could result in repeated single-stepping. So better to keep the TB around than keep having to regenerate it. +/* ??? When we begin running cpus in parallel, we should + stop all cpus, clear parallel_cpus, and interpret a + single insn with cpu_exec_step. In the meantime, + we should never get here. */ +abort(); Possibly more correct would be: g_assert(parallel_cpus == false); abort(); No, since it is here that we would *set* parallel_cpus to false. Or did you mean assert parallel_cpus true? Not that that helps for the moment... +static void step_atomic(CPUState *cpu) +{ +start_exclusive(); + +/* Since we got here, we know that parallel_cpus must be true. */ +parallel_cpus = false; +cpu_exec_step(cpu); +parallel_cpus = true; + +end_exclusive(); +} + Paolo's safe work patches bring the start/end_exclusive functions into cpu-exec-common so I think after that has been merged this function can also be moved and called directly from the MTTCG loop on an EXCP_ATOMIC exit. Excellent. Perhaps I should rebase this upon that right away. Have you got a pointer to a tree handy? +bool parallel_cpus; So lets add some words to this to distinguish between this and the mttcg enabled flags and its relation to -smp. Something like: parallel_cpus indicates to the front-ends if code needs to be generated taking into account multiple threads of execution. It will be true for linux-user after the first thread clone and if system mode if MTTCG is enabled. On the transition from false->true any code generated while false needs to be invalidated. It may be temporally set to false when generating non-cached code in an exclusive context. Sure. r~
[Qemu-devel] [Bug 674740] Re: qemu segfaults when security_model=none using virtio-9p-pci driver
Current QEMU 2.7 does not segfault here anymore, and the "thesecurity" problem is also not available in the sources anymore ==> I think this can be closed nowadays. ** Changed in: qemu Status: New => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/674740 Title: qemu segfaults when security_model=none using virtio-9p-pci driver Status in QEMU: Fix Released Bug description: qemu version: 0.13 commit-id: 6ed912999d6ef636a5be5ccb266d7d3c0f0310b4 example invocation: $ qemu -virtfs local,path=/tmp,security_model=none,mount_tag=mmm r.img one of the following must be specified as thesecurity option: security_model=passthrough security_model=mapped Segmentation fault Patch is attached. Also attached is a patch that addes the space between 'the' and 'security' in 'thesecurity'. Does not affect trunk. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/674740/+subscriptions
Re: [Qemu-devel] [PATCH 1/3] vfio/pci: conversion to realize
On Mon, 12 Sep 2016 21:39:22 +0200 Auger Eric wrote: > Hi, > On 12/09/2016 18:17, Alex Williamson wrote: > > On Mon, 12 Sep 2016 16:00:18 +0200 > > Auger Eric wrote: > > > >> Hi Markus, > >> > >> On 12/09/2016 14:45, Markus Armbruster wrote: > >>> Eric Auger writes: > >>> > This patch converts VFIO PCI to realize function. > > Also original initfn errors now are propagated using QEMU > error objects. All errors are formatted with the same pattern: > "vfio: %s: the error description" > >>> > >>> Example: > >>> > >>> $ upstream-qemu -device vfio-pci > >>> upstream-qemu: -device vfio-pci: vfio: :00:00.0: no iommu_group > >>> found: Unknown error -2 > >>> > >>> Two remarks: > >>> > >>> * "Unknown error -2" should be "No such file or directory". See below. > >>> > >> Hum. I noticed that but I didn't have the presence of mind to get it was > >> due to -errno! > >>> > >>> * Five colons, not counting the ones in the PCI address. Do we need the > >>> "vfio: :00:00.0:" part? If yes, could we find a nicer way to > >>> print it? Up to you. > >> Well we have quite a lot of traces with such format, hence my choice. > >> Alex do you want to change this? > > > > Well, we need to identify the component with the error, it's not > > uncommon to have multiple assigned devices. The PCI address is just > > the basename in vfio code, it might also be the name of a device node > > in sysfs, such as a uuid of an mdev devices. AFAIK we cannot count on > > a id and even if we could libvirt assigns them based on order in the > > xml, making them a bit magic. Maybe I'm not understanding the > > question. Regarding trace vs error message, I expect that it's going > > to be a more advanced user/developer enabling tracing, error reports > > should try a little harder to be comprehensible to an average user. > On my side I would be inclined to keep the "vfio: BDF" prefix. Maybe the > PCI domain may be omitted? I don't really see the point of making the device name smaller. If a user happens to have multiple domains, they're going to care about that component of the address. Is QEMU going to probe the host system to see if multiple domains are available and update if a new PCI chassis handled as a separate domain is hot attached? Even an approach like only printing the domain if it's non-zero devolves into needing logic to know that basename is a PCI path and not a random sysfs device path. And then if we only print the domain when non-zero, what about the bus number or slot number. It's a lot of logic for a problem that I'm not even convinced is a problem. Thanks, Alex
Re: [Qemu-devel] [PATCH v6 01/18] build-sys: define QEMU_VERSION_{MAJOR, MINOR, MICRO}
On 09/12/2016 04:18 AM, Marc-André Lureau wrote: > There are better chances to find what went wrong at build time than a > later assert in qmp_query_version > > Signed-off-by: Marc-André Lureau > --- > qmp.c | 16 +++- > scripts/create_config | 6 ++ > 2 files changed, 9 insertions(+), 13 deletions(-) > > - > -err = qemu_strtoll(tmp, &tmp, 10, &info->qemu->micro); > -assert(err == 0); > +info->qemu->major = QEMU_VERSION_MAJOR; > +info->qemu->minor = QEMU_VERSION_MINOR; > +info->qemu->micro = QEMU_VERSION_MICRO; > info->package = g_strdup(QEMU_PKGVERSION); > > return info; The old code silently ignores any garbage after the third integer (it populates &tmp, but never checks the value of tmp). > diff --git a/scripts/create_config b/scripts/create_config > index 1dd6a35..e6929dd 100755 > --- a/scripts/create_config > +++ b/scripts/create_config > @@ -7,7 +7,13 @@ while read line; do > case $line in > VERSION=*) # configuration > version=${line#*=} > +major=$(echo "$version" | cut -d. -f1) > +minor=$(echo "$version" | cut -d. -f2) > +micro=$(echo "$version" | cut -d. -f3) > echo "#define QEMU_VERSION \"$version\"" > +echo "#define QEMU_VERSION_MAJOR $major" > +echo "#define QEMU_VERSION_MINOR $minor" > +echo "#define QEMU_VERSION_MICRO $micro" The new code likewise ignores any fourth field. Do we care either way? Unless someone else has a reason for why we should care, I'm fine with: Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] virtio-bus: Plug devices after features are negotiated
On Mon, Sep 12, 2016 at 08:22:50PM +0200, Maxime Coquelin wrote: > > > On 09/12/2016 10:51 AM, Cornelia Huck wrote: > > On Sat, 10 Sep 2016 10:23:37 +0200 > > Maxime Coquelin wrote: > > > > > > Currently, devices are plugged before features are negotiated. > > > > If the backend doesn't support VIRTIO_F_VERSION_1, the transport > > > > need to rewind some settings. > > > > > > > > This is the case for CCW, for which a post_plugged callback had > > > > been introduced, where max_rev field is just updated if > > > > VIRTIO_F_VERSION_1 is not supported by the backend. > > > > For PCI, implementing the post_plugged would be much more > > s/the// > > > > > > complicated, so it needs to know whether the backend supports > > > > VIRTIO_F_VERSION_1 at plug time. > > > > > > > > Currently, nothing is done for PCI. Modern capabilitities get > > > > exposed to the guest even if VIRTIO_F_VERSION_1 is not supported > > > > by the backend, which confuses the guest. > > > > > > > > This patch proposes to replace existing post_plugged solution > > Nit: The patch does not propose anything, it just does it :) > > > Michael, > > Should I send a v2 fixing the above comments, or you can handle them > when applying the patch? > > Thanks, > Maxime It's easier if you post v2 including all acks. Note - after --- that no code was changed. -- MST
[Qemu-devel] [PATCH v5 3/5] linux-user: Fix structure target_flock definition for Mips
From: Aleksandar Markovic Structure flock is defined for Mips in a way different from any other platform. For reference, see Linux kernel source code files: arch/mips/include/uapi/asm/fcntl.h#L63 (for Mips) include/uapi/asm-generic/fcntl.h#L195 (for all other platforms) This patch fix this problem, by amending structure target_flock, for Mips only. Besides, this patch fixes LTP tests fcntl11, fcntl17, fcntl19, fcntl20, and fcntl21, which are currently failing, if executed in Qemu user mode for Mips platforms. Signed-off-by: Aleksandar Markovic --- linux-user/syscall_defs.h | 6 ++ 1 file changed, 6 insertions(+) diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index 44b1197..c40b725 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -2327,7 +2327,13 @@ struct target_flock { short l_whence; abi_long l_start; abi_long l_len; +#if defined(TARGET_MIPS) +target_long l_sysid; +#endif int l_pid; +#if defined(TARGET_MIPS) +target_long pad[4]; +#endif }; struct target_flock64 { -- 2.9.3
[Qemu-devel] [PATCH v3 10/10] linux-user: Remove a duplicate item from strace.list
From: Aleksandar Markovic There is a duplicate item in strace.list. It is benign, but it shouldn't be there. It is the only duplicate in strace.list. This patch removes it. Signed-off-by: Aleksandar Markovic --- linux-user/strace.list | 3 --- 1 file changed, 3 deletions(-) diff --git a/linux-user/strace.list b/linux-user/strace.list index fadeb29..27d7006 100644 --- a/linux-user/strace.list +++ b/linux-user/strace.list @@ -1529,9 +1529,6 @@ #ifdef TARGET_NR_utimensat { TARGET_NR_utimensat, "utimensat", NULL, print_utimensat, NULL }, #endif -#ifdef TARGET_NR_sync_file_range -{ TARGET_NR_sync_file_range, "sync_file_range", NULL, NULL, NULL }, -#endif #ifdef TARGET_NR_sync_file_range2 { TARGET_NR_sync_file_range2, "sync_file_range2", NULL, NULL, NULL }, #endif -- 2.9.3
Re: [Qemu-devel] [PATCH 5/5] QMP: fixup typos and whitespace damage
On 09/12/2016 06:00 AM, Cornelia Huck wrote: > From: Christian Borntraeger > > Fixup some typos and whitespace damage introduced by the CPU model > patches for s390. > > Reported-by: Eric Blake > Signed-off-by: Christian Borntraeger > Signed-off-by: Cornelia Huck > --- > qapi-schema.json | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v3 03/10] linux-user: Add support for ustat() syscall
From: Aleksandar Markovic This patch implements Qemu user mode ustat() syscall support. Syscall ustat() returns information about a mounted filesystem. The implementation is similar to the implementations of statfs(), fstatfs() and other related syscalls. It is based on invocation of host's ustat(), and its key part is in the correspondent case segment of the main switch statement of the function do_syscall(), in file linux-user/syscalls.c. All necessary conversions of data structures from target to host and from host to target are covered. Sufficient support for "-strace" option for this syscall is already present, and this patch does not change it. This patch also fixes failures of LTP tests ustat01, and ustat02, if executed on Qemu-emulated systems. Signed-off-by: Aleksandar Markovic --- linux-user/syscall.c | 25 - 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 38d69f1..b118bd9 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -48,6 +48,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base, #include #include #include +#include #include #include #include @@ -8098,7 +8099,29 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, break; #ifdef TARGET_NR_ustat case TARGET_NR_ustat: -goto unimplemented; +{ +struct ustat ust; +int cnt; +ret = get_errno(ustat(arg1, &ust)); + +if (!is_error(ret)) { +struct ustat *target_ust; + +if (!lock_user_struct(VERIFY_WRITE, target_ust, arg2, 0)) { +goto efault; +} + +__put_user(ust.f_tfree, &target_ust->f_tfree); +__put_user(ust.f_tinode, &target_ust->f_tinode); + +for (cnt = 0; cnt < 6; cnt++) { +__put_user(ust.f_fname[cnt], &target_ust->f_fname[cnt]); +__put_user(ust.f_fpack[cnt], &target_ust->f_fpack[cnt]); +} +unlock_user_struct(target_ust, arg2, 1); +} +break; + } #endif #ifdef TARGET_NR_dup2 case TARGET_NR_dup2: -- 2.9.3
[Qemu-devel] [Bug 672934] Re: FPU incorrect on Mac OS X
Looks like the ISO from comment #4 (thanks for attaching that one!) shows the correct behavior with up to date QEMU 2.7. Also, the affected softfloat code has been completely reworked in between (e.g. with commit cf67c6bad56d43e6d60), so I assume this has been fixed sometimes in the past years. ** Changed in: qemu Status: New => Fix Released -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/672934 Title: FPU incorrect on Mac OS X Status in QEMU: Fix Released Bug description: I am using the 0.13.0 release version of QEMU on Mac OS X 10.6.4. I work for a university and the affected guest OS is our own research OS. I believe I found a bug in QEMU's FPU emulation, which only triggers on the Mac. You can reproduce the problem by booting the attached ISO image. Investigating the problem, I found that the lua interpreter in our loader component (called "ned") internally uses doubles to represent all lua-numbers. These doubles are showing completely wrong values on QEMU/Mac, resulting in the lua code not processing properly. I also attached a patch which fixes the problem for me. The attached ZIP-file also contains "before" and "after" screenshots. Note that booting the ISO on a real machine or on a Linux-QEMU always shows the correct "after" behavior. Only QEMU on the Mac exhibits the wrong "before" behavior without my patch. The patch might break other systems setting the CONFIG_BSD flag, so maybe the preprocessor should check for __APPLE__ instead to make the fix Mac-only. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/672934/+subscriptions
[Qemu-devel] [PATCH v5 4/5] linux-user: Fix structure target_semid64_ds definition for Mips
From: Aleksandar Markovic This patch corrects target_semid64_ds structure definition for Mips. See, for example definition of semid64_ds for Mips in Linux kernel: arch/mips/include/uapi/asm/sembuf.h#L13. This patch will also fix certain semaphore-related LTP tests for Mips, if they are executed in Qemu user mode for any Mips platform. Signed-off-by: Miodrag Dinic Signed-off-by: Aleksandar Markovic Reviewed-by: Peter Maydell --- linux-user/mips/target_structs.h | 16 1 file changed, 16 insertions(+) diff --git a/linux-user/mips/target_structs.h b/linux-user/mips/target_structs.h index fbd9955..5ba124d 100644 --- a/linux-user/mips/target_structs.h +++ b/linux-user/mips/target_structs.h @@ -45,4 +45,20 @@ struct target_shmid_ds { abi_ulong __unused2; }; +#define TARGET_SEMID64_DS + +/* + * The semid64_ds structure for the MIPS architecture. + * Note extra padding because this structure is passed back and forth + * between kernel and user space. + */ +struct target_semid64_ds { +struct target_ipc_perm sem_perm; +abi_ulong sem_otime; +abi_ulong sem_ctime; +abi_ulong sem_nsems; +abi_ulong __unused3; +abi_ulong __unused4; +}; + #endif -- 2.9.3
[Qemu-devel] [PATCH v3 07/10] linux-user: Remove tabs and trailing spaces from linux-user/main.c
From: Aleksandar Markovic File main.c is frequently a starting point of debugging or analysing Qemu code for novice devevelopers, and it would be nice if it had format as clean as posible. This patch starts improving its format by removing tabs and trailing spaces. This patch is obtained in this way: 1. All tabs replaced with spaces, using a tool. 2. All trailing spaces removed, using a tool. 3. If step 1 resulted in visually untidy code segments, that was manually corrected by inserting or removing spaces only. Signed-off-by: Aleksandar Markovic --- linux-user/main.c | 812 +++--- 1 file changed, 406 insertions(+), 406 deletions(-) diff --git a/linux-user/main.c b/linux-user/main.c index 6004ece..9d26b3e 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -2043,349 +2043,349 @@ void cpu_loop(CPUPPCState *env) # ifdef TARGET_ABI_MIPSO32 # define MIPS_SYS(name, args) args, static const uint8_t mips_syscall_args[] = { - MIPS_SYS(sys_syscall, 8)/* 4000 */ - MIPS_SYS(sys_exit , 1) - MIPS_SYS(sys_fork , 0) - MIPS_SYS(sys_read , 3) - MIPS_SYS(sys_write , 3) - MIPS_SYS(sys_open , 3)/* 4005 */ - MIPS_SYS(sys_close , 1) - MIPS_SYS(sys_waitpid, 3) - MIPS_SYS(sys_creat , 2) - MIPS_SYS(sys_link , 2) - MIPS_SYS(sys_unlink , 1)/* 4010 */ - MIPS_SYS(sys_execve , 0) - MIPS_SYS(sys_chdir , 1) - MIPS_SYS(sys_time , 1) - MIPS_SYS(sys_mknod , 3) - MIPS_SYS(sys_chmod , 2)/* 4015 */ - MIPS_SYS(sys_lchown , 3) - MIPS_SYS(sys_ni_syscall , 0) - MIPS_SYS(sys_ni_syscall , 0)/* was sys_stat */ - MIPS_SYS(sys_lseek , 3) - MIPS_SYS(sys_getpid , 0)/* 4020 */ - MIPS_SYS(sys_mount , 5) - MIPS_SYS(sys_umount , 1) - MIPS_SYS(sys_setuid , 1) - MIPS_SYS(sys_getuid , 0) - MIPS_SYS(sys_stime , 1)/* 4025 */ - MIPS_SYS(sys_ptrace , 4) - MIPS_SYS(sys_alarm , 1) - MIPS_SYS(sys_ni_syscall , 0)/* was sys_fstat */ - MIPS_SYS(sys_pause , 0) - MIPS_SYS(sys_utime , 2)/* 4030 */ - MIPS_SYS(sys_ni_syscall , 0) - MIPS_SYS(sys_ni_syscall , 0) - MIPS_SYS(sys_access , 2) - MIPS_SYS(sys_nice , 1) - MIPS_SYS(sys_ni_syscall , 0)/* 4035 */ - MIPS_SYS(sys_sync , 0) - MIPS_SYS(sys_kill , 2) - MIPS_SYS(sys_rename , 2) - MIPS_SYS(sys_mkdir , 2) - MIPS_SYS(sys_rmdir , 1)/* 4040 */ - MIPS_SYS(sys_dup, 1) - MIPS_SYS(sys_pipe , 0) - MIPS_SYS(sys_times , 1) - MIPS_SYS(sys_ni_syscall , 0) - MIPS_SYS(sys_brk, 1)/* 4045 */ - MIPS_SYS(sys_setgid , 1) - MIPS_SYS(sys_getgid , 0) - MIPS_SYS(sys_ni_syscall , 0)/* was signal(2) */ - MIPS_SYS(sys_geteuid, 0) - MIPS_SYS(sys_getegid, 0)/* 4050 */ - MIPS_SYS(sys_acct , 0) - MIPS_SYS(sys_umount2, 2) - MIPS_SYS(sys_ni_syscall , 0) - MIPS_SYS(sys_ioctl , 3) - MIPS_SYS(sys_fcntl , 3)/* 4055 */ - MIPS_SYS(sys_ni_syscall , 2) - MIPS_SYS(sys_setpgid, 2) - MIPS_SYS(sys_ni_syscall , 0) - MIPS_SYS(sys_olduname , 1) - MIPS_SYS(sys_umask , 1)/* 4060 */ - MIPS_SYS(sys_chroot , 1) - MIPS_SYS(sys_ustat , 2) - MIPS_SYS(sys_dup2 , 2) - MIPS_SYS(sys_getppid, 0) - MIPS_SYS(sys_getpgrp, 0)/* 4065 */ - MIPS_SYS(sys_setsid , 0) - MIPS_SYS(sys_sigaction , 3) - MIPS_SYS(sys_sgetmask , 0) - MIPS_SYS(sys_ssetmask , 1) - MIPS_SYS(sys_setreuid , 2)/* 4070 */ - MIPS_SYS(sys_setregid , 2) - MIPS_SYS(sys_sigsuspend , 0) - MIPS_SYS(sys_sigpending , 1) - MIPS_SYS(sys_sethostname, 2) - MIPS_SYS(sys_setrlimit , 2)/* 4075 */ - MIPS_SYS(sys_getrlimit , 2) - MIPS_SYS(sys_getrusage , 2) - MIPS_SYS(sys_gettimeofday, 2) - MIPS_SYS(sys_settimeofday, 2) - MIPS_SYS(sys_getgroups , 2)/* 4080 */ - MIPS_SYS(sys_setgroups , 2) - MIPS_SYS(sys_ni_syscall , 0)/* old_select */ - MIPS_SYS(sys_symlink, 2) - MIPS_SYS(sys_ni_syscall , 0)/* was sys_lstat */ - MIPS_SYS(sys_readlink , 3)/* 4085 */ - MIPS_SYS(sys_uselib , 1) - MIPS_SYS(sys_swapon , 2) - MIPS_SYS(sys_reboot , 3) - MIPS_SYS(old_readdir, 3) - MIPS_SYS(old_mmap , 6)/* 4090 */ - MIPS_SYS(sys_munmap , 2) - MIPS_SYS(sys_truncate , 2) - MIPS_SYS(sys_ftruncate , 2) - MIPS_SYS(sys_fchmod , 2) - MIPS_SYS(sys_fchown , 3)/* 4095 */ - MIPS_SYS(sys_ge
Re: [Qemu-devel] [PATCH v4 0/4] Introduce error_report_{fatal|abort}
On 09/09/2016 12:05 PM, Markus Armbruster wrote: > > You effectively propose to revise this coding rule from error.h: > > * Please don't error_setg(&error_fatal, ...), use error_report() and > * exit(), because that's more obvious. > * Likewise, don't error_setg(&error_abort, ...), use assert(). > > If we accept your proposal, you get to add a patch to update the rule :) > > We've discussed the preferred way to report fatal errors to the human > user before. With actual patches, we can see how a change of rules > changes the code. Do we like the change shown by this patch set? That includes diffstats, to help gauge the extent of the change (not as easy is gauging the ratio of the changed code to the rest of the code that did not need to change - if we are touching 40% of all callers, it is invasive because the remaining 60% is not that much more dominant; if we are touching 2% of all callers it is a great patch for keeping us consistent with the remaining 98%). error_report_fatal() had a lot of hits: 76 files changed, 557 insertions(+), 799 deletions(-) create mode 100644 scripts/coccinelle/error_report_fatal.cocci while error_report_abort() was not as common: 12 files changed, 41 insertions(+), 32 deletions(-) create mode 100644 scripts/coccinelle/error_report_abort.cocci > > I believe there are a number of separate issues to discuss here: > > * Shall we get rid of error_setg(&error_fatal, ...)? > > This is a no-brainer for me. Such a simple thing should be done in > one way, not two ways. I count 14 instances of > error_setg(&error_fatal, ...), but more than 300 of error_report(...); > exit(1). So this adds some of the percentages that I allude to above: 14/300 is definitely a case where the outliers are worth making common (so getting rid of error_setg(&error_fatal) makes sense). Now, whether we get rid of the differences by making it all error_setg()/exit() (to match the 300), or whether we change ALL of these to a new error_report_fatal (for 314 changes) is up for a bit more debate: > > * Shall we fuse error_report() and exit() into error_report_fatal()? > > Saves ~200 lines, not counting the Coccinelle semantic patch. > > I think the real question is what's easier to read and to write. Do > you prefer something like > > error_report("ISA bus not available for %s", c->name); > exit(1); > > or something like > > error_report_fatal("ISA bus not available for %s", >c->name); > > The second form saves a tiny bit of instruction space, I guess. I can live with error_report_fatal(). It makes indentation a bit more awkward to think about, and hides the fact that it is exit()ing, but if done commonly enough (and 314 instances in the code base seems relatively common) along with a Coccinelle script to enforce it, it seems like it would be a usable idiom. > > * Shall we get rid of error_setg(&error_abort, ...)? > > Getting rid of it is again a no-brainer, but what to replace it with > isn't. > > In my personal opinion, abort() is a perfectly fine way to handle > "this cannot happen" conditions, and printing pretty messages right > before abort() is a waste of time. If the abort() happens, the > program is broken, and all the end user needs to know is that he needs > to find someone to debug and fix it. If the end user really needs to > know more, use of abort() is usually wrong. > > But others have different opinions. If you want to print pretty > messages before abort(), you get to print them. > > The question is whether to provide a fused error_report_abort(). I'd > be willing to provide it just for symmetry with error_report_fatal(), > if we decide we want error_report_fatal(). I'm leaning towards error_report_abort() as useless, agreeing with the argument that reporting a nice message before abort()ing is a waste of time (the ideal program never aborts, so the nice message is either dead code, or the error is reachable in situations such that you should not have been trying to abort). But if we don't convert error_report_abort(), then having JUST error_report_fatal() as shorthand on its own merits becomes a bit tougher sell. I don't know that I have swayed any opinions, so much as just added commentary to the discussion. I guess we could easily split this into a trivial patch (get rid of the 14 error_setg(&error_fatal) via Coccinelle to error_report()/exit()) as a patch worth applying now, and a second Coccinelle conversion of error_report()/exit() to error_report_fatal() as a more ambiguous change of whether we like it or not. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v5 2/5] linux-user: Fix TARGET_F_GETOWN definition for Mips
From: Aleksandar Markovic For some reason, Qemu's TARGET_F_GETOWN constant for Mips does not match the correct value of correspondant F_GETOWN. This patch fixes this problem. For reference, see Mips' F_GETOWN definition in Linux kernel at arch/mips/include/uapi/asm/fcntl.h#L44. This patch also fixes some fcntl()-related LTP tests for Qemu user mode for Mips. Signed-off-by: Miodrag Dinic Signed-off-by: Aleksandar Markovic --- linux-user/syscall_defs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index cf89f16..44b1197 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -2158,7 +2158,7 @@ struct target_statfs64 { #define TARGET_F_SETLK 6 #define TARGET_F_SETLKW7 #define TARGET_F_SETOWN24 /* for sockets. */ -#define TARGET_F_GETOWN25 /* for sockets. */ +#define TARGET_F_GETOWN23 /* for sockets. */ #else #define TARGET_F_GETLK 5 #define TARGET_F_SETLK 6 -- 2.9.3
[Qemu-devel] [PATCH v3 02/10] linux-user: Add support for sysfs() syscall
From: Aleksandar Markovic This patch implements Qemu user mode sysfs() syscall support. Syscall sysfs() involves returning information about the filesystem types currently present in the kernel, and can operate in three distinct flavors, depending on its first argument. The implementation is based on invocation of host's sysfs(), and its key part is in the correspondent case segment of the main switch statement of the function do_syscall(), in file linux-user/syscalls.c. All necessary conversions of data structures from target to host and from host to target are covered. Based on the value of the first argument, three cases are distinguished, and such conversions are implemented separately for each case. Relevant support for "-strace" option is included in files linux-user/strace.c and linux-user/strace.list. This patch also fixes failures of LTP tests sysfs01, sysfs02, sysfs03, sysfs04, sysfs05, and sysfs06, if executed in Qemu user mode. Signed-off-by: Aleksandar Markovic --- linux-user/strace.c| 25 + linux-user/strace.list | 2 +- linux-user/syscall.c | 42 +- 3 files changed, 67 insertions(+), 2 deletions(-) diff --git a/linux-user/strace.c b/linux-user/strace.c index 7ddcaf8..a52ff58 100644 --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -2138,6 +2138,31 @@ print_kill(const struct syscallname *name, } #endif +#if defined(TARGET_NR_sysfs) +static void +print_sysfs(const struct syscallname *name, +abi_long arg0, abi_long arg1, abi_long arg2, +abi_long arg3, abi_long arg4, abi_long arg5) +{ +print_syscall_prologue(name); +switch (arg0) { +case 1: +print_raw_param("%d", arg0, 1); +print_string(arg1, 1); +break; +case 2: +print_raw_param("%d", arg0, 0); +print_raw_param("%u", arg1, 0); +print_pointer(arg2, 1); +break; +default: +print_raw_param("%d", arg0, 1); +break; +} +print_syscall_epilogue(name); +} +#endif + /* * An array of all of the syscalls we know about */ diff --git a/linux-user/strace.list b/linux-user/strace.list index 9a665a8..e8133b0 100644 --- a/linux-user/strace.list +++ b/linux-user/strace.list @@ -1368,7 +1368,7 @@ { TARGET_NR_sys_epoll_wait, "sys_epoll_wait" , NULL, NULL, NULL }, #endif #ifdef TARGET_NR_sysfs -{ TARGET_NR_sysfs, "sysfs" , NULL, NULL, NULL }, +{ TARGET_NR_sysfs, "sysfs" , NULL, print_sysfs, NULL }, #endif #ifdef TARGET_NR_sysinfo { TARGET_NR_sysinfo, "sysinfo" , NULL, NULL, NULL }, diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 5643840..38d69f1 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9532,7 +9532,47 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, #endif #ifdef TARGET_NR_sysfs case TARGET_NR_sysfs: -goto unimplemented; +switch (arg1) { +case 1: +{ +if (arg2 != 0) { +p = lock_user_string(arg2); +if (!p) { +goto efault; +} +ret = get_errno(syscall(__NR_sysfs, arg1, p)); +unlock_user(p, arg2, 0); +} else { +ret = get_errno(syscall(__NR_sysfs, arg1, NULL)); +} +} +break; +case 2: +{ +if (arg3 != 0) { +char buf[PATH_MAX]; +int len; +memset(buf, 0, PATH_MAX); +ret = get_errno(syscall(__NR_sysfs, arg1, arg2, buf)); +len = PATH_MAX; +if (len > strlen(buf)) { +len = strlen(buf); +} +if (copy_to_user(arg3, buf, len) != 0) { +goto efault; +} +} else { +ret = get_errno(syscall(__NR_sysfs, arg1, arg2, NULL)); +} +} +break; +case 3: +ret = get_errno(syscall(__NR_sysfs, arg1)); +break; +default: +ret = -EINVAL; +} +break; #endif case TARGET_NR_personality: ret = get_errno(personality(arg1)); -- 2.9.3
[Qemu-devel] [PATCH v3 09/10] linux-user: Improve usage of spaces in linux-user/main.c
From: Aleksandar Markovic This patch removes all spaces-related errors (reported by checkpatch.pl) from linux-user/main.c. Signed-off-by: Aleksandar Markovic --- linux-user/main.c | 96 +++ 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/linux-user/main.c b/linux-user/main.c index bb581fc..24c9206 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -292,11 +292,11 @@ void cpu_loop(CPUX86State *env) abi_ulong ret; target_siginfo_t info; -for(;;) { +for (;;) { cpu_exec_start(cs); trapnr = cpu_exec(cs); cpu_exec_end(cs); -switch(trapnr) { +switch (trapnr) { case 0x80: /* linux syscall from int $0x80 */ ret = do_syscall(env, @@ -738,11 +738,11 @@ void cpu_loop(CPUARMState *env) uint32_t addr; abi_ulong ret; -for(;;) { +for (;;) { cpu_exec_start(cs); trapnr = cpu_exec(cs); cpu_exec_end(cs); -switch(trapnr) { +switch (trapnr) { case EXCP_UDEF: { TaskState *ts = cs->opaque; @@ -762,7 +762,7 @@ void cpu_loop(CPUARMState *env) info._sifields._sigfault._addr = env->regs[15]; queue_signal(env, info.si_signo, &info); } else if (rc < 0) { /* FP exception */ -int arm_fpe=0; +int arm_fpe = 0; /* translate softfloat flags to FPSR flags */ if (-rc & float_flag_invalid) { @@ -827,7 +827,7 @@ void cpu_loop(CPUARMState *env) if ((!(fpsr & BIT_IOE)) && (arm_fpe & BIT_IOC)) { fpsr |= BIT_IOC; } -ts->fpa.fpsr=fpsr; +ts->fpa.fpsr = fpsr; } else { /* everything OK */ /* increment PC */ env->regs[15] += 4; @@ -867,7 +867,7 @@ void cpu_loop(CPUARMState *env) /* nop */ } else if (n == ARM_NR_semihosting || n == ARM_NR_thumb_semihosting) { -env->regs[0] = do_arm_semihosting (env); +env->regs[0] = do_arm_semihosting(env); } else if (n == 0 || n >= ARM_SYSCALL_BASE || env->thumb) { /* linux syscall */ if (env->thumb || n == 0) { @@ -876,7 +876,7 @@ void cpu_loop(CPUARMState *env) n -= ARM_SYSCALL_BASE; env->eabi = 0; } -if ( n > ARM_NR_BASE) { +if (n > ARM_NR_BASE) { switch (n) { case ARM_NR_cacheflush: /* nop */ @@ -1291,7 +1291,7 @@ static inline void save_window_offset(CPUSPARCState *env, int cwp1) printf("win_overflow: sp_ptr=0x" TARGET_ABI_FMT_lx " save_cwp=%d\n", sp_ptr, cwp1); #endif -for(i = 0; i < 16; i++) { +for (i = 0; i < 16; i++) { /* FIXME - what to do if put_user() fails? */ put_user_ual(env->regbase[get_reg_index(env, cwp1, 8 + i)], sp_ptr); sp_ptr += sizeof(abi_ulong); @@ -1338,7 +1338,7 @@ static void restore_window(CPUSPARCState *env) printf("win_underflow: sp_ptr=0x" TARGET_ABI_FMT_lx " load_cwp=%d\n", sp_ptr, cwp1); #endif -for(i = 0; i < 16; i++) { +for (i = 0; i < 16; i++) { /* FIXME - what to do if get_user() fails? */ get_user_ual(env->regbase[get_reg_index(env, cwp1, 8 + i)], sp_ptr); sp_ptr += sizeof(abi_ulong); @@ -1359,7 +1359,7 @@ static void flush_windows(CPUSPARCState *env) int offset, cwp1; offset = 1; -for(;;) { +for (;;) { /* if restore would invoke restore_window(), then we can stop */ cwp1 = cpu_cwp_inc(env, env->cwp + offset); #ifndef TARGET_SPARC64 @@ -1386,7 +1386,7 @@ static void flush_windows(CPUSPARCState *env) #endif } -void cpu_loop (CPUSPARCState *env) +void cpu_loop(CPUSPARCState *env) { CPUState *cs = CPU(sparc_env_get_cpu(env)); int trapnr; @@ -1411,7 +1411,7 @@ void cpu_loop (CPUSPARCState *env) case 0x110: case 0x16d: #endif -ret = do_syscall (env, env->gregs[1], +ret = do_syscall(env, env->gregs[1], env->regwptr[0], env->regwptr[1], env->regwptr[2], env->regwptr[3], env->regwptr[4], env->regwptr[5], @@ -1524,11 +1524,11 @@ void cpu_loop (CPUSPARCState *env) } break; default: -printf ("Unhandled trap: 0x%x\n", trapnr); +printf("Unhandled trap: 0x%x\n", trapnr); cpu_dump_state(cs, stderr, fprintf, 0); exit(EXIT_FAILURE); } -process_pending_signals (env); +process_
[Qemu-devel] [PATCH v3 01/10] linux-user: Add support for adjtimex() syscall
From: Aleksandar Markovic This patch implements Qemu user mode adjtimex() syscall support. Syscall adjtimex() reads and optionally sets parameters for a clock adjustment algorithm used in network synchonization or similar scenarios. The implementation is based on invocation of host's adjtimex(), and its key part is in the correspondent case segment of the main switch statement of the function do_syscall(), in file linux-user/syscalls.c. Also, support for related structure "timex" is added to the file linux-user/syscall_defs.h, based on its definition in Linux kernel. All necessary conversions of the data structures from target to host and from host to target are covered. Two new functions, target_to_host_timex() and host_to_target_timex(), are provided for the purpose of such conversions. Moreover, the relevant support for "-strace" Qemu option is included in files linux-user/strace.c and linux-user/strace.list. This patch also fixes failures of LTP tests adjtimex01 and adjtimex02, if executed in Qemu user mode. Signed-off-by: Aleksandar Rikalo Signed-off-by: Aleksandar Markovic --- linux-user/strace.c | 12 +++ linux-user/strace.list| 2 +- linux-user/syscall.c | 90 ++- linux-user/syscall_defs.h | 28 +++ 4 files changed, 130 insertions(+), 2 deletions(-) diff --git a/linux-user/strace.c b/linux-user/strace.c index cc10dc4..7ddcaf8 100644 --- a/linux-user/strace.c +++ b/linux-user/strace.c @@ -919,6 +919,18 @@ print_access(const struct syscallname *name, } #endif +#ifdef TARGET_NR_adjtimex +static void +print_adjtimex(const struct syscallname *name, +abi_long arg0, abi_long arg1, abi_long arg2, +abi_long arg3, abi_long arg4, abi_long arg5) +{ +print_syscall_prologue(name); +print_pointer(arg0, 1); +print_syscall_epilogue(name); +} +#endif + #ifdef TARGET_NR_brk static void print_brk(const struct syscallname *name, diff --git a/linux-user/strace.list b/linux-user/strace.list index aa967a2..9a665a8 100644 --- a/linux-user/strace.list +++ b/linux-user/strace.list @@ -16,7 +16,7 @@ { TARGET_NR_add_key, "add_key" , NULL, NULL, NULL }, #endif #ifdef TARGET_NR_adjtimex -{ TARGET_NR_adjtimex, "adjtimex" , NULL, NULL, NULL }, +{ TARGET_NR_adjtimex, "adjtimex" , NULL, print_adjtimex, NULL }, #endif #ifdef TARGET_NR_afs_syscall { TARGET_NR_afs_syscall, "afs_syscall" , NULL, NULL, NULL }, diff --git a/linux-user/syscall.c b/linux-user/syscall.c index ca06943..5643840 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -35,6 +35,7 @@ #include #include #include +#include #ifdef __ia64__ int __clone2(int (*fn)(void *), void *child_stack_base, size_t stack_size, int flags, void *arg, ...); @@ -6578,6 +6579,78 @@ static inline abi_long target_ftruncate64(void *cpu_env, abi_long arg1, } #endif +#ifdef TARGET_NR_adjtimex +static inline abi_long target_to_host_timex(struct timex *host_buf, +abi_long target_addr) +{ +struct target_timex *target_buf; + +if (!lock_user_struct(VERIFY_READ, target_buf, target_addr, 1)) { +return -TARGET_EFAULT; +} + +host_buf->modes = tswap32(target_buf->modes); +host_buf->offset = tswapal(target_buf->offset); +host_buf->freq = tswapal(target_buf->freq); +host_buf->maxerror = tswapal(target_buf->maxerror); +host_buf->esterror = tswapal(target_buf->esterror); +host_buf->status = tswap32(target_buf->status); +host_buf->constant = tswapal(target_buf->constant); +host_buf->precision = tswapal(target_buf->precision); +host_buf->tolerance = tswapal(target_buf->tolerance); +host_buf->time.tv_sec = tswapal(target_buf->time.tv_sec); +host_buf->time.tv_usec = tswapal(target_buf->time.tv_usec); +host_buf->tick = tswapal(target_buf->tick); +host_buf->ppsfreq = tswapal(target_buf->ppsfreq); +host_buf->jitter = tswapal(target_buf->jitter); +host_buf->shift = tswap32(target_buf->shift); +host_buf->stabil = tswapal(target_buf->stabil); +host_buf->jitcnt = tswapal(target_buf->jitcnt); +host_buf->calcnt = tswapal(target_buf->calcnt); +host_buf->errcnt = tswapal(target_buf->errcnt); +host_buf->stbcnt = tswapal(target_buf->stbcnt); +host_buf->tai = tswap32(target_buf->tai); + +unlock_user_struct(target_buf, target_addr, 0); +return 0; +} + +static inline abi_long host_to_target_timex(abi_long target_addr, +struct timex *host_buf) +{ +struct target_timex *target_buf; + +if (!lock_user_struct(VERIFY_WRITE, target_buf, target_addr, 0)) { +return -TARGET_EFAULT; +} + +target_buf->modes = tswap32(host_buf->modes); +target_buf->offset = tswapal(host_buf->offset); +target_buf->freq = tswapal(host_buf->freq); +target_buf->maxerror = tswapal(host_buf->maxerror); +target_buf->esterror = tswapal(host_buf->esterror); +target_buf->s
[Qemu-devel] [PATCH v5 5/5] linux-user: Fix certain argument alignment cases for Mips64
From: Aleksandar Markovic The function that is changed in this patch is supposed to indicate that there was certaing argument rearangement related to 64-bit arguments on 32-bit platforms. The background on such rearangements can be found, for example, in the man page for syscall(2). However, for 64-bit Mips architectures there is no such rearangement, and this patch reflects it. Signed-off-by: Aleksandar Rikalo Signed-off-by: Aleksandar Markovic --- linux-user/syscall.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index ca06943..ee23b29 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -620,7 +620,14 @@ static inline int regpairs_aligned(void *cpu_env) { return CPUARMState *)cpu_env)->eabi) == 1) ; } #elif defined(TARGET_MIPS) -static inline int regpairs_aligned(void *cpu_env) { return 1; } +static inline int regpairs_aligned(void *cpu_env) +{ +#if TARGET_ABI_BITS == 32 +return 1; +#else +return 0; +#endif +} #elif defined(TARGET_PPC) && !defined(TARGET_PPC64) /* SysV AVI for PPC32 expects 64bit parameters to be passed on odd/even pairs * of registers which translates to the same as ARM/MIPS, because we start with -- 2.9.3