RE: [XEN PATCH for-4.14] Config: Update QEMU
> -Original Message- > From: Wei Liu > Sent: 03 July 2020 21:11 > To: Anthony PERARD > Cc: Paul Durrant ; xen-devel@lists.xenproject.org; Roger Pau > Monné > ; Ian Jackson ; Wei Liu > > Subject: Re: [XEN PATCH for-4.14] Config: Update QEMU > > On Fri, Jul 03, 2020 at 02:55:33PM +0100, Anthony PERARD wrote: > > Backport 2 commits to fix building QEMU without PCI passthrough > > support. > > > > Signed-off-by: Anthony PERARD > > FWIW: > > Acked-by: Wei Liu Release-acked-by: Paul Durrant
RE: vPT rework (and timer mode)
> -Original Message- > From: Andrew Cooper > Sent: 03 July 2020 16:03 > To: Jan Beulich ; Roger Pau Monné > Cc: xen-devel@lists.xenproject.org; Wei Liu ; Paul Durrant > > Subject: Re: vPT rework (and timer mode) > > On 03/07/2020 15:50, Jan Beulich wrote: > > On 01.07.2020 11:02, Roger Pau Monné wrote: > >> It's my understanding that the purpose of pt_update_irq and > >> pt_intr_post is to attempt to implement the "delay for missed ticks" > >> mode, where Xen will accumulate timer interrupts if they cannot be > >> injected. As shown by the patch above, this is all broken when the > >> timer is added to a vCPU (pt->vcpu) different than the actual target > >> vCPU where the interrupt gets delivered (note this can also be a list > >> of vCPUs if routed from the IO-APIC using Fixed mode). > >> > >> I'm at lost at how to fix this so that virtual timers work properly > >> and we also keep the "delay for missed ticks" mode without doing a > >> massive rework and somehow keeping track of where injected interrupts > >> originated, which seems an overly complicated solution. > >> > >> My proposal hence would be to completely remove the timer_mode, and > >> just treat virtual timer interrupts as other interrupts, ie: they will > >> be injected from the callback (pt_timer_fn) and the vCPU(s) would be > >> kicked. Whether interrupts would get lost (ie: injected when a > >> previous one is still pending) depends on the contention on the > >> system. I'm not aware of any current OS that uses timer interrupts as > >> a way to track time. I think current OSes know the differences between > >> a timer counter and an event timer, and will use them appropriately. > > Fundamentally - why not, the more that this promises to be a > > simplification. The question we need to answer up front is whether > > we're happy to possibly break old OSes (presumably ones no-one > > ought to be using anymore these days, due to their support life > > cycles long having ended). > > The various timer modes were all compatibility, and IIRC, mostly for > Windows XP and older which told time by counting the number of timer > interrupts. > > Paul - you might remember better than me? I think it is only quite recently that Windows has started favouring enlightened time sources rather than counting ticks but an admin may still turn all the viridian enlightenments off so just dropping ticks will probably still cause time to drift backwards. Paul > > Its possibly worth noting that issues in this are cause triple faults in > OVMF (it seems to enable interrupts in its timer handler), and breaks > in-guest kexec (because our timer-targetting logic doesn't work in a way > remotely close to real hardware when the kexec kernel is booting on a > non-zero vCPU). > > ~Andrew
[qemu-mainline test] 151656: regressions - FAIL
flight 151656 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/151656/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 151065 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qcow210 debian-di-installfail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-i386-libvirt-pair 21 guest-start/debian fail REGR. vs. 151065 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 151065 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 151065 test-amd64-amd64-libvirt 12 guest-start fail REGR. vs. 151065 test-amd64-amd64-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-amd64-i386-libvirt 12 guest-start fail REGR. vs. 151065 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 151065 test-armhf-armhf-xl-vhd 10 debian-di-installfail REGR. vs. 151065 test-amd64-amd64-libvirt-pair 21 guest-start/debian fail REGR. vs. 151065 test-armhf-armhf-libvirt 12 guest-start fail REGR. vs. 151065 test-arm64-arm64-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 151065 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-sup
Re: [PATCH v11 8/8] xen: introduce ERRP_AUTO_PROPAGATE
Philippe Mathieu-Daudé writes:
> On 7/3/20 11:08 AM, Vladimir Sementsov-Ogievskiy wrote:
>> If we want to add some info to errp (by error_prepend() or
>> error_append_hint()), we must use the ERRP_AUTO_PROPAGATE macro.
>> Otherwise, this info will not be added when errp == &error_fatal
>> (the program will exit prior to the error_append_hint() or
>> error_prepend() call). Fix such cases.
>>
>> If we want to check error after errp-function call, we need to
>> introduce local_err and then propagate it to errp. Instead, use
>> ERRP_AUTO_PROPAGATE macro, benefits are:
>> 1. No need of explicit error_propagate call
>> 2. No need of explicit local_err variable: use errp directly
>> 3. ERRP_AUTO_PROPAGATE leaves errp as is if it's not NULL or
>>&error_fatal, this means that we don't break error_abort
>>(we'll abort on error_set, not on error_propagate)
>>
>> This commit is generated by command
>>
>> sed -n '/^X86 Xen CPUs$/,/^$/{s/^F: //p}' MAINTAINERS | \
>> xargs git ls-files | grep '\.[hc]$' | \
>> xargs spatch \
>> --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
>> --macro-file scripts/cocci-macro-file.h \
>> --in-place --no-show-diff --max-width 80
>>
>> Reported-by: Kevin Wolf
>> Reported-by: Greg Kurz
>> Signed-off-by: Vladimir Sementsov-Ogievskiy
>> ---
>> hw/block/dataplane/xen-block.c | 17 +++---
>> hw/block/xen-block.c | 102 ++---
>> hw/pci-host/xen_igd_pt.c | 7 +--
>> hw/xen/xen-backend.c | 7 +--
>> hw/xen/xen-bus.c | 92 +
>> hw/xen/xen-host-pci-device.c | 27 +
>> hw/xen/xen_pt.c| 25
>> hw/xen/xen_pt_config_init.c| 17 +++---
>> 8 files changed, 128 insertions(+), 166 deletions(-)
>
> Without the description, this patch has 800 lines of diff...
> It killed me, I don't have the energy to review patch #7 of this
> series after that, sorry.
> Consider splitting such mechanical patches next time. Here it
> could have been hw/block, hw/pci-host, hw/xen.
Probably my fault; I asked for less fine-grained splitting.
Finding a split of a tree-wide transformation that pleases everyone is
basically impossible.
The conversion to ERRP_AUTO_PROPAGATE() could be one patch per function,
but that would be excessive.
Vladimir chose to split along maintenance boundaries, so he can cc: the
right people on the right code. I agree with the idea. The difficulty
is which boundaries. Our code is not partitioned into maintenance
domains. Instead, we have overlapping sets. Makes sense, because it
mirrors how we actually maintain it.
Because of that, a blind split guided by MAINTAINERS won't work well. A
split that makes sense needs a bit of human judgement, too.
This part makes perfect sense to me from the cc: point of view: it's
Xen, the whole of Xen, and nothing but Xen.
I acknowledge that its size made it exhausting for you to review. I
didn't expect that, probably because after having spent hours on
reviewing and improving the macro and the Coccinelle script, I know
exactly what to look for, and also consider the script trustworthy[*].
> Reviewed-by: Philippe Mathieu-Daudé
Thank you, much appreciated!
[*] I've learned not to trust Coccinelle 100%, ever.
Re: [PATCH v11 8/8] xen: introduce ERRP_AUTO_PROPAGATE
04.07.2020 19:36, Philippe Mathieu-Daudé wrote:
On 7/3/20 11:08 AM, Vladimir Sementsov-Ogievskiy wrote:
If we want to add some info to errp (by error_prepend() or
error_append_hint()), we must use the ERRP_AUTO_PROPAGATE macro.
Otherwise, this info will not be added when errp == &error_fatal
(the program will exit prior to the error_append_hint() or
error_prepend() call). Fix such cases.
If we want to check error after errp-function call, we need to
introduce local_err and then propagate it to errp. Instead, use
ERRP_AUTO_PROPAGATE macro, benefits are:
1. No need of explicit error_propagate call
2. No need of explicit local_err variable: use errp directly
3. ERRP_AUTO_PROPAGATE leaves errp as is if it's not NULL or
&error_fatal, this means that we don't break error_abort
(we'll abort on error_set, not on error_propagate)
This commit is generated by command
sed -n '/^X86 Xen CPUs$/,/^$/{s/^F: //p}' MAINTAINERS | \
xargs git ls-files | grep '\.[hc]$' | \
xargs spatch \
--sp-file scripts/coccinelle/auto-propagated-errp.cocci \
--macro-file scripts/cocci-macro-file.h \
--in-place --no-show-diff --max-width 80
Reported-by: Kevin Wolf
Reported-by: Greg Kurz
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
hw/block/dataplane/xen-block.c | 17 +++---
hw/block/xen-block.c | 102 ++---
hw/pci-host/xen_igd_pt.c | 7 +--
hw/xen/xen-backend.c | 7 +--
hw/xen/xen-bus.c | 92 +
hw/xen/xen-host-pci-device.c | 27 +
hw/xen/xen_pt.c| 25
hw/xen/xen_pt_config_init.c| 17 +++---
8 files changed, 128 insertions(+), 166 deletions(-)
Without the description, this patch has 800 lines of diff...
It killed me, I don't have the energy to review patch #7 of this
series after that, sorry.
Sorry for that! I really understand you, take a look at Markus's
"[PATCH v2 00/44] Less clumsy error checking", which I'm trying to review
currently..
Still, the patch exists in such form since
"[RFC v5 000/126] error: auto propagated local_err", where it was reviewed
by Anthony, and I suggested to split it, but it was not needed.
Unfortunately, I've dropped r-bs due to changes..
Consider splitting such mechanical patches next time. Here it
could have been hw/block, hw/pci-host, hw/xen.
Reviewed-by: Philippe Mathieu-Daudé
Thanks a lot!
--
Best regards,
Vladimir
Re: [PATCH v5 06/11] x86/hvm: processor trace interface in HVM
On 05.07.2020 21:11, Michał Leszczyński wrote:
> - 5 lip 2020 o 20:54, Michał Leszczyński michal.leszczyn...@cert.pl
> napisał(a):
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -2199,6 +2199,25 @@ int domain_relinquish_resources(struct domain *d)
>> altp2m_vcpu_disable_ve(v);
>> }
>>
>> +for_each_vcpu ( d, v )
>> +{
>> +unsigned int i;
>> +
>> +if ( !v->vmtrace.pt_buf )
>> +continue;
>> +
>> +for ( i = 0; i < (v->domain->vmtrace_pt_size >> PAGE_SHIFT);
>> i++ )
>> +{
>> +struct page_info *pg = mfn_to_page(
>> +mfn_add(page_to_mfn(v->vmtrace.pt_buf), i));
>> +if ( (pg->count_info & PGC_count_mask) != 1 )
>> +return -EBUSY;
>> +}
>> +
>> +free_domheap_pages(v->vmtrace.pt_buf,
>> +get_order_from_bytes(v->domain->vmtrace_pt_size));
>
>
> While this works, I don't feel that this is a good solution with this loop
> returning -EBUSY here. I would like to kindly ask for suggestions regarding
> this topic.
I'm sorry to ask, but with the previously give suggestions to mirror
existing code, why do you still need to play with this function? You
really shouldn't have a need to, just like e.g. the ioreq server page
handling code didn't.
Jan
Re: vPT rework (and timer mode)
On Mon, Jul 06, 2020 at 08:03:50AM +0100, Paul Durrant wrote: > > -Original Message- > > From: Andrew Cooper > > Sent: 03 July 2020 16:03 > > To: Jan Beulich ; Roger Pau Monné > > Cc: xen-devel@lists.xenproject.org; Wei Liu ; Paul Durrant > > > > Subject: Re: vPT rework (and timer mode) > > > > On 03/07/2020 15:50, Jan Beulich wrote: > > > On 01.07.2020 11:02, Roger Pau Monné wrote: > > >> It's my understanding that the purpose of pt_update_irq and > > >> pt_intr_post is to attempt to implement the "delay for missed ticks" > > >> mode, where Xen will accumulate timer interrupts if they cannot be > > >> injected. As shown by the patch above, this is all broken when the > > >> timer is added to a vCPU (pt->vcpu) different than the actual target > > >> vCPU where the interrupt gets delivered (note this can also be a list > > >> of vCPUs if routed from the IO-APIC using Fixed mode). > > >> > > >> I'm at lost at how to fix this so that virtual timers work properly > > >> and we also keep the "delay for missed ticks" mode without doing a > > >> massive rework and somehow keeping track of where injected interrupts > > >> originated, which seems an overly complicated solution. > > >> > > >> My proposal hence would be to completely remove the timer_mode, and > > >> just treat virtual timer interrupts as other interrupts, ie: they will > > >> be injected from the callback (pt_timer_fn) and the vCPU(s) would be > > >> kicked. Whether interrupts would get lost (ie: injected when a > > >> previous one is still pending) depends on the contention on the > > >> system. I'm not aware of any current OS that uses timer interrupts as > > >> a way to track time. I think current OSes know the differences between > > >> a timer counter and an event timer, and will use them appropriately. > > > Fundamentally - why not, the more that this promises to be a > > > simplification. The question we need to answer up front is whether > > > we're happy to possibly break old OSes (presumably ones no-one > > > ought to be using anymore these days, due to their support life > > > cycles long having ended). > > > > The various timer modes were all compatibility, and IIRC, mostly for > > Windows XP and older which told time by counting the number of timer > > interrupts. > > > > Paul - you might remember better than me? > > I think it is only quite recently that Windows has started favouring > enlightened time sources rather than counting ticks but an admin may still > turn all the viridian enlightenments off so just dropping ticks will probably > still cause time to drift backwards. Even when not using the viridian enlightenments, Windows should rely on emulated time counters (or the TSC) rather than counting ticks? I guess I could give it a try with one of the emulated Windows versions that we test on osstest. Thanks, Roger.
Re: [PATCH v5 11/11] tools/proctrace: add proctrace tool
On 05.07.2020 20:58, Michał Leszczyński wrote: > - 5 lip 2020 o 20:55, Michał Leszczyński michal.leszczyn...@cert.pl > napisał(a): >> --- /dev/null >> +++ b/tools/proctrace/proctrace.c >> +#include >> +#include >> +#include >> +#include >> + >> +#include >> +#include >> +#include >> + >> +#define BUF_SIZE (16384 * XC_PAGE_SIZE) > > I would like to discuss here, how we should retrieve the trace buffer size > in runtime? Should there be a hypercall for it, or some extension to > acquire_resource logic? Personally I'd prefer the latter, but the question is whether one can be made in a backwards compatible way. Jan
Re: [PATCH v5 06/11] x86/hvm: processor trace interface in HVM
On Sun, Jul 05, 2020 at 08:54:59PM +0200, Michał Leszczyński wrote:
> From: Michal Leszczynski
>
> Implement necessary changes in common code/HVM to support
> processor trace features. Define vmtrace_pt_* API and
> implement trace buffer allocation/deallocation in common
> code.
>
> Signed-off-by: Michal Leszczynski
> ---
> xen/arch/x86/domain.c | 19 +++
> xen/common/domain.c | 19 +++
> xen/include/asm-x86/hvm/hvm.h | 20
> xen/include/xen/sched.h | 4
> 4 files changed, 62 insertions(+)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index fee6c3931a..79c9794408 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -2199,6 +2199,25 @@ int domain_relinquish_resources(struct domain *d)
> altp2m_vcpu_disable_ve(v);
> }
>
> +for_each_vcpu ( d, v )
> +{
> +unsigned int i;
> +
> +if ( !v->vmtrace.pt_buf )
> +continue;
> +
> +for ( i = 0; i < (v->domain->vmtrace_pt_size >> PAGE_SHIFT); i++
> )
> +{
> +struct page_info *pg = mfn_to_page(
> +mfn_add(page_to_mfn(v->vmtrace.pt_buf), i));
> +if ( (pg->count_info & PGC_count_mask) != 1 )
> +return -EBUSY;
> +}
> +
> +free_domheap_pages(v->vmtrace.pt_buf,
> +get_order_from_bytes(v->domain->vmtrace_pt_size));
This is racy as a control domain could take a reference between the
check and the freeing.
> +}
> +
> if ( is_pv_domain(d) )
> {
> for_each_vcpu ( d, v )
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 25d3359c5b..f480c4e033 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -137,6 +137,21 @@ static void vcpu_destroy(struct vcpu *v)
> free_vcpu_struct(v);
> }
>
> +static int vmtrace_alloc_buffers(struct vcpu *v)
> +{
> +struct page_info *pg;
> +uint64_t size = v->domain->vmtrace_pt_size;
> +
> +pg = alloc_domheap_pages(v->domain, get_order_from_bytes(size),
> + MEMF_no_refcount);
> +
> +if ( !pg )
> +return -ENOMEM;
> +
> +v->vmtrace.pt_buf = pg;
> +return 0;
> +}
I think we already agreed that you would use the same model as ioreq
servers, where a reference is taken on allocation and then the pages
are not explicitly freed on domain destruction and put_page_and_type
is used. Is there some reason why that model doesn't work in this
case?
If not, please see hvm_alloc_ioreq_mfn and hvm_free_ioreq_mfn.
Roger.
Re: [PATCH v4 03/10] tools/libxl: add vmtrace_pt_size parameter
On 04.07.2020 19:23, Julien Grall wrote:
> Hi,
>
> On 03/07/2020 11:11, Roger Pau Monné wrote:
>> On Fri, Jul 03, 2020 at 11:56:38AM +0200, Jan Beulich wrote:
>>> On 03.07.2020 11:44, Roger Pau Monné wrote:
On Thu, Jul 02, 2020 at 06:23:28PM +0200, Michał Leszczyński wrote:
> - 2 lip 2020 o 11:00, Roger Pau Monné roger@citrix.com napisał(a):
>
>> On Tue, Jun 30, 2020 at 02:33:46PM +0200, Michał Leszczyński wrote:
>>> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
>>> index 59bdc28c89..7b8289d436 100644
>>> --- a/xen/include/public/domctl.h
>>> +++ b/xen/include/public/domctl.h
>>> @@ -92,6 +92,7 @@ struct xen_domctl_createdomain {
>>> uint32_t max_evtchn_port;
>>> int32_t max_grant_frames;
>>> int32_t max_maptrack_frames;
>>> +uint8_t vmtrace_pt_order;
>>
>> I've been thinking about this, and even though this is a domctl (so
>> not a stable interface) we might want to consider using a size (or a
>> number of pages) here rather than an order. IPT also supports
>> TOPA mode (kind of a linked list of buffers) that would allow for
>> sizes not rounded to order boundaries to be used, since then only each
>> item in the linked list needs to be rounded to an order boundary, so
>> you could for example use three 4K pages in TOPA mode AFAICT.
>>
>> Roger.
>
> In previous versions it was "size" but it was requested to change it
> to "order" in order to shrink the variable size from uint64_t to
> uint8_t, because there is limited space for xen_domctl_createdomain
> structure.
It's likely I'm missing something here, but I wasn't aware
xen_domctl_createdomain had any constrains regarding it's size. It's
currently 48bytes which seems fairly small.
>>>
>>> Additionally I would guess a uint32_t could do here, if the value
>>> passed was "number of pages" rather than "number of bytes"?
> Looking at the rest of the code, the toolstack accepts a 64-bit value.
> So this would lead to truncation of the buffer if it is bigger than 2^44
> bytes.
>
> I agree such buffer is unlikely, yet I still think we want to harden the
> code whenever we can. So the solution is to either prevent check
> truncation in libxl or directly use 64-bit in the domctl.
>
> My preference is the latter.
>
>>
>> That could work, not sure if it needs to state however that those will
>> be 4K pages, since Arm can have a different minimum page size IIRC?
>> (or that's already the assumption for all number of frames fields)
>> vmtrace_nr_frames seems fine to me.
>
> The hypercalls interface is using the same page granularity as the
> hypervisor (i.e 4KB).
>
> While we already support guest using 64KB page granularity, it is
> impossible to have a 64KB Arm hypervisor in the current state. You are
> going to either break existing guest (if you switch to 64KB page
> granularity for the hypercall ABI) or render them insecure (the mimimum
> mapping in the P2M would be 64KB).
>
> DOMCTLs are not stable yet, so using a number of pages is OK. However, I
> would strongly suggest to use a number of bytes for any xl/libxl/stable
> libraries interfaces as this avoids confusion and also make more
> futureproof.
If we can't settle on what "page size" means in the public interface
(which imo is embarrassing), then how about going with number of kb,
like other memory libxl controls do? (I guess using Mb, in line with
other config file controls, may end up being too coarse here.) This
would likely still allow for a 32-bit field to be wide enough.
Jan
RE: vPT rework (and timer mode)
> -Original Message- > From: Roger Pau Monné > Sent: 06 July 2020 09:32 > To: p...@xen.org > Cc: 'Andrew Cooper' ; 'Jan Beulich' > ; xen- > de...@lists.xenproject.org; 'Wei Liu' > Subject: Re: vPT rework (and timer mode) > > On Mon, Jul 06, 2020 at 08:03:50AM +0100, Paul Durrant wrote: > > > -Original Message- > > > From: Andrew Cooper > > > Sent: 03 July 2020 16:03 > > > To: Jan Beulich ; Roger Pau Monné > > > > > > Cc: xen-devel@lists.xenproject.org; Wei Liu ; Paul Durrant > > > > > > Subject: Re: vPT rework (and timer mode) > > > > > > On 03/07/2020 15:50, Jan Beulich wrote: > > > > On 01.07.2020 11:02, Roger Pau Monné wrote: > > > >> It's my understanding that the purpose of pt_update_irq and > > > >> pt_intr_post is to attempt to implement the "delay for missed ticks" > > > >> mode, where Xen will accumulate timer interrupts if they cannot be > > > >> injected. As shown by the patch above, this is all broken when the > > > >> timer is added to a vCPU (pt->vcpu) different than the actual target > > > >> vCPU where the interrupt gets delivered (note this can also be a list > > > >> of vCPUs if routed from the IO-APIC using Fixed mode). > > > >> > > > >> I'm at lost at how to fix this so that virtual timers work properly > > > >> and we also keep the "delay for missed ticks" mode without doing a > > > >> massive rework and somehow keeping track of where injected interrupts > > > >> originated, which seems an overly complicated solution. > > > >> > > > >> My proposal hence would be to completely remove the timer_mode, and > > > >> just treat virtual timer interrupts as other interrupts, ie: they will > > > >> be injected from the callback (pt_timer_fn) and the vCPU(s) would be > > > >> kicked. Whether interrupts would get lost (ie: injected when a > > > >> previous one is still pending) depends on the contention on the > > > >> system. I'm not aware of any current OS that uses timer interrupts as > > > >> a way to track time. I think current OSes know the differences between > > > >> a timer counter and an event timer, and will use them appropriately. > > > > Fundamentally - why not, the more that this promises to be a > > > > simplification. The question we need to answer up front is whether > > > > we're happy to possibly break old OSes (presumably ones no-one > > > > ought to be using anymore these days, due to their support life > > > > cycles long having ended). > > > > > > The various timer modes were all compatibility, and IIRC, mostly for > > > Windows XP and older which told time by counting the number of timer > > > interrupts. > > > > > > Paul - you might remember better than me? > > > > I think it is only quite recently that Windows has started favouring > > enlightened time sources rather > than counting ticks but an admin may still turn all the viridian > enlightenments off so just dropping > ticks will probably still cause time to drift backwards. > > Even when not using the viridian enlightenments, Windows should rely > on emulated time counters (or the TSC) rather than counting ticks? Microsoft implementations... sensible... two different things. > > I guess I could give it a try with one of the emulated Windows versions > that we test on osstest. > Pick an old-ish version. I think osstest has copy of Windows 7. Cheers, Paul > Thanks, Roger.
Re: [PATCH v5 11/11] tools/proctrace: add proctrace tool
On 06/07/2020 09:33, Jan Beulich wrote: > On 05.07.2020 20:58, Michał Leszczyński wrote: >> - 5 lip 2020 o 20:55, Michał Leszczyński michal.leszczyn...@cert.pl >> napisał(a): >>> --- /dev/null >>> +++ b/tools/proctrace/proctrace.c >>> +#include >>> +#include >>> +#include >>> +#include >>> + >>> +#include >>> +#include >>> +#include >>> + >>> +#define BUF_SIZE (16384 * XC_PAGE_SIZE) >> I would like to discuss here, how we should retrieve the trace buffer size >> in runtime? Should there be a hypercall for it, or some extension to >> acquire_resource logic? > Personally I'd prefer the latter, but the question is whether one can > be made in a backwards compatible way. I already covered this in v4. ~Andrew
Re: [PATCH v5 06/11] x86/hvm: processor trace interface in HVM
- 6 lip 2020 o 10:42, Roger Pau Monné roger@citrix.com napisał(a):
> On Sun, Jul 05, 2020 at 08:54:59PM +0200, Michał Leszczyński wrote:
>> From: Michal Leszczynski
>>
>> Implement necessary changes in common code/HVM to support
>> processor trace features. Define vmtrace_pt_* API and
>> implement trace buffer allocation/deallocation in common
>> code.
>>
>> Signed-off-by: Michal Leszczynski
>> ---
>> xen/arch/x86/domain.c | 19 +++
>> xen/common/domain.c | 19 +++
>> xen/include/asm-x86/hvm/hvm.h | 20
>> xen/include/xen/sched.h | 4
>> 4 files changed, 62 insertions(+)
>>
>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index fee6c3931a..79c9794408 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -2199,6 +2199,25 @@ int domain_relinquish_resources(struct domain *d)
>> altp2m_vcpu_disable_ve(v);
>> }
>>
>> +for_each_vcpu ( d, v )
>> +{
>> +unsigned int i;
>> +
>> +if ( !v->vmtrace.pt_buf )
>> +continue;
>> +
>> +for ( i = 0; i < (v->domain->vmtrace_pt_size >> PAGE_SHIFT);
>> i++ )
>> +{
>> +struct page_info *pg = mfn_to_page(
>> +mfn_add(page_to_mfn(v->vmtrace.pt_buf), i));
>> +if ( (pg->count_info & PGC_count_mask) != 1 )
>> +return -EBUSY;
>> +}
>> +
>> +free_domheap_pages(v->vmtrace.pt_buf,
>> +get_order_from_bytes(v->domain->vmtrace_pt_size));
>
> This is racy as a control domain could take a reference between the
> check and the freeing.
>
>> +}
>> +
>> if ( is_pv_domain(d) )
>> {
>> for_each_vcpu ( d, v )
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index 25d3359c5b..f480c4e033 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -137,6 +137,21 @@ static void vcpu_destroy(struct vcpu *v)
>> free_vcpu_struct(v);
>> }
>>
>> +static int vmtrace_alloc_buffers(struct vcpu *v)
>> +{
>> +struct page_info *pg;
>> +uint64_t size = v->domain->vmtrace_pt_size;
>> +
>> +pg = alloc_domheap_pages(v->domain, get_order_from_bytes(size),
>> + MEMF_no_refcount);
>> +
>> +if ( !pg )
>> +return -ENOMEM;
>> +
>> +v->vmtrace.pt_buf = pg;
>> +return 0;
>> +}
>
> I think we already agreed that you would use the same model as ioreq
> servers, where a reference is taken on allocation and then the pages
> are not explicitly freed on domain destruction and put_page_and_type
> is used. Is there some reason why that model doesn't work in this
> case?
>
> If not, please see hvm_alloc_ioreq_mfn and hvm_free_ioreq_mfn.
>
> Roger.
Ok, I've got it, will do. Thanks for pointing out the examples.
One thing that is confusing to me is that I don't get what is
the meaning of MEMF_no_refcount flag.
In the hvm_{alloc,free}_ioreq_mfn the memory is allocated
explicitly but freed just by putting out the reference, so
I guess it's automatically detected that the refcount dropped to 0
and the page should be freed? If so, why the flag is named "no refcount"?
Best regards,
Michał Leszczyński
CERT Polska
Re: [PATCH v5 04/11] common: add vmtrace_pt_size domain parameter
- 5 lip 2020 o 20:54, Michał Leszczyński michal.leszczyn...@cert.pl
napisał(a):
> From: Michal Leszczynski
>
> Add vmtrace_pt_size domain parameter in live domain and
> vmtrace_pt_order parameter in xen_domctl_createdomain.
>
> Signed-off-by: Michal Leszczynski
> ---
> xen/common/domain.c | 12
> xen/include/public/domctl.h | 1 +
> xen/include/xen/sched.h | 4
> 3 files changed, 17 insertions(+)
>
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index a45cf023f7..25d3359c5b 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -338,6 +338,12 @@ static int sanitise_domain_config(struct
> xen_domctl_createdomain *config)
> return -EINVAL;
> }
>
> +if ( config->vmtrace_pt_order && !vmtrace_supported )
> +{
> +dprintk(XENLOG_INFO, "Processor tracing is not supported\n");
> +return -EINVAL;
> +}
> +
> return arch_sanitise_domain_config(config);
> }
>
> @@ -443,6 +449,12 @@ struct domain *domain_create(domid_t domid,
> d->nr_pirqs = min(d->nr_pirqs, nr_irqs);
>
> radix_tree_init(&d->pirq_tree);
> +
> +if ( config->vmtrace_pt_order )
> +{
> +uint32_t shift_val = config->vmtrace_pt_order + PAGE_SHIFT;
> +d->vmtrace_pt_size = (1ULL << shift_val);
> +}
> }
>
> if ( (err = arch_domain_create(d, config)) != 0 )
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 59bdc28c89..7b8289d436 100644
> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -92,6 +92,7 @@ struct xen_domctl_createdomain {
> uint32_t max_evtchn_port;
> int32_t max_grant_frames;
> int32_t max_maptrack_frames;
> +uint8_t vmtrace_pt_order;
>
> struct xen_arch_domainconfig arch;
> };
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index ac53519d7f..48f0a61bbd 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -457,6 +457,10 @@ struct domain
> unsignedpbuf_idx;
> spinlock_t pbuf_lock;
>
> +/* Used by vmtrace features */
> +spinlock_t vmtrace_lock;
> +uint64_tvmtrace_pt_size;
> +
> /* OProfile support. */
> struct xenoprof *xenoprof;
>
> --
> 2.17.1
Just a note to myself: in v4 it was suggested by Jan that we should
go with "number of kB" instead of "number of bytes" and the type
could be uint32_t.
I will modify it in such way within the next version.
Best regards,
Michał Leszczyński
CERT Polska
Re: [PATCH v5 11/11] tools/proctrace: add proctrace tool
- 6 lip 2020 o 11:47, Andrew Cooper andrew.coop...@citrix.com napisał(a): > On 06/07/2020 09:33, Jan Beulich wrote: >> On 05.07.2020 20:58, Michał Leszczyński wrote: >>> - 5 lip 2020 o 20:55, Michał Leszczyński michal.leszczyn...@cert.pl >>> napisał(a): --- /dev/null +++ b/tools/proctrace/proctrace.c +#include +#include +#include +#include + +#include +#include +#include + +#define BUF_SIZE (16384 * XC_PAGE_SIZE) >>> I would like to discuss here, how we should retrieve the trace buffer size >>> in runtime? Should there be a hypercall for it, or some extension to >>> acquire_resource logic? >> Personally I'd prefer the latter, but the question is whether one can >> be made in a backwards compatible way. > > I already covered this in v4. > > ~Andrew Ok, sorry, I see: > The guest_handle_is_null(xmar.frame_list) path > in Xen is supposed to report the size of the resource, not the size of > Xen's local buffer, so userspace can ask "how large is this resource". > > I'll try and find some time to fix this and arrange for backports, but > the current behaviour is nonsense, and problematic for new users. So to make it clear: should I modify the acquire_resource logic in such way that NULL guest handle would report the actual size of the resource? If I got it right, here: https://lists.xen.org/archives/html/xen-devel/2020-07/msg00159.html it was suggested that it should report the constant value of UINT_MAX >> MEMOP_EXTENT_SHIFT and as far as I understood, the expectation is that it would report how many frames might be requested at once, not what is the size of the resource we're asking for. Best regards, Michał Leszczyński CERT Polska
Re: [PATCH v5 08/11] x86/mm: add vmtrace_buf resource type
Hi,
On 05/07/2020 19:55, Michał Leszczyński wrote:
From: Michal Leszczynski
Allow to map processor trace buffer using
acquire_resource().
Signed-off-by: Michal Leszczynski
---
xen/common/memory.c | 28
xen/include/public/memory.h | 1 +
2 files changed, 29 insertions(+)
diff --git a/xen/common/memory.c b/xen/common/memory.c
index eb42f883df..04f4e152c0 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1007,6 +1007,29 @@ static long xatp_permission_check(struct domain *d,
unsigned int space)
return xsm_add_to_physmap(XSM_TARGET, current->domain, d);
}
+static int acquire_vmtrace_buf(struct domain *d, unsigned int id,
+ unsigned long frame,
Shouldn't this be uint64_t to avoid truncation?
+ unsigned int nr_frames,
+ xen_pfn_t mfn_list[])
+{
+mfn_t mfn;
+unsigned int i;
+struct vcpu *v = domain_vcpu(d, id);
+
+if ( !v || !v->vmtrace.pt_buf )
+return -EINVAL;
+
+mfn = page_to_mfn(v->vmtrace.pt_buf);
+
+if ( frame + nr_frames > (v->domain->vmtrace_pt_size >> PAGE_SHIFT) )
frame + nr_frames could possibly overflow a 64-bit value and therefore
still pass the check.
So I would suggest to use:
(frame > (v->domain_vm_ptrace_pt_size >> PAGE_SHIFT)) ||
(nr_frames > ((v->domain_vm_ptrace_pt_size >> PAGE_SHIFT) - frame))
Cheers,
--
Julien Grall
Re: [PATCH v5 09/11] x86/domctl: add XEN_DOMCTL_vmtrace_op
Hi Michal,
On 05/07/2020 19:55, Michał Leszczyński wrote:
+/* XEN_DOMCTL_vmtrace_op: Perform VM tracing related operation */
+#if defined(__XEN__) || defined(__XEN_TOOLS__)
+
+struct xen_domctl_vmtrace_op {
+/* IN variable */
+uint32_t cmd;
+/* Enable/disable external vmtrace for given domain */
+#define XEN_DOMCTL_vmtrace_pt_enable 1
+#define XEN_DOMCTL_vmtrace_pt_disable 2
+#define XEN_DOMCTL_vmtrace_pt_get_offset 3
+domid_t domain;
AFAICT, there is a 16-bit implicit padding here and ...
+uint32_t vcpu;
... a 32-bit implicit padding here. I would suggest to make
the two explicit.
We possibly want to check they are also always zero.
Cheers,
--
Julien Grall
Re: [PATCH v5 06/11] x86/hvm: processor trace interface in HVM
- 6 lip 2020 o 10:31, Jan Beulich jbeul...@suse.com napisał(a):
> On 05.07.2020 21:11, Michał Leszczyński wrote:
>> - 5 lip 2020 o 20:54, Michał Leszczyński michal.leszczyn...@cert.pl
>> napisał(a):
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -2199,6 +2199,25 @@ int domain_relinquish_resources(struct domain *d)
>>> altp2m_vcpu_disable_ve(v);
>>> }
>>>
>>> +for_each_vcpu ( d, v )
>>> +{
>>> +unsigned int i;
>>> +
>>> +if ( !v->vmtrace.pt_buf )
>>> +continue;
>>> +
>>> +for ( i = 0; i < (v->domain->vmtrace_pt_size >> PAGE_SHIFT);
>>> i++ )
>>> +{
>>> +struct page_info *pg = mfn_to_page(
>>> +mfn_add(page_to_mfn(v->vmtrace.pt_buf), i));
>>> +if ( (pg->count_info & PGC_count_mask) != 1 )
>>> +return -EBUSY;
>>> +}
>>> +
>>> +free_domheap_pages(v->vmtrace.pt_buf,
>>> +get_order_from_bytes(v->domain->vmtrace_pt_size));
>>
>>
>> While this works, I don't feel that this is a good solution with this loop
>> returning -EBUSY here. I would like to kindly ask for suggestions regarding
>> this topic.
>
> I'm sorry to ask, but with the previously give suggestions to mirror
> existing code, why do you still need to play with this function? You
> really shouldn't have a need to, just like e.g. the ioreq server page
> handling code didn't.
>
> Jan
Ok, sorry. I think I've finally got it after latest Roger's suggestions :P
This will be fixed in the next version.
Best regards,
Michał Leszczyński
CERT Polska
Re: [PATCH v5 09/11] x86/domctl: add XEN_DOMCTL_vmtrace_op
On 06.07.2020 12:31, Julien Grall wrote:
> On 05/07/2020 19:55, Michał Leszczyński wrote:
>> +/* XEN_DOMCTL_vmtrace_op: Perform VM tracing related operation */
>> +#if defined(__XEN__) || defined(__XEN_TOOLS__)
>> +
>> +struct xen_domctl_vmtrace_op {
>> +/* IN variable */
>> +uint32_t cmd;
>> +/* Enable/disable external vmtrace for given domain */
>> +#define XEN_DOMCTL_vmtrace_pt_enable 1
>> +#define XEN_DOMCTL_vmtrace_pt_disable 2
>> +#define XEN_DOMCTL_vmtrace_pt_get_offset 3
>> +domid_t domain;
>
> AFAICT, there is a 16-bit implicit padding here and ...
>
>
>> +uint32_t vcpu;
>
> ... a 32-bit implicit padding here. I would suggest to make
> the two explicit.
>
> We possibly want to check they are also always zero.
Not just possibly imo - we should.
Jan
Re: [PATCH v5 09/11] x86/domctl: add XEN_DOMCTL_vmtrace_op
On 06/07/2020 11:37, Jan Beulich wrote:
On 06.07.2020 12:31, Julien Grall wrote:
On 05/07/2020 19:55, Michał Leszczyński wrote:
+/* XEN_DOMCTL_vmtrace_op: Perform VM tracing related operation */
+#if defined(__XEN__) || defined(__XEN_TOOLS__)
+
+struct xen_domctl_vmtrace_op {
+/* IN variable */
+uint32_t cmd;
+/* Enable/disable external vmtrace for given domain */
+#define XEN_DOMCTL_vmtrace_pt_enable 1
+#define XEN_DOMCTL_vmtrace_pt_disable 2
+#define XEN_DOMCTL_vmtrace_pt_get_offset 3
+domid_t domain;
AFAICT, there is a 16-bit implicit padding here and ...
+uint32_t vcpu;
... a 32-bit implicit padding here. I would suggest to make
the two explicit.
We possibly want to check they are also always zero.
Not just possibly imo - we should.
I wasn't sure given that DOMCTL is not a stable interface.
Cheers,
--
Julien Grall
Re: [PATCH v5 04/11] common: add vmtrace_pt_size domain parameter
Hi, On 05/07/2020 19:54, Michał Leszczyński wrote: From: Michal Leszczynski Add vmtrace_pt_size domain parameter in live domain and vmtrace_pt_order parameter in xen_domctl_createdomain. Signed-off-by: Michal Leszczynski --- xen/common/domain.c | 12 xen/include/public/domctl.h | 1 + xen/include/xen/sched.h | 4 3 files changed, 17 insertions(+) diff --git a/xen/common/domain.c b/xen/common/domain.c index a45cf023f7..25d3359c5b 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -338,6 +338,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config) return -EINVAL; } +if ( config->vmtrace_pt_order && !vmtrace_supported ) Looking at the rest of the series, vmtrace will only be supported for x86 HVM guest. So don't you want to return -EINVAL for PV guests here? This could be done in a new helper arch_vmtrace_supported() or possibly in the existing arch_sanitise_domain_config(). Cheers, -- Julien Grall
Re: [PATCH v5 05/11] tools/libxl: add vmtrace_pt_size parameter
Hi, On 05/07/2020 20:02, Michał Leszczyński wrote: - 5 lip 2020 o 20:54, Michał Leszczyński michal.leszczyn...@cert.pl napisał(a): From: Michal Leszczynski Allow to specify the size of per-vCPU trace buffer upon domain creation. This is zero by default (meaning: not enabled). Signed-off-by: Michal Leszczynski --- docs/man/xl.cfg.5.pod.in | 11 +++ tools/golang/xenlight/helpers.gen.go | 2 ++ tools/golang/xenlight/types.gen.go | 1 + tools/libxl/libxl.h | 8 tools/libxl/libxl_create.c | 1 + tools/libxl/libxl_types.idl | 2 ++ tools/xl/xl_parse.c | 22 ++ 7 files changed, 47 insertions(+) diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 0532739c1f..670759f6bd 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -278,6 +278,17 @@ memory=8096 will report significantly less memory available for use than a system with maxmem=8096 memory=8096 due to the memory overhead of having to track the unused pages. +=item B + +Specifies the size of processor trace buffer that would be allocated +for each vCPU belonging to this domain. Disabled (i.e. +B by default. This must be set to +non-zero value in order to be able to use processor tracing features +with this domain. + +B: The size value must be between 4 kB and 4 GB and it must +be also a power of 2. This seems to suggest that 4 kB is allowed. But looking at the code below, you are forbidding the value. [...] As there were many different ideas about how the naming scheme should be and what kinds of values should be passed where, I would like to discuss this particular topic. Right now we have it pretty confusing: * user sets "processor_trace_buffer_size" option in xl.cfg * domain creation hypercall uses "vmtrace_pt_order" (derived from above) You don't only use the order in the hypercall but also the public interface of libxl. * hypervisor side stores "vmtrace_pt_size" (converted back to bytes) My preference would be to use the size everywhere, but if one still prefer to use the order in the hypercall then the libxl interface should use the size. See my comment in v4 for the rationale. Cheers, -- Julien Grall
Re: [PATCH v5 09/11] x86/domctl: add XEN_DOMCTL_vmtrace_op
On 06.07.2020 12:38, Julien Grall wrote:
> On 06/07/2020 11:37, Jan Beulich wrote:
>> On 06.07.2020 12:31, Julien Grall wrote:
>>> On 05/07/2020 19:55, Michał Leszczyński wrote:
+/* XEN_DOMCTL_vmtrace_op: Perform VM tracing related operation */
+#if defined(__XEN__) || defined(__XEN_TOOLS__)
+
+struct xen_domctl_vmtrace_op {
+/* IN variable */
+uint32_t cmd;
+/* Enable/disable external vmtrace for given domain */
+#define XEN_DOMCTL_vmtrace_pt_enable 1
+#define XEN_DOMCTL_vmtrace_pt_disable 2
+#define XEN_DOMCTL_vmtrace_pt_get_offset 3
+domid_t domain;
>>>
>>> AFAICT, there is a 16-bit implicit padding here and ...
>>>
>>>
+uint32_t vcpu;
>>>
>>> ... a 32-bit implicit padding here. I would suggest to make
>>> the two explicit.
>>>
>>> We possibly want to check they are also always zero.
>>
>> Not just possibly imo - we should.
>
> I wasn't sure given that DOMCTL is not a stable interface.
True; checking padding fields allows assigning meaning to them
without bumping the domctl interface version.
Jan
[xen-unstable test] 151661: tolerable FAIL
flight 151661 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/151661/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-amd64-amd64-examine4 memdisk-try-append fail in 151635 pass in 151661 test-amd64-i386-libvirt-xsm 18 guest-start/debian.repeat fail pass in 151635 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 151635 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 151635 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 151635 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail like 151635 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 151635 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 151635 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 151635 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 151635 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 151635 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 151635 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass version targeted for testing: xen be63d9d47f571a60d70f8fb630c03871312d9655 baseline version: xen be63d9d47f571a60d70f8fb630c03871312d9655 Last test of basis 151661 2020-07-06 01:54:10 Z0 days Testing same since (not found) 0 attempts jobs: build-amd64-xsm pass build-arm64-xsm
[xen-unstable-smoke test] 151671: regressions - FAIL
flight 151671 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/151671/
Regressions :-(
Tests which did not succeed and are blocking,
including tests which could not be run:
test-arm64-arm64-xl-xsm 7 xen-boot fail REGR. vs. 151535
Tests which did not succeed, but are not blocking:
test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass
test-armhf-armhf-xl 13 migrate-support-checkfail never pass
test-armhf-armhf-xl 14 saverestore-support-checkfail never pass
version targeted for testing:
xen 158912a532fe98f448c688d3571241c9033553bd
baseline version:
xen be63d9d47f571a60d70f8fb630c03871312d9655
Last test of basis 151535 2020-07-02 10:00:52 Z4 days
Testing same since 151671 2020-07-06 09:02:11 Z0 days1 attempts
People who touched revisions under test:
Anthony PERARD
Wei Liu
jobs:
build-arm64-xsm pass
build-amd64 pass
build-armhf pass
build-amd64-libvirt pass
test-armhf-armhf-xl pass
test-arm64-arm64-xl-xsm fail
test-amd64-amd64-xl-qemuu-debianhvm-amd64pass
test-amd64-amd64-libvirt pass
sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images
Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs
Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master
Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary
Not pushing.
commit 158912a532fe98f448c688d3571241c9033553bd
Author: Anthony PERARD
Date: Fri Jul 3 14:55:33 2020 +0100
Config: Update QEMU
Backport 2 commits to fix building QEMU without PCI passthrough
support.
Signed-off-by: Anthony PERARD
Acked-by: Wei Liu
Release-acked-by: Paul Durrant
commit d44cbbe0f3243afcc56e47dcfa97bbfe23e46fbb
Author: Wei Liu
Date: Fri Jul 3 20:10:01 2020 +
kdd: fix build again
Restore Tim's patch. The one that was committed was recreated by me
because git didn't accept my saved copy. I made some mistakes while
recreating that patch and here we are.
Fixes: 3471cafbdda3 ("kdd: stop using [0] arrays to access packet contents")
Reported-by: Michael Young
Signed-off-by: Wei Liu
Reviewed-by: Tim Deegan
Release-acked-by: Paul Durrant
(qemu changes not included)
[libvirt test] 151665: regressions - FAIL
flight 151665 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/151665/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 151496 test-amd64-i386-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 151496 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 151496 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 151496 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-arm64-arm64-libvirt-qcow2 12 migrate-support-checkfail never pass test-arm64-arm64-libvirt-qcow2 13 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass version targeted for testing: libvirt 852ee1950aee5f31c9656b30c5fe9124f734c38c baseline version: libvirt e7998ebeaf15e4e8825be0dd97aa1316f194f00d Last test of basis 151496 2020-07-01 04:23:43 Z5 days Failing since151527 2020-07-02 04:29:15 Z4 days5 attempts Testing same since 151665 2020-07-06 04:18:46 Z0 days1 attempts People who touched revisions under test: Andrea Bolognani Daniel Henrique Barboza Daniel P. Berrangé Daniel Veillard Laine Stump Michal Privoznik Yanqiu Zhang jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386 pass build-amd64-libvirt pass build-arm64-libvirt pass build-armhf-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmpass test-amd64-amd64-libvirt-xsm pass test-arm64-arm64-libvirt-xsm pass test-amd64-i386-libvirt-xsm pass test-amd64-amd64-libvirt pass test-arm64-arm64-libvirt pass test-armhf-armhf-libvirt pass test-amd64-i386-libvirt pass test-amd64-amd64-libvirt-pairfail test-amd64-i386-libvirt-pair fail test-arm64-arm64-libvirt-qcow2 pass test-armhf-armhf-libvirt-raw pass test-amd64-amd64-libvirt-vhd pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness
[PATCH 0/2] block/block-backend: Let blk_attach_dev() provide helpful error
A pair of patches which helps me debug an issue with block drive already attached. Suggestions to correctly/better use the Error API welcome, in particular in qdev-properties-system::set_drive_helper(). Philippe Mathieu-Daudé (2): block/block-backend: Trace blk_attach_dev() block/block-backend: Let blk_attach_dev() provide helpful error include/sysemu/block-backend.h | 2 +- block/block-backend.c| 12 +++- hw/block/fdc.c | 4 +--- hw/block/swim.c | 4 +--- hw/block/xen-block.c | 5 +++-- hw/core/qdev-properties-system.c | 17 ++--- hw/ide/qdev.c| 4 +--- hw/scsi/scsi-disk.c | 4 +--- block/trace-events | 1 + 9 files changed, 30 insertions(+), 23 deletions(-) -- 2.21.3
[RFC PATCH 2/2] block/block-backend: Let blk_attach_dev() provide helpful error
Let blk_attach_dev() take an Error* object to return helpful
information. Adapt the callers.
$ qemu-system-arm -M n800
qemu-system-arm: sd_init failed: cannot attach blk 'sd0' to device 'sd-card'
because it is already attached by device 'omap2-mmc'
Drive 'sd0' is already in use because it has been automatically connected to
another device (did you need 'if=none' in the drive options?)
Signed-off-by: Philippe Mathieu-Daudé
---
RFC: I'm not sure the error API is correctly used in
set_drive_helper().
include/sysemu/block-backend.h | 2 +-
block/block-backend.c| 11 ++-
hw/block/fdc.c | 4 +---
hw/block/swim.c | 4 +---
hw/block/xen-block.c | 5 +++--
hw/core/qdev-properties-system.c | 17 ++---
hw/ide/qdev.c| 4 +---
hw/scsi/scsi-disk.c | 4 +---
8 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 8203d7f6f9..118fbad0b4 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -113,7 +113,7 @@ BlockDeviceIoStatus blk_iostatus(const BlockBackend *blk);
void blk_iostatus_disable(BlockBackend *blk);
void blk_iostatus_reset(BlockBackend *blk);
void blk_iostatus_set_err(BlockBackend *blk, int error);
-int blk_attach_dev(BlockBackend *blk, DeviceState *dev);
+int blk_attach_dev(BlockBackend *blk, DeviceState *dev, Error **errp);
void blk_detach_dev(BlockBackend *blk, DeviceState *dev);
DeviceState *blk_get_attached_dev(BlockBackend *blk);
char *blk_get_attached_dev_id(BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index 23caa45e6a..29480a121d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -882,12 +882,21 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm,
uint64_t *shared_perm)
/*
* Attach device model @dev to @blk.
+ *
+ * @blk: Block backend
+ * @dev: Device to attach the block backend to
+ * @errp: pointer to NULL initialized error object
+ *
* Return 0 on success, -EBUSY when a device model is attached already.
*/
-int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
+int blk_attach_dev(BlockBackend *blk, DeviceState *dev, Error **errp)
{
trace_blk_attach_dev(blk_name(blk), object_get_typename(OBJECT(dev)));
if (blk->dev) {
+error_setg(errp, "cannot attach blk '%s' to device '%s' because it is "
+ "already attached by device '%s'",
+ blk_name(blk), object_get_typename(OBJECT(dev)),
+ object_get_typename(OBJECT(blk->dev)));
return -EBUSY;
}
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 3425d56e2a..5765b5d4f2 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -519,7 +519,6 @@ static void floppy_drive_realize(DeviceState *qdev, Error
**errp)
FloppyBus *bus = FLOPPY_BUS(qdev->parent_bus);
FDrive *drive;
bool read_only;
-int ret;
if (dev->unit == -1) {
for (dev->unit = 0; dev->unit < MAX_FD; dev->unit++) {
@@ -545,8 +544,7 @@ static void floppy_drive_realize(DeviceState *qdev, Error
**errp)
if (!dev->conf.blk) {
/* Anonymous BlockBackend for an empty drive */
dev->conf.blk = blk_new(qemu_get_aio_context(), 0, BLK_PERM_ALL);
-ret = blk_attach_dev(dev->conf.blk, qdev);
-assert(ret == 0);
+blk_attach_dev(dev->conf.blk, qdev, &error_abort);
/* Don't take write permissions on an empty drive to allow attaching a
* read-only node later */
diff --git a/hw/block/swim.c b/hw/block/swim.c
index 74f56e8f46..2f1ecd0bb2 100644
--- a/hw/block/swim.c
+++ b/hw/block/swim.c
@@ -159,7 +159,6 @@ static void swim_drive_realize(DeviceState *qdev, Error
**errp)
SWIMDrive *dev = SWIM_DRIVE(qdev);
SWIMBus *bus = SWIM_BUS(qdev->parent_bus);
FDrive *drive;
-int ret;
if (dev->unit == -1) {
for (dev->unit = 0; dev->unit < SWIM_MAX_FD; dev->unit++) {
@@ -185,8 +184,7 @@ static void swim_drive_realize(DeviceState *qdev, Error
**errp)
if (!dev->conf.blk) {
/* Anonymous BlockBackend for an empty drive */
dev->conf.blk = blk_new(qemu_get_aio_context(), 0, BLK_PERM_ALL);
-ret = blk_attach_dev(dev->conf.blk, qdev);
-assert(ret == 0);
+blk_attach_dev(dev->conf.blk, qdev, &error_abort);
}
if (!blkconf_blocksizes(&dev->conf, errp)) {
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 1b7bc5de08..81650208dc 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -616,14 +616,15 @@ static void xen_cdrom_realize(XenBlockDevice *blockdev,
Error **errp)
blockdev->device_type = "cdrom";
if (!conf->blk) {
+Error *local_err = NULL;
int rc;
/* Set up an empty drive */
conf->blk = blk_new(qemu_get_aio_context(), 0, BLK_PERM_ALL);
-rc = blk_attach_dev(conf->blk, DEVICE(blockd
[PATCH 1/2] block/block-backend: Trace blk_attach_dev()
Add a trace event to follow devices attaching block drives:
$ qemu-system-arm -M n800 -trace blk_\*
9513@1594040428.738162:blk_attach_dev attaching blk 'sd0' to device
'omap2-mmc'
9513@1594040428.738189:blk_attach_dev attaching blk 'sd0' to device 'sd-card'
qemu-system-arm: sd_init failed: blk 'sd0' already attached by device
'sd-card'
Signed-off-by: Philippe Mathieu-Daudé
---
block/block-backend.c | 1 +
block/trace-events| 1 +
2 files changed, 2 insertions(+)
diff --git a/block/block-backend.c b/block/block-backend.c
index 6936b25c83..23caa45e6a 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -886,6 +886,7 @@ void blk_get_perm(BlockBackend *blk, uint64_t *perm,
uint64_t *shared_perm)
*/
int blk_attach_dev(BlockBackend *blk, DeviceState *dev)
{
+trace_blk_attach_dev(blk_name(blk), object_get_typename(OBJECT(dev)));
if (blk->dev) {
return -EBUSY;
}
diff --git a/block/trace-events b/block/trace-events
index dbe76a7613..aa641c2034 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -9,6 +9,7 @@ blk_co_preadv(void *blk, void *bs, int64_t offset, unsigned int
bytes, int flags
blk_co_pwritev(void *blk, void *bs, int64_t offset, unsigned int bytes, int
flags) "blk %p bs %p offset %"PRId64" bytes %u flags 0x%x"
blk_root_attach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
blk_root_detach(void *child, void *blk, void *bs) "child %p blk %p bs %p"
+blk_attach_dev(const char *blk_name, const char *dev_name) "attaching blk '%s'
to device '%s'"
# io.c
bdrv_co_preadv(void *bs, int64_t offset, int64_t nbytes, unsigned int flags)
"bs %p offset %"PRId64" nbytes %"PRId64" flags 0x%x"
--
2.21.3
Re: [PATCH v5 06/11] x86/hvm: processor trace interface in HVM
On Mon, Jul 06, 2020 at 12:09:02PM +0200, Michał Leszczyński wrote:
> - 6 lip 2020 o 10:42, Roger Pau Monné roger@citrix.com napisał(a):
>
> > On Sun, Jul 05, 2020 at 08:54:59PM +0200, Michał Leszczyński wrote:
> >> From: Michal Leszczynski
> >>
> >> Implement necessary changes in common code/HVM to support
> >> processor trace features. Define vmtrace_pt_* API and
> >> implement trace buffer allocation/deallocation in common
> >> code.
> >>
> >> Signed-off-by: Michal Leszczynski
> >> ---
> >> xen/arch/x86/domain.c | 19 +++
> >> xen/common/domain.c | 19 +++
> >> xen/include/asm-x86/hvm/hvm.h | 20
> >> xen/include/xen/sched.h | 4
> >> 4 files changed, 62 insertions(+)
> >>
> >> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> >> index fee6c3931a..79c9794408 100644
> >> --- a/xen/arch/x86/domain.c
> >> +++ b/xen/arch/x86/domain.c
> >> @@ -2199,6 +2199,25 @@ int domain_relinquish_resources(struct domain *d)
> >> altp2m_vcpu_disable_ve(v);
> >> }
> >>
> >> +for_each_vcpu ( d, v )
> >> +{
> >> +unsigned int i;
> >> +
> >> +if ( !v->vmtrace.pt_buf )
> >> +continue;
> >> +
> >> +for ( i = 0; i < (v->domain->vmtrace_pt_size >> PAGE_SHIFT);
> >> i++ )
> >> +{
> >> +struct page_info *pg = mfn_to_page(
> >> +mfn_add(page_to_mfn(v->vmtrace.pt_buf), i));
> >> +if ( (pg->count_info & PGC_count_mask) != 1 )
> >> +return -EBUSY;
> >> +}
> >> +
> >> +free_domheap_pages(v->vmtrace.pt_buf,
> >> +get_order_from_bytes(v->domain->vmtrace_pt_size));
> >
> > This is racy as a control domain could take a reference between the
> > check and the freeing.
> >
> >> +}
> >> +
> >> if ( is_pv_domain(d) )
> >> {
> >> for_each_vcpu ( d, v )
> >> diff --git a/xen/common/domain.c b/xen/common/domain.c
> >> index 25d3359c5b..f480c4e033 100644
> >> --- a/xen/common/domain.c
> >> +++ b/xen/common/domain.c
> >> @@ -137,6 +137,21 @@ static void vcpu_destroy(struct vcpu *v)
> >> free_vcpu_struct(v);
> >> }
> >>
> >> +static int vmtrace_alloc_buffers(struct vcpu *v)
> >> +{
> >> +struct page_info *pg;
> >> +uint64_t size = v->domain->vmtrace_pt_size;
> >> +
> >> +pg = alloc_domheap_pages(v->domain, get_order_from_bytes(size),
> >> + MEMF_no_refcount);
> >> +
> >> +if ( !pg )
> >> +return -ENOMEM;
> >> +
> >> +v->vmtrace.pt_buf = pg;
> >> +return 0;
> >> +}
> >
> > I think we already agreed that you would use the same model as ioreq
> > servers, where a reference is taken on allocation and then the pages
> > are not explicitly freed on domain destruction and put_page_and_type
> > is used. Is there some reason why that model doesn't work in this
> > case?
> >
> > If not, please see hvm_alloc_ioreq_mfn and hvm_free_ioreq_mfn.
> >
> > Roger.
>
>
> Ok, I've got it, will do. Thanks for pointing out the examples.
>
>
> One thing that is confusing to me is that I don't get what is
> the meaning of MEMF_no_refcount flag.
That flag prevents the memory from being counted towards the amount of
memory assigned to the domain. You want it that way so that trace
buffers are not accounted as part of the memory assigned to the
domain.
You then need to get a (extra) reference to the pages (there's always
the 'allocated' reference AFAICT) so that when the last reference is
dropped (either by the domain being destroyed or the memory being
unmapped from the control domain) it will be freed.
> In the hvm_{alloc,free}_ioreq_mfn the memory is allocated
> explicitly but freed just by putting out the reference, so
> I guess it's automatically detected that the refcount dropped to 0
> and the page should be freed?
Yes, put_page_alloc_ref will remove the allocated flag and then when
the last reference is dropped the page will be freed.
> If so, why the flag is named "no refcount"?
I'm not sure about the naming, but you can get references to pages
allocated as MEMF_no_refcount, and change their types. They are
however not accounted towards the memory usage of a domain.
Roger.
[linux-linus test] 151662: regressions - trouble: blocked/broken/fail/pass
flight 151662 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/151662/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-cubietruck broken test-armhf-armhf-xl-cubietruck 4 host-install(4) broken REGR. vs. 151214 test-arm64-arm64-libvirt-xsm 17 guest-start.2fail REGR. vs. 151214 build-i386-pvops 6 kernel-build fail REGR. vs. 151214 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail REGR. vs. 151214 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail REGR. vs. 151214 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-pvshim 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-examine 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 151214 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 151214 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 151214 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 151214 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 151214 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 151214 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-
[PATCH] x86emul: fix FXRSTOR test for most AMD CPUs
AMD CPUs that we classify as X86_BUG_FPU_PTRS don't touch the selector/ offset portion of the save image during FXSAVE unless an unmasked exception is pending. Hence the selector zapping done between the initial FXSAVE and the emulated FXRSTOR needs to be mirrored onto the second FXSAVE, output of which gets fed into memcmp() to compare with the input image. Reported-by: Andrew Cooper Signed-off-by: Jan Beulich --- a/tools/tests/x86_emulator/test_x86_emulator.c +++ b/tools/tests/x86_emulator/test_x86_emulator.c @@ -2577,6 +2577,7 @@ int main(int argc, char **argv) regs.ecx = (unsigned long)(res + 0x81); rc = x86_emulate(&ctxt, &emulops); asm volatile ( "fxsave %0" : "=m" (res[0x100]) :: "memory" ); +zap_xfpsel(&res[0x100]); if ( (rc != X86EMUL_OKAY) || memcmp(res + 0x100, res + 0x80, 0x200) || (regs.eip != (unsigned long)&instr[4]) )
Re: [PATCH] x86emul: fix FXRSTOR test for most AMD CPUs
On 06/07/2020 16:14, Jan Beulich wrote: > AMD CPUs that we classify as X86_BUG_FPU_PTRS don't touch the selector/ > offset portion of the save image during FXSAVE unless an unmasked > exception is pending. Hence the selector zapping done between the > initial FXSAVE and the emulated FXRSTOR needs to be mirrored onto the > second FXSAVE, output of which gets fed into memcmp() to compare with > the input image. > > Reported-by: Andrew Cooper > Signed-off-by: Jan Beulich Acked-by: Andrew Cooper Tested-by: Andrew Cooper
RE: [PATCH] x86emul: fix FXRSTOR test for most AMD CPUs
> -Original Message- > From: Andrew Cooper > Sent: 06 July 2020 16:47 > To: Jan Beulich ; xen-devel@lists.xenproject.org > Cc: Wei Liu ; Roger Pau Monné ; Paul > Durrant > Subject: Re: [PATCH] x86emul: fix FXRSTOR test for most AMD CPUs > > On 06/07/2020 16:14, Jan Beulich wrote: > > AMD CPUs that we classify as X86_BUG_FPU_PTRS don't touch the selector/ > > offset portion of the save image during FXSAVE unless an unmasked > > exception is pending. Hence the selector zapping done between the > > initial FXSAVE and the emulated FXRSTOR needs to be mirrored onto the > > second FXSAVE, output of which gets fed into memcmp() to compare with > > the input image. > > > > Reported-by: Andrew Cooper > > Signed-off-by: Jan Beulich > > Acked-by: Andrew Cooper > Tested-by: Andrew Cooper Release-acked-by: Paul Durrant
Design Session Notes - "How best to upstream Bareflank's implementation of the Xen VMM into the Xen Project"
# Design Session Notes - "How best to upstream Bareflank's implementation of the Xen VMM into the Xen Project" ## Design Session Description Assured Information Security, Inc. has been working on a new implementation of the Xen VMM (i.e., just the root component of the Xen hypervisor) using Bareflank/Boxy (https://github.com/Bareflank/hypervisor). The goals of this VMM include the ability to reload the hypervisor without having to reboot, support for a Windows Dom0 (or any Dom0 really), removal of the Xen scheduling and power management code and instead using the scheduler and power management logic built into the Dom0 kernel, and removal of PV support in favor of a pure PVH/HVM implementation. Although there is still a lot of work to do, we can demonstrate this capability today. The goal of this design session is to discuss the design of our new approach, ways in which we can improve it, and ultimately how best to upstream our work into the Xen Project. ## Current Status of Xen compatibility in Bareflank Bareflank has compatibility layer to work with the Xen PVH hypercalls, with the goal of re-using existing Xen VMs, improved performance, less complex scheduling and power management, and the ability to run non-traditional dom0s, such as Windows. * Prototype is up and running: * Removal of scheduling/power management code * Windows dom0 * Hotloading the hypervisor (optional late launch, is desirable by gwd) * Doesn't share any code directly with Xen at the moment, was referenced throughout the implementation * No legacy PV mode, PVH only (currently) though HVM is planned down the road * APIC not managed by the hypervisor, but by dom0 * Mostly MSIs are passed directly to dom0 * Minimal interference with power management, lets the operating system deal with it * Removal of possible schedule aliasing * libxl toolstack runs outside of dom0 ## Discussion of upstreamability (desirable by andyhhp) * How to do it organizationally: * Subproject (feedback indicated this was undesirable, there would be too much overhead) * Directly bring capabilities into the mainline (feedback indicated this was more desirable) * Potential technical requirements: * Add scheduling API to allow the use of a host OS scheduler rather than the built in Xen scheduler (may be difficult) ## Proposed Actions: * KConfig options to strip out undesirable code portions * Add ability to do the rootkit like loading of Xen (like hxen/bareflank support) * Addition of more hypercalls to support dom0 (or more generally non-hypervisor context) scheduling * Stay engaged with xen-devel to align changes with potential ABI changes that are coming down the road to support updating libxl and the hypervisor in stages and supporting encrypted memory schemes that are incorporated in newer architectures
[xen-unstable-smoke test] 151679: tolerable all pass - PUSHED
flight 151679 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/151679/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen 158912a532fe98f448c688d3571241c9033553bd baseline version: xen be63d9d47f571a60d70f8fb630c03871312d9655 Last test of basis 151535 2020-07-02 10:00:52 Z4 days Testing same since 151671 2020-07-06 09:02:11 Z0 days2 attempts People who touched revisions under test: Anthony PERARD Wei Liu jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git be63d9d47f..158912a532 158912a532fe98f448c688d3571241c9033553bd -> smoke
Re: [PATCH] trivial: Remove trailing whitespaces
Patchew URL:
https://patchew.org/QEMU/20200706162300.1084753-1-dinec...@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH] trivial: Remove trailing whitespaces
Type: series
Message-id: 20200706162300.1084753-1-dinec...@redhat.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
From https://github.com/patchew-project/qemu
* [new tag] patchew/20200706162300.1084753-1-dinec...@redhat.com ->
patchew/20200706162300.1084753-1-dinec...@redhat.com
Switched to a new branch 'test'
9af3e90 trivial: Remove trailing whitespaces
=== OUTPUT BEGIN ===
WARNING: line over 80 characters
#121: FILE: disas/microblaze.c:112:
+ fcmp_lt, fcmp_eq, fcmp_le, fcmp_gt, fcmp_ne, fcmp_ge, fcmp_un, flt, fint,
fsqrt,
ERROR: line over 90 characters
#148: FILE: disas/microblaze.c:280:
+ unsigned long bit_sequence; /* all the fixed bits for the op are set and all
the variable bits (reg names, imm vals) are set to 0 */
ERROR: space prohibited between function name and open parenthesis '('
#289: FILE: disas/microblaze.c:728:
+read_insn_microblaze (bfd_vma memaddr,
ERROR: suspect code indent for conditional statements (2, 6)
#298: FILE: disas/microblaze.c:739:
+ if (status != 0)
{
ERROR: code indent should never use tabs
#334: FILE: disas/microblaze.c:854:
+^I /* The non-pc relative instructions are returns, which shouldn't$
WARNING: Block comments use a leading /* on a separate line
#334: FILE: disas/microblaze.c:854:
+ /* The non-pc relative instructions are returns, which shouldn't
ERROR: code indent should never use tabs
#343: FILE: disas/microblaze.c:889:
+^I}$
WARNING: Block comments use a leading /* on a separate line
#365: FILE: disas/nios2.c:99:
+/* This structure holds information for a particular instruction.
ERROR: code indent should never use tabs
#374: FILE: disas/nios2.c:155:
+ const char *args;^I^I/* A string describing the arguments for this$
WARNING: Block comments use a leading /* on a separate line
#374: FILE: disas/nios2.c:155:
+ const char *args;/* A string describing the arguments for this
ERROR: code indent should never use tabs
#377: FILE: disas/nios2.c:157:
+ const char *args_test;^I/* Like args, but with an extra argument for$
WARNING: Block comments use a leading /* on a separate line
#377: FILE: disas/nios2.c:157:
+ const char *args_test; /* Like args, but with an extra argument for
ERROR: code indent should never use tabs
#380: FILE: disas/nios2.c:159:
+ unsigned long num_args;^I/* The number of arguments the instruction$
WARNING: Block comments use a leading /* on a separate line
#380: FILE: disas/nios2.c:159:
+ unsigned long num_args; /* The number of arguments the instruction
ERROR: code indent should never use tabs
#386: FILE: disas/nios2.c:164:
+ unsigned long mask;^I^I/* Mask for the opcode field of the$
WARNING: Block comments use a leading /* on a separate line
#386: FILE: disas/nios2.c:164:
+ unsigned long mask; /* Mask for the opcode field of the
ERROR: code indent should never use tabs
#389: FILE: disas/nios2.c:166:
+ unsigned long pinfo;^I^I/* Is this a real instruction or instruction$
WARNING: Block comments use a leading /* on a separate line
#389: FILE: disas/nios2.c:166:
+ unsigned long pinfo; /* Is this a real instruction or instruction
WARNING: Block comments use a leading /* on a separate line
#392: FILE: disas/nios2.c:168:
+ enum overflow_type overflow_msg; /* Used to generate informative
WARNING: Block comments use a leading /* on a separate line
#399: FILE: disas/nios2.c:172:
+/* This value is used in the nios2_opcode.pinfo field to indicate that the
WARNING: Block comments use * on subsequent lines
#400: FILE: disas/nios2.c:173:
+/* This value is used in the nios2_opcode.pinfo field to indicate that the
+ instruction is a macro or pseudo-op. This requires special treatment by
WARNING: line over 80 characters
#549: FILE: disas/nios2.c:284:
+#define GET_IW_CUSTOM_A(W) (((W) >> IW_CUSTOM_A_LSB) &
IW_CUSTOM_A_UNSHIFTED_MASK)
WARNING: line over 80 characters
#550: FILE: disas/nios2.c:285:
+#define SET_IW_CUSTOM_A(V) (((V) & IW_CUSTOM_A_UNSHIFTED_MASK) <<
IW_CUSTOM_A_LSB)
WARNING: line over 80 characters
#562: FILE: disas/nios2.c:291:
+#define GET_IW_CUSTOM_B(W) (((W) >> IW_CUSTOM_B_LSB) &
IW_CUSTOM_B_UNSHIFTED_MASK)
WARNING: line over 80 characters
#563: FILE: disas/nios2.c:292:
+#define SET_IW_CUSTOM_B(V) (((V) & IW_CUSTOM_B_UNSHIFTED_MASK) <<
IW_CUSTOM_B_LSB)
WARNING: line over 80 characters
#575: FILE: disas/nios2.c:298:
+#define GET_IW_CUSTOM_C(W) (((W) >> IW_CUSTOM_C_LSB) &
IW_CUSTOM_C_UNSHIFTED_MASK)
WARNING: line over 80 characters
#576: FILE: disas/nios2.c:299:
+#define SET_IW_
Re: [PATCH 08/26] hw/usb/hcd-dwc2: Restrict 'dwc2-regs.h' scope
On Sat, Jul 4, 2020 at 7:53 AM Philippe Mathieu-Daudé wrote:
>
> We only use these register definitions in files under the
> hw/usb/ directory. Keep that header local by moving it there.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> {include/hw => hw}/usb/dwc2-regs.h | 0
> hw/usb/hcd-dwc2.c | 2 +-
> 2 files changed, 1 insertion(+), 1 deletion(-)
> rename {include/hw => hw}/usb/dwc2-regs.h (100%)
>
> diff --git a/include/hw/usb/dwc2-regs.h b/hw/usb/dwc2-regs.h
> similarity index 100%
> rename from include/hw/usb/dwc2-regs.h
> rename to hw/usb/dwc2-regs.h
> diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c
> index ccf05d0823..252b60ef65 100644
> --- a/hw/usb/hcd-dwc2.c
> +++ b/hw/usb/hcd-dwc2.c
> @@ -34,7 +34,6 @@
> #include "qemu/osdep.h"
> #include "qemu/units.h"
> #include "qapi/error.h"
> -#include "hw/usb/dwc2-regs.h"
> #include "hw/usb/hcd-dwc2.h"
> #include "hw/irq.h"
> #include "sysemu/dma.h"
> @@ -43,6 +42,7 @@
> #include "qemu/timer.h"
> #include "qemu/log.h"
> #include "hw/qdev-properties.h"
> +#include "dwc2-regs.h"
>
> #define USB_HZ_FS 1200
> #define USB_HZ_HS 9600
> --
> 2.21.3
>
>
Re: [PATCH 01/26] hw/arm/sbsa-ref: Remove unused 'hw/usb.h' header
On Sat, Jul 4, 2020 at 7:51 AM Philippe Mathieu-Daudé wrote: > > This file doesn't access anything from "hw/usb.h", remove its > inclusion. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > hw/arm/sbsa-ref.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c > index e40c868a82..021e7c1b8b 100644 > --- a/hw/arm/sbsa-ref.c > +++ b/hw/arm/sbsa-ref.c > @@ -38,7 +38,6 @@ > #include "hw/loader.h" > #include "hw/pci-host/gpex.h" > #include "hw/qdev-properties.h" > -#include "hw/usb.h" > #include "hw/char/pl011.h" > #include "net/net.h" > > -- > 2.21.3 > >
Re: [PATCH 02/26] hw/ppc/sam460ex: Add missing 'hw/pci/pci.h' header
On Sat, Jul 4, 2020 at 7:50 AM Philippe Mathieu-Daudé wrote: > > This file uses pci_create_simple() and PCI_DEVFN() which are both > declared in "hw/pci/pci.h". This include is indirectly included > by an USB header. As we want to reduce the USB header inclusions > later, include the PCI header now, to avoid later: > > hw/ppc/sam460ex.c:397:5: error: implicit declaration of function > ‘pci_create_simple’; did you mean ‘sysbus_create_simple’? > [-Werror=implicit-function-declaration] > 397 | pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501"); > | ^ > | sysbus_create_simple > hw/ppc/sam460ex.c:397:5: error: nested extern declaration of > ‘pci_create_simple’ [-Werror=nested-externs] > hw/ppc/sam460ex.c:397:32: error: implicit declaration of function > ‘PCI_DEVFN’ [-Werror=implicit-function-declaration] > 397 | pci_create_simple(pci_bus, PCI_DEVFN(6, 0), "sm501"); > |^ > hw/ppc/sam460ex.c:397:32: error: nested extern declaration of ‘PCI_DEVFN’ > [-Werror=nested-externs] > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > hw/ppc/sam460ex.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c > index 1a106a68de..fae970b142 100644 > --- a/hw/ppc/sam460ex.c > +++ b/hw/ppc/sam460ex.c > @@ -38,6 +38,7 @@ > #include "hw/usb/hcd-ehci.h" > #include "hw/ppc/fdt.h" > #include "hw/qdev-properties.h" > +#include "hw/pci/pci.h" > > #include > > -- > 2.21.3 > >
Re: [PATCH 03/26] hw/usb: Remove unused VM_USB_HUB_SIZE definition
On Sat, Jul 4, 2020 at 7:50 AM Philippe Mathieu-Daudé wrote:
>
> Commit a5d2f7273c ("qdev/usb: make qemu aware of usb busses")
> removed the last use of VM_USB_HUB_SIZE, 11 years ago. Time
> to drop it.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> include/hw/usb.h | 4
> 1 file changed, 4 deletions(-)
>
> diff --git a/include/hw/usb.h b/include/hw/usb.h
> index e29a37635b..4f04a1a879 100644
> --- a/include/hw/usb.h
> +++ b/include/hw/usb.h
> @@ -470,10 +470,6 @@ void usb_generic_async_ctrl_complete(USBDevice *s,
> USBPacket *p);
> void hmp_info_usbhost(Monitor *mon, const QDict *qdict);
> bool usb_host_dev_is_scsi_storage(USBDevice *usbdev);
>
> -/* usb ports of the VM */
> -
> -#define VM_USB_HUB_SIZE 8
> -
> /* usb-bus.c */
>
> #define TYPE_USB_BUS "usb-bus"
> --
> 2.21.3
>
>
Re: [PATCH 04/26] hw/usb: Reduce 'exec/memory.h' inclusion
On Sat, Jul 4, 2020 at 7:52 AM Philippe Mathieu-Daudé wrote:
>
> "exec/memory.h" is only required by "hw/usb/hcd-musb.h".
> Include it there directly.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> include/hw/usb.h | 1 -
> include/hw/usb/hcd-musb.h | 2 ++
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/usb.h b/include/hw/usb.h
> index 4f04a1a879..15b2ef300a 100644
> --- a/include/hw/usb.h
> +++ b/include/hw/usb.h
> @@ -25,7 +25,6 @@
> * THE SOFTWARE.
> */
>
> -#include "exec/memory.h"
> #include "hw/qdev-core.h"
> #include "qemu/iov.h"
> #include "qemu/queue.h"
> diff --git a/include/hw/usb/hcd-musb.h b/include/hw/usb/hcd-musb.h
> index c874b9f292..ec3ee5c4b0 100644
> --- a/include/hw/usb/hcd-musb.h
> +++ b/include/hw/usb/hcd-musb.h
> @@ -13,6 +13,8 @@
> #ifndef HW_USB_MUSB_H
> #define HW_USB_MUSB_H
>
> +#include "exec/memory.h"
> +
> enum musb_irq_source_e {
> musb_irq_suspend = 0,
> musb_irq_resume,
> --
> 2.21.3
>
>
Re: [PATCH 06/26] hw/usb/hcd-dwc2: Remove unnecessary includes
On Sat, Jul 4, 2020 at 7:53 AM Philippe Mathieu-Daudé wrote: > > "qemu/error-report.h" and "qemu/main-loop.h" are not used. > Remove them. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > hw/usb/hcd-dwc2.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c > index 72cbd051f3..590e75b455 100644 > --- a/hw/usb/hcd-dwc2.c > +++ b/hw/usb/hcd-dwc2.c > @@ -39,8 +39,6 @@ > #include "migration/vmstate.h" > #include "trace.h" > #include "qemu/log.h" > -#include "qemu/error-report.h" > -#include "qemu/main-loop.h" > #include "hw/qdev-properties.h" > > #define USB_HZ_FS 1200 > -- > 2.21.3 > >
Re: [PATCH 05/26] hw/usb/desc: Add missing header
On Sat, Jul 4, 2020 at 7:52 AM Philippe Mathieu-Daudé wrote:
>
> This header uses the USBPacket and USBDevice types which are
> forward declared in "hw/usb.h".
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/usb/desc.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/usb/desc.h b/hw/usb/desc.h
> index 4d81c68e0e..92594fbe29 100644
> --- a/hw/usb/desc.h
> +++ b/hw/usb/desc.h
> @@ -2,6 +2,7 @@
> #define QEMU_HW_USB_DESC_H
>
> #include
> +#include "hw/usb.h"
>
> /* binary representation */
> typedef struct USBDescriptor {
> --
> 2.21.3
>
>
Re: [PATCH 07/26] hw/usb/hcd-dwc2: Restrict some headers to source
On Sat, Jul 4, 2020 at 7:52 AM Philippe Mathieu-Daudé wrote: > > The header "usb/hcd-dwc2.h" doesn't need to include "qemu/timer.h", > "sysemu/dma.h", "hw/irq.h" (the types required are forward declared). > Include them in the source file which is the only one requiring the > function declarations. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > hw/usb/hcd-dwc2.h | 3 --- > hw/usb/hcd-dwc2.c | 3 +++ > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/usb/hcd-dwc2.h b/hw/usb/hcd-dwc2.h > index 4ba809a07b..2adf0f53c7 100644 > --- a/hw/usb/hcd-dwc2.h > +++ b/hw/usb/hcd-dwc2.h > @@ -19,11 +19,8 @@ > #ifndef HW_USB_DWC2_H > #define HW_USB_DWC2_H > > -#include "qemu/timer.h" > -#include "hw/irq.h" > #include "hw/sysbus.h" > #include "hw/usb.h" > -#include "sysemu/dma.h" > > #define DWC2_MMIO_SIZE 0x11000 > > diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c > index 590e75b455..ccf05d0823 100644 > --- a/hw/usb/hcd-dwc2.c > +++ b/hw/usb/hcd-dwc2.c > @@ -36,8 +36,11 @@ > #include "qapi/error.h" > #include "hw/usb/dwc2-regs.h" > #include "hw/usb/hcd-dwc2.h" > +#include "hw/irq.h" > +#include "sysemu/dma.h" > #include "migration/vmstate.h" > #include "trace.h" > +#include "qemu/timer.h" > #include "qemu/log.h" > #include "hw/qdev-properties.h" > > -- > 2.21.3 > >
Re: [PATCH 09/26] hw/usb/hcd-ehci: Remove unnecessary include
On Sat, Jul 4, 2020 at 7:55 AM Philippe Mathieu-Daudé wrote: > > As "qemu/main-loop.h" is not used, remove it. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > hw/usb/hcd-ehci.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c > index 1495e8f7fa..256fb91e0c 100644 > --- a/hw/usb/hcd-ehci.c > +++ b/hw/usb/hcd-ehci.c > @@ -34,7 +34,6 @@ > #include "migration/vmstate.h" > #include "trace.h" > #include "qemu/error-report.h" > -#include "qemu/main-loop.h" > #include "sysemu/runstate.h" > > #define FRAME_TIMER_FREQ 1000 > -- > 2.21.3 > >
Re: [PATCH 11/26] hw/usb/hcd-xhci: Add missing header
On Sat, Jul 4, 2020 at 7:54 AM Philippe Mathieu-Daudé wrote: > > This header uses the USBPort type which is forward declared > by "hw/usb.h". > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > hw/usb/hcd-xhci.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h > index 946af51fc2..8edbdc2c3e 100644 > --- a/hw/usb/hcd-xhci.h > +++ b/hw/usb/hcd-xhci.h > @@ -22,6 +22,8 @@ > #ifndef HW_USB_HCD_XHCI_H > #define HW_USB_HCD_XHCI_H > > +#include "hw/usb.h" > + > #define TYPE_XHCI "base-xhci" > #define TYPE_NEC_XHCI "nec-usb-xhci" > #define TYPE_QEMU_XHCI "qemu-xhci" > -- > 2.21.3 > >
Re: [PATCH 12/26] hw/usb/hcd-musb: Restrict header scope
On Sat, Jul 4, 2020 at 7:56 AM Philippe Mathieu-Daudé wrote:
>
> "hcd-musb.h" is only required by USB device implementions.
> As we keep these implementations in the hw/usb/ directory,
> move the header there.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> {include/hw => hw}/usb/hcd-musb.h | 0
> hw/usb/hcd-musb.c | 2 +-
> hw/usb/tusb6010.c | 2 +-
> 3 files changed, 2 insertions(+), 2 deletions(-)
> rename {include/hw => hw}/usb/hcd-musb.h (100%)
>
> diff --git a/include/hw/usb/hcd-musb.h b/hw/usb/hcd-musb.h
> similarity index 100%
> rename from include/hw/usb/hcd-musb.h
> rename to hw/usb/hcd-musb.h
> diff --git a/hw/usb/hcd-musb.c b/hw/usb/hcd-musb.c
> index 85f5ff5bd4..b8d8766a4a 100644
> --- a/hw/usb/hcd-musb.c
> +++ b/hw/usb/hcd-musb.c
> @@ -23,9 +23,9 @@
> #include "qemu/osdep.h"
> #include "qemu/timer.h"
> #include "hw/usb.h"
> -#include "hw/usb/hcd-musb.h"
> #include "hw/irq.h"
> #include "hw/hw.h"
> +#include "hcd-musb.h"
>
> /* Common USB registers */
> #define MUSB_HDRC_FADDR0x00/* 8-bit */
> diff --git a/hw/usb/tusb6010.c b/hw/usb/tusb6010.c
> index 27eb28d3e4..9f9b81b09d 100644
> --- a/hw/usb/tusb6010.c
> +++ b/hw/usb/tusb6010.c
> @@ -23,11 +23,11 @@
> #include "qemu/module.h"
> #include "qemu/timer.h"
> #include "hw/usb.h"
> -#include "hw/usb/hcd-musb.h"
> #include "hw/arm/omap.h"
> #include "hw/hw.h"
> #include "hw/irq.h"
> #include "hw/sysbus.h"
> +#include "hcd-musb.h"
>
> #define TYPE_TUSB6010 "tusb6010"
> #define TUSB(obj) OBJECT_CHECK(TUSBState, (obj), TYPE_TUSB6010)
> --
> 2.21.3
>
>
Re: [PATCH 10/26] hw/usb/hcd-ehci: Move few definitions from header to source
On Sat, Jul 4, 2020 at 7:53 AM Philippe Mathieu-Daudé wrote: > > Move definitions only useful for hcd-ehci.c to this source file. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > hw/usb/hcd-ehci.h | 11 --- > hw/usb/hcd-ehci.c | 12 > 2 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h > index 57b38cfc05..4577f5e31d 100644 > --- a/hw/usb/hcd-ehci.h > +++ b/hw/usb/hcd-ehci.h > @@ -24,17 +24,6 @@ > #include "hw/pci/pci.h" > #include "hw/sysbus.h" > > -#ifndef EHCI_DEBUG > -#define EHCI_DEBUG 0 > -#endif > - > -#if EHCI_DEBUG > -#define DPRINTF printf > -#else > -#define DPRINTF(...) > -#endif > - > -#define MMIO_SIZE0x1000 > #define CAPA_SIZE0x10 > > #define NB_PORTS 6/* Max. Number of downstream ports */ > diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c > index 256fb91e0c..a0beee527c 100644 > --- a/hw/usb/hcd-ehci.c > +++ b/hw/usb/hcd-ehci.c > @@ -36,6 +36,18 @@ > #include "qemu/error-report.h" > #include "sysemu/runstate.h" > > +#ifndef EHCI_DEBUG > +#define EHCI_DEBUG 0 > +#endif > + > +#if EHCI_DEBUG > +#define DPRINTF printf > +#else > +#define DPRINTF(...) > +#endif > + > +#define MMIO_SIZE0x1000 > + > #define FRAME_TIMER_FREQ 1000 > #define FRAME_TIMER_NS (NANOSECONDS_PER_SECOND / FRAME_TIMER_FREQ) > #define UFRAME_TIMER_NS (FRAME_TIMER_NS / 8) > -- > 2.21.3 > >
Re: [PATCH 13/26] hw/usb/desc: Reduce some declarations scope
On Sat, Jul 4, 2020 at 7:59 AM Philippe Mathieu-Daudé wrote:
>
> USBDescString is forward-declared. Only bus.c uses the
> usb_device_get_product_desc() and usb_device_get_usb_desc()
> function. Move all that to the "desc.h" header to reduce
> the big "hw/usb.h" header a bit.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/usb/desc.h| 10 ++
> include/hw/usb.h | 10 --
> hw/usb/bus.c | 1 +
> 3 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/hw/usb/desc.h b/hw/usb/desc.h
> index 92594fbe29..4bf6966c4b 100644
> --- a/hw/usb/desc.h
> +++ b/hw/usb/desc.h
> @@ -242,4 +242,14 @@ int usb_desc_get_descriptor(USBDevice *dev, USBPacket *p,
> int usb_desc_handle_control(USBDevice *dev, USBPacket *p,
> int request, int value, int index, int length, uint8_t *data);
>
> +const char *usb_device_get_product_desc(USBDevice *dev);
> +
> +const USBDesc *usb_device_get_usb_desc(USBDevice *dev);
> +
> +struct USBDescString {
> +uint8_t index;
> +char *str;
> +QLIST_ENTRY(USBDescString) next;
> +};
> +
> #endif /* QEMU_HW_USB_DESC_H */
> diff --git a/include/hw/usb.h b/include/hw/usb.h
> index 15b2ef300a..18f1349bdc 100644
> --- a/include/hw/usb.h
> +++ b/include/hw/usb.h
> @@ -192,12 +192,6 @@ typedef struct USBDescOther USBDescOther;
> typedef struct USBDescString USBDescString;
> typedef struct USBDescMSOS USBDescMSOS;
>
> -struct USBDescString {
> -uint8_t index;
> -char *str;
> -QLIST_ENTRY(USBDescString) next;
> -};
> -
> #define USB_MAX_ENDPOINTS 15
> #define USB_MAX_INTERFACES 16
>
> @@ -555,10 +549,6 @@ int usb_device_alloc_streams(USBDevice *dev, USBEndpoint
> **eps, int nr_eps,
> int streams);
> void usb_device_free_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps);
>
> -const char *usb_device_get_product_desc(USBDevice *dev);
> -
> -const USBDesc *usb_device_get_usb_desc(USBDevice *dev);
> -
> /* quirks.c */
>
> /* In bulk endpoints are streaming data sources (iow behave like isoc eps) */
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index 957559b18d..111c3af7c1 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -9,6 +9,7 @@
> #include "monitor/monitor.h"
> #include "trace.h"
> #include "qemu/cutils.h"
> +#include "desc.h"
>
> static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
>
> --
> 2.21.3
>
>
Re: [PATCH 14/26] hw/usb/quirks: Rename included source with '.inc.c' suffix
On Sat, Jul 4, 2020 at 7:58 AM Philippe Mathieu-Daudé wrote:
>
> This file is not a header, but contains source code which is
> included and compiled once. We use the '.inc.c' suffix in few
> other cases in the repository. Follow the same convention with
> this file.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/usb/quirks.c | 2 +-
> hw/usb/{quirks.h => quirks.inc.c} | 5 -
> 2 files changed, 1 insertion(+), 6 deletions(-)
> rename hw/usb/{quirks.h => quirks.inc.c} (99%)
>
> diff --git a/hw/usb/quirks.c b/hw/usb/quirks.c
> index 23ea7a23ea..655b36f2d5 100644
> --- a/hw/usb/quirks.c
> +++ b/hw/usb/quirks.c
> @@ -13,7 +13,7 @@
> */
>
> #include "qemu/osdep.h"
> -#include "quirks.h"
> +#include "quirks.inc.c"
> #include "hw/usb.h"
>
> static bool usb_id_match(const struct usb_device_id *ids,
> diff --git a/hw/usb/quirks.h b/hw/usb/quirks.inc.c
> similarity index 99%
> rename from hw/usb/quirks.h
> rename to hw/usb/quirks.inc.c
> index 50ef2f9c2e..004b228aba 100644
> --- a/hw/usb/quirks.h
> +++ b/hw/usb/quirks.inc.c
> @@ -12,9 +12,6 @@
> * (at your option) any later version.
> */
>
> -#ifndef HW_USB_QUIRKS_H
> -#define HW_USB_QUIRKS_H
> -
> /* 1 on 1 copy of linux/drivers/usb/serial/ftdi_sio_ids.h */
> #include "quirks-ftdi-ids.h"
> /* 1 on 1 copy of linux/drivers/usb/serial/pl2303.h */
> @@ -915,5 +912,3 @@ static const struct usb_device_id
> usbredir_ftdi_serial_ids[] = {
>
> #undef USB_DEVICE
> #undef USB_DEVICE_AND_INTERFACE_INFO
> -
> -#endif
> --
> 2.21.3
>
>
[PATCH] trivial: Remove trailing whitespaces
There are a number of unnecessary trailing whitespaces that have accumulated over time in the source code. They cause stray changes in patches if you use tools that automatically remove them. Tested by doing a `git diff -w` after the change. This could probably be turned into a pre-commit hook. Signed-off-by: Christophe de Dinechin --- block/iscsi.c | 2 +- disas/cris.c | 2 +- disas/microblaze.c| 80 +++--- disas/nios2.c | 256 +- hmp-commands.hx | 2 +- hw/alpha/typhoon.c| 6 +- hw/arm/gumstix.c | 6 +- hw/arm/omap1.c| 2 +- hw/arm/stellaris.c| 2 +- hw/char/etraxfs_ser.c | 2 +- hw/core/ptimer.c | 2 +- hw/cris/axis_dev88.c | 2 +- hw/cris/boot.c| 2 +- hw/display/qxl.c | 2 +- hw/dma/etraxfs_dma.c | 18 +- hw/dma/i82374.c | 2 +- hw/i2c/bitbang_i2c.c | 2 +- hw/input/tsc2005.c| 2 +- hw/input/tsc210x.c| 2 +- hw/intc/etraxfs_pic.c | 8 +- hw/intc/sh_intc.c | 10 +- hw/intc/xilinx_intc.c | 2 +- hw/misc/imx25_ccm.c | 6 +- hw/misc/imx31_ccm.c | 2 +- hw/net/vmxnet3.h | 2 +- hw/net/xilinx_ethlite.c | 2 +- hw/pci/pcie.c | 2 +- hw/sd/omap_mmc.c | 2 +- hw/sh4/shix.c | 2 +- hw/sparc64/sun4u.c| 2 +- hw/timer/etraxfs_timer.c | 2 +- hw/timer/xilinx_timer.c | 4 +- hw/usb/hcd-musb.c | 10 +- hw/usb/hcd-ohci.c | 6 +- hw/usb/hcd-uhci.c | 2 +- hw/virtio/virtio-pci.c| 2 +- include/hw/cris/etraxfs_dma.h | 4 +- include/hw/net/lance.h| 2 +- include/hw/ppc/spapr.h| 2 +- include/hw/xen/interface/io/ring.h| 34 +-- include/qemu/log.h| 2 +- include/qom/object.h | 4 +- linux-user/cris/cpu_loop.c| 16 +- linux-user/microblaze/cpu_loop.c | 16 +- linux-user/mmap.c | 8 +- linux-user/sparc/signal.c | 4 +- linux-user/syscall.c | 24 +- linux-user/syscall_defs.h | 2 +- linux-user/uaccess.c | 2 +- os-posix.c| 2 +- qapi/qapi-util.c | 2 +- qemu-img.c| 2 +- qemu-options.hx | 26 +- qom/object.c | 2 +- target/cris/translate.c | 28 +- target/cris/translate_v10.inc.c | 6 +- target/i386/hvf/hvf.c | 4 +- target/i386/hvf/x86.c | 4 +- target/i386/hvf/x86_decode.c | 20 +- target/i386/hvf/x86_decode.h | 4 +- target/i386/hvf/x86_descr.c | 2 +- target/i386/hvf/x86_emu.c | 2 +- target/i386/hvf/x86_mmu.c | 6 +- target/i386/hvf/x86_task.c| 2 +- target/i386/hvf/x86hvf.c | 42 +-- target/i386/translate.c | 8 +- target/microblaze/mmu.c | 2 +- target/microblaze/translate.c | 2 +- target/sh4/op_helper.c| 4 +- target/xtensa/core-de212/core-isa.h | 6 +- .../xtensa/core-sample_controller/core-isa.h | 6 +- target/xtensa/core-test_kc705_be/core-isa.h | 2 +- tcg/sparc/tcg-target.inc.c| 2 +- tcg/tcg.c | 32 +-- tests/tcg/multiarch/test-mmap.c | 72 ++--- ui/curses.c | 4 +- ui/curses_keys.h | 4 +- util/cutils.c | 2 +- 78 files changed, 440 insertions(+), 440 deletions(-) diff --git a/block/iscsi.c b/block/iscsi.c index a8b76979d8..884075f4e1 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -1412,7 +1412,7 @@ static void iscsi_readcapacity_sync(IscsiLun *iscsilun, Error **e
[qemu-mainline test] 151669: regressions - FAIL
flight 151669 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/151669/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 151065 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-xl-qcow210 debian-di-installfail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 151065 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 151065 test-amd64-i386-libvirt-pair 21 guest-start/debian fail REGR. vs. 151065 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 151065 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 151065 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 151065 test-amd64-amd64-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-amd64-amd64-libvirt 12 guest-start fail REGR. vs. 151065 test-amd64-i386-libvirt 12 guest-start fail REGR. vs. 151065 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 151065 test-armhf-armhf-xl-vhd 10 debian-di-installfail REGR. vs. 151065 test-amd64-amd64-libvirt-pair 21 guest-start/debian fail REGR. vs. 151065 test-armhf-armhf-libvirt 12 guest-start fail REGR. vs. 151065 test-arm64-arm64-libvirt-xsm 12 guest-start fail REGR. vs. 151065 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 151065 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-pvshim12 guest-start fail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-sup
Re: [PATCH 15/26] hw/usb: Add new 'usb-quirks.h' local header
On Sat, Jul 4, 2020 at 7:56 AM Philippe Mathieu-Daudé wrote: > > Only redirect.c consumes the quirks API. Reduce the big "hw/usb.h" > header by moving the quirks related declaration into their own > header. As nothing out of hw/usb/ requires it, keep it local. > > Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis Alistair > --- > hw/usb/usb-quirks.h | 27 +++ > include/hw/usb.h| 11 --- > hw/usb/quirks.c | 1 + > hw/usb/redirect.c | 1 + > 4 files changed, 29 insertions(+), 11 deletions(-) > create mode 100644 hw/usb/usb-quirks.h > > diff --git a/hw/usb/usb-quirks.h b/hw/usb/usb-quirks.h > new file mode 100644 > index 00..542889efc4 > --- /dev/null > +++ b/hw/usb/usb-quirks.h > @@ -0,0 +1,27 @@ > +/* > + * USB quirk handling > + * > + * Copyright (c) 2012 Red Hat, Inc. > + * > + * Red Hat Authors: > + * Hans de Goede > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ > + > +#ifndef HW_USB_QUIRKS_H > +#define HW_USB_QUIRKS_H > + > +/* In bulk endpoints are streaming data sources (iow behave like isoc eps) */ > +#define USB_QUIRK_BUFFER_BULK_IN0x01 > +/* Bulk pkts in FTDI format, need special handling when combining packets */ > +#define USB_QUIRK_IS_FTDI 0x02 > + > +int usb_get_quirks(uint16_t vendor_id, uint16_t product_id, > + uint8_t interface_class, uint8_t interface_subclass, > + uint8_t interface_protocol); > + > +#endif > diff --git a/include/hw/usb.h b/include/hw/usb.h > index 18f1349bdc..8c3bc920ff 100644 > --- a/include/hw/usb.h > +++ b/include/hw/usb.h > @@ -549,15 +549,4 @@ int usb_device_alloc_streams(USBDevice *dev, USBEndpoint > **eps, int nr_eps, > int streams); > void usb_device_free_streams(USBDevice *dev, USBEndpoint **eps, int nr_eps); > > -/* quirks.c */ > - > -/* In bulk endpoints are streaming data sources (iow behave like isoc eps) */ > -#define USB_QUIRK_BUFFER_BULK_IN 0x01 > -/* Bulk pkts in FTDI format, need special handling when combining packets */ > -#define USB_QUIRK_IS_FTDI 0x02 > - > -int usb_get_quirks(uint16_t vendor_id, uint16_t product_id, > - uint8_t interface_class, uint8_t interface_subclass, > - uint8_t interface_protocol); > - > #endif > diff --git a/hw/usb/quirks.c b/hw/usb/quirks.c > index 655b36f2d5..b0d0f87e35 100644 > --- a/hw/usb/quirks.c > +++ b/hw/usb/quirks.c > @@ -15,6 +15,7 @@ > #include "qemu/osdep.h" > #include "quirks.inc.c" > #include "hw/usb.h" > +#include "usb-quirks.h" > > static bool usb_id_match(const struct usb_device_id *ids, > uint16_t vendor_id, uint16_t product_id, > diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c > index 417a60a2e6..4c5925a039 100644 > --- a/hw/usb/redirect.c > +++ b/hw/usb/redirect.c > @@ -45,6 +45,7 @@ > #include "hw/usb.h" > #include "migration/qemu-file-types.h" > #include "migration/vmstate.h" > +#include "usb-quirks.h" > > /* ERROR is defined below. Remove any previous definition. */ > #undef ERROR > -- > 2.21.3 > >
Re: [PATCH 17/26] hw/usb/bus: Rename usb_get_dev_path() as usb_get_full_dev_path()
On Sat, Jul 4, 2020 at 7:58 AM Philippe Mathieu-Daudé wrote:
>
> If the device has USB_DEV_FLAG_FULL_PATH set, usb_get_dev_path()
> returns the full port path. Rename the function accordingly.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/usb/bus.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index f8901e822c..fad8194bf5 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -13,7 +13,7 @@
>
> static void usb_bus_dev_print(Monitor *mon, DeviceState *qdev, int indent);
>
> -static char *usb_get_dev_path(DeviceState *dev);
> +static char *usb_get_full_dev_path(DeviceState *dev);
> static char *usb_get_fw_dev_path(DeviceState *qdev);
> static void usb_qdev_unrealize(DeviceState *qdev);
>
> @@ -33,7 +33,7 @@ static void usb_bus_class_init(ObjectClass *klass, void
> *data)
> HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
>
> k->print_dev = usb_bus_dev_print;
> -k->get_dev_path = usb_get_dev_path;
> +k->get_dev_path = usb_get_full_dev_path;
> k->get_fw_dev_path = usb_get_fw_dev_path;
> hc->unplug = qdev_simple_device_unplug_cb;
> }
> @@ -577,7 +577,7 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState
> *qdev, int indent)
> dev->attached ? ", attached" : "");
> }
>
> -static char *usb_get_dev_path(DeviceState *qdev)
> +static char *usb_get_full_dev_path(DeviceState *qdev)
> {
> USBDevice *dev = USB_DEVICE(qdev);
>
> --
> 2.21.3
>
>
Re: [PATCH 16/26] hw/usb/bus: Simplify usb_get_dev_path()
On Sat, Jul 4, 2020 at 8:00 AM Philippe Mathieu-Daudé wrote:
>
> Simplify usb_get_dev_path() a bit.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/usb/bus.c | 19 +--
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index 111c3af7c1..f8901e822c 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -580,19 +580,18 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState
> *qdev, int indent)
> static char *usb_get_dev_path(DeviceState *qdev)
> {
> USBDevice *dev = USB_DEVICE(qdev);
> -DeviceState *hcd = qdev->parent_bus->parent;
> -char *id = NULL;
>
> if (dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) {
> -id = qdev_get_dev_path(hcd);
> -}
> -if (id) {
> -char *ret = g_strdup_printf("%s/%s", id, dev->port->path);
> -g_free(id);
> -return ret;
> -} else {
> -return g_strdup(dev->port->path);
> +DeviceState *hcd = qdev->parent_bus->parent;
> +char *id = qdev_get_dev_path(hcd);
> +
> +if (id) {
> +char *ret = g_strdup_printf("%s/%s", id, dev->port->path);
> +g_free(id);
> +return ret;
> +}
> }
> +return g_strdup(dev->port->path);
> }
>
> static char *usb_get_fw_dev_path(DeviceState *qdev)
> --
> 2.21.3
>
>
[PATCH v2 1/3] xen/privcmd: Corrected error handling path
Previously, if lock_pages() end up partially mapping pages, it used
to return -ERRNO due to which unlock_pages() have to go through
each pages[i] till *nr_pages* to validate them. This can be avoided
by passing correct number of partially mapped pages & -ERRNO separately,
while returning from lock_pages() due to error.
With this fix unlock_pages() doesn't need to validate pages[i] till
*nr_pages* for error scenario and few condition checks can be ignored.
Signed-off-by: Souptick Joarder
Cc: John Hubbard
Cc: Boris Ostrovsky
Cc: Paul Durrant
---
drivers/xen/privcmd.c | 31 +++
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index a250d11..33677ea 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -580,13 +580,13 @@ static long privcmd_ioctl_mmap_batch(
static int lock_pages(
struct privcmd_dm_op_buf kbufs[], unsigned int num,
- struct page *pages[], unsigned int nr_pages)
+ struct page *pages[], unsigned int nr_pages, unsigned int *pinned)
{
unsigned int i;
+ int page_count = 0;
for (i = 0; i < num; i++) {
unsigned int requested;
- int pinned;
requested = DIV_ROUND_UP(
offset_in_page(kbufs[i].uptr) + kbufs[i].size,
@@ -594,14 +594,15 @@ static int lock_pages(
if (requested > nr_pages)
return -ENOSPC;
- pinned = get_user_pages_fast(
+ page_count = get_user_pages_fast(
(unsigned long) kbufs[i].uptr,
requested, FOLL_WRITE, pages);
- if (pinned < 0)
- return pinned;
+ if (page_count < 0)
+ return page_count;
- nr_pages -= pinned;
- pages += pinned;
+ *pinned += page_count;
+ nr_pages -= page_count;
+ pages += page_count;
}
return 0;
@@ -611,13 +612,8 @@ static void unlock_pages(struct page *pages[], unsigned
int nr_pages)
{
unsigned int i;
- if (!pages)
- return;
-
- for (i = 0; i < nr_pages; i++) {
- if (pages[i])
- put_page(pages[i]);
- }
+ for (i = 0; i < nr_pages; i++)
+ put_page(pages[i]);
}
static long privcmd_ioctl_dm_op(struct file *file, void __user *udata)
@@ -630,6 +626,7 @@ static long privcmd_ioctl_dm_op(struct file *file, void
__user *udata)
struct xen_dm_op_buf *xbufs = NULL;
unsigned int i;
long rc;
+ unsigned int pinned = 0;
if (copy_from_user(&kdata, udata, sizeof(kdata)))
return -EFAULT;
@@ -683,9 +680,11 @@ static long privcmd_ioctl_dm_op(struct file *file, void
__user *udata)
goto out;
}
- rc = lock_pages(kbufs, kdata.num, pages, nr_pages);
- if (rc)
+ rc = lock_pages(kbufs, kdata.num, pages, nr_pages, &pinned);
+ if (rc < 0) {
+ nr_pages = pinned;
goto out;
+ }
for (i = 0; i < kdata.num; i++) {
set_xen_guest_handle(xbufs[i].h, kbufs[i].uptr);
--
1.9.1
[PATCH v2 2/3] xen/privcmd: Mark pages as dirty
pages need to be marked as dirty before unpinned it in
unlock_pages() which was oversight. This is fixed now.
Signed-off-by: Souptick Joarder
Suggested-by: John Hubbard
Cc: John Hubbard
Cc: Boris Ostrovsky
Cc: Paul Durrant
---
drivers/xen/privcmd.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 33677ea..f6c1543 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -612,8 +612,11 @@ static void unlock_pages(struct page *pages[], unsigned
int nr_pages)
{
unsigned int i;
- for (i = 0; i < nr_pages; i++)
+ for (i = 0; i < nr_pages; i++) {
+ if (!PageDirty(pages[i]))
+ set_page_dirty_lock(pages[i]);
put_page(pages[i]);
+ }
}
static long privcmd_ioctl_dm_op(struct file *file, void __user *udata)
--
1.9.1
[PATCH v2 0/3] Few bug fixes and Convert to pin_user_pages*()
This series contains few clean up, minor bug fixes and Convert get_user_pages() to pin_user_pages(). I'm compile tested this, but unable to run-time test, so any testing help is much appriciated. v2: Addressed few review comments and compile issue. Patch[1/2] from v1 split into 2 in v2. Cc: John Hubbard Cc: Boris Ostrovsky Cc: Paul Durrant Souptick Joarder (3): xen/privcmd: Corrected error handling path xen/privcmd: Mark pages as dirty xen/privcmd: Convert get_user_pages*() to pin_user_pages*() drivers/xen/privcmd.c | 32 ++-- 1 file changed, 14 insertions(+), 18 deletions(-) -- 1.9.1
[PATCH v2 3/3] xen/privcmd: Convert get_user_pages*() to pin_user_pages*()
In 2019, we introduced pin_user_pages*() and now we are converting
get_user_pages*() to the new API as appropriate. [1] & [2] could
be referred for more information. This is case 5 as per document [1].
[1] Documentation/core-api/pin_user_pages.rst
[2] "Explicit pinning of user-space pages":
https://lwn.net/Articles/807108/
Signed-off-by: Souptick Joarder
Cc: John Hubbard
Cc: Boris Ostrovsky
Cc: Paul Durrant
---
drivers/xen/privcmd.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index f6c1543..5c5cd24 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -594,7 +594,7 @@ static int lock_pages(
if (requested > nr_pages)
return -ENOSPC;
- page_count = get_user_pages_fast(
+ page_count = pin_user_pages_fast(
(unsigned long) kbufs[i].uptr,
requested, FOLL_WRITE, pages);
if (page_count < 0)
@@ -610,13 +610,7 @@ static int lock_pages(
static void unlock_pages(struct page *pages[], unsigned int nr_pages)
{
- unsigned int i;
-
- for (i = 0; i < nr_pages; i++) {
- if (!PageDirty(pages[i]))
- set_page_dirty_lock(pages[i]);
- put_page(pages[i]);
- }
+ unpin_user_pages_dirty_lock(pages, nr_pages, true);
}
static long privcmd_ioctl_dm_op(struct file *file, void __user *udata)
--
1.9.1
[xen-unstable-smoke test] 151687: tolerable all pass - PUSHED
flight 151687 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/151687/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen f97f99c8d88ebc108f6adc3ba74e87d53ba57c70 baseline version: xen 158912a532fe98f448c688d3571241c9033553bd Last test of basis 151679 2020-07-06 14:01:18 Z0 days Testing same since 151687 2020-07-06 19:01:28 Z0 days1 attempts People who touched revisions under test: Andrew Cooper Jan Beulich jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 158912a532..f97f99c8d8 f97f99c8d88ebc108f6adc3ba74e87d53ba57c70 -> smoke
[linux-linus test] 151681: regressions - FAIL
flight 151681 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/151681/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-cubietruckbroken in 151662 build-i386-pvops 6 kernel-build fail REGR. vs. 151214 test-arm64-arm64-libvirt-xsm 17 guest-start.2 fail in 151662 REGR. vs. 151214 test-armhf-armhf-xl-vhd 15 guest-start/debian.repeat fail in 151662 REGR. vs. 151214 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-cubietruck 4 host-install(4) broken in 151662 pass in 151681 test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail in 151662 pass in 151681 test-arm64-arm64-libvirt-xsm 16 guest-start/debian.repeat fail pass in 151662 test-amd64-amd64-examine 4 memdisk-try-append fail pass in 151662 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail pass in 151662 test-armhf-armhf-xl-vhd 11 guest-startfail pass in 151662 Tests which did not succeed, but are not blocking: test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-coresched-i386-xl 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-pvshim 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-examine 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-i386-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a test-armhf-armhf-xl-vhd 12 migrate-support-check fail in 151662 never pass test-armhf-armhf-xl-vhd 13 saverestore-support-check fail in 151662 never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 151214 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 151214 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 151214 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 151214 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 151214 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 151214 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-th
Re: [PATCH 07/26] hw/usb/hcd-dwc2: Restrict some headers to source
On Sat, Jul 4, 2020 at 7:50 AM Philippe Mathieu-Daudé wrote: > The header "usb/hcd-dwc2.h" doesn't need to include "qemu/timer.h", > "sysemu/dma.h", "hw/irq.h" (the types required are forward declared). > Include them in the source file which is the only one requiring the > function declarations. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/usb/hcd-dwc2.h | 3 --- > hw/usb/hcd-dwc2.c | 3 +++ > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/usb/hcd-dwc2.h b/hw/usb/hcd-dwc2.h > index 4ba809a07b..2adf0f53c7 100644 > --- a/hw/usb/hcd-dwc2.h > +++ b/hw/usb/hcd-dwc2.h > @@ -19,11 +19,8 @@ > #ifndef HW_USB_DWC2_H > #define HW_USB_DWC2_H > > -#include "qemu/timer.h" > -#include "hw/irq.h" > #include "hw/sysbus.h" > #include "hw/usb.h" > -#include "sysemu/dma.h" > > #define DWC2_MMIO_SIZE 0x11000 > > diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c > index 590e75b455..ccf05d0823 100644 > --- a/hw/usb/hcd-dwc2.c > +++ b/hw/usb/hcd-dwc2.c > @@ -36,8 +36,11 @@ > #include "qapi/error.h" > #include "hw/usb/dwc2-regs.h" > #include "hw/usb/hcd-dwc2.h" > +#include "hw/irq.h" > +#include "sysemu/dma.h" > #include "migration/vmstate.h" > #include "trace.h" > +#include "qemu/timer.h" > #include "qemu/log.h" > #include "hw/qdev-properties.h" > > -- > 2.21.3 > > Reviewed-by: Paul Zimmerman
Re: [PATCH 06/26] hw/usb/hcd-dwc2: Remove unnecessary includes
On Sat, Jul 4, 2020 at 7:50 AM Philippe Mathieu-Daudé wrote: > "qemu/error-report.h" and "qemu/main-loop.h" are not used. > Remove them. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/usb/hcd-dwc2.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c > index 72cbd051f3..590e75b455 100644 > --- a/hw/usb/hcd-dwc2.c > +++ b/hw/usb/hcd-dwc2.c > @@ -39,8 +39,6 @@ > #include "migration/vmstate.h" > #include "trace.h" > #include "qemu/log.h" > -#include "qemu/error-report.h" > -#include "qemu/main-loop.h" > #include "hw/qdev-properties.h" > > #define USB_HZ_FS 1200 > -- > 2.21.3 > > Reviewed-by: Paul Zimmerman
Re: [PATCH 08/26] hw/usb/hcd-dwc2: Restrict 'dwc2-regs.h' scope
On Sat, Jul 4, 2020 at 7:50 AM Philippe Mathieu-Daudé
wrote:
> We only use these register definitions in files under the
> hw/usb/ directory. Keep that header local by moving it there.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> {include/hw => hw}/usb/dwc2-regs.h | 0
> hw/usb/hcd-dwc2.c | 2 +-
> 2 files changed, 1 insertion(+), 1 deletion(-)
> rename {include/hw => hw}/usb/dwc2-regs.h (100%)
>
> diff --git a/include/hw/usb/dwc2-regs.h b/hw/usb/dwc2-regs.h
> similarity index 100%
> rename from include/hw/usb/dwc2-regs.h
> rename to hw/usb/dwc2-regs.h
> diff --git a/hw/usb/hcd-dwc2.c b/hw/usb/hcd-dwc2.c
> index ccf05d0823..252b60ef65 100644
> --- a/hw/usb/hcd-dwc2.c
> +++ b/hw/usb/hcd-dwc2.c
> @@ -34,7 +34,6 @@
> #include "qemu/osdep.h"
> #include "qemu/units.h"
> #include "qapi/error.h"
> -#include "hw/usb/dwc2-regs.h"
> #include "hw/usb/hcd-dwc2.h"
> #include "hw/irq.h"
> #include "sysemu/dma.h"
> @@ -43,6 +42,7 @@
> #include "qemu/timer.h"
> #include "qemu/log.h"
> #include "hw/qdev-properties.h"
> +#include "dwc2-regs.h"
>
> #define USB_HZ_FS 1200
> #define USB_HZ_HS 9600
> --
> 2.21.3
>
>
Reviewed-by: Paul Zimmerman
Re: [PATCH 19/26] hw/ppc/spapr: Use usb_get_port_path()
On Sat, Jul 4, 2020 at 7:59 AM Philippe Mathieu-Daudé wrote:
>
> To avoid to access the USBDevice internals, and use the
> recently added usb_get_port_path() helper instead.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> hw/ppc/spapr.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index f6f034d039..221d3e7a8c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3121,7 +3121,8 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p,
> BusState *bus,
> * We use SRP luns of the form 0100 | (usb-port << 16) | lun
> * in the top 32 bits of the 64-bit LUN
> */
> -unsigned usb_port = atoi(usb->port->path);
> +g_autofree char *usb_port_path = usb_get_port_path(usb);
> +unsigned usb_port = atoi(usb_port_path);
> unsigned id = 0x100 | (usb_port << 16) | d->lun;
> return g_strdup_printf("%s@%"PRIX64, qdev_fw_name(dev),
> (uint64_t)id << 32);
> @@ -3137,7 +3138,8 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p,
> BusState *bus,
> if (strcmp("usb-host", qdev_fw_name(dev)) == 0) {
> USBDevice *usbdev = CAST(USBDevice, dev, TYPE_USB_DEVICE);
> if (usb_host_dev_is_scsi_storage(usbdev)) {
> -return g_strdup_printf("storage@%s/disk", usbdev->port->path);
> +g_autofree char *usb_port_path = usb_get_port_path(usbdev);
> +return g_strdup_printf("storage@%s/disk", usb_port_path);
> }
> }
>
> --
> 2.21.3
>
>
Re: [PATCH 18/26] hw/usb/bus: Add usb_get_port_path()
On Sat, Jul 4, 2020 at 8:00 AM Philippe Mathieu-Daudé wrote:
>
> Refactor usb_get_full_dev_path() to take a 'want_full_path'
> argument, and add usb_get_port_path() which returns a short
> path.
>
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Alistair Francis
Alistair
> ---
> include/hw/usb.h | 10 ++
> hw/usb/bus.c | 18 +-
> 2 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/include/hw/usb.h b/include/hw/usb.h
> index 8c3bc920ff..7ea502d421 100644
> --- a/include/hw/usb.h
> +++ b/include/hw/usb.h
> @@ -506,6 +506,16 @@ void usb_port_location(USBPort *downstream, USBPort
> *upstream, int portnr);
> void usb_unregister_port(USBBus *bus, USBPort *port);
> void usb_claim_port(USBDevice *dev, Error **errp);
> void usb_release_port(USBDevice *dev);
> +/**
> + * usb_get_port_path:
> + * @dev: the USB device
> + *
> + * The returned data must be released with g_free()
> + * when no longer required.
> + *
> + * Returns: a dynamically allocated pathname.
> + */
> +char *usb_get_port_path(USBDevice *dev);
> void usb_device_attach(USBDevice *dev, Error **errp);
> int usb_device_detach(USBDevice *dev);
> void usb_check_attach(USBDevice *dev, Error **errp);
> diff --git a/hw/usb/bus.c b/hw/usb/bus.c
> index fad8194bf5..518e5b94ed 100644
> --- a/hw/usb/bus.c
> +++ b/hw/usb/bus.c
> @@ -577,12 +577,10 @@ static void usb_bus_dev_print(Monitor *mon, DeviceState
> *qdev, int indent)
> dev->attached ? ", attached" : "");
> }
>
> -static char *usb_get_full_dev_path(DeviceState *qdev)
> +static char *usb_get_dev_path(USBDevice *dev, bool want_full_path)
> {
> -USBDevice *dev = USB_DEVICE(qdev);
> -
> -if (dev->flags & (1 << USB_DEV_FLAG_FULL_PATH)) {
> -DeviceState *hcd = qdev->parent_bus->parent;
> +if (want_full_path && (dev->flags & (1 << USB_DEV_FLAG_FULL_PATH))) {
> +DeviceState *hcd = DEVICE(dev)->parent_bus->parent;
> char *id = qdev_get_dev_path(hcd);
>
> if (id) {
> @@ -594,6 +592,16 @@ static char *usb_get_full_dev_path(DeviceState *qdev)
> return g_strdup(dev->port->path);
> }
>
> +static char *usb_get_full_dev_path(DeviceState *qdev)
> +{
> +return usb_get_dev_path(USB_DEVICE(qdev), true);
> +}
> +
> +char *usb_get_port_path(USBDevice *dev)
> +{
> +return usb_get_dev_path(dev, false);
> +}
> +
> static char *usb_get_fw_dev_path(DeviceState *qdev)
> {
> USBDevice *dev = USB_DEVICE(qdev);
> --
> 2.21.3
>
>
[xen-unstable test] 151684: regressions - FAIL
flight 151684 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/151684/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-armhf-armhf-xl-vhd 11 guest-start fail REGR. vs. 151661 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 17 guest-start.2fail REGR. vs. 151586 Tests which did not succeed, but are not blocking: test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 151661 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 151661 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 151661 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 151661 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 151661 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 151661 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 151661 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 151661 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 151661 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 14 saverestore-support-checkfail never pass test-arm64-arm64-xl 13 migrate-support-checkfail never pass test-arm64-arm64-xl 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-arm64-arm64-xl-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 14 saverestore-support-checkfail never pass version targeted for testing: xen 158912a532fe98f448c688d3571241c9033553bd baseline version: xen be63d9d47f571a60d70f8fb630c03871312d9655 Last test of basis 151661 2020-07-06 01:54:10 Z1 days Testing same since 151684 2020-07-06 16:36:21 Z0 days1 attempts People who touched revisions under test: Anthony PERARD Wei Liu jobs: build-amd64-xsm pass build-arm64-xsm
