Re: [Qemu-devel] [PATCH v4 0/2] Rewrite TCP packet comparison in colo
On 2018年01月04日 15:42, Mao Zhongyi wrote: Hi, Jason Long time no news, Ping... Thanks, Mao Sorry for the delay. Queued. Thanks On 12/25/2017 10:54 AM, Mao Zhongyi wrote: v4: p2: fix some typo [Zhang Chen] v3: p1: merged the patch1 and patch2 from v2 p2: -merged the patch3 and patch4 from v2 -implement the same process flow for tcp, udp and icmp [Zhang Chen] v2: p1: a new patch p2: a new patch p3: -rename the fill_pkt_seq to fill_pkt_tcp_info -rename pdsize & hdsize to payload_size & header_size -reuse duplicated code -modified colo_packet_compare_common() to suit the tcp packet comparison instead of build a new function service for tcp. -add more comments for the 'max_ack' [Zhang Chen] Cc: Zhang Chen Cc: Li Zhijian Cc: Jason Wang Mao Zhongyi (2): colo: modified the payload compare function colo: compare the packet based on the tcp sequence number net/colo-compare.c | 411 + net/colo.c | 9 ++ net/colo.h | 15 ++ net/trace-events | 2 +- 4 files changed, 284 insertions(+), 153 deletions(-)
Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches
Am 04.01.2018 um 08:27 schrieb Alexandre DERUMIER: > does somebody have a redhat account to see te content of: > > https://access.redhat.com/solutions/3307851 > "Impacts of CVE-2017-5754, CVE-2017-5753, and CVE-2017-5715 to Red Hat > Virtualization products" i don't have one but the content might be something like this: https://www.suse.com/de-de/support/kb/doc/?id=7022512 So you need: 1.) intel / amd cpu microcode update 2.) qemu update to pass the new MSR and CPU flags from the microcode update 3.) host kernel update 4.) guest kernel update The microcode update and the kernel update is publicly available but i'm missing the qemu one. Greets, Stefan > - Mail original - > De: "aderumier" > À: "Stefan Priebe, Profihost AG" > Cc: "qemu-devel" > Envoyé: Jeudi 4 Janvier 2018 08:24:34 > Objet: Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches > >>> Can anybody point me to the relevant qemu patches? > > I don't have find them yet. > > Do you known if a vm using kvm64 cpu model is protected or not ? > > - Mail original - > De: "Stefan Priebe, Profihost AG" > À: "qemu-devel" > Envoyé: Jeudi 4 Janvier 2018 07:27:01 > Objet: [Qemu-devel] CVE-2017-5715: relevant qemu patches > > Hello, > > i've seen some vendors have updated qemu regarding meltdown / spectre. > > f.e.: > > CVE-2017-5715: QEMU was updated to allow passing through new MSR and > CPUID flags from the host VM to the CPU, to allow enabling/disabling > branch prediction features in the Intel CPU. (bsc#1068032) > > Can anybody point me to the relevant qemu patches? > > Thanks! > > Greets, > Stefan >
Re: [Qemu-devel] [PATCH v4 0/2] Rewrite TCP packet comparison in colo
On 01/04/2018 04:13 PM, Jason Wang wrote: On 2018年01月04日 15:42, Mao Zhongyi wrote: Hi, Jason Long time no news, Ping... Thanks, Mao Sorry for the delay. No worries, thanks :) Queued. Thanks On 12/25/2017 10:54 AM, Mao Zhongyi wrote: v4: p2: fix some typo [Zhang Chen] v3: p1: merged the patch1 and patch2 from v2 p2: -merged the patch3 and patch4 from v2 -implement the same process flow for tcp, udp and icmp [Zhang Chen] v2: p1: a new patch p2: a new patch p3: -rename the fill_pkt_seq to fill_pkt_tcp_info -rename pdsize & hdsize to payload_size & header_size -reuse duplicated code -modified colo_packet_compare_common() to suit the tcp packet comparison instead of build a new function service for tcp. -add more comments for the 'max_ack' [Zhang Chen] Cc: Zhang Chen Cc: Li Zhijian Cc: Jason Wang Mao Zhongyi (2): colo: modified the payload compare function colo: compare the packet based on the tcp sequence number net/colo-compare.c | 411 + net/colo.c | 9 ++ net/colo.h | 15 ++ net/trace-events | 2 +- 4 files changed, 284 insertions(+), 153 deletions(-)
Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches
>>So you need: >>1.) intel / amd cpu microcode update >>2.) qemu update to pass the new MSR and CPU flags from the microcode update >>3.) host kernel update >>4.) guest kernel update are you sure we need to patch guest kernel if we are able to patch qemu ? I have some pretty old guest (linux and windows) If I understand, patching the host kernel, should avoid that a vm is reading memory of another vm. (the most critical) patching the guest kernel, to avoid that a process from the vm have access to memory of another process of same vm. right ? - Mail original - De: "Stefan Priebe, Profihost AG" À: "aderumier" Cc: "qemu-devel" Envoyé: Jeudi 4 Janvier 2018 09:17:41 Objet: Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches Am 04.01.2018 um 08:27 schrieb Alexandre DERUMIER: > does somebody have a redhat account to see te content of: > > https://access.redhat.com/solutions/3307851 > "Impacts of CVE-2017-5754, CVE-2017-5753, and CVE-2017-5715 to Red Hat > Virtualization products" i don't have one but the content might be something like this: https://www.suse.com/de-de/support/kb/doc/?id=7022512 So you need: 1.) intel / amd cpu microcode update 2.) qemu update to pass the new MSR and CPU flags from the microcode update 3.) host kernel update 4.) guest kernel update The microcode update and the kernel update is publicly available but i'm missing the qemu one. Greets, Stefan > - Mail original - > De: "aderumier" > À: "Stefan Priebe, Profihost AG" > Cc: "qemu-devel" > Envoyé: Jeudi 4 Janvier 2018 08:24:34 > Objet: Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches > >>> Can anybody point me to the relevant qemu patches? > > I don't have find them yet. > > Do you known if a vm using kvm64 cpu model is protected or not ? > > - Mail original - > De: "Stefan Priebe, Profihost AG" > À: "qemu-devel" > Envoyé: Jeudi 4 Janvier 2018 07:27:01 > Objet: [Qemu-devel] CVE-2017-5715: relevant qemu patches > > Hello, > > i've seen some vendors have updated qemu regarding meltdown / spectre. > > f.e.: > > CVE-2017-5715: QEMU was updated to allow passing through new MSR and > CPUID flags from the host VM to the CPU, to allow enabling/disabling > branch prediction features in the Intel CPU. (bsc#1068032) > > Can anybody point me to the relevant qemu patches? > > Thanks! > > Greets, > Stefan >
Re: [Qemu-devel] [virtio-dev] Re: [v22 1/2] virtio-crypto: Add virtio crypto device specification
On 2018/1/4 4:43, Halil Pasic wrote: > > > On 12/30/2017 08:57 AM, Longpeng (Mike) wrote: >>> What you actually do is the following. You define a >>> 'VIRTIO_CRYPTO_F__STATELESS_MODE feature bit is negotiated' >>> (A) mode and a '... bit is not negotiated (B)' mode for each service. >> >>> In >>> mode A the driver has to use type A sateful requests (to which you refer >>> as session mode). In mode B however the driver can use both stateless >>> requests (to which you refer as stateless mode) and B type stateful >>> requests (to which you also refer as session mode). >> Sorry, I think the driver can use both in mode A and has to use stateful >> mode in >> mode B. >> > > You are right I've mixed up A and B. I think you got my point despite > of this slip up. > > I'm on holiday this week. Will try to review v23 next week. > OK. Hoping you have a good holiday :) > Regards, > Halil > > > - > To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org > > > . > -- Regards, Longpeng(Mike)
Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches
Am 04.01.2018 um 09:35 schrieb Alexandre DERUMIER: >>> So you need: >>> 1.) intel / amd cpu microcode update >>> 2.) qemu update to pass the new MSR and CPU flags from the microcode update >>> 3.) host kernel update >>> 4.) guest kernel update > > are you sure we need to patch guest kernel if we are able to patch qemu ? >> I have some pretty old guest (linux and windows) > > If I understand, patching the host kernel, should avoid that a vm is reading > memory of another vm. > (the most critical) Yes - this was just to complete the mitigation on all layers. > > patching the guest kernel, to avoid that a process from the vm have access to > memory of another process of same vm. Yes. Stefan > > > > - Mail original - > De: "Stefan Priebe, Profihost AG" > À: "aderumier" > Cc: "qemu-devel" > Envoyé: Jeudi 4 Janvier 2018 09:17:41 > Objet: Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches > > Am 04.01.2018 um 08:27 schrieb Alexandre DERUMIER: >> does somebody have a redhat account to see te content of: >> >> https://access.redhat.com/solutions/3307851 >> "Impacts of CVE-2017-5754, CVE-2017-5753, and CVE-2017-5715 to Red Hat >> Virtualization products" > > i don't have one but the content might be something like this: > https://www.suse.com/de-de/support/kb/doc/?id=7022512 > > So you need: > 1.) intel / amd cpu microcode update > 2.) qemu update to pass the new MSR and CPU flags from the microcode update > 3.) host kernel update > 4.) guest kernel update > > The microcode update and the kernel update is publicly available but i'm > missing the qemu one. > > Greets, > Stefan > >> - Mail original - >> De: "aderumier" >> À: "Stefan Priebe, Profihost AG" >> Cc: "qemu-devel" >> Envoyé: Jeudi 4 Janvier 2018 08:24:34 >> Objet: Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches >> Can anybody point me to the relevant qemu patches? >> >> I don't have find them yet. >> >> Do you known if a vm using kvm64 cpu model is protected or not ? >> >> - Mail original - >> De: "Stefan Priebe, Profihost AG" >> À: "qemu-devel" >> Envoyé: Jeudi 4 Janvier 2018 07:27:01 >> Objet: [Qemu-devel] CVE-2017-5715: relevant qemu patches >> >> Hello, >> >> i've seen some vendors have updated qemu regarding meltdown / spectre. >> >> f.e.: >> >> CVE-2017-5715: QEMU was updated to allow passing through new MSR and >> CPUID flags from the host VM to the CPU, to allow enabling/disabling >> branch prediction features in the Intel CPU. (bsc#1068032) >> >> Can anybody point me to the relevant qemu patches? >> >> Thanks! >> >> Greets, >> Stefan >> >
Re: [Qemu-devel] [PATCH v2.1 3/3] chardev: introduce qemu_chr_timeout_add() and use
On Thu, Jan 04, 2018 at 10:31:58AM +0800, Peter Xu wrote:
> On Wed, Jan 03, 2018 at 05:41:53PM +, Stefan Hajnoczi wrote:
> > On Wed, Jan 03, 2018 at 10:24:18AM +0800, Peter Xu wrote:
> > > It's a replacement of g_timeout_add[_seconds]() for chardevs. Chardevs
> > > now can have dedicated gcontext, we should always bind chardev tasks
> > > onto those gcontext rather than the default main context. Since there
> > > are quite a few of g_timeout_add[_seconds]() callers, a new function
> > > qemu_chr_timeout_add() is introduced.
> > >
> > > One thing to mention is that, terminal3270 is still always running on
> > > main gcontext. However let's convert that as well since it's still part
> > > of chardev codes and in case one day we'll miss that when we move it out
> > > of main gcontext too.
> > >
> > > Signed-off-by: Peter Xu
> > > ---
> > >
> > > v2 -> v2.1: Sorry I forgot to do the move in char.h. Did it in this
> > > minor version.
> > >
> > > chardev/char-pty.c | 9 ++---
> > > chardev/char-socket.c | 4 ++--
> > > chardev/char.c | 20
> > > hw/char/terminal3270.c | 7 ---
> > > include/chardev/char.h | 3 +++
> > > 5 files changed, 31 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/chardev/char-pty.c b/chardev/char-pty.c
> > > index dd17b1b823..cbd8ac5eb7 100644
> > > --- a/chardev/char-pty.c
> > > +++ b/chardev/char-pty.c
> > > @@ -78,13 +78,8 @@ static void pty_chr_rearm_timer(Chardev *chr, int ms)
> > > s->timer_tag = 0;
> > > }
> > >
> > > -if (ms == 1000) {
> > > -name = g_strdup_printf("pty-timer-secs-%s", chr->label);
> > > -s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr);
> > > -} else {
> > > -name = g_strdup_printf("pty-timer-ms-%s", chr->label);
> > > -s->timer_tag = g_timeout_add(ms, pty_chr_timer, chr);
> > > -}
> > > +name = g_strdup_printf("pty-timer-ms-%s", chr->label);
> > > +s->timer_tag = qemu_chr_timeout_add(chr, ms, pty_chr_timer, chr);
> >
> > The label is user-visible. Why did you remove the seconds label format?
>
> It's used for g_source_set_name_by_id() below, and that's not
> user-visible AFAICT?
>
> I removed it because I thought it was not user visible and actually I
> didn't see a point on doing that. Please let me know if I made a
> mistake.
>
> >
> > Please either include justification in the commit description or avoid
> > spurious changes like this so reviewers don't need to worry about code
> > changes that are not essential.
>
> Yes. I can add this into commit message after confirmed with you on above.
You are right, the GSource name isn't visible (it's only used for
debugging). I was thinking of chr->label.
It's still helpful to mention that the ms == 1000 special case is not
user-visible in the commit description.
Thanks,
Stefan
signature.asc
Description: PGP signature
Re: [Qemu-devel] [RESEND PATCH 2/6] memory: introduce AddressSpaceOps and IOMMUObject
On Wed, Jan 03, 2018 at 11:28:17AM +1100, David Gibson wrote: > On Thu, Dec 21, 2017 at 04:40:19PM +0800, Liu, Yi L wrote: > > On Wed, Dec 20, 2017 at 10:18:16PM +1100, David Gibson wrote: > > > On Wed, Dec 20, 2017 at 02:47:30PM +0800, Liu, Yi L wrote: > > > > On Mon, Dec 18, 2017 at 10:35:31PM +1100, David Gibson wrote: > > > > > On Wed, Nov 15, 2017 at 03:16:32PM +0800, Peter Xu wrote: > > > > > > On Tue, Nov 14, 2017 at 10:52:54PM +0100, Auger Eric wrote: > > > > > > > > > > > > [...] > > > > > > I'm ok with calling it a "PASID context". > > > > > > Thinking about this some more, here are some extra observations: > > > > > > * I think each device needs both a PASID context and an ordinary > > >address space. The PASID context would be used for bus > > >transactions which include a process id, the address space for > > >those that don't. > > > > Correct. Also virt-SVM still needs the PCI Address space. And the PCI > > Address space == Guest physical Address space. > > Not necessarily. That's true if you're only making the L1 translation > guest visible. But I think we need to at least think about the case > where both L1 and L2 translations are guest visible, in which case the > PCI address space is not the same as the guest physical address space. > > > For virt-SVM, requires > > pt mode to ensure the nested translation. > > What is pt mode? The pt mode here means the kernel parameter "iommu=pt" which means iommu do 1:1 mapping for iova. For t virt-SVM on VT-d, with guest set iommu=pt, the 2nd level page table in host would be a GPA->HPA mapping. If not, the host 2nd level page table would be a GIOVA->HPA mapping which is not expected in nested translation. > > > * Theoretically, the PASID context could be modelled as an array/map > > >of AddressSpace objects for each process ID. However, creating all > > > > I also thought about creating AddressSpace objects for each process ID. > > But I don't think we need to do it. My reason as below: > > > > In theory, it is correct to have AddressSpace object for each process > > virtual address space in Qemu, and this is what we are doing for PCI > > address space so far. However, this is necessary when we want to mirror > > the mapping to host. Each time there is mapping changed within the PCI > > address space, we need to mirror it to host. > > > > While for virt-SVM, we don't need to mirror the changes within the guest > > process address space. The nested translation capability in HW brings us > > a convenience. In nested translation, HW can access a guest PASID table > > with a GPA(this is what Intel and AMD does, not sure about ARM, maybe or > > maybe not). For VT-d and AMD-IOMMU, even any change in guest PASID table, > > it is not necessary to mirror it to host. Based on the statements above, > > there is a candidate function to be included in PASIDContext. It could be > > bind_guest_pasid_table(). And be used to set the guest PASID Table to host > > translation structure when guest finds a device is has SVM > > capability. > > That's true for passthrough devices, but not for qemu emulated > devices. None of those support SVM yet, but there's no reason they > couldn't in future. > > Even though that will never be the main production case, I think we'll > get a better model if we think about these edge cases carefully. Yeah, it's a quite good edge case. If an emulated device want to do DMA to a guest process virtual address space, Qemu needs to know it and do necessary address translation for it just as iova. If we want to support emulated SVM capable device, not sure if the current AddressSpace in Qemu can fit well. Honestly, SVM capable devices are major complicated accelerators. So I think we need to balance the efort. But I agree it is helpful to understand more about the edge cases. > > >those AddressSpace objects in advance might be too expensive. I > > >can see a couple of options to avoid this: > > > > > > 1) Have the PASID context class include a 'translate' method similar > > > to the one in IOMMUMemoryRegionClass, but taking a process ID as well > > > as an address. This would avoid creating extra AddressSpace objects, > > > but might require duplicating a bunch of the translation code that > > > already exists for AddressSpace. > > > > Translation code actually walks the guest CR3 table to get a > > GVA->GPA map. > > Right, but that's clearly x86 specific. Yes, CR3 table is x86 specific. While for page table walking, I think it can be vendor-agnostic. > > > But it is not really required so far due to HW capability. > > > > > > > > 2) "Lazily" create AddressSpace objects. The generic part of the > > > PASID aware DMA helper functions would use a cache of AddressSpace's > > > for particular process IDs, using the AddressSpace (and MemoryRegion > > > within) to translate accesses for a particular process ID. However, > > > these AddressSpace and MemoryRegion objects would only be created
Re: [Qemu-devel] [PATCH v5 0/2] qemu-img: Document --force-share / -U
On Tue, Dec 26, 2017 at 10:52:45AM +0800, Fam Zheng wrote: > v5: Clean up the @table @var section first. [Kevin, Peter] > > Fam Zheng (2): > qemu-img.texi: Clean up parameter list > qemu-img: Document --force-share / -U > > qemu-img.texi | 75 > ++- > 1 file changed, 48 insertions(+), 27 deletions(-) > > -- > 2.14.3 > Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[Qemu-devel] [PATCH v3 1/3] chardev: use backend chr context when watch for fe
In commit 6bbb6c0644 ("chardev: use per-dev context for
io_add_watch_poll", 2017-09-22) all the chardev watches are converted to
use per-chardev gcontext to support chardev to be run outside default
main thread. However that's still missing one call from the frontend
code. Touch that up.
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Marc-André Lureau
Signed-off-by: Peter Xu
---
chardev/char-fe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index ee6d596100..c611b3fa3e 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -356,7 +356,7 @@ guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition
cond,
}
g_source_set_callback(src, (GSourceFunc)func, user_data, NULL);
-tag = g_source_attach(src, NULL);
+tag = g_source_attach(src, s->gcontext);
g_source_unref(src);
return tag;
--
2.14.3
[Qemu-devel] [PATCH v3 0/3] chardev: convert leftover glib APIs to use dedicate gcontext
v3: - rename function to qemu_chr_timeout_add_ms() [Stefan] - add comment on return code [Stefan] - add comment in commit message on why change GSource name [Stefan] v2: - add r-bs - fix patch 3 on some s->ms conversion [Marc-André] There were existing work that tried to allow chardev to be run in a dedicated gcontext rather than the default main context/thread. Basically that work passed in the correct gcontext during g_source_attach(). However, I found something missing along the way, that some legacy glib APIs are used by chardev code which take the main context as default: g_timeout_add_seconds g_timeout_add g_idle_add To fully allow the chardevs to be run in dedicated gcontext, we need to convert all these legacy APIs into g_source_attach() calls as well, with the correct gcontext passed in. This series tries to clean the rest of things up. I picked up patch 1 from monitor-oob series into this series (which is a missing of chardev frontend call fix for g_source_attach()), so that this series can be a complete fix. Please review. Thanks, Peter Xu (3): chardev: use backend chr context when watch for fe chardev: let g_idle_add() be with chardev gcontext chardev: introduce qemu_chr_timeout_add_ms() chardev/char-fe.c | 2 +- chardev/char-pty.c | 16 chardev/char-socket.c | 6 -- chardev/char.c | 21 + hw/char/terminal3270.c | 7 --- include/chardev/char.h | 3 +++ 6 files changed, 41 insertions(+), 14 deletions(-) -- 2.14.3
[Qemu-devel] [PATCH v3 2/3] chardev: let g_idle_add() be with chardev gcontext
The idle task will be attached to main gcontext even if the chardev
backend is running in another gcontext. Fix the only caller by
extending the g_idle_add() logic into the more powerful
g_source_attach(). It's basically g_idle_add_full() implementation, but
with the chardev's gcontext passed in.
Reviewed-by: Marc-André Lureau
Reviewed-by: Stefan Hajnoczi
Signed-off-by: Peter Xu
---
chardev/char-pty.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 761ae6dec1..dd17b1b823 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -210,9 +210,14 @@ static void pty_chr_state(Chardev *chr, int connected)
s->timer_tag = 0;
}
if (!s->connected) {
+GSource *source = g_idle_source_new();
+
g_assert(s->open_tag == 0);
s->connected = 1;
-s->open_tag = g_idle_add(qemu_chr_be_generic_open_func, chr);
+g_source_set_callback(source, qemu_chr_be_generic_open_func,
+ chr, NULL);
+s->open_tag = g_source_attach(source, chr->gcontext);
+g_source_unref(source);
}
if (!chr->gsource) {
chr->gsource = io_add_watch_poll(chr, s->ioc,
--
2.14.3
[Qemu-devel] [PATCH v3 3/3] chardev: introduce qemu_chr_timeout_add_ms()
It's a replacement of g_timeout_add[_seconds]() for chardevs. Chardevs
now can have dedicated gcontext, we should always bind chardev tasks
onto those gcontext rather than the default main context. Since there
are quite a few of g_timeout_add[_seconds]() callers, a new function
qemu_chr_timeout_add_ms() is introduced.
One thing to mention is that, terminal3270 is still always running on
main gcontext. However let's convert that as well since it's still part
of chardev codes and in case one day we'll miss that when we move it out
of main gcontext too.
Also, in pty_chr_rearm_timer() the GSource name is not user visible, and
does not really matter much. Unify the two calls into one.
Signed-off-by: Peter Xu
---
chardev/char-pty.c | 9 ++---
chardev/char-socket.c | 6 --
chardev/char.c | 21 +
hw/char/terminal3270.c | 7 ---
include/chardev/char.h | 3 +++
5 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index dd17b1b823..2b37ad55ed 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -78,13 +78,8 @@ static void pty_chr_rearm_timer(Chardev *chr, int ms)
s->timer_tag = 0;
}
-if (ms == 1000) {
-name = g_strdup_printf("pty-timer-secs-%s", chr->label);
-s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr);
-} else {
-name = g_strdup_printf("pty-timer-ms-%s", chr->label);
-s->timer_tag = g_timeout_add(ms, pty_chr_timer, chr);
-}
+name = g_strdup_printf("pty-timer-ms-%s", chr->label);
+s->timer_tag = qemu_chr_timeout_add_ms(chr, ms, pty_chr_timer, chr);
g_source_set_name_by_id(s->timer_tag, name);
g_free(name);
}
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 630a7f2995..9527314708 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -73,8 +73,10 @@ static void qemu_chr_socket_restart_timer(Chardev *chr)
char *name;
assert(s->connected == 0);
-s->reconnect_timer = g_timeout_add_seconds(s->reconnect_time,
- socket_reconnect_timeout, chr);
+s->reconnect_timer = qemu_chr_timeout_add_ms(chr,
+ s->reconnect_time * 1000,
+ socket_reconnect_timeout,
+ chr);
name = g_strdup_printf("chardev-socket-reconnect-%s", chr->label);
g_source_set_name_by_id(s->reconnect_timer, name);
g_free(name);
diff --git a/chardev/char.c b/chardev/char.c
index 8c3765ee99..d77787d67a 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -1084,6 +1084,27 @@ void qmp_chardev_send_break(const char *id, Error **errp)
qemu_chr_be_event(chr, CHR_EVENT_BREAK);
}
+/*
+ * Add a timeout callback for the chardev (in milliseconds). Please
+ * use this to add timeout hook for chardev instead of g_timeout_add()
+ * and g_timeout_add_seconds(), to make sure the gcontext that the
+ * task bound to is correct. Returns the ID of the timeout GSource in
+ * gcontext of the chardev.
+ */
+guint qemu_chr_timeout_add_ms(Chardev *chr, guint ms, GSourceFunc func,
+ void *private)
+{
+GSource *source = g_timeout_source_new(ms);
+guint id;
+
+assert(func);
+g_source_set_callback(source, func, private, NULL);
+id = g_source_attach(source, chr->gcontext);
+g_source_unref(source);
+
+return id;
+}
+
void qemu_chr_cleanup(void)
{
object_unparent(get_chardevs_root());
diff --git a/hw/char/terminal3270.c b/hw/char/terminal3270.c
index a109ce5987..b0f81d0d16 100644
--- a/hw/char/terminal3270.c
+++ b/hw/char/terminal3270.c
@@ -94,8 +94,8 @@ static void terminal_read(void *opaque, const uint8_t *buf,
int size)
g_source_remove(t->timer_tag);
t->timer_tag = 0;
}
-t->timer_tag = g_timeout_add_seconds(600, send_timing_mark_cb, t);
-
+t->timer_tag = qemu_chr_timeout_add_ms(t->chr.chr, 600 * 1000,
+ send_timing_mark_cb, t);
memcpy(&t->inv[t->in_len], buf, size);
t->in_len += size;
if (t->in_len < 2) {
@@ -157,7 +157,8 @@ static void chr_event(void *opaque, int event)
* char-socket.c. Once qemu receives the terminal-type of the
* client, mark handshake done and trigger everything rolling again.
*/
-t->timer_tag = g_timeout_add_seconds(600, send_timing_mark_cb, t);
+t->timer_tag = qemu_chr_timeout_add_ms(t->chr.chr, 600 * 1000,
+ send_timing_mark_cb, t);
break;
case CHR_EVENT_CLOSED:
sch->curr_status.scsw.dstat = SCSW_DSTAT_DEVICE_END;
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 778d610295..f1b67af4d2 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -256,6 +256,9 @@ Chardev *qemu_chardev_new(const char *id, const char
*typena
Re: [Qemu-devel] vhost-user graceful connect/disconnect
On Thu, Dec 21, 2017 at 06:01:29AM -0500, Marc-André Lureau wrote: > Hi > > - Original Message - > > Hi, > > Vhost-user implementations assume the slave is already running before > > the master starts. The slave is required during virtio device > > initialization (e.g. feature bit negotiation) and so it is simplest to > > assume that the master is already available and will respond immediately > > to the VHOST_USER_GET_FEATURES message. > > > > I have thought about how to let master and slave start in any order. > > Some approaches involve changes to the VIRTIO specification so that > > guest drivers can wait until the vhost-user connection is established. > > > > We can avoid spec changes using PCI hotplug: > > > > 1. Introduce a new vhost-user object that manages a connection: > > > >-chardev ... > >-object vhost-user,id=vhost-user0,chardev=chr0 > > > >Note this object is *not* a NetClient. It's a resource for managing > >a vhost-user connection and can be used with any device type (net, > >scsi, blk, etc). > > > >This object tries to establish a connection. When a connection is > >established it sends vhost-user protocol messages to fetch > >information needed for virtio device initialization (like the number > >of virtqueues supported, features bits, etc). This information is > >stashed so that vhost_*() calls later on do not require synchronous > >communication with the vhost-user slave. > > This share similarities with vhost-user-backend I proposed here: > https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg01014.html > > (instead of spawning a slave, talk to a chardev - both could eventually > co-exist) Yes, it's similar. Thanks for the link! > > > > 2. When the vhost-user connection is established the object emits a QMP > >event so management software can hotplug the virtio device: > > > >VHOST_USER_CONNECTED source=vhost-user0 > > > > 3. The management software hotplugs the virtio device: > > > >(qmp) netdev_add vhost-user,id=netdev0,vhost-user=vhost-user0 > >(qmp) device_add virtio-net-pci,netdev=netdev0 > > > > Advantages of this approach: > > > > * Does not require spec changes. > > * Can be implemented without vhost-user.c qemu_chr_fe_read/write_all() > >calls that hang QEMU until the slave sends data. > > * Allows slave to set the number of queues, feature bits, etc via the > >vhost-user socket. It is not necessary to manually specify feature > >bitmasks and other slave-specific information on the QEMU > >command-line. > > > > I haven't thought through disconnection but I imagine the vhost-user > > object would emit a VHOST_USER_DISCONNECTED event so the management > > software can hot unplug the device. > > > > Thoughts? > > I am sorry, I can't think through all the potential issues easily. But that > sounds interesting enough to start a proof-of-concept. > > And we need more tests for vhost-user (including fixing the existing > tests!)... I'm not going to prototype this yet, I'm working on virtio-vhost-user first, but eventually I might get back to -object vhost-user(-backend). Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v9 4/4] contrib/vhost-user-blk: introduce a vhost-user-blk sample application
Hi
On Thu, Jan 4, 2018 at 2:24 AM, Changpeng Liu wrote:
> This commit introduces a vhost-user-blk backend device, it uses UNIX
> domain socket to communicate with QEMU. The vhost-user-blk sample
> application should be used with QEMU vhost-user-blk-pci device.
>
> To use it, complie with:
> make vhost-user-blk
>
> and start like this:
> vhost-user-blk -b /dev/sdb -s /path/vhost.socket
>
> Signed-off-by: Changpeng Liu
> ---
> .gitignore | 1 +
> Makefile| 3 +
> Makefile.objs | 1 +
> contrib/vhost-user-blk/Makefile.objs| 1 +
> contrib/vhost-user-blk/vhost-user-blk.c | 545
>
> 5 files changed, 551 insertions(+)
> create mode 100644 contrib/vhost-user-blk/Makefile.objs
> create mode 100644 contrib/vhost-user-blk/vhost-user-blk.c
>
> diff --git a/.gitignore b/.gitignore
> index 433f64f..704b222 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -54,6 +54,7 @@
> /module_block.h
> /scsi/qemu-pr-helper
> /vhost-user-scsi
> +/vhost-user-blk
> /fsdev/virtfs-proxy-helper
> *.tmp
> *.[1-9]
> diff --git a/Makefile b/Makefile
> index d86ecd2..f021fc8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -331,6 +331,7 @@ dummy := $(call unnest-vars,, \
> ivshmem-server-obj-y \
> libvhost-user-obj-y \
> vhost-user-scsi-obj-y \
> +vhost-user-blk-obj-y \
> qga-vss-dll-obj-y \
> block-obj-y \
> block-obj-m \
> @@ -562,6 +563,8 @@ ivshmem-server$(EXESUF): $(ivshmem-server-obj-y)
> $(COMMON_LDADDS)
> endif
> vhost-user-scsi$(EXESUF): $(vhost-user-scsi-obj-y) libvhost-user.a
> $(call LINK, $^)
> +vhost-user-blk$(EXESUF): $(vhost-user-blk-obj-y) libvhost-user.a
> + $(call LINK, $^)
>
> module_block.h: $(SRC_PATH)/scripts/modules/module_block.py config-host.mak
> $(call quiet-command,$(PYTHON) $< $@ \
> diff --git a/Makefile.objs b/Makefile.objs
> index 285c6f3..ae9aef7 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -115,6 +115,7 @@ libvhost-user-obj-y = contrib/libvhost-user/
> vhost-user-scsi.o-cflags := $(LIBISCSI_CFLAGS)
> vhost-user-scsi.o-libs := $(LIBISCSI_LIBS)
> vhost-user-scsi-obj-y = contrib/vhost-user-scsi/
> +vhost-user-blk-obj-y = contrib/vhost-user-blk/
>
> ##
> trace-events-subdirs =
> diff --git a/contrib/vhost-user-blk/Makefile.objs
> b/contrib/vhost-user-blk/Makefile.objs
> new file mode 100644
> index 000..72e2cdc
> --- /dev/null
> +++ b/contrib/vhost-user-blk/Makefile.objs
> @@ -0,0 +1 @@
> +vhost-user-blk-obj-y = vhost-user-blk.o
> diff --git a/contrib/vhost-user-blk/vhost-user-blk.c
> b/contrib/vhost-user-blk/vhost-user-blk.c
> new file mode 100644
> index 000..0b889fb
> --- /dev/null
> +++ b/contrib/vhost-user-blk/vhost-user-blk.c
> @@ -0,0 +1,545 @@
> +/*
> + * vhost-user-blk sample application
> + *
> + * Copyright (c) 2017 Intel Corporation. All rights reserved.
> + *
> + * Author:
> + * Changpeng Liu
> + *
> + * This work is based on the "vhost-user-scsi" sample and "virtio-blk" driver
> + * implementation by:
> + * Felipe Franciosi
> + * Anthony Liguori
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 only.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "standard-headers/linux/virtio_blk.h"
> +#include "contrib/libvhost-user/libvhost-user-glib.h"
> +#include "contrib/libvhost-user/libvhost-user.h"
> +
> +#include
> +
> +struct virtio_blk_inhdr {
> +unsigned char status;
> +};
> +
> +/* vhost user block device */
> +typedef struct VubDev {
> +VugDev parent;
> +int blk_fd;
> +struct virtio_blk_config blkcfg;
> +char *blk_name;
> +GMainLoop *loop;
> +} VubDev;
> +
> +typedef struct VubReq {
> +VuVirtqElement *elem;
> +int64_t sector_num;
> +size_t size;
> +struct virtio_blk_inhdr *in;
> +struct virtio_blk_outhdr *out;
> +VubDev *vdev_blk;
> +struct VuVirtq *vq;
> +} VubReq;
> +
> +/* refer util/iov.c */
> +static size_t vub_iov_size(const struct iovec *iov,
> + const unsigned int iov_cnt)
> +{
> +size_t len;
> +unsigned int i;
> +
> +len = 0;
> +for (i = 0; i < iov_cnt; i++) {
> +len += iov[i].iov_len;
> +}
> +return len;
> +}
> +
> +static void vub_panic_cb(VuDev *vu_dev, const char *buf)
> +{
> +VugDev *gdev;
> +VubDev *vdev_blk;
> +
> +assert(vu_dev);
> +
> +gdev = container_of(vu_dev, VugDev, parent);
> +vdev_blk = container_of(gdev, VubDev, parent);
> +if (buf) {
> +g_warning("vu_panic: %s", buf);
> +}
> +
> +g_main_loop_quit(vdev_blk->loop);
> +}
> +
> +static void vub_req_complete(VubReq *req)
> +{
> +VugDev *gdev = &req->vdev_blk->parent;
> +VuDev *vu_dev = &gdev->parent;
> +
> +
Re: [Qemu-devel] [PATCH v10 0/4] Introduce a new vhost-user-blk host device to QEMU
Hi On Thu, Jan 4, 2018 at 2:53 AM, Changpeng Liu wrote: > Although virtio scsi specification was designed as a replacement for > virtio_blk, > there are still many users using virtio_blk. QEMU 2.9 introduced a new device > vhost user scsi which can process I/O in user space for virtio_scsi, this > commit > introduces a new vhost user block host device, which can support virtio_blk in > Guest OS, and I/O processing in another I/O target. > > Due to the limitation for virtio_blk specification, virtio_blk device cannot > get > block information such as capacity, block size etc via the specification, > several > new vhost user messages were added to deliver virtio config space > information between Qemu and I/O target, > VHOST_USER_GET_CONFIG/VHOST_USER_SET_CONFIG > messages used for get/set config space from/to I/O target, > VHOST_USER_SLAVE_CONFIG_CHANGE_MSG > slave message was added for the event notifier in case the change of virtio > config space. Also, > those messages can be used for vhost device live migration as well. > > CHANGES: > v10: fix the code style error. > v8-v9: Several small optimization and code cleanup according to the comments. > v7-v8: Instead using an event file descriptor for event notifier in case of > virtio > configuration space changed, while here used a new vhost-user slave message > to deliver > such event. Several small optimizations to address the comments from v7. > v6-v7: change the parameter of set configuration function let it only contain > valid data buffer. > v5-v6: add header flags for vhost-user master so that the slave can know the > purpose for > set config, also vhost-user get/set messages' payload doesn't contain invalid > data buffers. > v4-v5: add header offset and size for virtio config space. > v3-v4: refactoring the vhost user block example patch based on new > libvhost-user library. > v2-v3: add new vhost user message to get/set virtio config space. > > Changpeng Liu (4): > vhost-user: add new vhost user messages to support virtio config space > vhost-user-blk: introduce a new vhost-user-blk host device > contrib/libvhost-user: enable virtio config space messages > contrib/vhost-user-blk: introduce a vhost-user-blk sample application Series: Reviewed-by: Marc-André Lureau Michael, do you take it? thanks > > .gitignore | 1 + > Makefile| 3 + > Makefile.objs | 1 + > contrib/libvhost-user/libvhost-user.c | 42 +++ > contrib/libvhost-user/libvhost-user.h | 33 ++ > contrib/vhost-user-blk/Makefile.objs| 1 + > contrib/vhost-user-blk/vhost-user-blk.c | 545 > > default-configs/pci.mak | 1 + > docs/interop/vhost-user.txt | 55 > hw/block/Makefile.objs | 3 + > hw/block/vhost-user-blk.c | 359 + > hw/virtio/vhost-user.c | 118 +++ > hw/virtio/vhost.c | 32 ++ > hw/virtio/virtio-pci.c | 55 > hw/virtio/virtio-pci.h | 18 ++ > include/hw/virtio/vhost-backend.h | 12 + > include/hw/virtio/vhost-user-blk.h | 41 +++ > include/hw/virtio/vhost.h | 15 + > 18 files changed, 1335 insertions(+) > create mode 100644 contrib/vhost-user-blk/Makefile.objs > create mode 100644 contrib/vhost-user-blk/vhost-user-blk.c > create mode 100644 hw/block/vhost-user-blk.c > create mode 100644 include/hw/virtio/vhost-user-blk.h > > -- > 1.9.3 > > -- Marc-André Lureau
Re: [Qemu-devel] MTTCG External Halt
Alistair Francis writes: > Hey guys, I'm super stuck with an ugly MTTCG issue and was wondering > if anyone had any ideas. > > In the Xilinx fork of QEMU (based on 2.11) we have a way for CPUs to > halt other CPUs. This is used for example when the power control unit > halts the ARM A53s. To do this we have internal GPIO signals that end > up calling a function that basically does this: > > To halt: > cpu->halted = true; > cpu_interrupt(cpu, CPU_INTERRUPT_HALT); Hmm I don't think you should be setting cpu->halted unless you know it is safe to do so. As the other CPUs free-run during BQL this isn't enough for a cross vCPU interaction. However you can schedule work to run in the target vCPUs context safely. That said isn't the cpu_interrupt enough to trigger the target vCPU to halt? > > To un-halt > cpu->halted = false; > cpu_reset_interrupt(cpu, CPU_INTERRUPT_HALT); Again if cross vCPU context this needs to be scheduled against the target vCPU. > > We also have the standard ARM WFI (Wait For Interrupt) implementation > in op_helper.c: > cs->halted = 1; > cs->exception_index = EXCP_HLT; > cpu_loop_exit(cs); > > Before MTTCG this used to work great, but now either we end up with > the guest Linux complaining about CPU stalls or we hit: > ERROR:/scratch/alistai/master-qemu/cpus.c:1516:qemu_tcg_cpu_thread_fn: > assertion failed: (cpu->halted) > > If I remove the instances of manually setting cpu->halted then I don't > see the asserts(), but the the WFI instruction doesn't work correctly. > So it seems like setting the halted status externally from the CPU > causes the issue. /* during start-up the vCPU is reset and the thread is * kicked several times. If we don't ensure we go back * to sleep in the halted state we won't cleanly * start-up when the vCPU is enabled. * * cpu->halted should ensure we sleep in wait_io_event */ I think what I'm trying to say is we should never be halted without having gone via wait_io_event where we can sleep. > I have tried setting it inside a lock, using atomic > operations and running the setter async on the CPU, but nothing works. > > Any chance any one has some insight into a way to externally set a > vCPU as halted/un-halted? See the PSCI code which uses the async interface for exactly this. > > Thanks, > Alistair -- Alex Bennée
Re: [Qemu-devel] vhost-user graceful connect/disconnect
On 01/04/2018 06:47 PM, Stefan Hajnoczi wrote: On Thu, Dec 21, 2017 at 06:01:29AM -0500, Marc-André Lureau wrote: I'm not going to prototype this yet, I'm working on virtio-vhost-user first, but eventually I might get back to -object vhost-user(-backend). Hi Stefan, are you implementing the guest slave and vhost-pci driver (we've posted to the dpdk mailinglist) as well? and do you have an estimation when would the prototype be ready? Best, Wei
Re: [Qemu-devel] [PATCH 1/5] docs/interop/prl-xml: description of Parallels Disk format
On Mon, Dec 18, 2017 at 02:09:07PM +0300, Denis V. Lunev wrote: > From: Klim Kireev > > This patch adds main information about Parallels Disk > format, which consists of DiskDescriptor.xml and other files. > > Signed-off-by: Edgar Kaziakhmedov > Signed-off-by: Klim Kireev > Signed-off-by: Vladimir Sementsov-Ogievskiy > Signed-off-by: Denis V. Lunev > CC: Stefan Hajnoczi > --- > docs/interop/prl-xml.txt | 155 > +++ > 1 file changed, 155 insertions(+) > create mode 100644 docs/interop/prl-xml.txt > > diff --git a/docs/interop/prl-xml.txt b/docs/interop/prl-xml.txt > new file mode 100644 > index 000..8ccb91a > --- /dev/null > +++ b/docs/interop/prl-xml.txt > @@ -0,0 +1,155 @@ > += License = > + > +Copyright (c) 2015 Denis Lunev > +Copyright (c) 2015 Vladimir Sementsov-Ogievskiy > +Copyright (c) 2016-2017 Klim Kireev > +Copyright (c) 2016-2017 Edgar Kaziakhmedov > + > +This work is licensed under the terms of the GNU GPL, version 2 or later. > +See the COPYING file in the top-level directory. > + > +This specification contains minimal information about Parallels Disk Format, > +which is enough to proper work with QEMU. Nevertheless, Parallels Cloud > Server > +and Parallels Desktop are able to add some unspecified nodes to xml and use > +them, but they are for internal work and don't affect functionality. Also it > +uses auxiliary xml "Snapshot.xml", which allows to store optional snapshot > +information, but it doesn't influence open/read/write functionality. QEMU and > +other software should not use unspecified here fields and Snapshot.xml file s/unspecified here fields/fields not covered in this document/ > +and must leave them as is. > + > += Parallels Disk Format = > + > +Parallels disk consists of two parts: the set of snapshots and the disk > +descriptor file, which stores information about all files and snapshots. > + > +== Definitions == > +Snapshot a record of the contents captured at a particular time, > + capable of storing current state. A snapshot has UUID and > + parent UUID. > + > + Snapshot imagean overlay representing the difference between this > + snapshot and some earlier snapshot > + > +Overlayan image storing the different sectors between two > captured > + states. > + > + Root image snapshot image without parent, the root of snapshot tree. s/without parent/with no parent/ > + > +Storagea special conception of data, which consists of disk > + parameters and a list of images. One of this image is > root, > + others are overlays. Images must be expandable (parallels > + image file), however root image could be expandable or > + plain. There may be more then one storage in the Parallels s/then/than/ > + disk and it is defined as a split image. I was having trouble understanding this paragraph. At this point I can begin to see that split images consist of multiple storages. That makes the concept of storage clearer. Perhaps rephrase this paragraph as: the backing storage for a subset of the virtual disk. When there is more than one storage in a Parallels disk then that is referred to as a split image. Each storage consists of disk parameters and a list of images. The list of images always contains a root image and may also contain overlays. The root image can be an expandable Parallels image file or plain. Overlays must be expandable. > + In this case every storage covers specific address > + space area of the disk and has its particular root image. > + Split images are not considered here and isn't supported s/isn't/aren't/ > + in QEMU. > + > + Description DiskDescriptor.xml stores information about disk > parameters, > + file snapshots, storages. > + > + Top The overlay between actual state and some previous > snapshot. > + SnapshotIt is not a snapshot in classical meaning. To make this clearer the last line could be: It is not a snapshot in the classical sense because it serves as the active image that the guest writes to. > + > +Sector a 512-byte data chunk. > + > +== Description file == > +All information is placed in single XML section Parallels_disk_image. > +The section has only one attribute "Version", that must be 1.0. > +General structure of DiskDescriptor.xml: > + > + > + > +... > + > + > +... > + > + > +... > + > + > + > +== Disk_Parameters section == > +The Disk_Parameters section describes the physical layout of the virtual disk > +and some general settings. > + > +The Disk_Parameters section MUST contain the following subsections: > +* Disk_size - number of sectors in the disk, > + desired
Re: [Qemu-devel] [PATCH] util/mmap-alloc: support MAP_SYNC in qemu_ram_mmap()
On Thu, Jan 04, 2018 at 09:23:28AM +0800, Haozhong Zhang wrote: > On 01/03/18 11:45 -0200, Eduardo Habkost wrote: > > On Wed, Jan 03, 2018 at 11:16:39AM +0800, Haozhong Zhang wrote: > > > On 01/02/18 18:02 +0200, Michael S. Tsirkin wrote: > > > > On Wed, Dec 27, 2017 at 02:56:20PM +0800, Haozhong Zhang wrote: > > > > > When a file supporting DAX is used as vNVDIMM backend, mmap it with > > > > > MAP_SYNC flag in addition can guarantee the persistence of guest write > > > > > to the backend file without other QEMU actions (e.g., periodic fsync() > > > > > by QEMU). > > > > > > > > > > By using MAP_SHARED_VALIDATE flag with MAP_SYNC, we can ensure mmap > > > > > with MAP_SYNC fails if MAP_SYNC is not supported by the kernel or the > > > > > backend file. On such failures, QEMU retries mmap without MAP_SYNC and > > > > > MAP_SHARED_VALIDATE. > > > > > > > > > > Signed-off-by: Haozhong Zhang > > > > > > > > If users rely on MAP_SYNC then don't you need to fail allocation > > > > if you can't use it? > > > > > > MAP_SYNC is supported since Linux kernel 4.15 and only needed for mmap > > > files on nvdimm. qemu_ram_mmap() has no way to check whether its > > > parameter 'fd' points to files on nvdimm, except by looking up > > > sysfs. However, accessing sysfs may be denied by certain SELinux > > > policies. > > > > > > The missing of MAP_SYNC should not affect the primary functionality of > > > vNVDIMM when using files on host nvdimm as backend, except the > > > guarantee of write persistence in case of qemu/host crash. > > > > > > We may check the kernel support of MAP_SYNC and the type of vNVDIMM > > > backend in some management utility (e.g., libvirt?), and deny to > > > launch QEMU if MAP_SYNC is not supported while files on host NVDIMM > > > are in use. > > > > Instead of making libvirt check if MAP_SYNC is supported and just > > hope it won't fail, it would be safer to let libvirt tell QEMU > > that MAP_SYNC must never fail. > > For example, add an option "sync" to memory-backend-file, and pass the > it to qemu_ram_mmap()? Yes. It could be a OnOffAuto option, "auto" would make QEMU try to use MAP_SYNC but not fail if it's unavailable. "on" would make QEMU ensure MAP_SYNC is really enabled. > > > > > However, it looks like kernel 4.14 won't even fail if MAP_SYNC is > > specified. How exactly can userspace detect if MAP_SYNC is > > really supported? > > Use MAP_SYNC with MAP_SHARED_VALIDATE (both introduced in 4.15 > kernel). Linux kernel 4.15 and later validate whether the MAP_SYNC is > supported. Because MAP_SHARED_VALIDATE is defined equally to > (MAP_SHARED | MAP_PRIVATE), it always fails on older kernels which do > not support MAP_SYNC as well. Nice. > > If we agree to introduce an option "sync" or likelihood, we can do the > above check in qemu_ram_mmap(). Sounds good to me. Thanks! -- Eduardo
[Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
The option has been marked as deprecated since QEMU 2.10, and so far
nobody complained that it is urgently required anymore. So let's now
get rid of this legacy pile, to simplify the usb code quite a bit.
Signed-off-by: Thomas Huth
---
hw/usb/Makefile.objs | 2 +-
hw/usb/bus.c | 98
hw/usb/dev-audio.c| 1 -
hw/usb/dev-bluetooth.c| 22 ---
hw/usb/dev-hid.c | 3 -
hw/usb/dev-network.c | 26
hw/usb/dev-serial.c | 45 -
hw/usb/dev-smartcard-reader.c | 1 -
hw/usb/dev-storage.c | 58 -
hw/usb/dev-wacom.c| 1 -
hw/usb/host-legacy.c | 144 --
include/hw/usb.h | 5 --
qemu-doc.texi | 8 ---
qemu-options.hx | 46 --
vl.c | 57 -
15 files changed, 1 insertion(+), 516 deletions(-)
delete mode 100644 hw/usb/host-legacy.c
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index bdfead6..fbcd498 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -43,7 +43,7 @@ redirect.o-libs = $(USB_REDIR_LIBS)
# usb pass-through
ifeq ($(CONFIG_USB_LIBUSB)$(CONFIG_USB),yy)
-common-obj-y += host-libusb.o host-legacy.o
+common-obj-y += host-libusb.o
else
common-obj-y += host-stub.o
endif
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 11f7720..0f1b16f 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -297,36 +297,6 @@ static void usb_qdev_unrealize(DeviceState *qdev, Error
**errp)
}
}
-typedef struct LegacyUSBFactory
-{
-const char *name;
-const char *usbdevice_name;
-USBDevice *(*usbdevice_init)(USBBus *bus, const char *params);
-} LegacyUSBFactory;
-
-static GSList *legacy_usb_factory;
-
-void usb_legacy_register(const char *typename, const char *usbdevice_name,
- USBDevice *(*usbdevice_init)(USBBus *bus,
- const char *params))
-{
-if (usbdevice_name) {
-LegacyUSBFactory *f = g_malloc0(sizeof(*f));
-f->name = typename;
-f->usbdevice_name = usbdevice_name;
-f->usbdevice_init = usbdevice_init;
-legacy_usb_factory = g_slist_append(legacy_usb_factory, f);
-}
-}
-
-USBDevice *usb_create(USBBus *bus, const char *name)
-{
-DeviceState *dev;
-
-dev = qdev_create(&bus->qbus, name);
-return USB_DEVICE(dev);
-}
-
static USBDevice *usb_try_create_simple(USBBus *bus, const char *name,
Error **errp)
{
@@ -654,74 +624,6 @@ void hmp_info_usb(Monitor *mon, const QDict *qdict)
}
}
-/* handle legacy -usbdevice cmd line option */
-USBDevice *usbdevice_create(const char *cmdline)
-{
-USBBus *bus = usb_bus_find(-1 /* any */);
-LegacyUSBFactory *f = NULL;
-Error *err = NULL;
-GSList *i;
-char driver[32];
-const char *params;
-int len;
-USBDevice *dev;
-
-params = strchr(cmdline,':');
-if (params) {
-params++;
-len = params - cmdline;
-if (len > sizeof(driver))
-len = sizeof(driver);
-pstrcpy(driver, len, cmdline);
-} else {
-params = "";
-pstrcpy(driver, sizeof(driver), cmdline);
-}
-
-for (i = legacy_usb_factory; i; i = i->next) {
-f = i->data;
-if (strcmp(f->usbdevice_name, driver) == 0) {
-break;
-}
-}
-if (i == NULL) {
-#if 0
-/* no error because some drivers are not converted (yet) */
-error_report("usbdevice %s not found", driver);
-#endif
-return NULL;
-}
-
-if (!bus) {
-error_report("Error: no usb bus to attach usbdevice %s, "
- "please try -machine usb=on and check that "
- "the machine model supports USB", driver);
-return NULL;
-}
-
-if (f->usbdevice_init) {
-dev = f->usbdevice_init(bus, params);
-} else {
-if (*params) {
-error_report("usbdevice %s accepts no params", driver);
-return NULL;
-}
-dev = usb_create(bus, f->name);
-}
-if (!dev) {
-error_report("Failed to create USB device '%s'", f->name);
-return NULL;
-}
-object_property_set_bool(OBJECT(dev), true, "realized", &err);
-if (err) {
-error_reportf_err(err, "Failed to initialize USB device '%s': ",
- f->name);
-object_unparent(OBJECT(dev));
-return NULL;
-}
-return dev;
-}
-
static bool usb_get_attached(Object *obj, Error **errp)
{
USBDevice *dev = USB_DEVICE(obj);
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index 3433452..ea4edd1 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -697,7 +697,6 @@ static const TypeInfo usb_audio_info = {
static void usb_audio_register_types(void)
{
type_register_sta
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
On 4 January 2018 at 12:27, Thomas Huth wrote: > The option has been marked as deprecated since QEMU 2.10, and so far > nobody complained that it is urgently required anymore. So let's now > get rid of this legacy pile, to simplify the usb code quite a bit. > > Signed-off-by: Thomas Huth The diffstat is nice, but can we retain the "please use $NEWOPTION instead of $OLDOPTION" message for a bit? I'm pretty sure there will be users out there who've been happily ignoring deprecation notices, and I think we'll reduce the amount of support traffic if they get a new QEMU with a specific error message telling them what they need to fix, rather than one which just prints a generic "unknown option" message. thanks -- PMM
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
On 04.01.2018 13:33, Peter Maydell wrote: > On 4 January 2018 at 12:27, Thomas Huth wrote: >> The option has been marked as deprecated since QEMU 2.10, and so far >> nobody complained that it is urgently required anymore. So let's now >> get rid of this legacy pile, to simplify the usb code quite a bit. >> >> Signed-off-by: Thomas Huth > > The diffstat is nice, but can we retain the "please use > $NEWOPTION instead of $OLDOPTION" message for a bit? > I'm pretty sure there will be users out there who've > been happily ignoring deprecation notices, and I think > we'll reduce the amount of support traffic if they > get a new QEMU with a specific error message telling them > what they need to fix, rather than one which just prints > a generic "unknown option" message. Sounds like a good idea at the first glance ... but at a second thought: Don't we confuse the management layers like libvirt this way, which might probe for the availability of this option by executing QEMU with this parameter? Thomas
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
On 4 January 2018 at 12:44, Thomas Huth wrote: > On 04.01.2018 13:33, Peter Maydell wrote: >> The diffstat is nice, but can we retain the "please use >> $NEWOPTION instead of $OLDOPTION" message for a bit? >> I'm pretty sure there will be users out there who've >> been happily ignoring deprecation notices, and I think >> we'll reduce the amount of support traffic if they >> get a new QEMU with a specific error message telling them >> what they need to fix, rather than one which just prints >> a generic "unknown option" message. > > Sounds like a good idea at the first glance ... but at a second thought: > Don't we confuse the management layers like libvirt this way, which > might probe for the availability of this option by executing QEMU with > this parameter? If they do (which I'd hope they don't) then QEMU will exit with a failure status code, which they ought to identify as "not supported". thanks -- PMM
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
On 04.01.2018 13:49, Peter Maydell wrote: > On 4 January 2018 at 12:44, Thomas Huth wrote: >> On 04.01.2018 13:33, Peter Maydell wrote: >>> The diffstat is nice, but can we retain the "please use >>> $NEWOPTION instead of $OLDOPTION" message for a bit? >>> I'm pretty sure there will be users out there who've >>> been happily ignoring deprecation notices, and I think >>> we'll reduce the amount of support traffic if they >>> get a new QEMU with a specific error message telling them >>> what they need to fix, rather than one which just prints >>> a generic "unknown option" message. >> >> Sounds like a good idea at the first glance ... but at a second thought: >> Don't we confuse the management layers like libvirt this way, which >> might probe for the availability of this option by executing QEMU with >> this parameter? > > If they do (which I'd hope they don't) then QEMU will > exit with a failure status code, which they ought to > identify as "not supported". OK, makes sense. I'll cook a v2 of the patch accordingly... Thomas
Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches
Nobody? Is this something they did on their own? Stefan Am 04.01.2018 um 07:27 schrieb Stefan Priebe - Profihost AG: > Hello, > > i've seen some vendors have updated qemu regarding meltdown / spectre. > > f.e.: > > CVE-2017-5715: QEMU was updated to allow passing through new MSR and > CPUID flags from the host VM to the CPU, to allow enabling/disabling > branch prediction features in the Intel CPU. (bsc#1068032) > > Can anybody point me to the relevant qemu patches? > > Thanks! > > Greets, > Stefan >
Re: [Qemu-devel] [PATCH v3 0/3] chardev: convert leftover glib APIs to use dedicate gcontext
On Thu, Jan 04, 2018 at 06:43:33PM +0800, Peter Xu wrote: > v3: > - rename function to qemu_chr_timeout_add_ms() [Stefan] > - add comment on return code [Stefan] > - add comment in commit message on why change GSource name [Stefan] > > v2: > - add r-bs > - fix patch 3 on some s->ms conversion [Marc-André] > > There were existing work that tried to allow chardev to be run in a > dedicated gcontext rather than the default main context/thread. > Basically that work passed in the correct gcontext during > g_source_attach(). However, I found something missing along the way, > that some legacy glib APIs are used by chardev code which take the > main context as default: > >g_timeout_add_seconds >g_timeout_add >g_idle_add Self NAK. I just noticed something more critical: these calls are still managing gsources using IDs rather than GSource objects. That should not work, especially if we want to remove these events using g_source_remove(). That can be fairly dangerous after OOB series merged. I need to use GSource objects to replace the tag IDs. I'll prepare another version tomorrow. Sorry for the troublesome. > > To fully allow the chardevs to be run in dedicated gcontext, we need > to convert all these legacy APIs into g_source_attach() calls as well, > with the correct gcontext passed in. > > This series tries to clean the rest of things up. > > I picked up patch 1 from monitor-oob series into this series (which is > a missing of chardev frontend call fix for g_source_attach()), so that > this series can be a complete fix. > > Please review. Thanks, > > Peter Xu (3): > chardev: use backend chr context when watch for fe > chardev: let g_idle_add() be with chardev gcontext > chardev: introduce qemu_chr_timeout_add_ms() > > chardev/char-fe.c | 2 +- > chardev/char-pty.c | 16 > chardev/char-socket.c | 6 -- > chardev/char.c | 21 + > hw/char/terminal3270.c | 7 --- > include/chardev/char.h | 3 +++ > 6 files changed, 41 insertions(+), 14 deletions(-) > > -- > 2.14.3 > -- Peter Xu
Re: [Qemu-devel] [PATCH v3 01/16] fdc: move object structures to header file
On 29/12/2017 16:29, Hervé Poussineau wrote:
We are now able to embed floppy controllers in another object.
Hi Hervé,
Are you sure we need to move all the struct definitions to the header file?
I looked at patch 11/16 and it seems only FDCtrlISABus definition is needed.
And also only the typedef is needed and not the fields.
It may worth a look also patches 2/16 and 3/16.
Thanks,
Marcel
Signed-off-by: Hervé Poussineau
---
hw/block/fdc.c | 102
include/hw/block/fdc.h | 103 +
2 files changed, 103 insertions(+), 102 deletions(-)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 7b7dd41296..c81e0313c8 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -60,15 +60,8 @@
#define TYPE_FLOPPY_BUS "floppy-bus"
#define FLOPPY_BUS(obj) OBJECT_CHECK(FloppyBus, (obj), TYPE_FLOPPY_BUS)
-typedef struct FDCtrl FDCtrl;
-typedef struct FDrive FDrive;
static FDrive *get_drv(FDCtrl *fdctrl, int unit);
-typedef struct FloppyBus {
-BusState bus;
-FDCtrl *fdc;
-} FloppyBus;
-
static const TypeInfo floppy_bus_info = {
.name = TYPE_FLOPPY_BUS,
.parent = TYPE_BUS,
@@ -178,36 +171,6 @@ static FDriveSize drive_size(FloppyDriveType drive)
#define FD_SECTOR_SC 2 /* Sector size code */
#define FD_RESET_SENSEI_COUNT 4 /* Number of sense interrupts on RESET */
-/* Floppy disk drive emulation */
-typedef enum FDiskFlags {
-FDISK_DBL_SIDES = 0x01,
-} FDiskFlags;
-
-struct FDrive {
-FDCtrl *fdctrl;
-BlockBackend *blk;
-BlockConf *conf;
-/* Drive status */
-FloppyDriveType drive;/* CMOS drive type*/
-uint8_t perpendicular;/* 2.88 MB access mode*/
-/* Position */
-uint8_t head;
-uint8_t track;
-uint8_t sect;
-/* Media */
-FloppyDriveType disk; /* Current disk type */
-FDiskFlags flags;
-uint8_t last_sect;/* Nb sector per track*/
-uint8_t max_track;/* Nb of tracks */
-uint16_t bps; /* Bytes per sector */
-uint8_t ro; /* Is read-only */
-uint8_t media_changed;/* Is media changed */
-uint8_t media_rate; /* Data rate of medium*/
-
-bool media_validated; /* Have we validated the media? */
-};
-
-
static FloppyDriveType get_fallback_drive_type(FDrive *drv);
/* Hack: FD_SEEK is expected to work on empty drives. However, QEMU
@@ -819,60 +782,6 @@ enum {
#define FD_MULTI_TRACK(state) ((state) & FD_STATE_MULTI)
#define FD_FORMAT_CMD(state) ((state) & FD_STATE_FORMAT)
-struct FDCtrl {
-MemoryRegion iomem;
-qemu_irq irq;
-/* Controller state */
-QEMUTimer *result_timer;
-int dma_chann;
-uint8_t phase;
-IsaDma *dma;
-/* Controller's identification */
-uint8_t version;
-/* HW */
-uint8_t sra;
-uint8_t srb;
-uint8_t dor;
-uint8_t dor_vmstate; /* only used as temp during vmstate */
-uint8_t tdr;
-uint8_t dsr;
-uint8_t msr;
-uint8_t cur_drv;
-uint8_t status0;
-uint8_t status1;
-uint8_t status2;
-/* Command FIFO */
-uint8_t *fifo;
-int32_t fifo_size;
-uint32_t data_pos;
-uint32_t data_len;
-uint8_t data_state;
-uint8_t data_dir;
-uint8_t eot; /* last wanted sector */
-/* States kept only to be returned back */
-/* precompensation */
-uint8_t precomp_trk;
-uint8_t config;
-uint8_t lock;
-/* Power down config (also with status regB access mode */
-uint8_t pwrd;
-/* Floppy drives */
-FloppyBus bus;
-uint8_t num_floppies;
-FDrive drives[MAX_FD];
-struct {
-BlockBackend *blk;
-FloppyDriveType type;
-} qdev_for_drives[MAX_FD];
-int reset_sensei;
-uint32_t check_media_rate;
-FloppyDriveType fallback; /* type=auto failure fallback */
-/* Timers state */
-uint8_t timer0;
-uint8_t timer1;
-PortioList portio_list;
-};
-
static FloppyDriveType get_fallback_drive_type(FDrive *drv)
{
return drv->fdctrl->fallback;
@@ -891,17 +800,6 @@ typedef struct FDCtrlSysBus {
#define ISA_FDC(obj) OBJECT_CHECK(FDCtrlISABus, (obj), TYPE_ISA_FDC)
-typedef struct FDCtrlISABus {
-ISADevice parent_obj;
-
-uint32_t iobase;
-uint32_t irq;
-uint32_t dma;
-struct FDCtrl state;
-int32_t bootindexA;
-int32_t bootindexB;
-} FDCtrlISABus;
-
static uint32_t fdctrl_read (void *opaque, uint32_t reg)
{
FDCtrl *fdctrl = opaque;
diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
index 1749dabf25..d076b2fc1a 100644
--- a/include/hw/block/fdc.h
+++ b/include/hw/block/fdc.h
@@ -2,12 +2,115 @@
#define HW_FDC_H
#include "qemu-common.h"
+#include "hw/block/block.h"
+#include "hw/isa/isa.h"
/* fdc.c */
#define MAX_FD 2
+typedef struct FDCtrl FDCtrl;
+
+/* Floppy disk drive emulation */
+typedef enum FDiskFlags {
+FDI
[Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
The option has been marked as deprecated since QEMU 2.10, and so far
nobody complained that it is urgently required anymore. So let's now
get rid of this legacy pile, to simplify the usb code quite a bit.
Signed-off-by: Thomas Huth
---
v2: Keep a stub in vl.c that still prints out an error message
with the hint to use -device instead.
hw/usb/Makefile.objs | 2 +-
hw/usb/bus.c | 98
hw/usb/dev-audio.c| 1 -
hw/usb/dev-bluetooth.c| 22 ---
hw/usb/dev-hid.c | 3 -
hw/usb/dev-network.c | 26
hw/usb/dev-serial.c | 45 -
hw/usb/dev-smartcard-reader.c | 1 -
hw/usb/dev-storage.c | 58 -
hw/usb/dev-wacom.c| 1 -
hw/usb/host-legacy.c | 144 --
include/hw/usb.h | 5 --
qemu-doc.texi | 8 ---
qemu-options.hx | 47 +-
vl.c | 57 +
15 files changed, 5 insertions(+), 513 deletions(-)
delete mode 100644 hw/usb/host-legacy.c
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index bdfead6..fbcd498 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -43,7 +43,7 @@ redirect.o-libs = $(USB_REDIR_LIBS)
# usb pass-through
ifeq ($(CONFIG_USB_LIBUSB)$(CONFIG_USB),yy)
-common-obj-y += host-libusb.o host-legacy.o
+common-obj-y += host-libusb.o
else
common-obj-y += host-stub.o
endif
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 11f7720..0f1b16f 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -297,36 +297,6 @@ static void usb_qdev_unrealize(DeviceState *qdev, Error
**errp)
}
}
-typedef struct LegacyUSBFactory
-{
-const char *name;
-const char *usbdevice_name;
-USBDevice *(*usbdevice_init)(USBBus *bus, const char *params);
-} LegacyUSBFactory;
-
-static GSList *legacy_usb_factory;
-
-void usb_legacy_register(const char *typename, const char *usbdevice_name,
- USBDevice *(*usbdevice_init)(USBBus *bus,
- const char *params))
-{
-if (usbdevice_name) {
-LegacyUSBFactory *f = g_malloc0(sizeof(*f));
-f->name = typename;
-f->usbdevice_name = usbdevice_name;
-f->usbdevice_init = usbdevice_init;
-legacy_usb_factory = g_slist_append(legacy_usb_factory, f);
-}
-}
-
-USBDevice *usb_create(USBBus *bus, const char *name)
-{
-DeviceState *dev;
-
-dev = qdev_create(&bus->qbus, name);
-return USB_DEVICE(dev);
-}
-
static USBDevice *usb_try_create_simple(USBBus *bus, const char *name,
Error **errp)
{
@@ -654,74 +624,6 @@ void hmp_info_usb(Monitor *mon, const QDict *qdict)
}
}
-/* handle legacy -usbdevice cmd line option */
-USBDevice *usbdevice_create(const char *cmdline)
-{
-USBBus *bus = usb_bus_find(-1 /* any */);
-LegacyUSBFactory *f = NULL;
-Error *err = NULL;
-GSList *i;
-char driver[32];
-const char *params;
-int len;
-USBDevice *dev;
-
-params = strchr(cmdline,':');
-if (params) {
-params++;
-len = params - cmdline;
-if (len > sizeof(driver))
-len = sizeof(driver);
-pstrcpy(driver, len, cmdline);
-} else {
-params = "";
-pstrcpy(driver, sizeof(driver), cmdline);
-}
-
-for (i = legacy_usb_factory; i; i = i->next) {
-f = i->data;
-if (strcmp(f->usbdevice_name, driver) == 0) {
-break;
-}
-}
-if (i == NULL) {
-#if 0
-/* no error because some drivers are not converted (yet) */
-error_report("usbdevice %s not found", driver);
-#endif
-return NULL;
-}
-
-if (!bus) {
-error_report("Error: no usb bus to attach usbdevice %s, "
- "please try -machine usb=on and check that "
- "the machine model supports USB", driver);
-return NULL;
-}
-
-if (f->usbdevice_init) {
-dev = f->usbdevice_init(bus, params);
-} else {
-if (*params) {
-error_report("usbdevice %s accepts no params", driver);
-return NULL;
-}
-dev = usb_create(bus, f->name);
-}
-if (!dev) {
-error_report("Failed to create USB device '%s'", f->name);
-return NULL;
-}
-object_property_set_bool(OBJECT(dev), true, "realized", &err);
-if (err) {
-error_reportf_err(err, "Failed to initialize USB device '%s': ",
- f->name);
-object_unparent(OBJECT(dev));
-return NULL;
-}
-return dev;
-}
-
static bool usb_get_attached(Object *obj, Error **errp)
{
USBDevice *dev = USB_DEVICE(obj);
diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index 3433452..ea4edd1 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -697,7 +697,6 @@ s
Re: [Qemu-devel] [PATCH 3/5] block/parallels: move some structures into header
On Mon, Dec 18, 2017 at 02:09:09PM +0300, Denis V. Lunev wrote: > +#ifndef BLOCK_PARALLELS_H > +#define BLOCK_PARALLELS_H > +#include "qemu/module.h" Does anything in this file require module.h? Also, the include for CoMutex is missing. Please make sure this header file includes necessary dependencies except for "qemu/osdep.h". signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 4/5] block/parallels: replace some magic numbers
On Mon, Dec 18, 2017 at 02:09:10PM +0300, Denis V. Lunev wrote: > From: Klim Kireev > > Signed-off-by: Klim Kireev > Reviewed-by: Vladimir Sementsov-Ogievskiy > Signed-off-by: Denis V. Lunev > CC: Stefan Hajnoczi > --- > block/parallels.h | 2 ++ > block/parallels.c | 5 +++-- > 2 files changed, 5 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH 5/5] block/parallels: add backing support to readv/writev
On Mon, Dec 18, 2017 at 02:09:11PM +0300, Denis V. Lunev wrote: > From: Edgar Kaziakhmedov > > Since parallels format supports backing files, refine > readv/writev (allocate_clusters) to redirect read/write requests > to a backing file (if cluster is not available in the current bs). > > Signed-off-by: Edgar Kaziakhmedov > Signed-off-by: Vladimir Sementsov-Ogievskiy > Signed-off-by: Denis V. Lunev > CC: Stefan Hajnoczi > --- > block/parallels.c | 50 -- > 1 file changed, 44 insertions(+), 6 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
Hello, I'm afraid I can't even work out how to replace -usbdevice braille (or -usbdevice serial, all questions below apply, except making chardev default to braille): ... -usbdevice braille -usbdevice braille: '-usbdevice' is deprecated, please use '-device usb-...' instead but -device usb-serial is not actually documented in the manual page (same for usb-keyboard, usb-tablet, etc. for instance I can't find in the manpage anything about replacement for -usbdevice host::) ... -device usb-braille -device usb-braille: No 'usb-bus' bus found for device 'usb-braille' ... -device usb-bus -device usb-braille -device usb-bus: 'usb-bus' is not a valid device model name So the error message above should suggest -usb. But then ... -usb -device usb-braille -device usb-braille: Property chardev is required that used to be a default, could we keep it a default when using -device? Hardly anybody would want to use usb-braille with something else than chardev braille. ... -usb -device usb-braille,chardev=braille -device usb-serial,chardev=braille: Property 'usb-serial.chardev' can't find value 'braille' Uh? It seems there is a parsing issue here? So I'm afraid a lot is still missing here. Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Thomas Huth, on jeu. 04 janv. 2018 14:15:52 +0100, wrote:
> -@item host:@var{bus}.@var{addr}
> -Pass through the host device identified by @var{bus}.@var{addr} (Linux only).
> -
> -@item host:@var{vendor_id}:@var{product_id}
> -Pass through the host device identified by @var{vendor_id}:@var{product_id}
> -(Linux only).
> -
> -@item
> serial:[vendorid=@var{vendor_id}][,productid=@var{product_id}]:@var{dev}
> -Serial converter to host character device @var{dev}, see @code{-serial} for
> the
> -available devices.
as mentioned in my other mail, I don't find any place in the manpage
where that is now documented. I believe this -usbdevice foo
documentation needs to be converted to -device usb-foo instead of
removed. Otherwise as it is now there is no way to even know that
-device usb-mouse, -device usb-disk, etc. are available at all.
Samuel
Re: [Qemu-devel] [PATCH v1 05/19] include/fpu/softfloat: add some float16 contants
Richard Henderson writes:
> On 12/15/2017 05:37 AM, Philippe Mathieu-Daudé wrote:
>> Hi Alex,
>>
>> On 12/11/2017 09:56 AM, Alex Bennée wrote:
>>> This defines the same set of common constants for float 16 as defined
>>> for 32 and 64 bit floats. These are often used by target helper
>>> functions.
>>>
>>> Signed-off-by: Alex Bennée
>>> ---
>>> include/fpu/softfloat.h | 7 +++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
>>> index 17dfe60dbd..5a9258c57c 100644
>>> --- a/include/fpu/softfloat.h
>>> +++ b/include/fpu/softfloat.h
>>> @@ -395,6 +395,13 @@ static inline float16 float16_set_sign(float16 a, int
>>> sign)
>>> return make_float16((float16_val(a) & 0x7fff) | (sign << 15));
>>> }
>>>
>>> +#define float16_zero make_float16(0)
>>
>> ok
>>
>>> +#define float16_one make_float16(0x3a00)
>>
>> I'm a bit confused...
>>
> [np.fromstring(struct.pack("> [0, 0x3a00, 0x34d1, 0x4448, 0x3800, 0x7a00]]
>> [0.0, 0.75, 0.30103, 4.2812, 0.5, 49152.0]
>>
>> However:
>>
> ['0x' + binascii.hexlify(np.array([x], '>f2').tostring()) for x in
>> [0, 1, math.log(2), np.pi, 0.5, np.inf]]
>> ['0x', '0x3c00', '0x398c', '0x4248', '0x3800', '0x7c00']
>>
>> It seems the MSB bit of the mantissa got shifted as the LSB of the
>> biased exponent...
>>
>>> +#define float16_ln2 make_float16(0x34d1)
>>
>> incorrect? 0x398c
>>
>>> +#define float16_pi make_float16(0x4448)
>>
>> incorrect? 0x4248
>>
>>> +#define float16_half make_float16(0x3800)
>>
>> ok
>>
>>> +#define float16_infinity make_float16(0x7a00)
>>
>> incorrect? 0x7c00
>
> All of Phil's numbers are correct -- I double-checked with gcc.
>
> Other than 0, 1 and +inf, I doubt any of the others will actually be used.
> Perhaps we should just leave them out?
I was following the other sizes. It seems it's x80 which uses the
additional ones in helpers:
floatx80_ln2
floatx80_pi
Should I delete all the unused constants from the other sizes as well?
--
Alex Bennée
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
On 04.01.2018 15:09, Samuel Thibault wrote:
> Thomas Huth, on jeu. 04 janv. 2018 14:15:52 +0100, wrote:
>> -@item host:@var{bus}.@var{addr}
>> -Pass through the host device identified by @var{bus}.@var{addr} (Linux
>> only).
>> -
>> -@item host:@var{vendor_id}:@var{product_id}
>> -Pass through the host device identified by @var{vendor_id}:@var{product_id}
>> -(Linux only).
>> -
>> -@item
>> serial:[vendorid=@var{vendor_id}][,productid=@var{product_id}]:@var{dev}
>> -Serial converter to host character device @var{dev}, see @code{-serial} for
>> the
>> -available devices.
>
> as mentioned in my other mail, I don't find any place in the manpage
> where that is now documented. I believe this -usbdevice foo
> documentation needs to be converted to -device usb-foo instead of
> removed. Otherwise as it is now there is no way to even know that
> -device usb-mouse, -device usb-disk, etc. are available at all.
I already did convert the chapter that describes the USB devices a
couple of months ago, you can find it in our qemu-doc.html here:
https://qemu.weilnetz.de/doc/qemu-doc.html#usb_005fdevices
Thomas
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Thomas Huth, on jeu. 04 janv. 2018 15:12:56 +0100, wrote:
> On 04.01.2018 15:09, Samuel Thibault wrote:
> > Thomas Huth, on jeu. 04 janv. 2018 14:15:52 +0100, wrote:
> >> -@item host:@var{bus}.@var{addr}
> >> -Pass through the host device identified by @var{bus}.@var{addr} (Linux
> >> only).
> >> -
> >> -@item host:@var{vendor_id}:@var{product_id}
> >> -Pass through the host device identified by
> >> @var{vendor_id}:@var{product_id}
> >> -(Linux only).
> >> -
> >> -@item
> >> serial:[vendorid=@var{vendor_id}][,productid=@var{product_id}]:@var{dev}
> >> -Serial converter to host character device @var{dev}, see @code{-serial}
> >> for the
> >> -available devices.
> >
> > as mentioned in my other mail, I don't find any place in the manpage
> > where that is now documented. I believe this -usbdevice foo
> > documentation needs to be converted to -device usb-foo instead of
> > removed. Otherwise as it is now there is no way to even know that
> > -device usb-mouse, -device usb-disk, etc. are available at all.
>
> I already did convert the chapter that describes the USB devices a
> couple of months ago, you can find it in our qemu-doc.html here:
>
> https://qemu.weilnetz.de/doc/qemu-doc.html#usb_005fdevices
Ah, ok, thanks.
I'm however still unable to make usb-serial and usb-braille work, as
mentioned in my other mail :)
Samuel
[Qemu-devel] [PATCH v4 0/3] chardev: convert leftover glib APIs to use dedicate gcontext
v4: - replace all timers from gsource tag IDs into gsource objects, otherwise g_source_remove() on that tag ID can errornously remove an gsource from default main context rather than the timer itself. v3: - rename function to qemu_chr_timeout_add_ms() [Stefan] - add comment on return code [Stefan] - add comment in commit message on why change GSource name [Stefan] v2: - add r-bs - fix patch 3 on some s->ms conversion [Marc-André] There were existing work that tried to allow chardev to be run in a dedicated gcontext rather than the default main context/thread. Basically that work passed in the correct gcontext during g_source_attach(). However, I found something missing along the way, that some legacy glib APIs are used by chardev code which take the main context as default: g_timeout_add_seconds g_timeout_add g_idle_add To fully allow the chardevs to be run in dedicated gcontext, we need to convert all these legacy APIs into g_source_attach() calls as well, with the correct gcontext passed in. This series tries to clean the rest of things up. I picked up patch 1 from monitor-oob series into this series (which is a missing of chardev frontend call fix for g_source_attach()), so that this series can be a complete fix. Please review. Thanks, Peter Xu (3): chardev: use backend chr context when watch for fe chardev: let g_idle_add() be with chardev gcontext chardev: introduce qemu_chr_timeout_add_ms() chardev/char-fe.c | 2 +- chardev/char-pty.c | 64 +- chardev/char-socket.c | 28 ++ chardev/char.c | 18 ++ hw/char/terminal3270.c | 28 -- include/chardev/char.h | 3 +++ 6 files changed, 88 insertions(+), 55 deletions(-) -- 2.14.3
[Qemu-devel] [PATCH v4 2/3] chardev: let g_idle_add() be with chardev gcontext
The idle task will be attached to main gcontext even if the chardev
backend is running in another gcontext. Fix the only caller by
extending the g_idle_add() logic into the more powerful
g_source_attach(). It's basically g_idle_add_full() implementation, but
with the chardev's gcontext passed in.
Signed-off-by: Peter Xu
---
chardev/char-pty.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 761ae6dec1..8248e36ab9 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -43,7 +43,7 @@ typedef struct {
/* Protected by the Chardev chr_write_lock. */
int connected;
guint timer_tag;
-guint open_tag;
+GSource *open_source;
} PtyChardev;
#define PTY_CHARDEV(obj) OBJECT_CHECK(PtyChardev, (obj), TYPE_CHARDEV_PTY)
@@ -58,7 +58,7 @@ static gboolean pty_chr_timer(gpointer opaque)
qemu_mutex_lock(&chr->chr_write_lock);
s->timer_tag = 0;
-s->open_tag = 0;
+s->open_source = NULL;
if (!s->connected) {
/* Next poll ... */
pty_chr_update_read_handler_locked(chr);
@@ -183,7 +183,7 @@ static gboolean qemu_chr_be_generic_open_func(gpointer
opaque)
Chardev *chr = CHARDEV(opaque);
PtyChardev *s = PTY_CHARDEV(opaque);
-s->open_tag = 0;
+s->open_source = NULL;
qemu_chr_be_event(chr, CHR_EVENT_OPENED);
return FALSE;
}
@@ -194,9 +194,10 @@ static void pty_chr_state(Chardev *chr, int connected)
PtyChardev *s = PTY_CHARDEV(chr);
if (!connected) {
-if (s->open_tag) {
-g_source_remove(s->open_tag);
-s->open_tag = 0;
+if (s->open_source) {
+g_source_destroy(s->open_source);
+g_source_unref(s->open_source);
+s->open_source = NULL;
}
remove_fd_in_watch(chr);
s->connected = 0;
@@ -210,9 +211,13 @@ static void pty_chr_state(Chardev *chr, int connected)
s->timer_tag = 0;
}
if (!s->connected) {
-g_assert(s->open_tag == 0);
+g_assert(s->open_source == NULL);
+s->open_source = g_idle_source_new();
s->connected = 1;
-s->open_tag = g_idle_add(qemu_chr_be_generic_open_func, chr);
+g_source_set_callback(s->open_source,
+ qemu_chr_be_generic_open_func,
+ chr, NULL);
+g_source_attach(s->open_source, chr->gcontext);
}
if (!chr->gsource) {
chr->gsource = io_add_watch_poll(chr, s->ioc,
--
2.14.3
[Qemu-devel] [PATCH v4 1/3] chardev: use backend chr context when watch for fe
In commit 6bbb6c0644 ("chardev: use per-dev context for
io_add_watch_poll", 2017-09-22) all the chardev watches are converted to
use per-chardev gcontext to support chardev to be run outside default
main thread. However that's still missing one call from the frontend
code. Touch that up.
Reviewed-by: Stefan Hajnoczi
Reviewed-by: Marc-André Lureau
Signed-off-by: Peter Xu
---
chardev/char-fe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index ee6d596100..c611b3fa3e 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -356,7 +356,7 @@ guint qemu_chr_fe_add_watch(CharBackend *be, GIOCondition
cond,
}
g_source_set_callback(src, (GSourceFunc)func, user_data, NULL);
-tag = g_source_attach(src, NULL);
+tag = g_source_attach(src, s->gcontext);
g_source_unref(src);
return tag;
--
2.14.3
[Qemu-devel] [PATCH v4 3/3] chardev: introduce qemu_chr_timeout_add_ms()
It's a replacement of g_timeout_add[_seconds]() for chardevs. Chardevs
now can have dedicated gcontext, we should always bind chardev tasks
onto those gcontext rather than the default main context. Since there
are quite a few of g_timeout_add[_seconds]() callers, a new function
qemu_chr_timeout_add_ms() is introduced.
One thing to mention is that, terminal3270 is still always running on
main gcontext. However let's convert that as well since it's still part
of chardev codes and in case one day we'll miss that when we move it out
of main gcontext too.
Also, convert all the timers from GSource tags into GSource pointers.
Gsource tag IDs and g_source_remove()s can only work with default
gcontext, while now these GSources can logically be attached to other
contexts. So let's use explicit g_source_destroy() plus another
g_source_unref() to remove a timer.
Note: when in the timer handler, we don't need the g_source_destroy()
any more since that'll be done automatically if the timer handler
returns false (and that's what all the current handlers do).
Yet another note: in pty_chr_rearm_timer() we take special care for
ms=1000. This patch merged the two cases into one.
Signed-off-by: Peter Xu
---
chardev/char-pty.c | 43 +++
chardev/char-socket.c | 28 ++--
chardev/char.c | 18 ++
hw/char/terminal3270.c | 28
include/chardev/char.h | 3 +++
5 files changed, 74 insertions(+), 46 deletions(-)
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 8248e36ab9..89315e6807 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -42,7 +42,7 @@ typedef struct {
/* Protected by the Chardev chr_write_lock. */
int connected;
-guint timer_tag;
+GSource *timer_src;
GSource *open_source;
} PtyChardev;
@@ -57,7 +57,8 @@ static gboolean pty_chr_timer(gpointer opaque)
PtyChardev *s = PTY_CHARDEV(opaque);
qemu_mutex_lock(&chr->chr_write_lock);
-s->timer_tag = 0;
+s->timer_src = NULL;
+g_source_unref(s->open_source);
s->open_source = NULL;
if (!s->connected) {
/* Next poll ... */
@@ -67,25 +68,25 @@ static gboolean pty_chr_timer(gpointer opaque)
return FALSE;
}
+static void pty_chr_timer_cancel(PtyChardev *s)
+{
+if (s->timer_src) {
+g_source_destroy(s->timer_src);
+g_source_unref(s->timer_src);
+s->timer_src = NULL;
+}
+}
+
/* Called with chr_write_lock held. */
static void pty_chr_rearm_timer(Chardev *chr, int ms)
{
PtyChardev *s = PTY_CHARDEV(chr);
char *name;
-if (s->timer_tag) {
-g_source_remove(s->timer_tag);
-s->timer_tag = 0;
-}
-
-if (ms == 1000) {
-name = g_strdup_printf("pty-timer-secs-%s", chr->label);
-s->timer_tag = g_timeout_add_seconds(1, pty_chr_timer, chr);
-} else {
-name = g_strdup_printf("pty-timer-ms-%s", chr->label);
-s->timer_tag = g_timeout_add(ms, pty_chr_timer, chr);
-}
-g_source_set_name_by_id(s->timer_tag, name);
+pty_chr_timer_cancel(s);
+name = g_strdup_printf("pty-timer-%s", chr->label);
+s->timer_src = qemu_chr_timeout_add_ms(chr, ms, pty_chr_timer, chr);
+g_source_set_name(s->timer_src, name);
g_free(name);
}
@@ -206,10 +207,7 @@ static void pty_chr_state(Chardev *chr, int connected)
* the virtual device linked to our pty. */
pty_chr_rearm_timer(chr, 1000);
} else {
-if (s->timer_tag) {
-g_source_remove(s->timer_tag);
-s->timer_tag = 0;
-}
+pty_chr_timer_cancel(s);
if (!s->connected) {
g_assert(s->open_source == NULL);
s->open_source = g_idle_source_new();
@@ -236,10 +234,7 @@ static void char_pty_finalize(Object *obj)
qemu_mutex_lock(&chr->chr_write_lock);
pty_chr_state(chr, 0);
object_unref(OBJECT(s->ioc));
-if (s->timer_tag) {
-g_source_remove(s->timer_tag);
-s->timer_tag = 0;
-}
+pty_chr_timer_cancel(s);
qemu_mutex_unlock(&chr->chr_write_lock);
qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
}
@@ -272,7 +267,7 @@ static void char_pty_open(Chardev *chr,
name = g_strdup_printf("chardev-pty-%s", chr->label);
qio_channel_set_name(QIO_CHANNEL(s->ioc), name);
g_free(name);
-s->timer_tag = 0;
+s->timer_src = NULL;
*be_opened = false;
}
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 630a7f2995..77cdf487eb 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -57,7 +57,7 @@ typedef struct {
bool is_telnet;
bool is_tn3270;
-guint reconnect_timer;
+GSource *reconnect_timer;
int64_t reconnect_time;
bool connect_err_reported;
} SocketChardev;
@@ -67,16 +67,27 @@ typedef struct {
static gboolean socket_reconnect_timeout(gpointer opaque);
+static void tcp_chr_reconn_timer_cancel(SocketChardev *s)
+{
+
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Samuel Thibault, on jeu. 04 janv. 2018 15:15:24 +0100, wrote: > I'm however still unable to make usb-serial and usb-braille work, as > mentioned in my other mail :) Ah, now with the documentation I understand, one has to also pass -chardev braille,chardev=foobar, and then -device usb-braille,chardev=foobar works. Would it be possible to make this automatic? Before I could tell people "use -usbdevice braille to test braille", and now it would be "use -chardev braille,chardev=foobar -device usb-braille,chardev=foobar" ... Samuel
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
On 04.01.2018 14:26, Samuel Thibault wrote: > Hello, > > I'm afraid I can't even work out how to replace -usbdevice braille (or > -usbdevice serial, all questions below apply, except making chardev > default to braille): > > ... -usbdevice braille > > -usbdevice braille: '-usbdevice' is deprecated, please use '-device usb-...' > instead > > but -device usb-serial is not actually documented in the manual page > (same for usb-keyboard, usb-tablet, etc. for instance I can't find in > the manpage anything about replacement for -usbdevice host::) As mentioned in my other mail, it's in the qemu-doc here: https://qemu.weilnetz.de/doc/qemu-doc.html#usb_005fdevices > But then > > ... -usb -device usb-braille > > -device usb-braille: Property chardev is required > > that used to be a default, could we keep it a default when using > -device? Hardly anybody would want to use usb-braille with something > else than chardev braille. I think it's more common to not do any magic default setup with "-device", but if you think this should be the case here, it can certainly be added again (not with this patch here, though, since this is a separate issue). > ... -usb -device usb-braille,chardev=braille > > -device usb-serial,chardev=braille: Property 'usb-serial.chardev' can't find > value 'braille' > > Uh? It seems there is a parsing issue here? Chardevs have to be specified with the "-chardev" parameter: ... -usb -device usb-braille,chardev=br0 -chardev braille,id=br0 HTH, Thomas
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
Thomas Huth, on jeu. 04 janv. 2018 15:22:25 +0100, wrote: > On 04.01.2018 14:26, Samuel Thibault wrote: > I think it's more common to not do any magic default setup with > "-device", but if you think this should be the case here, it can > certainly be added again Yes, please: it will almost never be used differently than with just a fresh braille chardev. Samuel
[Qemu-devel] [PATCH] scsi-disk: release AioContext in unaligned WRITE SAME case
scsi_write_same_complete() can retry the write if the request was
unaligned. Make sure to release the AioContext when that code path is
taken!
This patch fixes a hang when QEMU terminates after an unaligned WRITE
SAME request has been processed with dataplane. The hang occurs because
iothread_stop_all() cannot acquire the AioContext lock that was leaked
by the IOThread in scsi_write_same_complete().
Fixes: b9e413dd37
("block: explicitly acquire aiocontext in aio callbacks that need it").
Cc: Paolo Bonzini
Reported-by: Cong Li
Signed-off-by: Stefan Hajnoczi
---
hw/scsi/scsi-disk.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e58833a087..49d2559d93 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1755,6 +1755,7 @@ static void scsi_write_same_complete(void *opaque, int
ret)
data->sector << BDRV_SECTOR_BITS,
&data->qiov, 0,
scsi_write_same_complete, data);
+aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
return;
}
--
2.14.3
Re: [Qemu-devel] [PATCH] scsi-disk: release AioContext in unaligned WRITE SAME case
On 04/01/2018 15:25, Stefan Hajnoczi wrote:
> scsi_write_same_complete() can retry the write if the request was
> unaligned. Make sure to release the AioContext when that code path is
> taken!
>
> This patch fixes a hang when QEMU terminates after an unaligned WRITE
> SAME request has been processed with dataplane. The hang occurs because
> iothread_stop_all() cannot acquire the AioContext lock that was leaked
> by the IOThread in scsi_write_same_complete().
>
> Fixes: b9e413dd37
>("block: explicitly acquire aiocontext in aio callbacks that need it").
> Cc: Paolo Bonzini
> Reported-by: Cong Li
> Signed-off-by: Stefan Hajnoczi
> ---
> hw/scsi/scsi-disk.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
> index e58833a087..49d2559d93 100644
> --- a/hw/scsi/scsi-disk.c
> +++ b/hw/scsi/scsi-disk.c
> @@ -1755,6 +1755,7 @@ static void scsi_write_same_complete(void *opaque, int
> ret)
> data->sector << BDRV_SECTOR_BITS,
> &data->qiov, 0,
> scsi_write_same_complete, data);
> +aio_context_release(blk_get_aio_context(s->qdev.conf.blk));
> return;
> }
>
>
Queued,t hanks!
Paolo
Re: [Qemu-devel] [PATCH v4 0/3] chardev: convert leftover glib APIs to use dedicate gcontext
On 04/01/2018 15:18, Peter Xu wrote: > v4: > - replace all timers from gsource tag IDs into gsource objects, > otherwise g_source_remove() on that tag ID can errornously remove an > gsource from default main context rather than the timer itself. > > v3: > - rename function to qemu_chr_timeout_add_ms() [Stefan] > - add comment on return code [Stefan] > - add comment in commit message on why change GSource name [Stefan] > > v2: > - add r-bs > - fix patch 3 on some s->ms conversion [Marc-André] > > There were existing work that tried to allow chardev to be run in a > dedicated gcontext rather than the default main context/thread. > Basically that work passed in the correct gcontext during > g_source_attach(). However, I found something missing along the way, > that some legacy glib APIs are used by chardev code which take the > main context as default: > >g_timeout_add_seconds >g_timeout_add >g_idle_add > > To fully allow the chardevs to be run in dedicated gcontext, we need > to convert all these legacy APIs into g_source_attach() calls as well, > with the correct gcontext passed in. > > This series tries to clean the rest of things up. > > I picked up patch 1 from monitor-oob series into this series (which is > a missing of chardev frontend call fix for g_source_attach()), so that > this series can be a complete fix. > > Please review. Thanks, > > Peter Xu (3): > chardev: use backend chr context when watch for fe > chardev: let g_idle_add() be with chardev gcontext > chardev: introduce qemu_chr_timeout_add_ms() > > chardev/char-fe.c | 2 +- > chardev/char-pty.c | 64 > +- > chardev/char-socket.c | 28 ++ > chardev/char.c | 18 ++ > hw/char/terminal3270.c | 28 -- > include/chardev/char.h | 3 +++ > 6 files changed, 88 insertions(+), 55 deletions(-) > Queued, thanks. Paolo
Re: [Qemu-devel] [PATCH v3 04/16] mc146818rtc: always register rtc to rtc list
On 29/12/2017 16:29, Hervé Poussineau wrote:
We are not required anymore to use rtc_init() function.
Signed-off-by: Hervé Poussineau
---
hw/timer/mc146818rtc.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index 3e8c0b7d33..0b0da691cc 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -905,6 +905,13 @@ static void rtc_get_date(Object *obj, struct tm
*current_tm, Error **errp)
rtc_get_time(s, current_tm);
}
+static int rtc_initfn(DeviceState *dev)
+{
+RTCState *s = MC146818_RTC(dev);
+QLIST_INSERT_HEAD(&rtc_devices, s, link);
+return 0;
+}
+
static void rtc_realizefn(DeviceState *dev, Error **errp)
{
ISADevice *isadev = ISA_DEVICE(dev);
@@ -973,11 +980,9 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
qemu_irq intercept_irq)
{
DeviceState *dev;
ISADevice *isadev;
-RTCState *s;
isadev = isa_create(bus, TYPE_MC146818_RTC);
dev = DEVICE(isadev);
-s = MC146818_RTC(isadev);
qdev_prop_set_int32(dev, "base_year", base_year);
qdev_init_nofail(dev);
if (intercept_irq) {
@@ -985,7 +990,6 @@ ISADevice *mc146818_rtc_init(ISABus *bus, int base_year,
qemu_irq intercept_irq)
} else {
isa_connect_gpio_out(isadev, 0, RTC_ISA_IRQ);
}
-QLIST_INSERT_HEAD(&rtc_devices, s, link);
return isadev;
}
@@ -1012,12 +1016,11 @@ static void rtc_class_initfn(ObjectClass *klass, void
*data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+dc->init = rtc_initfn;
I am not sure you want to use dc->init function.
I think you need TypeInfo's instance_init.
Thanks,
Marcel
dc->realize = rtc_realizefn;
dc->reset = rtc_resetdev;
dc->vmsd = &vmstate_rtc;
dc->props = mc146818rtc_properties;
-/* Reason: needs to be wired up by rtc_init() */
-dc->user_creatable = false;
}
static void rtc_finalize(Object *obj)
Re: [Qemu-devel] [PATCH v3 05/16] piix4: rename some variables in realize function
On 29/12/2017 16:29, Hervé Poussineau wrote:
PIIX4 structure is now 's'
PCI device is now 'pci'
Please don't use 'pci'. Use pci_dev', the former is too wide,
Thanks,
Marcel
DeviceState is now 'dev'
Signed-off-by: Hervé Poussineau
---
hw/isa/piix4.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 6b8bc3faf0..4f476dc7e6 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -87,16 +87,17 @@ static const VMStateDescription vmstate_piix4 = {
}
};
-static void piix4_realize(PCIDevice *dev, Error **errp)
+static void piix4_realize(PCIDevice *pci, Error **errp)
{
-PIIX4State *d = PIIX4_PCI_DEVICE(dev);
+DeviceState *dev = DEVICE(pci);
+PIIX4State *s = DO_UPCAST(PIIX4State, dev, pci);
-if (!isa_bus_new(DEVICE(d), pci_address_space(dev),
- pci_address_space_io(dev), errp)) {
+if (!isa_bus_new(dev, pci_address_space(pci),
+ pci_address_space_io(pci), errp)) {
return;
}
-piix4_dev = &d->dev;
-qemu_register_reset(piix4_reset, d);
+piix4_dev = pci;
+qemu_register_reset(piix4_reset, s);
}
int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn)
[Qemu-devel] [RFC PATCH 0/6] qom: introduce TypeInfo name aliases
Hi, This RFC series is intended to simplify Flattened Device Tree support, in particular the 'compatible' FDT entry, when Linux names mismatches QEMU ones, but this is the same device modelled. Eventually this might help to remove the QDevAlias qdev_alias_table[] in qdev-monitor.c. So far this is only a 'proof of concept'. To see how the qtests perform, I only modified 3 devices, 2 used by the Xilinx Zynq machines (Cadence), and the e1000 (used by the PXE test). Regards, Phil. Philippe Mathieu-Daudé (6): qom: introduce TypeInfo name aliases hw/net/e1000: real device name is 'e1000-82540em', 'e1000' is an alias hw/char/cadence_uart: add FDT aliases arm/xlnx-zynq: use FDT names for the Cadence UART hw/net/cadence_gem: add FDT names as alias hw/arm/xlnx-zynq: use FDT names for the Cadence GEM include/qom/object.h | 3 +++ hw/arm/xilinx_zynq.c | 2 ++ hw/arm/xlnx-zynqmp.c | 4 ++-- hw/char/cadence_uart.c | 7 +++ hw/net/cadence_gem.c | 6 ++ hw/net/e1000.c | 5 - qom/object.c | 18 -- 7 files changed, 40 insertions(+), 5 deletions(-) -- 2.15.1
[Qemu-devel] [RFC PATCH 1/6] qom: introduce TypeInfo name aliases
Signed-off-by: Philippe Mathieu-Daudé
---
include/qom/object.h | 3 +++
qom/object.c | 18 --
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index dc73d59660..5d6e8795d4 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -428,6 +428,8 @@ struct Object
* TypeInfo:
* @name: The name of the type.
* @parent: The name of the parent type.
+ * @aliases: A list of alias names for the type. This should point to a static
+ * array that's terminated with a zero filled element.
* @instance_size: The size of the object (derivative of #Object). If
* @instance_size is 0, then the size of the object will be the size of the
* parent object.
@@ -468,6 +470,7 @@ struct TypeInfo
{
const char *name;
const char *parent;
+const char **aliases;
size_t instance_size;
void (*instance_init)(Object *obj);
diff --git a/qom/object.c b/qom/object.c
index c58c52d518..c532a8aa65 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -83,10 +83,15 @@ static GHashTable *type_table_get(void)
static bool enumerating_types;
-static void type_table_add(TypeImpl *ti)
+static void type_table_add_with_key(TypeImpl *ti, const char *key_name)
{
assert(!enumerating_types);
-g_hash_table_insert(type_table_get(), (void *)ti->name, ti);
+g_hash_table_insert(type_table_get(), (void *)key_name, ti);
+}
+
+static void type_table_add(TypeImpl *ti)
+{
+type_table_add_with_key(ti, ti->name);
}
static TypeImpl *type_table_lookup(const char *name)
@@ -137,6 +142,15 @@ static TypeImpl *type_register_internal(const TypeInfo
*info)
ti = type_new(info);
type_table_add(ti);
+
+if (info->aliases) {
+int i;
+
+for (i = 0; info->aliases[i]; i++) {
+type_table_add_with_key(ti, info->aliases[i]);
+}
+}
+
return ti;
}
--
2.15.1
[Qemu-devel] [RFC PATCH 6/6] hw/arm/xlnx-zynq: use FDT names for the Cadence GEM
Signed-off-by: Philippe Mathieu-Daudé
---
hw/arm/xilinx_zynq.c | 1 +
hw/arm/xlnx-zynqmp.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index c45c364583..5ae18921c1 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -245,6 +245,7 @@ static void zynq_init(MachineState *machine)
sysbus_create_varargs("cadence_ttc", 0xF8002000,
pic[69-IRQ_OFFSET], pic[70-IRQ_OFFSET], pic[71-IRQ_OFFSET], NULL);
+/* cdns,gem */
gem_init(&nd_table[0], 0xE000B000, pic[54-IRQ_OFFSET]);
gem_init(&nd_table[1], 0xE000C000, pic[77-IRQ_OFFSET]);
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 38f038786c..1aed89707c 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -150,7 +150,7 @@ static void xlnx_zynqmp_init(Object *obj)
qdev_set_parent_bus(DEVICE(&s->gic), sysbus_get_default());
for (i = 0; i < XLNX_ZYNQMP_NUM_GEMS; i++) {
-object_initialize(&s->gem[i], sizeof(s->gem[i]), TYPE_CADENCE_GEM);
+object_initialize(&s->gem[i], sizeof(s->gem[i]), "cdns,zynqmp-gem");
qdev_set_parent_bus(DEVICE(&s->gem[i]), sysbus_get_default());
}
--
2.15.1
[Qemu-devel] [RFC PATCH 5/6] hw/net/cadence_gem: add FDT names as alias
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/cadence_gem.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 3943187572..b7b4540d91 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1544,6 +1544,12 @@ static void gem_class_init(ObjectClass *klass, void
*data)
static const TypeInfo gem_info = {
.name = TYPE_CADENCE_GEM,
+.aliases = (const char * []) {
+"cdns,gem",
+"cdns,zynq-gem",/* Zynq-7xxx SoC */
+"cdns,zynqmp-gem", /* Zynq Ultrascale+ MPSoC */
+NULL
+ },
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(CadenceGEMState),
.instance_init = gem_init,
--
2.15.1
[Qemu-devel] [RFC PATCH 3/6] hw/char/cadence_uart: add FDT aliases
Signed-off-by: Philippe Mathieu-Daudé
---
hw/char/cadence_uart.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index 6143494060..6880827947 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -552,6 +552,13 @@ static void cadence_uart_class_init(ObjectClass *klass,
void *data)
static const TypeInfo cadence_uart_info = {
.name = TYPE_CADENCE_UART,
+.aliases = (const char * []) {
+"cdns,uart-r1p8",
+"xlnx,xuartps", /* Zynq-7xxx SoC */
+"cdns,uart-r1p12",
+"xlnx,zynqmp-uart", /* Zynq Ultrascale+ MPSoC */
+NULL
+ },
.parent= TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(CadenceUARTState),
.instance_init = cadence_uart_init,
--
2.15.1
[Qemu-devel] [RFC PATCH 2/6] hw/net/e1000: real device name is 'e1000-82540em', 'e1000' is an alias
Signed-off-by: Philippe Mathieu-Daudé
---
hw/net/e1000.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 05a00cba31..2280f7fdf9 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -1648,6 +1648,7 @@ typedef struct E1000Info {
uint16_t device_id;
uint8_trevision;
uint16_t phy_id2;
+const char **aliases;
} E1000Info;
static void e1000_class_init(ObjectClass *klass, void *data)
@@ -1695,10 +1696,11 @@ static const TypeInfo e1000_base_info = {
static const E1000Info e1000_devices[] = {
{
-.name = "e1000",
+.name = "e1000-82540em",
.device_id = E1000_DEV_ID_82540EM,
.revision = 0x03,
.phy_id2 = E1000_PHY_ID2_8254xx_DEFAULT,
+.aliases = (const char * []) {"e1000", NULL},
},
{
.name = "e1000-82544gc",
@@ -1725,6 +1727,7 @@ static void e1000_register_types(void)
type_info.name = info->name;
type_info.parent = TYPE_E1000_BASE;
+type_info.aliases = info->aliases;
type_info.class_data = (void *)info;
type_info.class_init = e1000_class_init;
type_info.instance_init = e1000_instance_init;
--
2.15.1
[Qemu-devel] [RFC PATCH 4/6] arm/xlnx-zynq: use FDT names for the Cadence UART
Signed-off-by: Philippe Mathieu-Daudé
---
hw/arm/xilinx_zynq.c | 1 +
hw/arm/xlnx-zynqmp.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 1836a4ed45..c45c364583 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -236,6 +236,7 @@ static void zynq_init(MachineState *machine)
sysbus_create_simple("xlnx,ps7-usb", 0xE0002000, pic[53-IRQ_OFFSET]);
sysbus_create_simple("xlnx,ps7-usb", 0xE0003000, pic[76-IRQ_OFFSET]);
+/* "xlnx,xuartps" */
cadence_uart_create(0xE000, pic[59 - IRQ_OFFSET], serial_hds[0]);
cadence_uart_create(0xE0001000, pic[82 - IRQ_OFFSET], serial_hds[1]);
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 325642058b..38f038786c 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -155,7 +155,7 @@ static void xlnx_zynqmp_init(Object *obj)
}
for (i = 0; i < XLNX_ZYNQMP_NUM_UARTS; i++) {
-object_initialize(&s->uart[i], sizeof(s->uart[i]), TYPE_CADENCE_UART);
+object_initialize(&s->uart[i], sizeof(s->uart[i]), "xlnx,zynqmp-uart");
qdev_set_parent_bus(DEVICE(&s->uart[i]), sysbus_get_default());
}
--
2.15.1
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
On 04.01.2018 15:23, Samuel Thibault wrote:
> Thomas Huth, on jeu. 04 janv. 2018 15:22:25 +0100, wrote:
>> On 04.01.2018 14:26, Samuel Thibault wrote:
>> I think it's more common to not do any magic default setup with
>> "-device", but if you think this should be the case here, it can
>> certainly be added again
>
> Yes, please: it will almost never be used differently than with just a
> fresh braille chardev.
It's pretty much untested, but would a patch like this help?
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -556,6 +556,19 @@ static const TypeInfo serial_info = {
.class_init= usb_serial_class_initfn,
};
+static void usb_braille_realize(USBDevice *dev, Error **errp)
+{
+USBSerialState *uss = USB_SERIAL_DEV(dev);
+
+if (!qemu_chr_fe_backend_connected(&uss->cs)) {
+/* Create a default braille chardev if user did not specify one */
+Chardev *cdrv = qemu_chr_new("braille", "braille");
+qdev_prop_set_chr(&dev->qdev, "chardev", cdrv);
+}
+
+usb_serial_realize(dev, errp);
+}
+
static Property braille_properties[] = {
DEFINE_PROP_CHR("chardev", USBSerialState, cs),
DEFINE_PROP_END_OF_LIST(),
@@ -568,6 +581,7 @@ static void usb_braille_class_initfn(ObjectClass *klass,
void *data)
uc->product_desc = "QEMU USB Braille";
uc->usb_desc = &desc_braille;
+uc->realize= usb_braille_realize;
dc->props = braille_properties;
}
Thomas
Re: [Qemu-devel] [PATCH v1 05/19] include/fpu/softfloat: add some float16 contants
On 01/04/2018 06:09 AM, Alex Bennée wrote: > I was following the other sizes. It seems it's x80 which uses the > additional ones in helpers: > > floatx80_ln2 > floatx80_pi > > Should I delete all the unused constants from the other sizes as well? Yeah, let's do that. r~
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
On 04/01/2018 15:21, Samuel Thibault wrote:
> Samuel Thibault, on jeu. 04 janv. 2018 15:15:24 +0100, wrote:
>> I'm however still unable to make usb-serial and usb-braille work, as
>> mentioned in my other mail :)
>
> Ah, now with the documentation I understand, one has to also pass
> -chardev braille,chardev=foobar, and then
> -device usb-braille,chardev=foobar works.
>
> Would it be possible to make this automatic? Before I could tell people
> "use -usbdevice braille to test braille", and now it would be
> "use -chardev braille,chardev=foobar -device usb-braille,chardev=foobar"
Maybe we can add "-braille" instead. Something like this:
diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 94b5c34afe..a5239b7fe7 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -13,6 +13,7 @@
#include "qemu-common.h"
#include "qemu/cutils.h"
#include "qemu/error-report.h"
+#include "sysemu/sysemu.h"
#include "hw/usb.h"
#include "hw/usb/desc.h"
#include "chardev/char-serial.h"
@@ -552,6 +553,24 @@ static USBDevice *usb_braille_init(USBBus *bus, const char
*unused)
return dev;
}
+void usb_braille_create(Error **errp)
+{
+USBBus *bus = usb_bus_find(-1 /* any */);
+USBDevice *dev = usb_braille_init(bus, "braille");
+Error *local_err = NULL;
+
+if (!dev) {
+error_setg(errp, "Failed to create USB device");
+return;
+}
+object_property_set_bool(OBJECT(dev), true, "realized", &local_err);
+if (local_err) {
+error_prepend(&local_err, "Failed to initialize USB device: ");
+error_propagate(errp, local_err);
+object_unparent(OBJECT(dev));
+}
+}
+
static const VMStateDescription vmstate_usb_serial = {
.name = "usb-serial",
.unmigratable = 1,
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index b4c10e62ba..eae3618735 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -167,6 +167,7 @@ extern Chardev *serial_hds[MAX_SERIAL_PORTS];
extern Chardev *parallel_hds[MAX_PARALLEL_PORTS];
void hmp_info_usb(Monitor *mon, const QDict *qdict);
+void usb_braille_create(Error **errp);
void add_boot_device_path(int32_t bootindex, DeviceState *dev,
const char *suffix);
diff --git a/qemu-options.hx b/qemu-options.hx
index 94647e21e3..92886cb777 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1214,6 +1214,16 @@ STEXI
Enable the USB driver (if it is not used by default yet).
ETEXI
+DEF("braille", QEMU_OPTION_braille,
+"-brailleadd a USB braille reader\n",
+QEMU_ARCH_ALL)
+STEXI
+@item -braille
+@findex -braille
+Add a USB Braille device. The device will use BrlAPI to display the braille
+output on a real or fake device.
+ETEXI
+
DEF("usbdevice", HAS_ARG, QEMU_OPTION_usbdevice,
"-usbdevice name add the host or guest USB device 'name'\n",
QEMU_ARCH_ALL)
diff --git a/roms/seabios b/roms/seabios
index 63451fca13..cd47172a67 16
--- a/roms/seabios
+++ b/roms/seabios
@@ -1 +1 @@
-Subproject commit 63451fca13c75870e1703eb3e20584d91179aebc
+Subproject commit cd47172a673762a05a0c7bd27df6e3cc8febe8d6
diff --git a/vl.c b/vl.c
index 5a8482a3a0..92d8775234 100644
--- a/vl.c
+++ b/vl.c
@@ -1488,6 +1488,17 @@ static int usb_parse(const char *cmdline)
return r;
}
+static int do_braille(const char *unused)
+{
+Error *err = NULL;
+usb_braille_create(&err);
+if (err) {
+error_report_err(err);
+return -1;
+}
+return 0;
+}
+
/***/
/* machine registration */
@@ -2495,6 +2506,7 @@ struct device_config {
DEV_DEBUGCON, /* -debugcon */
DEV_GDB, /* -gdb, -s */
DEV_SCLP, /* s390 sclp */
+DEV_BRAILLE, /* -braille */
} type;
const char *cmdline;
Location loc;
@@ -3411,6 +3423,9 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_no_fd_bootchk:
fd_bootchk = 0;
break;
+case QEMU_OPTION_braille:
+add_device_config(DEV_BRAILLE, "braille");
+break;
case QEMU_OPTION_netdev:
default_net = 0;
if (net_client_parse(qemu_find_opts("netdev"), optarg) == -1) {
@@ -4728,6 +4743,9 @@ int main(int argc, char **argv, char **envp)
if (foreach_device_config(DEV_USB, usb_parse) < 0)
exit(1);
}
+if (foreach_device_config(DEV_BRAILLE, do_braille) < 0) {
+exit(1);
+}
/* Check if IGD GFX passthrough. */
igd_gfx_passthru();
Untested, but it compiles (and it keeps syntactic sugar out of device code).
It still would let Thomas remove over 400 lines of code, and would cater
for the specific use case.
Paolo
Re: [Qemu-devel] [PATCH] usb: Remove legacy -usbdevice option
Hello, Thomas Huth, on jeu. 04 janv. 2018 16:01:24 +0100, wrote: > On 04.01.2018 15:23, Samuel Thibault wrote: > > Thomas Huth, on jeu. 04 janv. 2018 15:22:25 +0100, wrote: > >> On 04.01.2018 14:26, Samuel Thibault wrote: > >> I think it's more common to not do any magic default setup with > >> "-device", but if you think this should be the case here, it can > >> certainly be added again > > > > Yes, please: it will almost never be used differently than with just a > > fresh braille chardev. > > It's pretty much untested, but would a patch like this help? Completely! -usb -device usb-braille just work fine with it :) Thanks, Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Hello, Paolo Bonzini, on jeu. 04 janv. 2018 16:19:02 +0100, wrote: > On 04/01/2018 15:21, Samuel Thibault wrote: > > Samuel Thibault, on jeu. 04 janv. 2018 15:15:24 +0100, wrote: > >> I'm however still unable to make usb-serial and usb-braille work, as > >> mentioned in my other mail :) > > > > Ah, now with the documentation I understand, one has to also pass > > -chardev braille,chardev=foobar, and then > > -device usb-braille,chardev=foobar works. > > > > Would it be possible to make this automatic? Before I could tell people > > "use -usbdevice braille to test braille", and now it would be > > "use -chardev braille,chardev=foobar -device usb-braille,chardev=foobar" > > Maybe we can add "-braille" instead. I guess such legacy-looking option would be frowned up :) And anyway we need to be able to specify either -serial braille or -device usb-braille. Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
On 04/01/2018 16:24, Samuel Thibault wrote: >> Maybe we can add "-braille" instead. > I guess such legacy-looking option would be frowned up :) Not really. As long as it doesn't propagate everywhere in device code, it's fine. We are not going to deprecate "-serial mon:stdio", so... > And anyway we need to be able to specify either -serial braille or > -device usb-braille. So "-braille serial" and "-braille usb"? Paolo
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Paolo Bonzini, on jeu. 04 janv. 2018 16:28:27 +0100, wrote: > On 04/01/2018 16:24, Samuel Thibault wrote: > >> Maybe we can add "-braille" instead. > > I guess such legacy-looking option would be frowned up :) > > Not really. As long as it doesn't propagate everywhere in device code, > it's fine. We are not going to deprecate "-serial mon:stdio", so... > > > And anyway we need to be able to specify either -serial braille or > > -device usb-braille. > > So "-braille serial" and "-braille usb"? Well, why not, it just doesn't seem to me much simpler than "-serial braille" and "-device usb-braille" :) Samuel
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
On 04/01/2018 16:32, Samuel Thibault wrote: >>> And anyway we need to be able to specify either -serial braille or >>> -device usb-braille. >> So "-braille serial" and "-braille usb"? > Well, why not, it just doesn't seem to me much simpler than > "-serial braille" and "-device usb-braille" :) The difference is that "-device usb-braille" would have magic character backend creation in usb_braille_realize. *That* is what I really frown upon. Paolo
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
On 04.01.2018 16:32, Samuel Thibault wrote: > Paolo Bonzini, on jeu. 04 janv. 2018 16:28:27 +0100, wrote: >> On 04/01/2018 16:24, Samuel Thibault wrote: Maybe we can add "-braille" instead. >>> I guess such legacy-looking option would be frowned up :) >> >> Not really. As long as it doesn't propagate everywhere in device code, >> it's fine. We are not going to deprecate "-serial mon:stdio", so... >> >>> And anyway we need to be able to specify either -serial braille or >>> -device usb-braille. >> >> So "-braille serial" and "-braille usb"? > > Well, why not, it just doesn't seem to me much simpler than > "-serial braille" and "-device usb-braille" :) FWIW, I think I'd also rather prefer to add some little bit of "magic" to "-device usb-braille" instead of introducing yet another command line parameter. QEMU's command line interface is already way too much overcrowded, so IMHO we should try to avoid new parameters if possible. Thomas
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
On 04.01.2018 16:28, Paolo Bonzini wrote: > On 04/01/2018 16:24, Samuel Thibault wrote: >>> Maybe we can add "-braille" instead. >> I guess such legacy-looking option would be frowned up :) > > Not really. As long as it doesn't propagate everywhere in device code, > it's fine. We are not going to deprecate "-serial mon:stdio", so... Hmm, maybe we could also introduce a "-serial usb:braille" command that automagically also creates an usb-braille device, to avoid to introduce a new "-braille" command? It's bending the meaning of "-serial" quite a bit, though... Thomas
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
On 04/01/2018 16:35, Thomas Huth wrote: >> Well, why not, it just doesn't seem to me much simpler than >> "-serial braille" and "-device usb-braille" :) > > FWIW, I think I'd also rather prefer to add some little bit of "magic" > to "-device usb-braille" instead of introducing yet another command line > parameter. QEMU's command line interface is already way too much > overcrowded, so IMHO we should try to avoid new parameters if possible. The point of deprecation is not to make the interface simpler, it is to avoid cases where one option is doing too much and/or crossing abstraction boundaries, for example -net creating both a device and a hub port. "-serial" is okay because it only creates the front-end, it's the board that decides how to attach it to the back-end. -usbdevice creating both a front-end and a back-end is not a problem per se. The issue with -usbdevice is just that it's too much code. However, adding magic to "-device usb-braille" that creates both a front-end and a back-end is completely the opposite of sane... Paolo
Re: [Qemu-devel] [PATCH v3 06/16] piix4: add Reset Control Register
On 29/12/2017 16:29, Hervé Poussineau wrote:
The RCR I/O port (0xcf9) is used to generate a hard reset or a soft reset.
Signed-off-by: Hervé Poussineau
---
hw/isa/piix4.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 4f476dc7e6..7c83e7c23d 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -2,6 +2,7 @@
* QEMU PIIX4 PCI Bridge Emulation
*
* Copyright (c) 2006 Fabrice Bellard
+ * Copyright (c) 2016 Hervé Poussineau
I think 2018 :)
*
* Permission is hereby granted, free of charge, to any person obtaining a
copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -33,6 +34,10 @@ PCIDevice *piix4_dev;
typedef struct PIIX4State {
PCIDevice dev;
+
+/* Reset Control Register */
+MemoryRegion rcr_mem;
+uint8_t rcr;
} PIIX4State;
#define TYPE_PIIX4_PCI_DEVICE "PIIX4"
@@ -87,6 +92,30 @@ static const VMStateDescription vmstate_piix4 = {
}
};
+static void piix4_rcr_write(void *opaque, hwaddr addr, uint64_t val,
+unsigned int len)
+{
+PIIX4State *s = opaque;
+
+if (val & 4) {
+qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
+return;
+}
+s->rcr = val & 2; /* keep System Reset type only */
How does it work? When do we reset the value?
+}
+
+static uint64_t piix4_rcr_read(void *opaque, hwaddr addr, unsigned int len)
+{
+PIIX4State *s = opaque;
+return s->rcr;
+}
+
+static const MemoryRegionOps piix4_rcr_ops = {
+.read = piix4_rcr_read,
+.write = piix4_rcr_write,
+.endianness = DEVICE_LITTLE_ENDIAN
Maybe we need to specify the access_size.
Something like:
.impl = {
.min_access_size = 1,
.max_access_size = 1,
},
Thanks,
Marcel
+};
+
static void piix4_realize(PCIDevice *pci, Error **errp)
{
DeviceState *dev = DEVICE(pci);
@@ -96,6 +125,12 @@ static void piix4_realize(PCIDevice *pci, Error **errp)
pci_address_space_io(pci), errp)) {
return;
}
+
+memory_region_init_io(&s->rcr_mem, OBJECT(dev), &piix4_rcr_ops, s,
+ "reset-control", 1);
+memory_region_add_subregion_overlap(pci_address_space_io(pci), 0xcf9,
+&s->rcr_mem, 1);
+
piix4_dev = pci;
qemu_register_reset(piix4_reset, s);
}
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
On 4 January 2018 at 15:47, Paolo Bonzini wrote: > The point of deprecation is not to make the interface simpler, it is to > avoid cases where one option is doing too much and/or crossing > abstraction boundaries, for example -net creating both a device and a > hub port. > > "-serial" is okay because it only creates the front-end, it's the board > that decides how to attach it to the back-end. > > -usbdevice creating both a front-end and a back-end is not a problem per > se. The issue with -usbdevice is just that it's too much code. > > However, adding magic to "-device usb-braille" that creates both a > front-end and a back-end is completely the opposite of sane... Agreed with this. Is there any mileage in considering the idea of a generic bit in the option-parsing code that has a table of mappings from legacy-style-simple-option to complex-set-of-device-etc options, so we can retain userfriendliness without per-kind-of-thing special casing, or is that too simplistic to work ? thanks -- PMM
Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches
On 04/01/2018 09:35, Alexandre DERUMIER wrote: >>> So you need: >>> 1.) intel / amd cpu microcode update >>> 2.) qemu update to pass the new MSR and CPU flags from the microcode update >>> 3.) host kernel update >>> 4.) guest kernel update > > are you sure we need to patch guest kernel if we are able to patch qemu ? Patching the guest kernel is only required to protect the guest kernel from guest usermode. > If I understand, patching the host kernel, should avoid that a vm is reading > memory of another vm. > (the most critical) Correct. > patching the guest kernel, to avoid that a process from the vm have access to > memory of another process of same vm. Correct. The QEMU updates are pretty boring, mostly taking care of new MSR and CPUID flags (and adding new CPU models). They are not needed to protect the guest from "Meltdown", only "Spectre"---the former only needs a guest kernel update. Also, to have any effect, the guest kernels must also have "Spectre" patches which aren't upstream yet for either KVM or the rest of Linux. So the QEMU patches are much less important than the kernel side. >> https://access.redhat.com/solutions/3307851 >> "Impacts of CVE-2017-5754, CVE-2017-5753, and CVE-2017-5715 to Red Hat >> Virtualization products" It mostly repeats the contents of the RHEL document https://access.redhat.com/security/vulnerabilities/speculativeexecution, with some information specific to RHV. Thanks, Paolo > i don't have one but the content might be something like this: > https://www.suse.com/de-de/support/kb/doc/?id=7022512 > > So you need: > 1.) intel / amd cpu microcode update > 2.) qemu update to pass the new MSR and CPU flags from the microcode update > 3.) host kernel update > 4.) guest kernel update > > The microcode update and the kernel update is publicly available but i'm > missing the qemu one. > > Greets, > Stefan > >> - Mail original - >> De: "aderumier" >> À: "Stefan Priebe, Profihost AG" >> Cc: "qemu-devel" >> Envoyé: Jeudi 4 Janvier 2018 08:24:34 >> Objet: Re: [Qemu-devel] CVE-2017-5715: relevant qemu patches >> Can anybody point me to the relevant qemu patches? >> >> I don't have find them yet. >> >> Do you known if a vm using kvm64 cpu model is protected or not ? >> >> - Mail original - >> De: "Stefan Priebe, Profihost AG" >> À: "qemu-devel" >> Envoyé: Jeudi 4 Janvier 2018 07:27:01 >> Objet: [Qemu-devel] CVE-2017-5715: relevant qemu patches >> >> Hello, >> >> i've seen some vendors have updated qemu regarding meltdown / spectre. >> >> f.e.: >> >> CVE-2017-5715: QEMU was updated to allow passing through new MSR and >> CPUID flags from the host VM to the CPU, to allow enabling/disabling >> branch prediction features in the Intel CPU. (bsc#1068032) >> >> Can anybody point me to the relevant qemu patches? >> >> Thanks! >> >> Greets, >> Stefan >> > >
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
Paolo Bonzini, on jeu. 04 janv. 2018 16:47:16 +0100, wrote: > The point of deprecation is not to make the interface simpler, it is to > avoid cases where one option is doing too much and/or crossing > abstraction boundaries, for example -net creating both a device and a > hub port. > > "-serial" is okay because it only creates the front-end, it's the board > that decides how to attach it to the back-end. > > -usbdevice creating both a front-end and a back-end is not a problem per > se. The issue with -usbdevice is just that it's too much code. > > However, adding magic to "-device usb-braille" that creates both a > front-end and a back-end is completely the opposite of sane... Well, this is also what happens with -device usb-mouse, usb-kbd etc.: they also plug with keyboard & mouse pipes of the qemu graphical frontend. braille is just the same vein for the user. Samuel
Re: [Qemu-devel] [PATCH] TPM: add CRB device
On 12/22/2017 09:55 AM, Marc-André Lureau wrote:
tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB)
Interface as defined in TCG PC Client Platform TPM Profile (PTP)
Specification Family “2.0” Level 00 Revision 01.03 v22.
The PTP allows device implementation to switch between TIS and CRB
model at run time, but given that CRB is a simpler device to
implement, I chose to implement it as a different device.
The device doesn't implement other locality than 0 for now (my laptop
TPM doesn't either, so I assume this isn't so bad)
The command/reply memory region is statically allocated after the CRB
registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I
wonder if the BIOS could or should allocate it instead, or what size
to use, again this seems to fit well expectations)
The PTP doesn't specify a particular bus to put the device. So I added
it on the system bus directly, so it could hopefully be used easily on
a different platform than x86. Currently, it fails to init on piix,
because error_on_sysbus_device() check. The check may be changed in a
near future, see discussion on the qemu-devel ML.
Tested with some success with Linux upstream and Windows 10, seabios &
modified ovmf. The device is recognized and correctly transmit
command/response with passthrough & emu.
When you say 'some success', you mean that a test suite wasn't passing
all tests, right? I didn't run the test suite so far, but what I tested
looks good to me so far.
We have one ACPI part missing in the implementation and that's this
mailbox thing that allows one to send a code to the firmware that the
firmware then reads and acts upon and reconfigures the TPM during the
next warm-boot.
https://trustedcomputinggroup.org/wp-content/uploads/Physical-Presence-Interface_1-30_0-52.pdf
It was challenging to implement this interface 'back then' using ACPI's
AML, but now it's even more low level with the programming interface.
I know we have to wait for that other patch. Nevertheless ...
Signed-off-by: Marc-André Lureau
Signed-off-by: Stefan Berger
---
qapi/tpm.json | 5 +-
include/hw/acpi/tpm.h | 72
include/sysemu/tpm.h | 3 +
hw/i386/acpi-build.c | 34 +++-
hw/tpm/tpm_crb.c | 325 +
default-configs/i386-softmmu.mak | 1 +
default-configs/x86_64-softmmu.mak | 1 +
hw/tpm/Makefile.objs | 1 +
8 files changed, 432 insertions(+), 10 deletions(-)
create mode 100644 hw/tpm/tpm_crb.c
diff --git a/qapi/tpm.json b/qapi/tpm.json
index 7093f268fb..d50deef5e9 100644
--- a/qapi/tpm.json
+++ b/qapi/tpm.json
@@ -11,10 +11,11 @@
# An enumeration of TPM models
#
# @tpm-tis: TPM TIS model
+# @tpm-crb: TPM CRB model (since 2.12)
#
# Since: 1.5
##
-{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] }
+{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] }
##
# @query-tpm-models:
@@ -28,7 +29,7 @@
# Example:
#
# -> { "execute": "query-tpm-models" }
-# <- { "return": [ "tpm-tis" ] }
+# <- { "return": [ "tpm-tis", "tpm-crb" ] }
#
##
{ 'command': 'query-tpm-models', 'returns': ['TpmModel'] }
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 6d516c6a7f..b0048515fa 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -16,11 +16,82 @@
#ifndef HW_ACPI_TPM_H
#define HW_ACPI_TPM_H
+#include "qemu/osdep.h"
+
#define TPM_TIS_ADDR_BASE 0xFED4
#define TPM_TIS_ADDR_SIZE 0x5000
#define TPM_TIS_IRQ 5
+struct crb_regs {
+union {
+uint32_t reg;
+struct {
+unsigned tpm_established:1;
+unsigned loc_assigned:1;
+unsigned active_locality:3;
+unsigned reserved:2;
+unsigned tpm_reg_valid_sts:1;
+} bits;
+} loc_state;
+uint32_t reserved1;
+uint32_t loc_ctrl;
+union {
+uint32_t reg;
+struct {
+unsigned granted:1;
+unsigned been_seized:1;
+} bits;
+} loc_sts;
+uint8_t reserved2[32];
+union {
+uint64_t reg;
+struct {
+unsigned type:4;
+unsigned version:4;
+unsigned cap_locality:1;
+unsigned cap_crb_idle_bypass:1;
+unsigned reserved1:1;
+unsigned cap_data_xfer_size_support:2;
+unsigned cap_fifo:1;
+unsigned cap_crb:1;
+unsigned cap_if_res:2;
+unsigned if_selector:2;
+unsigned if_selector_lock:1;
+unsigned reserved2:4;
+unsigned rid:8;
+unsigned vid:16;
+unsigned did:16;
+} bits;
+} intf_id;
+uint64_t ctrl_ext;
+
+uint32_t ctrl_req;
+union {
+uint32_t reg;
+struct {
+unsigned tpm_sts:1;
+unsigned tpm_idle:1;
+unsigned reserved:30;
+} bits;
+
Re: [Qemu-devel] [PATCH v2] usb: Remove legacy -usbdevice option
On 04.01.2018 16:47, Paolo Bonzini wrote: > On 04/01/2018 16:35, Thomas Huth wrote: >>> Well, why not, it just doesn't seem to me much simpler than >>> "-serial braille" and "-device usb-braille" :) >> >> FWIW, I think I'd also rather prefer to add some little bit of "magic" >> to "-device usb-braille" instead of introducing yet another command line >> parameter. QEMU's command line interface is already way too much >> overcrowded, so IMHO we should try to avoid new parameters if possible. > > The point of deprecation is not to make the interface simpler, it is to > avoid cases where one option is doing too much and/or crossing > abstraction boundaries, for example -net creating both a device and a > hub port. > > "-serial" is okay because it only creates the front-end, it's the board > that decides how to attach it to the back-end. > > -usbdevice creating both a front-end and a back-end is not a problem per > se. The issue with -usbdevice is just that it's too much code. > > However, adding magic to "-device usb-braille" that creates both a > front-end and a back-end is completely the opposite of sane... Ok, makes sense. But instead of introducing a new "-braille" parameter, maybe we should rather keep some of the convenience "-usbdevice" possibilities around? E.g. keep "-usbdevice braille", "-usbdevice mouse", etc. but remove things like "-usbdevice serial" and "-usbdevice host" where the code is rather ugly and the user do not gain much in comparison to "-device" ? Thomas
[Qemu-devel] [Bug 1716028] Re: qemu 2.10 locks images with no feature flag
Libvirt will make this "easier" to be avoided by commit 28907b0043fbf71085a798372ab9c816ba043b93 Author: Peter Krempa Date: Wed Nov 15 15:21:14 2017 +0100 qemu: command: Mark disks as such in qemu Qemu has now an internal mechanism for locking images to fix specific cases of disk corruption. This requires libvirt to mark the image as shared so that qemu lifts certain restrictions. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1378242 commit 860a3c4bea1d24773d8a495f213d5de3ac48a462 Author: Peter Krempa Date: Wed Nov 15 15:02:58 2017 +0100 qemu: caps: Add capability for 'share-rw' disk option 'share-rw' for the disk device configures qemu to allow concurrent access to the backing storage. The capability is checked in various supported disk frontend buses since it does not make sense to partially backport it. Once this is complete in bionic by taking the new upstream I'll take a look at how backportable that is (changes look ok, but might need some testing in the SRU spirit of things). ** Also affects: qemu (Ubuntu Artful) Importance: Undecided Status: New ** Also affects: libvirt (Ubuntu Artful) Importance: Undecided Status: New ** Changed in: libvirt (Ubuntu Artful) Status: New => Triaged ** Changed in: libvirt (Ubuntu Artful) Importance: Undecided => Low ** Bug watch added: Red Hat Bugzilla #1378242 https://bugzilla.redhat.com/show_bug.cgi?id=1378242 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1716028 Title: qemu 2.10 locks images with no feature flag Status in QEMU: Opinion Status in libvirt package in Ubuntu: Triaged Status in qemu package in Ubuntu: Opinion Status in libvirt source package in Artful: Triaged Status in qemu source package in Artful: New Bug description: 1) % lsb_release -rd Description: Ubuntu Artful Aardvark (development branch) Release: 17.10 2) % apt-cache policy qemu-system-x86 qemu-system-x86: Installed: 1:2.10~rc3+dfsg-0ubuntu1 Candidate: 1:2.10+dfsg-0ubuntu1 Version table: 1:2.10+dfsg-0ubuntu1 500 500 http://archive.ubuntu.com//ubuntu devel/main amd64 Packages *** 1:2.10~rc3+dfsg-0ubuntu1 100 100 /var/lib/dpkg/status 3) qemu locks image files with no way to discover this feature nor how to disable it 4) qemu provides a way to query if it supports image locking, and what the default value is, and how to disable the locking via cli qemu 2.10 now will lock image files and warn if an image is currently locked. This prevent qemu from running (and possibly corrupting said image). However, qemu does not provide any way to determine if a qemu binary actually has this capability. Normally behavior changing features are exposed via some change to the qemu help menu or QMP/QAPI output of capabilities. I believe this slipped through since libvirt already does image locking, but direct cli users will be caught by this change. In particular, we have a use-case where we simulate multipath disks by creating to disks which point to the same file which now breaks without adding the 'file.locking=off' to the -drive parameters; which is also completely undocumented and unexposed. Some parts of the cli like -device allow querying of settable options (qemu-system-x86 -device scsi_hd,?) but nothing equivalent exists for -drive parameters. ProblemType: Bug DistroRelease: Ubuntu 17.10 Package: qemu-system-x86 1:2.10~rc3+dfsg-0ubuntu1 ProcVersionSignature: Ubuntu 4.12.0-11.12-generic 4.12.5 Uname: Linux 4.12.0-11-generic x86_64 NonfreeKernelModules: zfs zunicode zavl zcommon znvpair ApportVersion: 2.20.6-0ubuntu7 Architecture: amd64 Date: Fri Sep 8 12:56:53 2017 JournalErrors: Hint: You are currently not seeing messages from other users and the system. Users in groups 'adm', 'systemd-journal' can see all messages. Pass -q to turn off this notice. -- Logs begin at Mon 2017-01-30 11:56:02 CST, end at Fri 2017-09-08 12:56:46 CDT. -- -- No entries -- KvmCmdLine: COMMAND STAT EUID RUID PID PPID %CPU COMMAND MachineType: HP ProLiant DL360 Gen9 ProcEnviron: TERM=xterm PATH=(custom, no user) XDG_RUNTIME_DIR= LANG=en_US.UTF-8 SHELL=/bin/bash ProcKernelCmdLine: BOOT_IMAGE=/vmlinuz-4.12.0-11-generic root=UUID=45354276-e0c0-4bf6-9083-f130b89411cc ro --- console=ttyS1,115200 SourcePackage: qemu UpgradeStatus: No upgrade log present (probably fresh install) dmi.bios.date: 03/05/2015 dmi.bios.vendor: HP dmi.bios.version: P89 dmi.chassis.type: 23 dmi.chassis.vendor: HP dmi.modalias: dmi:bvnHP:bvrP89:bd03/05/2015:svnHP:pnProLiantDL360Gen9:pvr:cvnHP:ct23:cvr: dmi.product.family: ProLiant dmi.product.name: ProLiant DL360 Gen9 dmi.sys.vendor: HP To manage notifications about this
Re: [Qemu-devel] [PATCH] TPM: add CRB device
Hi
- Original Message -
> On 12/22/2017 09:55 AM, Marc-André Lureau wrote:
> > tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB)
> > Interface as defined in TCG PC Client Platform TPM Profile (PTP)
> > Specification Family “2.0” Level 00 Revision 01.03 v22.
> >
> > The PTP allows device implementation to switch between TIS and CRB
> > model at run time, but given that CRB is a simpler device to
> > implement, I chose to implement it as a different device.
> >
> > The device doesn't implement other locality than 0 for now (my laptop
> > TPM doesn't either, so I assume this isn't so bad)
> >
> > The command/reply memory region is statically allocated after the CRB
> > registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I
> > wonder if the BIOS could or should allocate it instead, or what size
> > to use, again this seems to fit well expectations)
> >
> > The PTP doesn't specify a particular bus to put the device. So I added
> > it on the system bus directly, so it could hopefully be used easily on
> > a different platform than x86. Currently, it fails to init on piix,
> > because error_on_sysbus_device() check. The check may be changed in a
> > near future, see discussion on the qemu-devel ML.
> >
> > Tested with some success with Linux upstream and Windows 10, seabios &
> > modified ovmf. The device is recognized and correctly transmit
> > command/response with passthrough & emu.
>
> When you say 'some success', you mean that a test suite wasn't passing
> all tests, right? I didn't run the test suite so far, but what I tested
> looks good to me so far.
I don't have a test suite, and I don't have much clue how to use a TPM for real
on Linux.
> We have one ACPI part missing in the implementation and that's this
> mailbox thing that allows one to send a code to the firmware that the
> firmware then reads and acts upon and reconfigures the TPM during the
> next warm-boot.
>
> https://trustedcomputinggroup.org/wp-content/uploads/Physical-Presence-Interface_1-30_0-52.pdf
>
> It was challenging to implement this interface 'back then' using ACPI's
> AML, but now it's even more low level with the programming interface.
>
> I know we have to wait for that other patch. Nevertheless ...
Oh, so PPI is mandatory for CRB devices? Are you going to implement it?
>
> >
> > Signed-off-by: Marc-André Lureau
> Signed-off-by: Stefan Berger
>
Thanks
> > ---
> > qapi/tpm.json | 5 +-
> > include/hw/acpi/tpm.h | 72
> > include/sysemu/tpm.h | 3 +
> > hw/i386/acpi-build.c | 34 +++-
> > hw/tpm/tpm_crb.c | 325
> > +
> > default-configs/i386-softmmu.mak | 1 +
> > default-configs/x86_64-softmmu.mak | 1 +
> > hw/tpm/Makefile.objs | 1 +
> > 8 files changed, 432 insertions(+), 10 deletions(-)
> > create mode 100644 hw/tpm/tpm_crb.c
> >
> > diff --git a/qapi/tpm.json b/qapi/tpm.json
> > index 7093f268fb..d50deef5e9 100644
> > --- a/qapi/tpm.json
> > +++ b/qapi/tpm.json
> > @@ -11,10 +11,11 @@
> > # An enumeration of TPM models
> > #
> > # @tpm-tis: TPM TIS model
> > +# @tpm-crb: TPM CRB model (since 2.12)
> > #
> > # Since: 1.5
> > ##
> > -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] }
> > +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] }
> >
> > ##
> > # @query-tpm-models:
> > @@ -28,7 +29,7 @@
> > # Example:
> > #
> > # -> { "execute": "query-tpm-models" }
> > -# <- { "return": [ "tpm-tis" ] }
> > +# <- { "return": [ "tpm-tis", "tpm-crb" ] }
> > #
> > ##
> > { 'command': 'query-tpm-models', 'returns': ['TpmModel'] }
> > diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> > index 6d516c6a7f..b0048515fa 100644
> > --- a/include/hw/acpi/tpm.h
> > +++ b/include/hw/acpi/tpm.h
> > @@ -16,11 +16,82 @@
> > #ifndef HW_ACPI_TPM_H
> > #define HW_ACPI_TPM_H
> >
> > +#include "qemu/osdep.h"
> > +
> > #define TPM_TIS_ADDR_BASE 0xFED4
> > #define TPM_TIS_ADDR_SIZE 0x5000
> >
> > #define TPM_TIS_IRQ 5
> >
> > +struct crb_regs {
> > +union {
> > +uint32_t reg;
> > +struct {
> > +unsigned tpm_established:1;
> > +unsigned loc_assigned:1;
> > +unsigned active_locality:3;
> > +unsigned reserved:2;
> > +unsigned tpm_reg_valid_sts:1;
> > +} bits;
> > +} loc_state;
> > +uint32_t reserved1;
> > +uint32_t loc_ctrl;
> > +union {
> > +uint32_t reg;
> > +struct {
> > +unsigned granted:1;
> > +unsigned been_seized:1;
> > +} bits;
> > +} loc_sts;
> > +uint8_t reserved2[32];
> > +union {
> > +uint64_t reg;
> > +struct {
> > +unsigned type:4;
> > +unsigned version:4;
> > +unsigned cap_locality:1;
> > +unsigned cap_crb_idle_bypa
[Qemu-devel] [PATCH v3 01/18] build-sys: fix qemu-ga -pthread linking
When linking qemu-ga under some configuration (when gthread-2.0.pc doesn't have -pthread, as happening atm with meson build), you may have this linking issue: /usr/bin/ld: libqemuutil.a(qemu-thread-posix.o): undefined reference to symbol 'pthread_setname_np@@GLIBC_2.12' /usr/lib64/libpthread.so.0: error adding symbols: DSO missing from command line Make sure qemu-ga links with the pthread library, by adding correct flags to libs_qga. This is really a QEMU bug, because it's QEMU code that's using pthread functions, and so we must explicitly link against pthreads. The bug was just masked by the fact that often some pkg-config or another for one of our dependencies will add -pthread to the link line anyway. Signed-off-by: Marc-André Lureau Reviewed-by: Peter Maydell --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index 100309c33f..de1f939a28 100755 --- a/configure +++ b/configure @@ -3445,6 +3445,7 @@ else done if test "$found" = "no"; then LIBS="$pthread_lib $LIBS" +libs_qga="$pthread_lib $libs_qga" fi PTHREAD_LIB="$pthread_lib" break -- 2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 02/18] build-sys: silence make by default or V=0
Move generic make flags in MAKEFLAGS (SUBDIR_MAKEFLAGS is more qemu specific). Use --quiet to silence make 'is up to date' message. Signed-off-by: Marc-André Lureau Tested-by: Eric Blake Reviewed-by: Paolo Bonzini --- Makefile | 2 +- rules.mak | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index d86ecd2dd4..1671db3bdd 100644 --- a/Makefile +++ b/Makefile @@ -277,7 +277,7 @@ else DOCS= endif -SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory) BUILD_DIR=$(BUILD_DIR) +SUBDIR_MAKEFLAGS=BUILD_DIR=$(BUILD_DIR) SUBDIR_DEVICES_MAK=$(patsubst %, %/config-devices.mak, $(TARGET_DIRS)) SUBDIR_DEVICES_MAK_DEP=$(patsubst %, %-config-devices.mak.d, $(TARGET_DIRS)) diff --git a/rules.mak b/rules.mak index 6e943335f3..5fb4951561 100644 --- a/rules.mak +++ b/rules.mak @@ -131,6 +131,8 @@ modules: # If called with only a single argument, will print nothing in quiet mode. quiet-command = $(if $(V),$1,$(if $(2),@printf " %-7s %s\n" $2 $3 && $1, @$1)) +MAKEFLAGS += $(if $(V),,--no-print-directory --quiet) + # cc-option # Usage: CFLAGS+=$(call cc-option, -falign-functions=0, -malign-functions=0) -- 2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 06/18] tests/docker: add test-debug
Add a new test with --enable-debug using clang/asan/ubsan, remove
--enable-debug from test-clang & test-mingw.
Signed-off-by: Marc-André Lureau
---
tests/docker/test-clang | 2 +-
tests/docker/test-debug | 26 ++
tests/docker/test-mingw | 2 --
3 files changed, 27 insertions(+), 3 deletions(-)
create mode 100755 tests/docker/test-debug
diff --git a/tests/docker/test-clang b/tests/docker/test-clang
index 1eb61a3af7..e90a793178 100755
--- a/tests/docker/test-clang
+++ b/tests/docker/test-clang
@@ -17,7 +17,7 @@ requires clang
cd "$BUILD_DIR"
-OPTS="--enable-debug --cxx=clang++ --cc=clang --host-cc=clang"
+OPTS="--cxx=clang++ --cc=clang --host-cc=clang"
# -fsanitize=undefined is broken on Fedora 23, skip it for now
# See also: https://bugzilla.redhat.com/show_bug.cgi?id=1263834
#OPTS="$OPTS --extra-cflags=-fsanitize=undefined \
diff --git a/tests/docker/test-debug b/tests/docker/test-debug
new file mode 100755
index 00..d020b06917
--- /dev/null
+++ b/tests/docker/test-debug
@@ -0,0 +1,26 @@
+#!/bin/bash -e
+#
+# Compile and check with clang & --enable-debug.
+#
+# Copyright (c) 2016-2018 Red Hat Inc.
+#
+# Authors:
+# Fam Zheng
+# Marc-André Lureau
+#
+# 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.
+
+. common.rc
+
+requires clang asan
+
+cd "$BUILD_DIR"
+
+OPTS="--cxx=clang++ --cc=clang --host-cc=clang"
+OPTS="--enable-debug $OPTS"
+
+build_qemu $OPTS
+make $MAKEFLAGS check
+install_qemu
diff --git a/tests/docker/test-mingw b/tests/docker/test-mingw
index 39a1da448e..503a6bc6f7 100755
--- a/tests/docker/test-mingw
+++ b/tests/docker/test-mingw
@@ -22,7 +22,6 @@ for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do
TARGET_LIST=${TARGET_LIST:-$DEF_TARGET_LIST} \
build_qemu --cross-prefix=$prefix \
--enable-trace-backends=simple \
---enable-debug \
--enable-gnutls \
--enable-nettle \
--enable-curl \
@@ -35,4 +34,3 @@ for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do
make clean
done
-
--
2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 00/18] Various build-sys and sanitizer related fixes
Hi, This is a series that improves a bit the build system, and introduces ASAN/UBSan by default when --enable-debug. Then it fixes a few leaks that occur during make check: common and x86_64 target tests are leak free after this series. The other targets will need some work to fix the leaks & warnings. v3: - add ubsan too with --enable-debug, since it is recommended and has low runtime cost - add a patch "compile with -Og or -O1 when --enable-debug", suggested by Paolo - add 2 new fixes to pass full make check with asan/ubsan - modify docker tests to run --enable-debug & asan with new test-debug - add some r-b/a-b tags v2: - simplify "build-sys: silence make by default or V=0": make it a oneliner MAKEFLAGS, use --quiet. - document print-VAR rule in docs/devel/build-system.txt - fix ASAN coroutine instrumentation failure - should fix builds on gcc 4.4.7 (centos 6) - new coroutine test leak fix - add some r-b tags Marc-André Lureau (18): build-sys: fix qemu-ga -pthread linking build-sys: silence make by default or V=0 build-sys: add a rule to print a variable build-sys: compile with -Og or -O1 when --enable-debug tests/docker: add some sanitizers to fedora dockerfile tests/docker: add test-debug build-sys: add some sanitizers when --enable-debug if possible tests: fix check-qobject leak vl: fix direct firmware directories leak readline: add a free function tests: fix migration-test leak crypto: fix stack-buffer-overflow error qemu-config: fix leak in query-command-line-options tests: fix qmp-test leak ucontext: annotate coroutine stack for ASAN tests: fix coroutine leak in /basic/entered mips: fix potential fopen(NULL,...) disas/s390: fix global-buffer-overflow include/qemu/compiler.h| 4 +++ include/qemu/readline.h| 1 + crypto/ivgen-essiv.c | 2 +- disas/s390.c | 16 +--- hw/nvram/ds1225y.c | 4 +-- monitor.c | 2 +- tests/check-qobject.c | 2 ++ tests/migration-test.c | 3 ++- tests/qmp-test.c | 3 ++- tests/test-coroutine.c | 1 - util/coroutine-ucontext.c | 46 ++ util/qemu-config.c | 3 ++- util/readline.c| 18 - vl.c | 9 --- Makefile | 7 -- configure | 23 +++-- docs/devel/build-system.txt| 13 ++ rules.mak | 2 ++ tests/docker/dockerfiles/fedora.docker | 4 +-- tests/docker/test-clang| 2 +- tests/docker/test-debug| 26 +++ tests/docker/test-mingw| 2 -- 22 files changed, 162 insertions(+), 31 deletions(-) create mode 100755 tests/docker/test-debug -- 2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 03/18] build-sys: add a rule to print a variable
$ make print-CFLAGS CFLAGS=-fsanitize=address -Og -g Trick from various sources: https://stackoverflow.com/questions/16467718/how-to-print-out-a-variable-in-makefile https://www.cmcrossroads.com/article/printing-value-makefile-variable Signed-off-by: Marc-André Lureau Reviewed-by: Eric Blake --- Makefile| 5 - docs/devel/build-system.txt | 13 + 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 1671db3bdd..f26ef1b1df 100644 --- a/Makefile +++ b/Makefile @@ -8,9 +8,12 @@ SRC_PATH=. UNCHECKED_GOALS := %clean TAGS cscope ctags dist \ html info pdf txt \ -help check-help \ +help check-help print-% \ docker docker-% vm-test vm-build-% +print-%: + @echo '$*=$($*)' + # All following code might depend on configuration variables ifneq ($(wildcard config-host.mak),) # Put the all: rule here so that config-host.mak can contain dependencies. diff --git a/docs/devel/build-system.txt b/docs/devel/build-system.txt index 386ef36ee3..52501f2ad9 100644 --- a/docs/devel/build-system.txt +++ b/docs/devel/build-system.txt @@ -510,3 +510,16 @@ default-configs/$TARGET-NAME file as input. This is the entrypoint used when make recurses to build a single system or userspace emulator target. It is merely a symlink back to the Makefile.target in the top level. + + +Useful make targets +=== + +- help + + Print a help message for the most common build targets. + +- print-VAR + + Print the value of the variable VAR. Useful for debugging the build + system. -- 2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 04/18] build-sys: compile with -Og or -O1 when --enable-debug
When --enable-debug is turned on, configure doesn't set -O level, and
uses default compiler -O0 level, which is slow.
Instead, use -Og if supported by the compiler (optimize debugging
experience), or -O1 (keeps code somewhat debuggable and works around
compiler bugs).
Unfortunately, gcc has many false-positive maybe-uninitialized
errors with Og and O1 (f27 gcc 7.2.1 20170915):
/home/elmarco/src/qemu/hw/ipmi/isa_ipmi_kcs.c: In function
‘ipmi_kcs_ioport_read’:
/home/elmarco/src/qemu/hw/ipmi/isa_ipmi_kcs.c:279:12: error: ‘ret’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
return ret;
^~~
cc1: all warnings being treated as errors
make: *** [/home/elmarco/src/qemu/rules.mak:66: hw/ipmi/isa_ipmi_kcs.o] Error 1
make: *** Waiting for unfinished jobs
/home/elmarco/src/qemu/hw/ide/ahci.c: In function ‘ahci_populate_sglist’:
/home/elmarco/src/qemu/hw/ide/ahci.c:903:58: error: ‘tbl_entry_size’ may be
used uninitialized in this function [-Werror=maybe-uninitialized]
if ((off_idx == -1) || (off_pos < 0) || (off_pos > tbl_entry_size)) {
~^
cc1: all warnings being treated as errors
make: *** [/home/elmarco/src/qemu/rules.mak:66: hw/ide/ahci.o] Error 1
/home/elmarco/src/qemu/hw/display/qxl.c: In function ‘qxl_add_memslot’:
/home/elmarco/src/qemu/hw/display/qxl.c:1397:52: error: ‘pci_start’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
memslot.virt_end = virt_start + (guest_end - pci_start);
~^~~~
/home/elmarco/src/qemu/hw/display/qxl.c:1389:9: error: ‘pci_region’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
qxl_set_guest_bug(d, "%s: pci_region = %d", __func__, pci_region);
^
cc1: all warnings being treated as errors
There seems to be a long list of related bugs in upstream GCC, some of
them are being fixed very recently:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639
For now, let's workaround it by using Wno-maybe-uninitialized (gcc-only).
Suggested-by: Paolo Bonzini
Signed-off-by: Marc-André Lureau
---
configure | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index de1f939a28..3953859314 100755
--- a/configure
+++ b/configure
@@ -5160,8 +5160,19 @@ if test "$gcov" = "yes" ; then
LDFLAGS="-fprofile-arcs -ftest-coverage $LDFLAGS"
elif test "$fortify_source" = "yes" ; then
CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS"
-elif test "$debug" = "no"; then
- CFLAGS="-O2 $CFLAGS"
+elif test "$debug" = "yes"; then
+ if compile_prog "-Og" ""; then
+ CFLAGS="-Og $CFLAGS"
+ elif compile_prog "-O1" ""; then
+ CFLAGS="-O1 $CFLAGS"
+ fi
+ # Workaround GCC false-positive Wuninitialized bugs with Og or O1:
+ # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=24639
+ if cc_has_warning_flag "-Wno-maybe-uninitialized"; then
+ CFLAGS="-Wno-maybe-uninitialized $CFLAGS"
+ fi
+else
+CFLAGS="-O2 $CFLAGS"
fi
##
--
2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 07/18] build-sys: add some sanitizers when --enable-debug if possible
Enable ASAN/UBSan by default if the compiler supports it. Typical slowdown introduced by AddressSanitizer is 2x. UBSan shouldn't have much impact on runtime cost. Signed-off-by: Marc-André Lureau --- configure | 7 +++ 1 file changed, 7 insertions(+) diff --git a/configure b/configure index 3953859314..de1477c93a 100755 --- a/configure +++ b/configure @@ -5161,6 +5161,13 @@ if test "$gcov" = "yes" ; then elif test "$fortify_source" = "yes" ; then CFLAGS="-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 $CFLAGS" elif test "$debug" = "yes"; then + write_c_skeleton; + if compile_prog "-fsanitize=address" ""; then + CFLAGS="-fsanitize=address $CFLAGS" + fi + if compile_prog "-fsanitize=undefined" ""; then + CFLAGS="-fsanitize=undefined $CFLAGS" + fi if compile_prog "-Og" ""; then CFLAGS="-Og $CFLAGS" elif compile_prog "-O1" ""; then -- 2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 05/18] tests/docker: add some sanitizers to fedora dockerfile
Build fedora image with ASAN/UBSan support. Signed-off-by: Marc-André Lureau --- tests/docker/dockerfiles/fedora.docker | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index 4b26c3aded..32de731675 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -3,7 +3,7 @@ ENV PACKAGES \ ccache gettext git tar PyYAML sparse flex bison python2 bzip2 hostname \ glib2-devel pixman-devel zlib-devel SDL-devel libfdt-devel \ gcc gcc-c++ clang make perl which bc findutils libaio-devel \ -nettle-devel \ +nettle-devel libasan libubsan \ mingw32-pixman mingw32-glib2 mingw32-gmp mingw32-SDL mingw32-pkg-config \ mingw32-gtk2 mingw32-gtk3 mingw32-gnutls mingw32-nettle mingw32-libtasn1 \ mingw32-libjpeg-turbo mingw32-libpng mingw32-curl mingw32-libssh2 \ @@ -15,4 +15,4 @@ ENV PACKAGES \ RUN dnf install -y $PACKAGES RUN rpm -q $PACKAGES | sort > /packages.txt -ENV FEATURES mingw clang pyyaml +ENV FEATURES mingw clang pyyaml asan -- 2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 13/18] qemu-config: fix leak in query-command-line-options
Direct leak of 160 byte(s) in 4 object(s) allocated from:
#0 0x55ed7678cda8 in calloc
(/home/elmarco/src/qq/build/x86_64-softmmu/qemu-system-x86_64+0x797da8)
#1 0x7f3f5e725f75 in g_malloc0
/home/elmarco/src/gnome/glib/builddir/../glib/gmem.c:124
#2 0x55ed778aa3a7 in query_option_descs
/home/elmarco/src/qq/util/qemu-config.c:60:16
#3 0x55ed778aa307 in get_drive_infolist
/home/elmarco/src/qq/util/qemu-config.c:140:19
#4 0x55ed778a9f40 in qmp_query_command_line_options
/home/elmarco/src/qq/util/qemu-config.c:254:36
#5 0x55ed76d4868c in qmp_marshal_query_command_line_options
/home/elmarco/src/qq/build/qmp-marshal.c:3078:14
#6 0x55ed77855dd5 in do_qmp_dispatch
/home/elmarco/src/qq/qapi/qmp-dispatch.c:104:5
#7 0x55ed778558cc in qmp_dispatch
/home/elmarco/src/qq/qapi/qmp-dispatch.c:131:11
#8 0x55ed768b592f in handle_qmp_command
/home/elmarco/src/qq/monitor.c:3840:11
#9 0x55ed7786ccfe in json_message_process_token
/home/elmarco/src/qq/qobject/json-streamer.c:105:5
#10 0x55ed778fe37c in json_lexer_feed_char
/home/elmarco/src/qq/qobject/json-lexer.c:323:13
#11 0x55ed778fdde6 in json_lexer_feed
/home/elmarco/src/qq/qobject/json-lexer.c:373:15
#12 0x55ed7786cd83 in json_message_parser_feed
/home/elmarco/src/qq/qobject/json-streamer.c:124:12
#13 0x55ed768b559e in monitor_qmp_read /home/elmarco/src/qq/monitor.c:3882:5
#14 0x55ed77714f29 in qemu_chr_be_write_impl
/home/elmarco/src/qq/chardev/char.c:167:9
#15 0x55ed77714fde in qemu_chr_be_write
/home/elmarco/src/qq/chardev/char.c:179:9
#16 0x55ed7772ffad in tcp_chr_read
/home/elmarco/src/qq/chardev/char-socket.c:440:13
#17 0x55ed113b in qio_channel_fd_source_dispatch
/home/elmarco/src/qq/io/channel-watch.c:84:12
#18 0x7f3f5e71d90b in g_main_dispatch
/home/elmarco/src/gnome/glib/builddir/../glib/gmain.c:3182
#19 0x7f3f5e71e7ac in g_main_context_dispatch
/home/elmarco/src/gnome/glib/builddir/../glib/gmain.c:3847
#20 0x55ed77886ffc in glib_pollfds_poll
/home/elmarco/src/qq/util/main-loop.c:214:9
#21 0x55ed778865fd in os_host_main_loop_wait
/home/elmarco/src/qq/util/main-loop.c:261:5
#22 0x55ed77886222 in main_loop_wait
/home/elmarco/src/qq/util/main-loop.c:515:11
#23 0x55ed76d2a4df in main_loop /home/elmarco/src/qq/vl.c:1995:9
#24 0x55ed76d1cb4a in main /home/elmarco/src/qq/vl.c:4914:5
#25 0x7f3f555f6039 in __libc_start_main (/lib64/libc.so.6+0x21039)
Signed-off-by: Marc-André Lureau
Reviewed-by: Eric Blake
---
util/qemu-config.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 99b0e46fa3..029fec53a9 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -105,7 +105,8 @@ static void cleanup_infolist(CommandLineParameterInfoList
*head)
if (!strcmp(pre_entry->value->name, cur->next->value->name)) {
del_entry = cur->next;
cur->next = cur->next->next;
-g_free(del_entry);
+del_entry->next = NULL;
+qapi_free_CommandLineParameterInfoList(del_entry);
break;
}
pre_entry = pre_entry->next;
--
2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 11/18] tests: fix migration-test leak
Direct leak of 12 byte(s) in 2 object(s) allocated from:
#0 0x7f50d403c850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x7f50d1ddf98f in vasprintf (/lib64/libc.so.6+0x8098f)
Signed-off-by: Marc-André Lureau
Reviewed-by: Dr. David Alan Gilbert
Reviewed-by: Philippe Mathieu-Daudé
---
tests/migration-test.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/migration-test.c b/tests/migration-test.c
index be598d3257..799e24ebc6 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -358,13 +358,14 @@ static void migrate_check_parameter(QTestState *who,
const char *parameter,
const char *value)
{
QDict *rsp, *rsp_return;
-const char *result;
+char *result;
rsp = wait_command(who, "{ 'execute': 'query-migrate-parameters' }");
rsp_return = qdict_get_qdict(rsp, "return");
result = g_strdup_printf("%" PRId64,
qdict_get_try_int(rsp_return, parameter, -1));
g_assert_cmpstr(result, ==, value);
+g_free(result);
QDECREF(rsp);
}
--
2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 08/18] tests: fix check-qobject leak
/public/qobject_is_equal_conversion: OK = ==14396==ERROR: LeakSanitizer: detected memory leaks Direct leak of 56 byte(s) in 1 object(s) allocated from: #0 0x7f07682c5850 in malloc (/lib64/libasan.so.4+0xde850) #1 0x7f0767d12f0c in g_malloc ../glib/gmem.c:94 #2 0x7f0767d131cf in g_malloc_n ../glib/gmem.c:331 #3 0x562bd767371f in do_test_equality /home/elmarco/src/qq/tests/check-qobject.c:49 #4 0x562bd7674a35 in qobject_is_equal_dict_test /home/elmarco/src/qq/tests/check-qobject.c:267 #5 0x7f0767d37b04 in test_case_run ../glib/gtestutils.c:2237 #6 0x7f0767d37ec4 in g_test_run_suite_internal ../glib/gtestutils.c:2321 #7 0x7f0767d37f6d in g_test_run_suite_internal ../glib/gtestutils.c:2333 #8 0x7f0767d38184 in g_test_run_suite ../glib/gtestutils.c:2408 #9 0x7f0767d36e0d in g_test_run ../glib/gtestutils.c:1674 #10 0x562bd7674e75 in main /home/elmarco/src/qq/tests/check-qobject.c:327 #11 0x7f0766009039 in __libc_start_main (/lib64/libc.so.6+0x21039) Signed-off-by: Marc-André Lureau Reviewed-by: Markus Armbruster Reviewed-by: Philippe Mathieu-Daudé --- tests/check-qobject.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/check-qobject.c b/tests/check-qobject.c index 03e9175113..710f9e6b0a 100644 --- a/tests/check-qobject.c +++ b/tests/check-qobject.c @@ -59,6 +59,8 @@ static void do_test_equality(bool expected, int _, ...) g_assert(qobject_is_equal(args[i], args[j]) == expected); } } + +g_free(args); } #define check_equal(...) \ -- 2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 12/18] crypto: fix stack-buffer-overflow error
ASAN complains about:
==8856==ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7ffd8a1fe168 at pc 0x561136cb4451 bp 0x7ffd8a1fe130 sp 0x7ffd8a1fd8e0
READ of size 16 at 0x7ffd8a1fe168 thread T0
#0 0x561136cb4450 in __asan_memcpy
(/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x110450)
#1 0x561136d2a6a7 in qcrypto_ivgen_essiv_calculate
/home/elmarco/src/qq/crypto/ivgen-essiv.c:83:5
#2 0x561136d29af8 in qcrypto_ivgen_calculate
/home/elmarco/src/qq/crypto/ivgen.c:72:12
#3 0x561136d07c8e in test_ivgen
/home/elmarco/src/qq/tests/test-crypto-ivgen.c:148:5
#4 0x7f2c3b04 in test_case_run
/home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2237
#5 0x7f2c3ec4 in g_test_run_suite_internal
/home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2321
#6 0x7f2c3f6d in g_test_run_suite_internal
/home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
#7 0x7f2c3f6d in g_test_run_suite_internal
/home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
#8 0x7f2c3f6d in g_test_run_suite_internal
/home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2333
#9 0x7f2c4184 in g_test_run_suite
/home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:2408
#10 0x7f2c2e0d in g_test_run
/home/elmarco/src/gnome/glib/builddir/../glib/gtestutils.c:1674
#11 0x561136d0799b in main
/home/elmarco/src/qq/tests/test-crypto-ivgen.c:173:12
#12 0x7f77756e6039 in __libc_start_main (/lib64/libc.so.6+0x21039)
#13 0x561136c13d89 in _start
(/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x6fd89)
Address 0x7ffd8a1fe168 is located in stack of thread T0 at offset 40 in frame
#0 0x561136d2a40f in qcrypto_ivgen_essiv_calculate
/home/elmarco/src/qq/crypto/ivgen-essiv.c:76
This frame has 1 object(s):
[32, 40) 'sector.addr' <== Memory access at offset 40 overflows this
variable
HINT: this may be a false positive if your program uses some custom stack
unwind mechanism or swapcontext
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow
(/home/elmarco/src/qq/build/tests/test-crypto-ivgen+0x110450) in __asan_memcpy
Shadow bytes around the buggy address:
0x100031437bd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437be0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437bf0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x100031437c20: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00[f3]f3 f3
0x100031437c30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100031437c70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user:f7
Container overflow: fc
Array cookie:ac
Intra object redzone:bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone:cb
It looks like the rest of the code copes with ndata being larger than
sizeof(sector), so limit the memcpy() range.
Signed-off-by: Marc-André Lureau
Reviewed-by: Daniel P. Berrange
---
crypto/ivgen-essiv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/crypto/ivgen-essiv.c b/crypto/ivgen-essiv.c
index cba20bde6c..ad4d926c19 100644
--- a/crypto/ivgen-essiv.c
+++ b/crypto/ivgen-essiv.c
@@ -79,7 +79,7 @@ static int qcrypto_ivgen_essiv_calculate(QCryptoIVGen *ivgen,
uint8_t *data = g_new(uint8_t, ndata);
sector = cpu_to_le64(sector);
-memcpy(data, (uint8_t *)§or, ndata);
+memcpy(data, (uint8_t *)§or, MIN(sizeof(sector), ndata));
if (sizeof(sector) < ndata) {
memset(data + sizeof(sector), 0, ndata - sizeof(sector));
}
--
2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 14/18] tests: fix qmp-test leak
Direct leak of 913 byte(s) in 43 object(s) allocated from:
#0 0x55880a15df60 in __interceptor_malloc
(/home/elmarco/src/qq/build/tests/qmp-test+0x110f60)
#1 0x7f3f20fd098f in _IO_vasprintf (/lib64/libc.so.6+0x8098f)
Signed-off-by: Marc-André Lureau
Reviewed-by: Markus Armbruster
Reviewed-by: Philippe Mathieu-Daudé
---
tests/qmp-test.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index c5a5c10b41..36feb2204b 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -271,7 +271,7 @@ static void add_query_tests(QmpSchema *schema)
{
SchemaInfoList *tail;
SchemaInfo *si, *arg_type, *ret_type;
-const char *test_name;
+char *test_name;
/* Test the query-like commands */
for (tail = schema->list; tail; tail = tail->next) {
@@ -297,6 +297,7 @@ static void add_query_tests(QmpSchema *schema)
test_name = g_strdup_printf("qmp/%s", si->name);
qtest_add_data_func(test_name, si->name, test_query);
+g_free(test_name);
}
}
--
2.15.1.355.g36791d7216
Re: [Qemu-devel] [PATCH v3 03/18] build-sys: add a rule to print a variable
Hi Marc-André, On 01/04/2018 01:05 PM, Marc-André Lureau wrote: > $ make print-CFLAGS > CFLAGS=-fsanitize=address -Og -g smiley mode: $ make print-* *=* I didn't understood this one: $ make print-\$ CC tests/qemu-iotests/socket_scm_helper.o LINKtests/qemu-iotests/socket_scm_helper GEN qga/qapi-generated/qga-qapi-types.h GEN qga/qapi-generated/qga-qapi-visit.h GEN qga/qapi-generated/qga-qmp-commands.h CC qga/commands.o ... > Trick from various sources: > https://stackoverflow.com/questions/16467718/how-to-print-out-a-variable-in-makefile > https://www.cmcrossroads.com/article/printing-value-makefile-variable > > Signed-off-by: Marc-André Lureau > Reviewed-by: Eric Blake Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé > --- > Makefile| 5 - > docs/devel/build-system.txt | 13 + > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 1671db3bdd..f26ef1b1df 100644 > --- a/Makefile > +++ b/Makefile > @@ -8,9 +8,12 @@ SRC_PATH=. > > UNCHECKED_GOALS := %clean TAGS cscope ctags dist \ > html info pdf txt \ > -help check-help \ > +help check-help print-% \ > docker docker-% vm-test vm-build-% > > +print-%: > + @echo '$*=$($*)' > + > # All following code might depend on configuration variables > ifneq ($(wildcard config-host.mak),) > # Put the all: rule here so that config-host.mak can contain dependencies. > diff --git a/docs/devel/build-system.txt b/docs/devel/build-system.txt > index 386ef36ee3..52501f2ad9 100644 > --- a/docs/devel/build-system.txt > +++ b/docs/devel/build-system.txt > @@ -510,3 +510,16 @@ default-configs/$TARGET-NAME file as input. > This is the entrypoint used when make recurses to build a single system > or userspace emulator target. It is merely a symlink back to the > Makefile.target in the top level. > + > + > +Useful make targets > +=== > + > +- help > + > + Print a help message for the most common build targets. > + > +- print-VAR > + > + Print the value of the variable VAR. Useful for debugging the build > + system. >
[Qemu-devel] [PATCH v3 15/18] ucontext: annotate coroutine stack for ASAN
It helps ASAN to detect more leaks on coroutine stacks, as found in
the following patch.
A similar work would need to be done for sigaltstack & windows fibers
to have similar coverage. Since ucontext is preferred, I didn't bother
checking the other coroutine implementations for now.
Signed-off-by: Marc-André Lureau
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Stefan Hajnoczi
---
include/qemu/compiler.h | 4
util/coroutine-ucontext.c | 46 ++
2 files changed, 50 insertions(+)
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index 340e5fdc09..5fcc4f7ec7 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -111,4 +111,8 @@
#define GCC_FMT_ATTR(n, m)
#endif
+#ifndef __has_feature
+#define __has_feature(x) 0 /* compatibility with non-clang compilers */
+#endif
+
#endif /* COMPILER_H */
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 6621f3f692..e78eae8766 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -31,6 +31,11 @@
#include
#endif
+#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer)
+#define CONFIG_ASAN 1
+#include
+#endif
+
typedef struct {
Coroutine base;
void *stack;
@@ -59,11 +64,37 @@ union cc_arg {
int i[2];
};
+static void finish_switch_fiber(void *fake_stack_save)
+{
+#ifdef CONFIG_ASAN
+const void *bottom_old;
+size_t size_old;
+
+__sanitizer_finish_switch_fiber(fake_stack_save, &bottom_old, &size_old);
+
+if (!leader.stack) {
+leader.stack = (void *)bottom_old;
+leader.stack_size = size_old;
+}
+#endif
+}
+
+static void start_switch_fiber(void **fake_stack_save,
+ const void *bottom, size_t size)
+{
+#ifdef CONFIG_ASAN
+__sanitizer_start_switch_fiber(fake_stack_save, bottom, size);
+#endif
+}
+
static void coroutine_trampoline(int i0, int i1)
{
union cc_arg arg;
CoroutineUContext *self;
Coroutine *co;
+void *fake_stack_save = NULL;
+
+finish_switch_fiber(NULL);
arg.i[0] = i0;
arg.i[1] = i1;
@@ -72,9 +103,13 @@ static void coroutine_trampoline(int i0, int i1)
/* Initialize longjmp environment and switch back the caller */
if (!sigsetjmp(self->env, 0)) {
+start_switch_fiber(&fake_stack_save,
+ leader.stack, leader.stack_size);
siglongjmp(*(sigjmp_buf *)co->entry_arg, 1);
}
+finish_switch_fiber(fake_stack_save);
+
while (true) {
co->entry(co->entry_arg);
qemu_coroutine_switch(co, co->caller, COROUTINE_TERMINATE);
@@ -87,6 +122,7 @@ Coroutine *qemu_coroutine_new(void)
ucontext_t old_uc, uc;
sigjmp_buf old_env;
union cc_arg arg = {0};
+void *fake_stack_save = NULL;
/* The ucontext functions preserve signal masks which incurs a
* system call overhead. sigsetjmp(buf, 0)/siglongjmp() does not
@@ -122,8 +158,12 @@ Coroutine *qemu_coroutine_new(void)
/* swapcontext() in, siglongjmp() back out */
if (!sigsetjmp(old_env, 0)) {
+start_switch_fiber(&fake_stack_save, co->stack, co->stack_size);
swapcontext(&old_uc, &uc);
}
+
+finish_switch_fiber(fake_stack_save);
+
return &co->base;
}
@@ -169,13 +209,19 @@ qemu_coroutine_switch(Coroutine *from_, Coroutine *to_,
CoroutineUContext *from = DO_UPCAST(CoroutineUContext, base, from_);
CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, to_);
int ret;
+void *fake_stack_save = NULL;
current = to_;
ret = sigsetjmp(from->env, 0);
if (ret == 0) {
+start_switch_fiber(action == COROUTINE_TERMINATE ?
+ NULL : &fake_stack_save, to->stack, to->stack_size);
siglongjmp(to->env, action);
}
+
+finish_switch_fiber(fake_stack_save);
+
return ret;
}
--
2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 09/18] vl: fix direct firmware directories leak
Note that data_dir[] will now point to allocated strings.
Fixes:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x7f1448181850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x7f1446ed8f0c in g_malloc ../glib/gmem.c:94
#2 0x7f1446ed91cf in g_malloc_n ../glib/gmem.c:331
#3 0x7f1446ef739a in g_strsplit ../glib/gstrfuncs.c:2364
#4 0x55cf276439d7 in main /home/elmarco/src/qq/vl.c:4311
#5 0x7f143dfad039 in __libc_start_main (/lib64/libc.so.6+0x21039)
Signed-off-by: Marc-André Lureau
Reviewed-by: Eric Blake
---
vl.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/vl.c b/vl.c
index d3a5c5d021..8430b0c731 100644
--- a/vl.c
+++ b/vl.c
@@ -2318,7 +2318,7 @@ static void qemu_add_data_dir(const char *path)
return; /* duplicate */
}
}
-data_dir[data_dir_idx++] = path;
+data_dir[data_dir_idx++] = g_strdup(path);
}
static inline bool nonempty_str(const char *str)
@@ -3079,7 +3079,7 @@ int main(int argc, char **argv, char **envp)
Error *main_loop_err = NULL;
Error *err = NULL;
bool list_data_dirs = false;
-char **dirs;
+char *dir, **dirs;
typedef struct BlockdevOptions_queue {
BlockdevOptions *bdo;
Location loc;
@@ -4263,9 +4263,12 @@ int main(int argc, char **argv, char **envp)
for (i = 0; dirs[i] != NULL; i++) {
qemu_add_data_dir(dirs[i]);
}
+g_strfreev(dirs);
/* try to find datadir relative to the executable path */
-qemu_add_data_dir(os_find_datadir());
+dir = os_find_datadir();
+qemu_add_data_dir(dir);
+g_free(dir);
/* add the datadir specified when building */
qemu_add_data_dir(CONFIG_QEMU_DATADIR);
--
2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 17/18] mips: fix potential fopen(NULL,...)
Spotted thanks to ASAN.
Signed-off-by: Marc-André Lureau
---
hw/nvram/ds1225y.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/nvram/ds1225y.c b/hw/nvram/ds1225y.c
index 57d5ab2154..ad7345f288 100644
--- a/hw/nvram/ds1225y.c
+++ b/hw/nvram/ds1225y.c
@@ -80,7 +80,7 @@ static int nvram_post_load(void *opaque, int version_id)
}
/* Write back nvram contents */
-s->file = fopen(s->filename, "wb");
+s->file = s->filename ? fopen(s->filename, "wb") : NULL;
if (s->file) {
/* Write back contents, as 'wb' mode cleaned the file */
if (fwrite(s->contents, s->chip_size, 1, s->file) != 1) {
@@ -126,7 +126,7 @@ static int nvram_sysbus_initfn(SysBusDevice *dev)
sysbus_init_mmio(dev, &s->iomem);
/* Read current file */
-file = fopen(s->filename, "rb");
+file = s->filename ? fopen(s->filename, "rb") : NULL;
if (file) {
/* Read nvram contents */
if (fread(s->contents, s->chip_size, 1, file) != 1) {
--
2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 10/18] readline: add a free function
Fixes leaks such as:
Direct leak of 2 byte(s) in 1 object(s) allocated from:
#0 0x7eff58beb850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x7eff57942f0c in g_malloc ../glib/gmem.c:94
#2 0x7eff579431cf in g_malloc_n ../glib/gmem.c:331
#3 0x7eff5795f6eb in g_strdup ../glib/gstrfuncs.c:363
#4 0x55db720f1d46 in readline_hist_add
/home/elmarco/src/qq/util/readline.c:258
#5 0x55db720f2d34 in readline_handle_byte
/home/elmarco/src/qq/util/readline.c:387
#6 0x55db71539d00 in monitor_read /home/elmarco/src/qq/monitor.c:3896
#7 0x55db71f9be35 in qemu_chr_be_write_impl
/home/elmarco/src/qq/chardev/char.c:167
#8 0x55db71f9bed3 in qemu_chr_be_write
/home/elmarco/src/qq/chardev/char.c:179
#9 0x55db71fa013c in fd_chr_read /home/elmarco/src/qq/chardev/char-fd.c:66
#10 0x55db71fe18a8 in qio_channel_fd_source_dispatch
/home/elmarco/src/qq/io/channel-watch.c:84
#11 0x7eff5793a90b in g_main_dispatch ../glib/gmain.c:3182
#12 0x7eff5793b7ac in g_main_context_dispatch ../glib/gmain.c:3847
#13 0x55db720af3bd in glib_pollfds_poll
/home/elmarco/src/qq/util/main-loop.c:214
#14 0x55db720af505 in os_host_main_loop_wait
/home/elmarco/src/qq/util/main-loop.c:261
#15 0x55db720af6d6 in main_loop_wait
/home/elmarco/src/qq/util/main-loop.c:515
#16 0x55db7184e0de in main_loop /home/elmarco/src/qq/vl.c:1995
#17 0x55db7185e956 in main /home/elmarco/src/qq/vl.c:4914
#18 0x7eff4ea17039 in __libc_start_main (/lib64/libc.so.6+0x21039)
(while at it, use g_new0(ReadLineState), it's a bit easier to read)
Signed-off-by: Marc-André Lureau
Reviewed-by: Dr. David Alan Gilbert
Reviewed-by: Philippe Mathieu-Daudé
---
include/qemu/readline.h | 1 +
monitor.c | 2 +-
util/readline.c | 18 +-
3 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/include/qemu/readline.h b/include/qemu/readline.h
index c08cf7400e..e81258322b 100644
--- a/include/qemu/readline.h
+++ b/include/qemu/readline.h
@@ -59,5 +59,6 @@ ReadLineState *readline_init(ReadLinePrintfFunc *printf_func,
ReadLineFlushFunc *flush_func,
void *opaque,
ReadLineCompletionFunc *completion_finder);
+void readline_free(ReadLineState *rs);
#endif /* READLINE_H */
diff --git a/monitor.c b/monitor.c
index d682eee2d8..b9da5e20d1 100644
--- a/monitor.c
+++ b/monitor.c
@@ -583,7 +583,7 @@ static void monitor_data_destroy(Monitor *mon)
if (monitor_is_qmp(mon)) {
json_message_parser_destroy(&mon->qmp.parser);
}
-g_free(mon->rs);
+readline_free(mon->rs);
QDECREF(mon->outbuf);
qemu_mutex_destroy(&mon->out_lock);
}
diff --git a/util/readline.c b/util/readline.c
index bbdee790b0..24ec839854 100644
--- a/util/readline.c
+++ b/util/readline.c
@@ -500,12 +500,28 @@ const char *readline_get_history(ReadLineState *rs,
unsigned int index)
return rs->history[index];
}
+void readline_free(ReadLineState *rs)
+{
+int i;
+
+if (!rs) {
+return;
+}
+for (i = 0; i < READLINE_MAX_CMDS; i++) {
+g_free(rs->history[i]);
+}
+for (i = 0; i < READLINE_MAX_COMPLETIONS; i++) {
+g_free(rs->completions[i]);
+}
+g_free(rs);
+}
+
ReadLineState *readline_init(ReadLinePrintfFunc *printf_func,
ReadLineFlushFunc *flush_func,
void *opaque,
ReadLineCompletionFunc *completion_finder)
{
-ReadLineState *rs = g_malloc0(sizeof(*rs));
+ReadLineState *rs = g_new0(ReadLineState, 1);
rs->hist_entry = -1;
rs->opaque = opaque;
--
2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 18/18] disas/s390: fix global-buffer-overflow
Spotted thanks to ASAN:
==25226==ERROR: AddressSanitizer: global-buffer-overflow on address
0x556715a1f120 at pc 0x556714b6f6b1 bp 0x7ffcdfac1360 sp 0x7ffcdfac1350
READ of size 1 at 0x556715a1f120 thread T0
#0 0x556714b6f6b0 in init_disasm /home/elmarco/src/qemu/disas/s390.c:219
#1 0x556714b6fa6a in print_insn_s390 /home/elmarco/src/qemu/disas/s390.c:294
#2 0x55671484d031 in monitor_disas /home/elmarco/src/qemu/disas.c:635
#3 0x556714862ec0 in memory_dump /home/elmarco/src/qemu/monitor.c:1324
#4 0x55671486342a in hmp_memory_dump /home/elmarco/src/qemu/monitor.c:1418
#5 0x5567148670be in handle_hmp_command
/home/elmarco/src/qemu/monitor.c:3109
#6 0x5567148674ed in qmp_human_monitor_command
/home/elmarco/src/qemu/monitor.c:613
#7 0x556714b00918 in qmp_marshal_human_monitor_command
/home/elmarco/src/qemu/build/qmp-marshal.c:1704
#8 0x556715138a3e in do_qmp_dispatch
/home/elmarco/src/qemu/qapi/qmp-dispatch.c:104
#9 0x556715138f83 in qmp_dispatch
/home/elmarco/src/qemu/qapi/qmp-dispatch.c:131
#10 0x55671485cf88 in handle_qmp_command
/home/elmarco/src/qemu/monitor.c:3839
#11 0x55671514e80b in json_message_process_token
/home/elmarco/src/qemu/qobject/json-streamer.c:105
#12 0x5567151bf2dc in json_lexer_feed_char
/home/elmarco/src/qemu/qobject/json-lexer.c:323
#13 0x5567151bf827 in json_lexer_feed
/home/elmarco/src/qemu/qobject/json-lexer.c:373
#14 0x55671514ee62 in json_message_parser_feed
/home/elmarco/src/qemu/qobject/json-streamer.c:124
#15 0x556714854b1f in monitor_qmp_read /home/elmarco/src/qemu/monitor.c:3881
#16 0x556715045440 in qemu_chr_be_write_impl
/home/elmarco/src/qemu/chardev/char.c:172
#17 0x556715047184 in qemu_chr_be_write
/home/elmarco/src/qemu/chardev/char.c:184
#18 0x55671505a8e6 in tcp_chr_read
/home/elmarco/src/qemu/chardev/char-socket.c:440
#19 0x5567150943c3 in qio_channel_fd_source_dispatch
/home/elmarco/src/qemu/io/channel-watch.c:84
#20 0x7fb90292b90b in g_main_dispatch ../glib/gmain.c:3182
#21 0x7fb90292c7ac in g_main_context_dispatch ../glib/gmain.c:3847
#22 0x556715162eca in glib_pollfds_poll
/home/elmarco/src/qemu/util/main-loop.c:214
#23 0x556715163001 in os_host_main_loop_wait
/home/elmarco/src/qemu/util/main-loop.c:261
#24 0x5567151631fa in main_loop_wait
/home/elmarco/src/qemu/util/main-loop.c:515
#25 0x556714ad6d3b in main_loop /home/elmarco/src/qemu/vl.c:1950
#26 0x556714ade329 in main /home/elmarco/src/qemu/vl.c:4865
#27 0x7fb8fe5c9009 in __libc_start_main (/lib64/libc.so.6+0x21009)
#28 0x5567147af4d9 in _start
(/home/elmarco/src/qemu/build/s390x-softmmu/qemu-system-s390x+0xf674d9)
0x556715a1f120 is located 32 bytes to the left of global variable
'char_hci_type_info' defined in '/home/elmarco/src/qemu/hw/bt/hci-csr.c:493:23'
(0x556715a1f140) of size 104
0x556715a1f120 is located 8 bytes to the right of global variable
's390_opcodes' defined in '/home/elmarco/src/qemu/disas/s390.c:860:33'
(0x556715a15280) of size 40600
This fix is based on Andreas Arnez upstream
commit:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=9ace48f3d7d80ce09c5df60cccb433470410b11b
2014-08-19 Andreas Arnez
* s390-dis.c (init_disasm): Simplify initialization of
opc_index[]. This also fixes an access after the last element
of s390_opcodes[].
Signed-off-by: Marc-André Lureau
---
disas/s390.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/disas/s390.c b/disas/s390.c
index 1f167d2eaa..6393860239 100644
--- a/disas/s390.c
+++ b/disas/s390.c
@@ -207,18 +207,14 @@ static int opc_index[256];
static void
init_disasm (struct disassemble_info *info)
{
- const struct s390_opcode *opcode;
- const struct s390_opcode *opcode_end;
+ int i;
memset (opc_index, 0, sizeof (opc_index));
- opcode_end = s390_opcodes + s390_num_opcodes;
- for (opcode = s390_opcodes; opcode < opcode_end; opcode++)
-{
- opc_index[(int) opcode->opcode[0]] = opcode - s390_opcodes;
- while ((opcode < opcode_end) &&
-(opcode[1].opcode[0] == opcode->opcode[0]))
- opcode++;
-}
+
+ /* Reverse order, such that each opc_index ends up pointing to the
+ first matching entry instead of the last. */
+ for (i = s390_num_opcodes; i--; )
+opc_index[s390_opcodes[i].opcode[0]] = i;
#ifdef QEMU_DISABLE
switch (info->mach)
--
2.15.1.355.g36791d7216
[Qemu-devel] [PATCH v3 16/18] tests: fix coroutine leak in /basic/entered
The coroutine is not finished by the time the test ends, resulting in ASAN warning: ==7005==ERROR: LeakSanitizer: detected memory leaks Direct leak of 312 byte(s) in 1 object(s) allocated from: #0 0x7fd35290fa38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38) #1 0x7fd3506c5f75 in g_malloc0 ../glib/gmem.c:124 #2 0x55994af03e47 in qemu_coroutine_new /home/elmarco/src/qemu/util/coroutine-ucontext.c:144 #3 0x55994aefed99 in qemu_coroutine_create /home/elmarco/src/qemu/util/qemu-coroutine.c:76 #4 0x55994ac1eb50 in verify_entered_step_1 /home/elmarco/src/qemu/tests/test-coroutine.c:80 #5 0x55994af03c75 in coroutine_trampoline /home/elmarco/src/qemu/util/coroutine-ucontext.c:119 #6 0x7fd34ec02bef (/lib64/libc.so.6+0x50bef) Do not yield() to let the coroutine terminate. Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Hajnoczi --- tests/test-coroutine.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test-coroutine.c b/tests/test-coroutine.c index abd97c23c1..76c646107e 100644 --- a/tests/test-coroutine.c +++ b/tests/test-coroutine.c @@ -67,7 +67,6 @@ static void coroutine_fn verify_entered_step_2(void *opaque) /* Once more to check it still works after yielding */ g_assert(qemu_coroutine_entered(caller)); g_assert(qemu_coroutine_entered(qemu_coroutine_self())); -qemu_coroutine_yield(); } static void coroutine_fn verify_entered_step_1(void *opaque) -- 2.15.1.355.g36791d7216
Re: [Qemu-devel] [RFC PATCH 2/6] hw/net/e1000: real device name is 'e1000-82540em', 'e1000' is an alias
* Philippe Mathieu-Daudé (f4...@amsat.org) wrote:
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/net/e1000.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 05a00cba31..2280f7fdf9 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -1648,6 +1648,7 @@ typedef struct E1000Info {
> uint16_t device_id;
> uint8_trevision;
> uint16_t phy_id2;
> +const char **aliases;
> } E1000Info;
>
> static void e1000_class_init(ObjectClass *klass, void *data)
> @@ -1695,10 +1696,11 @@ static const TypeInfo e1000_base_info = {
>
> static const E1000Info e1000_devices[] = {
> {
> -.name = "e1000",
> +.name = "e1000-82540em",
> .device_id = E1000_DEV_ID_82540EM,
> .revision = 0x03,
> .phy_id2 = E1000_PHY_ID2_8254xx_DEFAULT,
> +.aliases = (const char * []) {"e1000", NULL},
> },
> {
> .name = "e1000-82544gc",
> @@ -1725,6 +1727,7 @@ static void e1000_register_types(void)
>
> type_info.name = info->name;
> type_info.parent = TYPE_E1000_BASE;
> +type_info.aliases = info->aliases;
> type_info.class_data = (void *)info;
> type_info.class_init = e1000_class_init;
> type_info.instance_init = e1000_instance_init;
Can you just check that this doesn't break migration compatibility
either way please; I think there's some alias code somewhere but I
can't remember the details.
One thing I do remember that broke when it previously changed was
entries in PC_COMPAT_* tables like the one in hw/i386/pc_piix.c's
PC_COMPAT_1_3 - so check with an 'info qtree' that the property is OK.
Dave
> --
> 2.15.1
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v1] hmp: 'info snapshots' not showing the id
On 01/02/2018 01:54 PM, Markus Armbruster wrote: seeteena writes: On 12/22/2017 01:07 PM, Markus Armbruster wrote: Eric Blake writes: On 12/19/2017 08:20 AM, Max Reitz wrote: So there are three things: (1) We probably should not allow snapshot names that could be IDs. Easiest way to solve this: Names have to start with a non-digit. Yes, that would be a nice change. It is not strictly backwards compatible (so we'd still have to cope with images that didn't follow the rule, whether created by older qemu or by non-qemu implementations of qcow2), but would alleviate a lot of confusion. I recommend to restrict ID strings to letters, digits, '-', '.', '_', starting with a letter. Use id_wellformed() to check. char *id_generate(IdSubSystems id) autogenates ID in the format eg- "#block146" with starting with '#' always. restricting ID strings to use id_wellformed() means we need to force ID to start with letter. #define ID_SPECIAL_CHAR '#' needs to change to (some letter) #define ID_SPECIAL_CHAR 'A' Ensuring auto-generated IDs cannot clash with IDs chosen by the user is a good idea. We generally fail to do that in QEMU. An obvious way to do it is to require the user's IDs to start with a letter, and auto-generated IDs to start with another suitable character. In theory, starting with '#' is problematic, because it needs to be quoted in shell at the beginning of words. Only matters if auto-generated IDs make sense in command lines, which I guess they currently don't. I'd prefer a 'shell-safe' character all the same. I was using loaned system from other team for recreate and it appears there is some issue in that system and I am not able to recreate the issue now. QEMU 2.11.50 monitor - type 'help' for more information (qemu) savevm 0 Error while creating snapshot on 'rootdisk' (qemu) savevm 0 Error while creating snapshot on 'rootdisk' (qemu) savevm 0 Error while creating snapshot on 'rootdisk' (qemu) savevm 1 Error while creating snapshot on 'rootdisk' (qemu) If backward compatibility is an issue, deprecate offending IDs (with a suitable warning), and kill them off after the customary grace period. IDs embedded in image files and such you may have to keep working somehow indefinitely.
Re: [Qemu-devel] [PATCH v3 17/18] mips: fix potential fopen(NULL, ...)
Hi Marc-André,
On 01/04/2018 01:05 PM, Marc-André Lureau wrote:
> Spotted thanks to ASAN.
>
> Signed-off-by: Marc-André Lureau
> ---
> hw/nvram/ds1225y.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/nvram/ds1225y.c b/hw/nvram/ds1225y.c
> index 57d5ab2154..ad7345f288 100644
> --- a/hw/nvram/ds1225y.c
> +++ b/hw/nvram/ds1225y.c
> @@ -80,7 +80,7 @@ static int nvram_post_load(void *opaque, int version_id)
> }
>
More diffstats, but this let the code simpler imho:
if (s->filename) {
> /* Write back nvram contents */
> -s->file = fopen(s->filename, "wb");
> +s->file = s->filename ? fopen(s->filename, "wb") : NULL;
> if (s->file) {
> /* Write back contents, as 'wb' mode cleaned the file */
> if (fwrite(s->contents, s->chip_size, 1, s->file) != 1) {
...
}
> @@ -126,7 +126,7 @@ static int nvram_sysbus_initfn(SysBusDevice *dev)
> sysbus_init_mmio(dev, &s->iomem);
>
> /* Read current file */
> -file = fopen(s->filename, "rb");
> +file = s->filename ? fopen(s->filename, "rb") : NULL;
> if (file) {
> /* Read nvram contents */
> if (fread(s->contents, s->chip_size, 1, file) != 1) {
>
ditto.
Re: [Qemu-devel] [PATCH net-next v3 1/3] virtio_net: propagate linkspeed/duplex settings from the hypervisor
On Thu, Jan 04, 2018 at 12:16:44AM -0500, Jason Baron wrote:
> The ability to set speed and duplex for virtio_net is useful in various
> scenarios as described here:
>
> 16032be virtio_net: add ethtool support for set and get of settings
>
> However, it would be nice to be able to set this from the hypervisor,
> such that virtio_net doesn't require custom guest ethtool commands.
>
> Introduce a new feature flag, VIRTIO_NET_F_SPEED_DUPLEX, which allows
> the hypervisor to export a linkspeed and duplex setting. The user can
> subsequently overwrite it later if desired via: 'ethtool -s'.
>
> Note that VIRTIO_NET_F_SPEED_DUPLEX is defined as bit 63, the intention
> is that device feature bits are to grow down from bit 63, since the
> transports are starting from bit 24 and growing up.
>
> Signed-off-by: Jason Baron
> Cc: "Michael S. Tsirkin"
> Cc: Jason Wang
> Cc: virtio-...@lists.oasis-open.org
> ---
> drivers/net/virtio_net.c| 19 ++-
> include/uapi/linux/virtio_net.h | 13 +
> 2 files changed, 31 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 6fb7b65..0b2d314 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2146,6 +2146,22 @@ static void virtnet_config_changed_work(struct
> work_struct *work)
>
> vi->status = v;
>
> + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_SPEED_DUPLEX)) {
> + u32 speed;
> + u8 duplex;
> +
> + speed = virtio_cread32(vi->vdev,
> +offsetof(struct virtio_net_config,
> + speed));
> + if (ethtool_validate_speed(speed))
> + vi->speed = speed;
> + duplex = virtio_cread8(vi->vdev,
> +offsetof(struct virtio_net_config,
> + duplex));
> + if (ethtool_validate_duplex(duplex))
> + vi->duplex = duplex;
> + }
> +
> if (vi->status & VIRTIO_NET_S_LINK_UP) {
> netif_carrier_on(vi->dev);
> netif_tx_wake_all_queues(vi->dev);
> @@ -2796,7 +2812,8 @@ static struct virtio_device_id id_table[] = {
> VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN, \
> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ, \
> VIRTIO_NET_F_CTRL_MAC_ADDR, \
> - VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS
> + VIRTIO_NET_F_MTU, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, \
> + VIRTIO_NET_F_SPEED_DUPLEX
>
> static unsigned int features[] = {
> VIRTNET_FEATURES,
Still missing the update from virtnet_config_changed_work, and I think
it's important to reflex host changes within guest when the
feature bit has been acked.
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index fc353b5..5de6ed3 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -57,6 +57,8 @@
>* Steering */
> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23/* Set MAC address */
>
> +#define VIRTIO_NET_F_SPEED_DUPLEX 63 /* Device set linkspeed and duplex */
> +
> #ifndef VIRTIO_NET_NO_LEGACY
> #define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
> #endif /* VIRTIO_NET_NO_LEGACY */
> @@ -76,6 +78,17 @@ struct virtio_net_config {
> __u16 max_virtqueue_pairs;
> /* Default maximum transmit unit advice */
> __u16 mtu;
> + /*
> + * speed, in units of 1Mb. All values 0 to INT_MAX are legal.
> + * Any other value stands for unknown.
> + */
> + __u32 speed;
> + /*
> + * 0x00 - half duplex
> + * 0x01 - full duplex
> + * Any other value stands for unknown.
> + */
> + __u8 duplex;
> } __attribute__((packed));
>
> /*
> --
> 2.6.1
Re: [Qemu-devel] [RFC PATCH 2/6] hw/net/e1000: real device name is 'e1000-82540em', 'e1000' is an alias
Hi David,
On 01/04/2018 01:24 PM, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (f4...@amsat.org) wrote:
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> hw/net/e1000.c | 5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
>> index 05a00cba31..2280f7fdf9 100644
>> --- a/hw/net/e1000.c
>> +++ b/hw/net/e1000.c
>> @@ -1648,6 +1648,7 @@ typedef struct E1000Info {
>> uint16_t device_id;
>> uint8_trevision;
>> uint16_t phy_id2;
>> +const char **aliases;
>> } E1000Info;
>>
>> static void e1000_class_init(ObjectClass *klass, void *data)
>> @@ -1695,10 +1696,11 @@ static const TypeInfo e1000_base_info = {
>>
>> static const E1000Info e1000_devices[] = {
>> {
>> -.name = "e1000",
>> +.name = "e1000-82540em",
>> .device_id = E1000_DEV_ID_82540EM,
>> .revision = 0x03,
>> .phy_id2 = E1000_PHY_ID2_8254xx_DEFAULT,
>> +.aliases = (const char * []) {"e1000", NULL},
>> },
>> {
>> .name = "e1000-82544gc",
>> @@ -1725,6 +1727,7 @@ static void e1000_register_types(void)
>>
>> type_info.name = info->name;
>> type_info.parent = TYPE_E1000_BASE;
>> +type_info.aliases = info->aliases;
>> type_info.class_data = (void *)info;
>> type_info.class_init = e1000_class_init;
>> type_info.instance_init = e1000_instance_init;
>
> Can you just check that this doesn't break migration compatibility
> either way please; I think there's some alias code somewhere but I
> can't remember the details.
Good point.
Something related to qdev-monitor.c?
> One thing I do remember that broke when it previously changed was
> entries in PC_COMPAT_* tables like the one in hw/i386/pc_piix.c's
> PC_COMPAT_1_3 - so check with an 'info qtree' that the property is OK.
Ok, I'll verify and think about a qtest for that.
I took this device because I wanted to test another arch than ARM (which
strongly use FDT).
If there is an issue migrating this particular device we can drop this
patch for this only device, and the series still stands for the ARM/FDT.
Regards,
Phil.
signature.asc
Description: OpenPGP digital signature
