Re: [Xen-devel] [PATCH 2/2] evtchn: Change evtchn port type to evtchn_port_t
On 23.03.2020 06:35, Yan Yankovskyi wrote: > struct evtchn_set_priority uses uint32_t type for event channel port. > Replace the type with evtchn_port_t. Such change is also done in Linux. > > Signed-off-by: Yan Yankovskyi Reviewed-by: Jan Beulich As a general remark, the order of changes would better be the other way around: The canonical header in the Xen repo be adjusted first, and the change then propagated to repos carrying clones. Thanks, Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/6] x86/ucode: Alter ops->free_patch() to free the entire patch
On 20.03.2020 17:48, Andrew Cooper wrote:
> On 20/03/2020 16:16, Jan Beulich wrote:
>> On 20.03.2020 17:10, Andrew Cooper wrote:
>>> On 20/03/2020 15:15, Jan Beulich wrote:
On 20.03.2020 15:50, Andrew Cooper wrote:
> On 20/03/2020 13:51, Jan Beulich wrote:
>> On 19.03.2020 16:26, Andrew Cooper wrote:
>>> The data layout for struct microcode_patch is extremely poor, and
>>> unnecessarily complicated. Almost all of it is opaque to core.c, with
>>> the
>>> exception of free_patch().
>>>
>>> Move the responsibility for freeing the patch into the free_patch()
>>> hook,
>>> which will allow each driver to do a better job.
>> But that wrapper structure is something common, i.e. to be
>> allocated as well as to be freed (preferably) by common code.
>> We did specifically move there during review of the most
>> recent re-work.
> The current behaviour of having it allocated by the request() hook, but
> "freed" in a mix of common code and a free() hook, cannot possibly have
> been an intended consequence from moving it.
>
> The free() hook is currently necessary, as is the vendor-specific
> allocation logic, so splitting freeing responsibility with the common
> code is wrong.
Hmm, yes, with the allocation done in vendor code, the freeing
could be, too. But the wrapper struct gets allocated last in
cpu_request_microcode() (for both Intel and AMD), and hence ought
to be relatively easy to get rid of, instead of moving around
the freeing (the common code part of the freeing would then
simply go away).
>>> I am working on removing all unnecessary allocations, including folding
>>> microcode_{intel,amd} into microcode_patch, but I'm still confident this
>>> wants to be done with microcode_patch being properly opaque to core.c
>> Oh, sure - I didn't mean to put this under question. It just seems
>> to me the the route there may better be somewhat different from this
>> and the following patch.
>
> How?
>
> We want to remove the pointer from microcode_patch, and don't want the
> current contents of microcode_{intel,amd} escaping from their current
> source files.
>
> I don't see any option but to rearrange it to be opaque.
I agree. But why do you need to first re-arrange freeing, if then you
drop the wrapper struct (which really is a union) anyway? By dropping
it right away, the split freeing will go away as a side effect.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] Revert "x86/vvmx: fix virtual interrupt injection when Ack on exit control is used"
On 20.03.2020 20:07, Roger Pau Monne wrote: > This reverts commit f96e1469ad06b61796c60193daaeb9f8a96d7458. > > The commit is wrong, as the whole point of nvmx_update_apicv is to > update the guest interrupt status field when the Ack on exit VMEXIT > control feature is enabled. > > Signed-off-by: Roger Pau Monné Before anyone gets to look at the other two patches, should this be thrown in right away? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] Xen crash after S3 suspend - Xen 4.13
On 23.03.2020 01:09, Marek Marczykowski-Górecki wrote: > But there is more: additionally, in most (all?) cases after resume I've got > soft lockup in Linux dom0 in smp_call_function_single() - see below. It > didn't happened before and the only change was Xen 4.13 -> master. Unless the Linux side manifestation rings a bell to someone, would there be any chance you could bisect this to the offending commit? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/4] x86/ucode: Fix error paths in apply_microcode()
On 20.03.2020 22:24, Andrew Cooper wrote:
> @@ -259,15 +260,14 @@ static int apply_microcode(const struct microcode_patch
> *patch)
> /* check current patch id and patch's id for match */
> if ( hw_err || (rev != hdr->patch_id) )
> {
> -printk(KERN_ERR "microcode: CPU%d update from revision "
> - "%#x to %#x failed\n", cpu, rev, hdr->patch_id);
> +printk(XENLOG_ERR
> + "microcode: CPU%u update rev %#x to %#x failed, result %#x\n",
> + cpu, old_rev, hdr->patch_id, rev);
> return -EIO;
> }
>
> -printk(KERN_WARNING "microcode: CPU%d updated from revision %#x to
> %#x\n",
> - cpu, sig->rev, hdr->patch_id);
> -
> -sig->rev = rev;
> +printk(XENLOG_WARNING "microcode: CPU%u updated from revision %#x to
> %#x\n",
> + cpu, old_rev, hdr->patch_id);
Prefer the local variable here over hdr->patch_id, just like you do
on the Intel side?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 17/17] xen: Switch parameter in get_page_from_gfn to use typesafe gfn
> -Original Message-
> From: jul...@xen.org
> Sent: 22 March 2020 16:14
> To: xen-devel@lists.xenproject.org
> Cc: jul...@xen.org; Julien Grall ; Stefano Stabellini
> ;
> Volodymyr Babchuk ; Andrew Cooper
> ; George
> Dunlap ; Ian Jackson ;
> Jan Beulich
> ; Wei Liu ; Roger Pau Monné
> ; Paul Durrant
> ; Jun Nakajima ; Kevin Tian
> ; Tim Deegan
>
> Subject: [PATCH 17/17] xen: Switch parameter in get_page_from_gfn to use
> typesafe gfn
>
> From: Julien Grall
>
> No functional change intended.
>
> Only reasonable clean-ups are done in this patch. The rest will use _gfn
> for the time being.
>
> Signed-off-by: Julien Grall
Definitely an improvement so...
Reviewed-by: Paul Durrant
But a couple of things I noticed...
[snip]
> diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
> index 5d5a746a25..3c29ff86be 100644
> --- a/xen/arch/x86/hvm/domain.c
> +++ b/xen/arch/x86/hvm/domain.c
> @@ -296,8 +296,10 @@ int arch_set_info_hvm_guest(struct vcpu *v, const
> vcpu_hvm_context_t *ctx)
> if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
> {
> /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
> -struct page_info *page = get_page_from_gfn(v->domain,
> - v->arch.hvm.guest_cr[3] >> PAGE_SHIFT,
> +struct page_info *page;
> +
> +page = get_page_from_gfn(v->domain,
> + gaddr_to_gfn(v->arch.hvm.guest_cr[3]),
Should this be cr3_to_gfn?
> NULL, P2M_ALLOC);
> if ( !page )
> {
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index a3d115b650..9f720e7aa1 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2216,7 +2216,7 @@ int hvm_set_cr0(unsigned long value, bool may_defer)
> {
> struct vcpu *v = current;
> struct domain *d = v->domain;
> -unsigned long gfn, old_value = v->arch.hvm.guest_cr[0];
> +unsigned long old_value = v->arch.hvm.guest_cr[0];
> struct page_info *page;
>
> HVM_DBG_LOG(DBG_LEVEL_VMMU, "Update CR0 value = %lx", value);
> @@ -2271,7 +2271,8 @@ int hvm_set_cr0(unsigned long value, bool may_defer)
> if ( !paging_mode_hap(d) )
> {
> /* The guest CR3 must be pointing to the guest physical. */
> -gfn = v->arch.hvm.guest_cr[3] >> PAGE_SHIFT;
> +gfn_t gfn = gaddr_to_gfn(v->arch.hvm.guest_cr[3]);
> +
Same here.
> page = get_page_from_gfn(d, gfn, NULL, P2M_ALLOC);
> if ( !page )
> {
> @@ -2363,7 +2364,7 @@ int hvm_set_cr3(unsigned long value, bool may_defer)
> {
> /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
> HVM_DBG_LOG(DBG_LEVEL_VMMU, "CR3 value = %lx", value);
> -page = get_page_from_gfn(v->domain, value >> PAGE_SHIFT,
> +page = get_page_from_gfn(v->domain, cr3_to_gfn(value),
> NULL, P2M_ALLOC);
> if ( !page )
> goto bad_cr3;
> @@ -3191,7 +3192,7 @@ enum hvm_translation_result hvm_translate_get_page(
> && hvm_mmio_internal(gfn_to_gaddr(gfn)) )
> return HVMTRANS_bad_gfn_to_mfn;
>
> -page = get_page_from_gfn(v->domain, gfn_x(gfn), &p2mt, P2M_UNSHARE);
> +page = get_page_from_gfn(v->domain, gfn, &p2mt, P2M_UNSHARE);
>
> if ( !page )
> return HVMTRANS_bad_gfn_to_mfn;
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 32d8d847f2..a9abd6d3f1 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -299,7 +299,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct
> hvm_hw_cpu *c)
> {
> if ( c->cr0 & X86_CR0_PG )
> {
> -page = get_page_from_gfn(v->domain, c->cr3 >> PAGE_SHIFT,
> +page = get_page_from_gfn(v->domain, cr3_to_gfn(c->cr3),
> NULL, P2M_ALLOC);
> if ( !page )
> {
> @@ -2230,9 +2230,9 @@ nsvm_get_nvmcb_page(struct vcpu *v, uint64_t vmcbaddr)
> return NULL;
>
> /* Need to translate L1-GPA to MPA */
> -page = get_page_from_gfn(v->domain,
> -nv->nv_vvmcxaddr >> PAGE_SHIFT,
> -&p2mt, P2M_ALLOC | P2M_UNSHARE);
> +page = get_page_from_gfn(v->domain,
> + gaddr_to_gfn(nv->nv_vvmcxaddr),
> + &p2mt, P2M_ALLOC | P2M_UNSHARE);
> if ( !page )
> return NULL;
>
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c
> b/xen/arch/x86/hvm/viridian/viridian.c
> index 977c1bc54f..3d75a0f133 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -242,16 +242,16 @@ static void dump_hypercall(const struct domain *d)
>
> static void enable_hypercall_page(struct domain *d)
> {
> -unsigned long gmfn = d->arch.hvm.viridian->hypercall_gpa.pfn;
Re: [Xen-devel] [PATCH 4/4] xen: Introduce a xmemdup_bytes() helper
On 21.03.2020 23:19, Julien Grall wrote:
> On Fri, 20 Mar 2020 at 21:26, Andrew Cooper wrote:
>> --- a/xen/include/xen/xmalloc.h
>> +++ b/xen/include/xen/xmalloc.h
>> @@ -51,6 +51,17 @@
>> #define xmalloc_bytes(_bytes) _xmalloc(_bytes, SMP_CACHE_BYTES)
>> #define xzalloc_bytes(_bytes) _xzalloc(_bytes, SMP_CACHE_BYTES)
>>
>> +/* Allocate untyped storage and copying an existing instance. */
>> +#define xmemdup_bytes(_src, _nr)\
>> +({ \
>> +unsigned long nr_ = (_nr); \
>> +void *dst_ = xmalloc_bytes(nr_);\
>
> The nr_ vs _nr is really confusing to read. Could you re-implement the
> function as a static inline?
And even if that wouldn't work out - what's the point of having
macro argument names with leading underscores? This isn't any
better standard-wise (afaict) than other uses of leading
underscores for identifiers which aren't CU-scope.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/4] x86/ucode: Cleanup - Part 2/n
On 20.03.2020 22:24, Andrew Cooper wrote: > Two minor bugfixes, and two minor cleanups with minor benefits to other code > as well. > > There is no dependency on the remaining pieces of the Part 1 series. > > Andrew Cooper (4): > x86/ucode/amd: Fix assertion in compare_patch() > x86/ucode: Fix error paths in apply_microcode() > xen: Drop raw_smp_processor_id() FAOD feel free to throw in with Wei's R-b, ideally with the small adjustment suggested for patch 2. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] xen/mm: Introduce PGC_state_uninitialised
> -Original Message- > > > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > > > index 62507ca651..5f0581c072 100644 > > > --- a/xen/arch/x86/mm.c > > > +++ b/xen/arch/x86/mm.c > > > @@ -491,7 +491,8 @@ void share_xen_page_with_guest(struct page_info > > > *page, struct domain *d, > > > > > > page_set_owner(page, d); > > > smp_wmb(); /* install valid domain ptr before updating refcnt. */ > > > -ASSERT((page->count_info & ~PGC_xen_heap) == 0); > > > +ASSERT((page->count_info & ~PGC_xen_heap) == PGC_state_inuse || > > > + (page->count_info & ~PGC_xen_heap) == > > > PGC_state_uninitialised); > > > > Could the page state perhaps be bumped to inuse in this case? It > > seems odd to leave state uninitialized yet succeed in sharing with a > > guest. > > No, that doesn't really work. > > You can't just *declare* that the page was properly initialised, > because it isn't true. There's a pathological case where the zone > hasn't been initialised at all, because *all* the pages in that zone > got handed out by the boot allocator or used for initrd etc. > > The first pages 'freed' in that zone end up being used (in > init_heap_pages) to create the zone structures. > > Likewise, it could include a page which init_heap_pages() doesn't > actually *put* into the buddy allocator, to work around the cross-zone > merge problem. It's fine to use that page and share it with a guest, > but it can't ever be freed into the buddy allocator. > Ok, so deferring the call to free_heap_pages() (and consequently init_heap_pages()) is safe to defer until the guest is torn down? (Or is this only safe if the page is being assigned to the initial domain?) Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] xen/mm: Introduce PGC_state_uninitialised
Hi David, On 20/03/2020 15:17, David Woodhouse wrote: On Fri, 2020-03-20 at 13:33 +, Paul Durrant wrote: -Original Message- From: Xen-devel On Behalf Of David Woodhouse Sent: 19 March 2020 21:22 To: xen-devel@lists.xenproject.org Cc: Stefano Stabellini ; Julien Grall ; Wei Liu ; Andrew Cooper ; Ian Jackson ; George Dunlap ; hongy...@amazon.com; Jan Beulich ; Volodymyr Babchuk ; Roger Pau Monné Subject: [Xen-devel] [PATCH 2/2] xen/mm: Introduce PGC_state_uninitialised From: David Woodhouse It is possible for pages to enter general circulation without ever being process by init_heap_pages(). For example, pages of the multiboot module containing the initramfs may be assigned via assign_pages() to dom0 as it is created. And some code including map_pages_to_xen() has checks on 'system_state' to determine whether to use the boot or the heap allocator, but it seems impossible to prove that pages allocated by the boot allocator are not subsequently freed with free_heap_pages(). This actually works fine in the majority of cases; there are only a few esoteric corner cases which init_heap_pages() handles before handing the page range off to free_heap_pages(): • Excluding MFN #0 to avoid inappropriate cross-zone merging. • Ensuring that the node information structures exist, when the first page(s) of a given node are handled. • High order allocations crossing from one node to another. To handle this case, shift PG_state_inuse from its current value of zero, to another value. Use zero, which is the initial state of the entire frame table, as PG_state_uninitialised. Fix a couple of assertions which were assuming that PG_state_inuse is zero, and make them cope with the PG_state_uninitialised case too where appopriate. Finally, make free_heap_pages() call through to init_heap_pages() when given a page range which has not been initialised. This cannot keep recursing because init_heap_pages() will set each page state to PGC_state_inuse before passing it back to free_heap_pages() for the second time. Signed-off-by: David Woodhouse --- xen/arch/x86/mm.c| 3 ++- xen/common/page_alloc.c | 44 +--- xen/include/asm-arm/mm.h | 3 ++- xen/include/asm-x86/mm.h | 3 ++- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 62507ca651..5f0581c072 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -491,7 +491,8 @@ void share_xen_page_with_guest(struct page_info *page, struct domain *d, page_set_owner(page, d); smp_wmb(); /* install valid domain ptr before updating refcnt. */ -ASSERT((page->count_info & ~PGC_xen_heap) == 0); +ASSERT((page->count_info & ~PGC_xen_heap) == PGC_state_inuse || + (page->count_info & ~PGC_xen_heap) == PGC_state_uninitialised); Could the page state perhaps be bumped to inuse in this case? It seems odd to leave state uninitialized yet succeed in sharing with a guest. No, that doesn't really work. You can't just *declare* that the page was properly initialised, because it isn't true. There's a pathological case where the zone hasn't been initialised at all, because *all* the pages in that zone got handed out by the boot allocator or used for initrd etc. The first pages 'freed' in that zone end up being used (in init_heap_pages) to create the zone structures. Likewise, it could include a page which init_heap_pages() doesn't actually *put* into the buddy allocator, to work around the cross-zone merge problem. It's fine to use that page and share it with a guest, but it can't ever be freed into the buddy allocator. For liveupdate, we will need a way to initialize a page but mark it as already inuse (i.e in the same state as they would be if allocated normally). It feels to me, this is also what we want in this case. The page would be first initialize and then we can use it normally including freeing later on. Would it make sense to introduce an helper for this purpose? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 3/5] x86_64/mm: map and unmap page tables in share_hotadd_m2p_table
From: Wei Liu
Fetch lYe by mapping and unmapping lXe instead of using the direct map,
which is now done via the new lYe_from_lXe() helpers.
Signed-off-by: Wei Liu
Signed-off-by: Hongyan Xia
---
xen/arch/x86/x86_64/mm.c | 12 ++--
xen/include/asm-x86/page.h | 18 ++
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index a440dac25e..2680173fab 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -173,14 +173,14 @@ static int share_hotadd_m2p_table(struct mem_hotadd_info
*info)
v += n << PAGE_SHIFT )
{
n = L2_PAGETABLE_ENTRIES * L1_PAGETABLE_ENTRIES;
-l3e = l4e_to_l3e(idle_pg_table[l4_table_offset(v)])[
-l3_table_offset(v)];
+l3e = l3e_from_l4e(idle_pg_table[l4_table_offset(v)],
+ l3_table_offset(v));
if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
continue;
if ( !(l3e_get_flags(l3e) & _PAGE_PSE) )
{
n = L1_PAGETABLE_ENTRIES;
-l2e = l3e_to_l2e(l3e)[l2_table_offset(v)];
+l2e = l2e_from_l3e(l3e, l2_table_offset(v));
if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) )
continue;
m2p_start_mfn = l2e_get_mfn(l2e);
@@ -201,11 +201,11 @@ static int share_hotadd_m2p_table(struct mem_hotadd_info
*info)
v != RDWR_COMPAT_MPT_VIRT_END;
v += 1 << L2_PAGETABLE_SHIFT )
{
-l3e = l4e_to_l3e(idle_pg_table[l4_table_offset(v)])[
-l3_table_offset(v)];
+l3e = l3e_from_l4e(idle_pg_table[l4_table_offset(v)],
+ l3_table_offset(v));
if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) )
continue;
-l2e = l3e_to_l2e(l3e)[l2_table_offset(v)];
+l2e = l2e_from_l3e(l3e, l2_table_offset(v));
if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) )
continue;
m2p_start_mfn = l2e_get_mfn(l2e);
diff --git a/xen/include/asm-x86/page.h b/xen/include/asm-x86/page.h
index c98d8f5ede..d4752a5925 100644
--- a/xen/include/asm-x86/page.h
+++ b/xen/include/asm-x86/page.h
@@ -196,6 +196,24 @@ static inline l4_pgentry_t l4e_from_paddr(paddr_t pa,
unsigned int flags)
#define map_l2t_from_l3e(x)(l2_pgentry_t
*)map_domain_page(l3e_get_mfn(x))
#define map_l3t_from_l4e(x)(l3_pgentry_t
*)map_domain_page(l4e_get_mfn(x))
+#define l1e_from_l2e(l2e, off) ({ \
+l1_pgentry_t *l1t = map_l1t_from_l2e(l2e); \
+l1_pgentry_t l1e = l1t[off];\
+UNMAP_DOMAIN_PAGE(l1t); \
+l1e; })
+
+#define l2e_from_l3e(l3e, off) ({ \
+l2_pgentry_t *l2t = map_l2t_from_l3e(l3e); \
+l2_pgentry_t l2e = l2t[off];\
+UNMAP_DOMAIN_PAGE(l2t); \
+l2e; })
+
+#define l3e_from_l4e(l4e, off) ({ \
+l3_pgentry_t *l3t = map_l3t_from_l4e(l4e); \
+l3_pgentry_t l3e = l3t[off];\
+UNMAP_DOMAIN_PAGE(l3t); \
+l3e; })
+
/* Given a virtual address, get an entry offset into a page table. */
#define l1_table_offset(a) \
(((a) >> L1_PAGETABLE_SHIFT) & (L1_PAGETABLE_ENTRIES - 1))
--
2.24.1.AMZN
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 0/5] use new API for Xen page tables
From: Hongyan Xia This small series is basically just rewriting functions using the new API to map and unmap PTEs. Each patch is independent. Apart from mapping and unmapping page tables, no other functional change intended. Wei Liu (5): x86/shim: map and unmap page tables in replace_va_mapping x86_64/mm: map and unmap page tables in m2p_mapped x86_64/mm: map and unmap page tables in share_hotadd_m2p_table x86_64/mm: map and unmap page tables in destroy_compat_m2p_mapping x86_64/mm: map and unmap page tables in destroy_m2p_mapping xen/arch/x86/pv/shim.c | 10 --- xen/arch/x86/x86_64/mm.c | 55 +- xen/include/asm-x86/page.h | 18 + 3 files changed, 62 insertions(+), 21 deletions(-) -- 2.24.1.AMZN ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/5] x86_64/mm: map and unmap page tables in m2p_mapped
From: Wei Liu
Signed-off-by: Wei Liu
Reviewed-by: Hongyan Xia
---
xen/arch/x86/x86_64/mm.c | 18 --
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index b7ce833ffc..a440dac25e 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -131,27 +131,33 @@ static int m2p_mapped(unsigned long spfn)
unsigned long va;
l3_pgentry_t *l3_ro_mpt;
l2_pgentry_t *l2_ro_mpt;
+int rc = M2P_NO_MAPPED;
va = RO_MPT_VIRT_START + spfn * sizeof(*machine_to_phys_mapping);
-l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(va)]);
+l3_ro_mpt = map_l3t_from_l4e(idle_pg_table[l4_table_offset(va)]);
switch ( l3e_get_flags(l3_ro_mpt[l3_table_offset(va)]) &
(_PAGE_PRESENT |_PAGE_PSE))
{
case _PAGE_PSE|_PAGE_PRESENT:
-return M2P_1G_MAPPED;
+rc = M2P_1G_MAPPED;
+goto out;
/* Check for next level */
case _PAGE_PRESENT:
break;
default:
-return M2P_NO_MAPPED;
+rc = M2P_NO_MAPPED;
+goto out;
}
-l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
+l2_ro_mpt = map_l2t_from_l3e(l3_ro_mpt[l3_table_offset(va)]);
if (l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT)
-return M2P_2M_MAPPED;
+rc = M2P_2M_MAPPED;
+UNMAP_DOMAIN_PAGE(l2_ro_mpt);
-return M2P_NO_MAPPED;
+ out:
+UNMAP_DOMAIN_PAGE(l3_ro_mpt);
+return rc;
}
static int share_hotadd_m2p_table(struct mem_hotadd_info *info)
--
2.24.1.AMZN
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 4/5] x86_64/mm: map and unmap page tables in destroy_compat_m2p_mapping
From: Wei Liu
Signed-off-by: Wei Liu
Reviewed-by: Hongyan Xia
---
xen/arch/x86/x86_64/mm.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 2680173fab..71c84ac593 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -235,11 +235,13 @@ static void destroy_compat_m2p_mapping(struct
mem_hotadd_info *info)
if ( emap > ((RDWR_COMPAT_MPT_VIRT_END - RDWR_COMPAT_MPT_VIRT_START) >> 2)
)
emap = (RDWR_COMPAT_MPT_VIRT_END - RDWR_COMPAT_MPT_VIRT_START) >> 2;
-l3_ro_mpt =
l4e_to_l3e(idle_pg_table[l4_table_offset(HIRO_COMPAT_MPT_VIRT_START)]);
+l3_ro_mpt = map_l3t_from_l4e(idle_pg_table[
+l4_table_offset(HIRO_COMPAT_MPT_VIRT_START)]);
ASSERT(l3e_get_flags(l3_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START)]) &
_PAGE_PRESENT);
-l2_ro_mpt =
l3e_to_l2e(l3_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START)]);
+l2_ro_mpt = map_l2t_from_l3e(
+l3_ro_mpt[l3_table_offset(HIRO_COMPAT_MPT_VIRT_START)]);
for ( i = smap; i < emap; )
{
@@ -261,6 +263,9 @@ static void destroy_compat_m2p_mapping(struct
mem_hotadd_info *info)
i += 1UL << (L2_PAGETABLE_SHIFT - 2);
}
+UNMAP_DOMAIN_PAGE(l2_ro_mpt);
+UNMAP_DOMAIN_PAGE(l3_ro_mpt);
+
return;
}
--
2.24.1.AMZN
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/5] x86/shim: map and unmap page tables in replace_va_mapping
From: Wei Liu
Signed-off-by: Wei Liu
Reviewed-by: Hongyan Xia
---
xen/arch/x86/pv/shim.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index ed2ece8a8a..1229d5ffb3 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -169,15 +169,19 @@ static void __init replace_va_mapping(struct domain *d,
l4_pgentry_t *l4start,
unsigned long va, mfn_t mfn)
{
l4_pgentry_t *pl4e = l4start + l4_table_offset(va);
-l3_pgentry_t *pl3e = l4e_to_l3e(*pl4e) + l3_table_offset(va);
-l2_pgentry_t *pl2e = l3e_to_l2e(*pl3e) + l2_table_offset(va);
-l1_pgentry_t *pl1e = l2e_to_l1e(*pl2e) + l1_table_offset(va);
+l3_pgentry_t *pl3e = map_l3t_from_l4e(*pl4e) + l3_table_offset(va);
+l2_pgentry_t *pl2e = map_l2t_from_l3e(*pl3e) + l2_table_offset(va);
+l1_pgentry_t *pl1e = map_l1t_from_l2e(*pl2e) + l1_table_offset(va);
struct page_info *page = mfn_to_page(l1e_get_mfn(*pl1e));
put_page_and_type(page);
*pl1e = l1e_from_mfn(mfn, (!is_pv_32bit_domain(d) ? L1_PROT
: COMPAT_L1_PROT));
+
+UNMAP_DOMAIN_PAGE(pl1e);
+UNMAP_DOMAIN_PAGE(pl2e);
+UNMAP_DOMAIN_PAGE(pl3e);
}
static void evtchn_reserve(struct domain *d, unsigned int port)
--
2.24.1.AMZN
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 5/5] x86_64/mm: map and unmap page tables in destroy_m2p_mapping
From: Wei Liu
Signed-off-by: Wei Liu
Reviewed-by: Hongyan Xia
---
xen/arch/x86/x86_64/mm.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index 71c84ac593..6a0ffe088b 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -275,7 +275,8 @@ static void destroy_m2p_mapping(struct mem_hotadd_info
*info)
unsigned long i, va, rwva;
unsigned long smap = info->spfn, emap = info->epfn;
-l3_ro_mpt = l4e_to_l3e(idle_pg_table[l4_table_offset(RO_MPT_VIRT_START)]);
+l3_ro_mpt = map_l3t_from_l4e(
+idle_pg_table[l4_table_offset(RO_MPT_VIRT_START)]);
/*
* No need to clean m2p structure existing before the hotplug
@@ -297,26 +298,33 @@ static void destroy_m2p_mapping(struct mem_hotadd_info
*info)
continue;
}
-l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
+l2_ro_mpt = map_l2t_from_l3e(l3_ro_mpt[l3_table_offset(va)]);
if (!(l2e_get_flags(l2_ro_mpt[l2_table_offset(va)]) & _PAGE_PRESENT))
{
i = ( i & ~((1UL << (L2_PAGETABLE_SHIFT - 3)) - 1)) +
(1UL << (L2_PAGETABLE_SHIFT - 3)) ;
+UNMAP_DOMAIN_PAGE(l2_ro_mpt);
continue;
}
pt_pfn = l2e_get_pfn(l2_ro_mpt[l2_table_offset(va)]);
if ( hotadd_mem_valid(pt_pfn, info) )
{
+l2_pgentry_t *l2t;
+
destroy_xen_mappings(rwva, rwva + (1UL << L2_PAGETABLE_SHIFT));
-l2_ro_mpt = l3e_to_l2e(l3_ro_mpt[l3_table_offset(va)]);
-l2e_write(&l2_ro_mpt[l2_table_offset(va)], l2e_empty());
+l2t = map_l2t_from_l3e(l3_ro_mpt[l3_table_offset(va)]);
+l2e_write(&l2t[l2_table_offset(va)], l2e_empty());
+UNMAP_DOMAIN_PAGE(l2t);
}
i = ( i & ~((1UL << (L2_PAGETABLE_SHIFT - 3)) - 1)) +
(1UL << (L2_PAGETABLE_SHIFT - 3));
+UNMAP_DOMAIN_PAGE(l2_ro_mpt);
}
+UNMAP_DOMAIN_PAGE(l3_ro_mpt);
+
destroy_compat_m2p_mapping(info);
/* Brute-Force flush all TLB */
--
2.24.1.AMZN
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [seabios test] 148853: regressions - FAIL
flight 148853 seabios real [real] http://logs.test-lab.xenproject.org/osstest/logs/148853/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 148666 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 148666 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 148666 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 148666 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail starved in 148666 version targeted for testing: seabios de88a9628426e82f1cee4b61b06e67e6787301b1 baseline version: seabios 066a9956097b54530888b88ab9aa1ea02e42af5a Last test of basis 148666 2020-03-17 13:39:45 Z5 days Failing since148690 2020-03-18 06:43:59 Z5 days7 attempts Testing same since 148794 2020-03-20 23:39:57 Z2 days3 attempts People who touched revisions under test: Gerd Hoffmann Matt DeVillier Paul Menzel jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-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-xl-qemuu-debianhvm-i386-xsm pass test-amd64-i386-xl-qemuu-debianhvm-i386-xsm pass test-amd64-amd64-qemuu-nested-amdfail test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-xl-qemuu-ws16-amd64 fail test-amd64-i386-xl-qemuu-ws16-amd64 fail test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrictpass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict pass test-amd64-amd64-qemuu-nested-intel fail test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow pass test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 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 de88a9628426e82f1cee4b61b06e67e6787301b1 Author: Paul Menzel Date: Wed Mar 4 14:51:27 2020 +0100 std/tcg: Replace zero-length array with flexible-array member GCC 10 gives the warnings below: In file included from out/ccode32flat.o.tmp.c:54: ./src/tcgbios.c: In function 'tpm20_write_EfiSpecIdEventStruct': ./src/tcgbios.c:290:30: warning: array subscript '() + 4294967295' is outside the bounds of an interior zero-length array 'struct TCG_EfiSpecIdEventAlgorithmSize[0]' [-Wzero-length-bounds] 290 | event.hdr.digestSizes[count].algorithmId = be16_to_cpu(sel->hashAlg); | ~^~~ In file included from ./src/tcgbios.c:22, from out/ccode32flat.o.tmp.c:54: ./src/std/tcg.h:527:7: note: while referencing 'digestSizes' 527 | } digestSizes[0]; | ^~~ In file included from out/ccode32flat.o.tmp.c:54: ./src/tcgbios.c:291:3
[Xen-devel] [PATCH 7/7] x86/ucode/intel: Fold structures together
Currently, we allocate an 8 byte struct microcode_patch to point at a
separately allocated struct microcode_intel. This is wasteful.
Fold struct microcode_header_intel and microcode_intel into microcode_patch to
simplify the code and remove a level of indirection.
The two semantic changes are in free_patch() and cpu_request_microcode() which
deal with the memory allocation aspects. Everything else is no functional
change.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/arch/x86/cpu/microcode/intel.c | 103 +
1 file changed, 37 insertions(+), 66 deletions(-)
diff --git a/xen/arch/x86/cpu/microcode/intel.c
b/xen/arch/x86/cpu/microcode/intel.c
index 2cccf9c26d..8f4ebbd759 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -32,17 +32,12 @@
#define pr_debug(x...) ((void)0)
-struct microcode_header_intel {
+struct microcode_patch {
unsigned int hdrver;
unsigned int rev;
-union {
-struct {
-uint16_t year;
-uint8_t day;
-uint8_t month;
-};
-unsigned int date;
-};
+uint16_t year;
+uint8_t day;
+uint8_t month;
unsigned int sig;
unsigned int cksum;
unsigned int ldrver;
@@ -57,10 +52,7 @@ struct microcode_header_intel {
unsigned int _datasize;
unsigned int _totalsize;
unsigned int reserved[3];
-};
-struct microcode_intel {
-struct microcode_header_intel hdr;
uint8_t data[];
};
@@ -76,21 +68,17 @@ struct extended_sigtable {
} sigs[];
};
-struct microcode_patch {
-struct microcode_intel *mc_intel;
-};
-
#define PPRO_UCODE_DATASIZE 2000
-#define MC_HEADER_SIZE (sizeof(struct microcode_header_intel))
+#define MC_HEADER_SIZE offsetof(struct microcode_patch, data)
-static uint32_t get_datasize(const struct microcode_header_intel *hdr)
+static uint32_t get_datasize(const struct microcode_patch *mc)
{
-return hdr->_datasize ?: PPRO_UCODE_DATASIZE;
+return mc->_datasize ?: PPRO_UCODE_DATASIZE;
}
-static uint32_t get_totalsize(const struct microcode_header_intel *hdr)
+static uint32_t get_totalsize(const struct microcode_patch *mc)
{
-return hdr->_totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
+return mc->_totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
}
/*
@@ -100,10 +88,10 @@ static uint32_t get_totalsize(const struct
microcode_header_intel *hdr)
* fields, and no extended signature table.)
*/
static const struct extended_sigtable *get_ext_sigtable(
-const struct microcode_intel *mc)
+const struct microcode_patch *mc)
{
-if ( mc->hdr._totalsize > (MC_HEADER_SIZE + mc->hdr._datasize) )
-return (void *)&mc->data[mc->hdr._datasize];
+if ( mc->_totalsize > (MC_HEADER_SIZE + mc->_datasize) )
+return (void *)&mc->data[mc->_datasize];
return NULL;
}
@@ -158,11 +146,11 @@ static int collect_cpu_info(struct cpu_signature *csig)
* header is of a known format, and together with totalsize are within the
* bounds of the container. Everything else is unchecked.
*/
-static int microcode_sanity_check(const struct microcode_intel *mc)
+static int microcode_sanity_check(const struct microcode_patch *mc)
{
const struct extended_sigtable *ext;
-unsigned int total_size = get_totalsize(&mc->hdr);
-unsigned int data_size = get_datasize(&mc->hdr);
+unsigned int total_size = get_totalsize(mc);
+unsigned int data_size = get_datasize(mc);
unsigned int i, ext_size;
uint32_t sum, *ptr;
@@ -211,7 +199,7 @@ static int microcode_sanity_check(const struct
microcode_intel *mc)
* Checksum each indiviudal extended signature as if it had been in the
* main header.
*/
-sum = mc->hdr.sig + mc->hdr.pf + mc->hdr.cksum;
+sum = mc->sig + mc->pf + mc->cksum;
for ( i = 0; i < ext->count; ++i )
if ( sum != (ext->sigs[i].sig + ext->sigs[i].pf + ext->sigs[i].cksum) )
return -EINVAL;
@@ -221,7 +209,7 @@ static int microcode_sanity_check(const struct
microcode_intel *mc)
/* Check an update against the CPU signature and current update revision */
static enum microcode_match_result microcode_update_match(
-const struct microcode_intel *mc)
+const struct microcode_patch *mc)
{
const struct extended_sigtable *ext;
unsigned int i;
@@ -230,7 +218,7 @@ static enum microcode_match_result microcode_update_match(
ASSERT(!microcode_sanity_check(mc));
/* Check the main microcode signature. */
-if ( signature_maches(cpu_sig, mc->hdr.sig, mc->hdr.pf) )
+if ( signature_maches(cpu_sig, mc->sig, mc->pf) )
goto found;
/* If there is an extended signature table, check each of them. */
@@ -242,7 +230,7 @@ static enum microcode_match_result microcode_update_match(
return MIS_UCODE;
found:
-return mc->hdr.rev > cpu_sig->rev ? NEW_UCODE : OLD_UCODE;
+
[Xen-devel] [PATCH 0/7] x86/ucode: Cleanup and fixes - Part 3/n (Intel)
This focuses on the Intel ucode driver, removing the gratuitous memory
allocations and indirection, as well as minor fixes in other areas of the
logic.
It depends on both the Part 1 and 2 series, and hopefully better demonstrates
why making struct microcode_patch opaque is a sensible move forward.
Andrew Cooper (7):
x86/ucode: Document the behaviour of the microcode_ops hooks
x86/ucode/intel: Adjust microcode_sanity_check() to not take void *
x86/ucode/intel: Remove gratuitous memory allocations from
cpu_request_microcode()
x86/ucode/intel: Reimplement get_{data,total}size() helpers
x86/ucode/intel: Clean up microcode_update_match()
x86/ucode/intel: Clean up microcode_sanity_check()
x86/ucode/intel: Fold structures together
xen/arch/x86/cpu/microcode/intel.c | 371 +++
xen/arch/x86/cpu/microcode/private.h | 46 +
xen/include/asm-x86/microcode.h | 5 +
3 files changed, 214 insertions(+), 208 deletions(-)
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/7] x86/ucode: Document the behaviour of the microcode_ops hooks
... and struct cpu_signature for good measure.
No comment is passed on the suitability of the behaviour...
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/arch/x86/cpu/microcode/private.h | 46
xen/include/asm-x86/microcode.h | 5
2 files changed, 51 insertions(+)
diff --git a/xen/arch/x86/cpu/microcode/private.h
b/xen/arch/x86/cpu/microcode/private.h
index e64168a502..a2aec53047 100644
--- a/xen/arch/x86/cpu/microcode/private.h
+++ b/xen/arch/x86/cpu/microcode/private.h
@@ -14,14 +14,60 @@ enum microcode_match_result {
struct microcode_patch; /* Opaque */
struct microcode_ops {
+/*
+ * Parse a microcode container. Format is vendor-specific.
+ *
+ * Search within the container for the patch, suitable for the current
+ * CPU, which has the highest revision. (Note: May be a patch which is
+ * older that what is running in the CPU. This is a feature, to better
+ * cope with corner cases from buggy firmware.)
+ *
+ * If one is found, allocate and return a struct microcode_patch
+ * encapsulating the appropriate microcode patch. Does not alias the
+ * original buffer.
+ *
+ * If one is not found, (nothing matches the current CPU), return NULL.
+ * Also may return ERR_PTR(-err), e.g. bad container, out of memory.
+ */
struct microcode_patch *(*cpu_request_microcode)(const void *buf,
size_t size);
+
+/* Obtain microcode-relevant details for the current CPU. */
int (*collect_cpu_info)(struct cpu_signature *csig);
+
+/*
+ * Attempt to load the provided patch into the CPU. Returns -EIO if
+ * anything didn't go as expected.
+ */
int (*apply_microcode)(const struct microcode_patch *patch);
+
+/*
+ * Optional. If provided and applicable to the specific update attempt,
+ * is run once by the initiating CPU. Returning an error will abort the
+ * load attempt.
+ */
int (*start_update)(void);
+
+/*
+ * Optional. If provided, called on every CPU which completes a microcode
+ * load. May be called in the case of some errors, and not others. May
+ * be called even if start_update() wasn't.
+ */
void (*end_update_percpu)(void);
+
+/* Free a patch previously allocated by cpu_request_microcode(). */
void (*free_patch)(struct microcode_patch *patch);
+
+/*
+ * Is the microcode patch applicable for the current CPU, and newer than
+ * the currently running patch?
+ */
bool (*match_cpu)(const struct microcode_patch *patch);
+
+/*
+ * Given two patches, are they both applicable to the current CPU, and is
+ * new a higher revision than old?
+ */
enum microcode_match_result (*compare_patch)(
const struct microcode_patch *new, const struct microcode_patch *old);
};
diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h
index 89b9aaa02d..41e85a24d2 100644
--- a/xen/include/asm-x86/microcode.h
+++ b/xen/include/asm-x86/microcode.h
@@ -7,8 +7,13 @@
#include
struct cpu_signature {
+/* CPU signature (CPUID.1.EAX). Only written on Intel. */
unsigned int sig;
+
+/* Platform Flags (only actually 1 bit). Only applicable to Intel. */
unsigned int pf;
+
+/* Microcode Revision. */
unsigned int rev;
};
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 4/7] x86/ucode/intel: Reimplement get_{data, total}size() helpers
Every caller actually passes a struct microcode_header_intel. Implement the
helpers with proper types, and leave a comment explaining the Pentium Pro/II
behaviour with empty {data,total}size fields.
No functional change.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/arch/x86/cpu/microcode/intel.c | 32
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/xen/arch/x86/cpu/microcode/intel.c
b/xen/arch/x86/cpu/microcode/intel.c
index 0cceac6255..dfe44679be 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -46,9 +46,16 @@ struct microcode_header_intel {
unsigned int sig;
unsigned int cksum;
unsigned int ldrver;
+
+/*
+ * Microcode for the Pentium Pro and II had all further fields in the
+ * header reserved, had a fixed datasize of 2000 and totalsize of 2048,
+ * and didn't use platform flags despite the availability of the MSR.
+ */
+
unsigned int pf;
-unsigned int datasize;
-unsigned int totalsize;
+unsigned int _datasize;
+unsigned int _totalsize;
unsigned int reserved[3];
};
@@ -75,20 +82,21 @@ struct microcode_patch {
struct microcode_intel *mc_intel;
};
-#define DEFAULT_UCODE_DATASIZE (2000)
+#define PPRO_UCODE_DATASIZE 2000
#define MC_HEADER_SIZE (sizeof(struct microcode_header_intel))
-#define DEFAULT_UCODE_TOTALSIZE (DEFAULT_UCODE_DATASIZE + MC_HEADER_SIZE)
#define EXT_HEADER_SIZE (sizeof(struct extended_sigtable))
#define EXT_SIGNATURE_SIZE (sizeof(struct extended_signature))
#define DWSIZE (sizeof(u32))
-#define get_totalsize(mc) \
-(((struct microcode_intel *)mc)->hdr.totalsize ? \
- ((struct microcode_intel *)mc)->hdr.totalsize : \
- DEFAULT_UCODE_TOTALSIZE)
-
-#define get_datasize(mc) \
-(((struct microcode_intel *)mc)->hdr.datasize ? \
- ((struct microcode_intel *)mc)->hdr.datasize : DEFAULT_UCODE_DATASIZE)
+
+static uint32_t get_datasize(const struct microcode_header_intel *hdr)
+{
+return hdr->_datasize ?: PPRO_UCODE_DATASIZE;
+}
+
+static uint32_t get_totalsize(const struct microcode_header_intel *hdr)
+{
+return hdr->_totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
+}
#define sigmatch(s1, s2, p1, p2) \
(((s1) == (s2)) && (((p1) & (p2)) || (((p1) == 0) && ((p2) == 0
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 5/7] x86/ucode/intel: Clean up microcode_update_match()
Implement a new get_ext_sigtable() helper to abstract the logic for
identifying whether an extended signature table exists. As part of this,
rename microcode_intel.bits to data and change its type so it can be usefully
used in combination with the datasize header field.
Also, replace the sigmatch() macro with a static inline with a more useful
API, and an explanation of why it is safe to drop one of the previous
conditionals.
No practical change in behaviour.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/arch/x86/cpu/microcode/intel.c | 75 +-
1 file changed, 49 insertions(+), 26 deletions(-)
diff --git a/xen/arch/x86/cpu/microcode/intel.c
b/xen/arch/x86/cpu/microcode/intel.c
index dfe44679be..bc3bbf139e 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -61,7 +61,7 @@ struct microcode_header_intel {
struct microcode_intel {
struct microcode_header_intel hdr;
-unsigned int bits[0];
+uint8_t data[];
};
/* microcode format is extended from prescott processors */
@@ -98,8 +98,41 @@ static uint32_t get_totalsize(const struct
microcode_header_intel *hdr)
return hdr->_totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
}
-#define sigmatch(s1, s2, p1, p2) \
-(((s1) == (s2)) && (((p1) & (p2)) || (((p1) == 0) && ((p2) == 0
+/*
+ * A piece of microcode has an extended signature table if there is space
+ * between the end of data[] and the total size. (This logic also works
+ * appropriately for Pentium Pro/II microcode, which has 0 for both size
+ * fields, and no extended signature table.)
+ */
+static const struct extended_sigtable *get_ext_sigtable(
+const struct microcode_intel *mc)
+{
+if ( mc->hdr._totalsize > (MC_HEADER_SIZE + mc->hdr._datasize) )
+return (void *)&mc->data[mc->hdr._datasize];
+
+return NULL;
+}
+
+/*
+ * A piece of microcode is applicable for a CPU if:
+ * 1) the signatures (CPUID.1.EAX - Family/Model/Stepping) match, and
+ * 2) The Platform Flags bitmap intersect.
+ *
+ * A CPU will have a single Platform Flag bit, while the microcode may be
+ * common to multiple platforms and have multiple bits set.
+ *
+ * Note: The Pentium Pro/II microcode didn't use platform flags, and should
+ * treat 0 as a match. However, Xen being 64bit means that the cpu signature
+ * won't match, allowing us to simplify the logic.
+ */
+static bool signature_maches(const struct cpu_signature *cpu_sig,
+ unsigned int ucode_sig, unsigned int ucode_pf)
+{
+if ( cpu_sig->sig != ucode_sig )
+return false;
+
+return cpu_sig->pf & ucode_pf;
+}
#define exttable_size(et) ((et)->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE)
@@ -221,36 +254,26 @@ static int microcode_sanity_check(const struct
microcode_intel *mc)
static enum microcode_match_result microcode_update_match(
const struct microcode_intel *mc)
{
-const struct microcode_header_intel *mc_header = &mc->hdr;
-const struct extended_sigtable *ext_header;
-const struct extended_signature *ext_sig;
+const struct extended_sigtable *ext;
unsigned int i;
struct cpu_signature *cpu_sig = &this_cpu(cpu_sig);
-unsigned int sig = cpu_sig->sig;
-unsigned int pf = cpu_sig->pf;
-unsigned int rev = cpu_sig->rev;
-unsigned long data_size = get_datasize(mc_header);
-const void *end = (const void *)mc_header + get_totalsize(mc_header);
ASSERT(!microcode_sanity_check(mc));
-if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
-return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
-ext_header = (const void *)(mc_header + 1) + data_size;
-ext_sig = (const void *)(ext_header + 1);
+/* Check the main microcode signature. */
+if ( signature_maches(cpu_sig, mc->hdr.sig, mc->hdr.pf) )
+goto found;
-/*
- * Make sure there is enough space to hold an extended header and enough
- * array elements.
- */
-if ( end <= (const void *)ext_sig )
-return MIS_UCODE;
-
-for ( i = 0; i < ext_header->count; i++ )
-if ( sigmatch(sig, ext_sig[i].sig, pf, ext_sig[i].pf) )
-return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
+/* If there is an extended signature table, check each of them. */
+if ( (ext = get_ext_sigtable(mc)) != NULL )
+for ( i = 0; i < ext->count; ++i )
+if ( signature_maches(cpu_sig, ext->sigs[i].sig, ext->sigs[i].pf) )
+goto found;
return MIS_UCODE;
+
+ found:
+return mc->hdr.rev > cpu_sig->rev ? NEW_UCODE : OLD_UCODE;
}
static bool match_cpu(const struct microcode_patch *patch)
@@ -303,7 +326,7 @@ static int apply_microcode(const struct microcode_patch
*patch)
BUG_ON(local_irq_is_enabled());
/* write microcode via MSR 0x79 */
-wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->bits);
+wrmsrl(MSR_IA32_UCODE_WRITE, (u
[Xen-devel] [PATCH 3/7] x86/ucode/intel: Remove gratuitous memory allocations from cpu_request_microcode()
cpu_request_microcode() needs to scan its container and duplicate one blob,
but the get_next_ucode_from_buffer() helper duplicates every blob in turn.
Furthermore, the length checking is only safe from overflow in 64bit builds.
Delete get_next_ucode_from_buffer() and alter the purpose of the saved
variable to simply point somewhere in buf until we're ready to return.
This is only a modest reduction in absolute code size (-144), but avoids
making memory allocations for every blob in the container.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/arch/x86/cpu/microcode/intel.c | 69 ++
1 file changed, 26 insertions(+), 43 deletions(-)
diff --git a/xen/arch/x86/cpu/microcode/intel.c
b/xen/arch/x86/cpu/microcode/intel.c
index f0beefe1bb..0cceac6255 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -321,75 +321,58 @@ static int apply_microcode(const struct microcode_patch
*patch)
return 0;
}
-static long get_next_ucode_from_buffer(struct microcode_intel **mc,
- const uint8_t *buf, unsigned long size,
- unsigned long offset)
-{
-struct microcode_header_intel *mc_header;
-unsigned long total_size;
-
-/* No more data */
-if ( offset >= size )
-return 0;
-mc_header = (struct microcode_header_intel *)(buf + offset);
-total_size = get_totalsize(mc_header);
-
-if ( (offset + total_size) > size )
-{
-printk(KERN_ERR "microcode: error! Bad data in microcode data file\n");
-return -EINVAL;
-}
-
-*mc = xmemdup_bytes(mc_header, total_size);
-if ( *mc == NULL )
-return -ENOMEM;
-
-return offset + total_size;
-}
-
static struct microcode_patch *cpu_request_microcode(const void *buf,
size_t size)
{
-long offset = 0;
int error = 0;
-struct microcode_intel *mc, *saved = NULL;
+const struct microcode_intel *saved = NULL;
struct microcode_patch *patch = NULL;
-while ( (offset = get_next_ucode_from_buffer(&mc, buf, size, offset)) > 0 )
+while ( size )
{
-error = microcode_sanity_check(mc);
-if ( error )
+const struct microcode_intel *mc;
+unsigned int blob_size;
+
+if ( size < MC_HEADER_SIZE || /* Insufficient space for header?
*/
+ (mc = buf)->hdr.hdrver != 1 || /* Unrecognised header version?
*/
+ mc->hdr.ldrver != 1 || /* Unrecognised loader version?
*/
+ size < (blob_size =/* Insufficient space for patch?
*/
+ get_totalsize(&mc->hdr)) )
{
-xfree(mc);
+error = -EINVAL;
break;
}
+error = microcode_sanity_check(mc);
+if ( error )
+break;
+
/*
* If the new update covers current CPU, compare updates and store the
* one with higher revision.
*/
if ( (microcode_update_match(mc) != MIS_UCODE) &&
(!saved || (mc->hdr.rev > saved->hdr.rev)) )
-{
-xfree(saved);
saved = mc;
-}
-else
-xfree(mc);
+
+buf += blob_size;
+size -= blob_size;
}
-if ( offset < 0 )
-error = offset;
if ( saved )
{
patch = xmalloc(struct microcode_patch);
if ( patch )
-patch->mc_intel = saved;
-else
{
-xfree(saved);
-error = -ENOMEM;
+patch->mc_intel = xmemdup_bytes(saved, get_totalsize(&saved->hdr));
+if ( !patch->mc_intel )
+{
+XFREE(patch);
+error = -ENOMEM;
+}
}
+else
+error = -ENOMEM;
}
if ( error && !patch )
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/7] x86/ucode/intel: Adjust microcode_sanity_check() to not take void *
microcode_sanity_check()'s callers actually call it with a mixture of
microcode_intel and microcode_header_intel pointers, which is fragile.
Rework it to take struct microcode_intel *, which in turn requires
microcode_update_match()'s type to be altered.
No functional change - compiled binary is identical.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/arch/x86/cpu/microcode/intel.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/cpu/microcode/intel.c
b/xen/arch/x86/cpu/microcode/intel.c
index f26511da98..f0beefe1bb 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -119,9 +119,9 @@ static int collect_cpu_info(struct cpu_signature *csig)
return 0;
}
-static int microcode_sanity_check(const void *mc)
+static int microcode_sanity_check(const struct microcode_intel *mc)
{
-const struct microcode_header_intel *mc_header = mc;
+const struct microcode_header_intel *mc_header = &mc->hdr;
const struct extended_sigtable *ext_header = NULL;
const struct extended_signature *ext_sig;
unsigned long total_size, data_size, ext_table_size;
@@ -153,7 +153,7 @@ static int microcode_sanity_check(const void *mc)
"Small exttable size in microcode data file\n");
return -EINVAL;
}
-ext_header = mc + MC_HEADER_SIZE + data_size;
+ext_header = (void *)mc + MC_HEADER_SIZE + data_size;
if ( ext_table_size != exttable_size(ext_header) )
{
printk(KERN_ERR "microcode: error! "
@@ -211,8 +211,9 @@ static int microcode_sanity_check(const void *mc)
/* Check an update against the CPU signature and current update revision */
static enum microcode_match_result microcode_update_match(
-const struct microcode_header_intel *mc_header)
+const struct microcode_intel *mc)
{
+const struct microcode_header_intel *mc_header = &mc->hdr;
const struct extended_sigtable *ext_header;
const struct extended_signature *ext_sig;
unsigned int i;
@@ -223,7 +224,7 @@ static enum microcode_match_result microcode_update_match(
unsigned long data_size = get_datasize(mc_header);
const void *end = (const void *)mc_header + get_totalsize(mc_header);
-ASSERT(!microcode_sanity_check(mc_header));
+ASSERT(!microcode_sanity_check(mc));
if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
@@ -249,7 +250,7 @@ static bool match_cpu(const struct microcode_patch *patch)
if ( !patch )
return false;
-return microcode_update_match(&patch->mc_intel->hdr) == NEW_UCODE;
+return microcode_update_match(patch->mc_intel) == NEW_UCODE;
}
static void free_patch(struct microcode_patch *patch)
@@ -268,8 +269,8 @@ static enum microcode_match_result compare_patch(
* Both patches to compare are supposed to be applicable to local CPU.
* Just compare the revision number.
*/
-ASSERT(microcode_update_match(&old->mc_intel->hdr) != MIS_UCODE);
-ASSERT(microcode_update_match(&new->mc_intel->hdr) != MIS_UCODE);
+ASSERT(microcode_update_match(old->mc_intel) != MIS_UCODE);
+ASSERT(microcode_update_match(new->mc_intel) != MIS_UCODE);
return (new->mc_intel->hdr.rev > old->mc_intel->hdr.rev) ? NEW_UCODE
: OLD_UCODE;
@@ -367,7 +368,7 @@ static struct microcode_patch *cpu_request_microcode(const
void *buf,
* If the new update covers current CPU, compare updates and store the
* one with higher revision.
*/
-if ( (microcode_update_match(&mc->hdr) != MIS_UCODE) &&
+if ( (microcode_update_match(mc) != MIS_UCODE) &&
(!saved || (mc->hdr.rev > saved->hdr.rev)) )
{
xfree(saved);
--
2.11.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 6/7] x86/ucode/intel: Clean up microcode_sanity_check()
Rewrite the size checks in a way which which doesn't depend on Xen being
compiled as 64bit.
Introduce a check missing from the old code, that total_size is a multiple of
1024 bytes, and drop unnecessarily defines/macros/structures.
No practical change in behaviour.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
CC: Wei Liu
CC: Roger Pau Monné
---
xen/arch/x86/cpu/microcode/intel.c | 147 +++--
1 file changed, 58 insertions(+), 89 deletions(-)
diff --git a/xen/arch/x86/cpu/microcode/intel.c
b/xen/arch/x86/cpu/microcode/intel.c
index bc3bbf139e..2cccf9c26d 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -65,17 +65,15 @@ struct microcode_intel {
};
/* microcode format is extended from prescott processors */
-struct extended_signature {
-unsigned int sig;
-unsigned int pf;
-unsigned int cksum;
-};
-
struct extended_sigtable {
unsigned int count;
unsigned int cksum;
unsigned int reserved[3];
-struct extended_signature sigs[0];
+struct {
+unsigned int sig;
+unsigned int pf;
+unsigned int cksum;
+} sigs[];
};
struct microcode_patch {
@@ -84,9 +82,6 @@ struct microcode_patch {
#define PPRO_UCODE_DATASIZE 2000
#define MC_HEADER_SIZE (sizeof(struct microcode_header_intel))
-#define EXT_HEADER_SIZE (sizeof(struct extended_sigtable))
-#define EXT_SIGNATURE_SIZE (sizeof(struct extended_signature))
-#define DWSIZE (sizeof(u32))
static uint32_t get_datasize(const struct microcode_header_intel *hdr)
{
@@ -134,8 +129,6 @@ static bool signature_maches(const struct cpu_signature
*cpu_sig,
return cpu_sig->pf & ucode_pf;
}
-#define exttable_size(et) ((et)->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE)
-
static int collect_cpu_info(struct cpu_signature *csig)
{
uint64_t msr_content;
@@ -160,93 +153,69 @@ static int collect_cpu_info(struct cpu_signature *csig)
return 0;
}
+/*
+ * Sanity check a blob which is expected to be a microcode patch. The 48 byte
+ * header is of a known format, and together with totalsize are within the
+ * bounds of the container. Everything else is unchecked.
+ */
static int microcode_sanity_check(const struct microcode_intel *mc)
{
-const struct microcode_header_intel *mc_header = &mc->hdr;
-const struct extended_sigtable *ext_header = NULL;
-const struct extended_signature *ext_sig;
-unsigned long total_size, data_size, ext_table_size;
-unsigned int ext_sigcount = 0, i;
-uint32_t sum, orig_sum;
-
-total_size = get_totalsize(mc_header);
-data_size = get_datasize(mc_header);
-if ( (data_size + MC_HEADER_SIZE) > total_size )
-{
-printk(KERN_ERR "microcode: error! "
- "Bad data size in microcode data file\n");
+const struct extended_sigtable *ext;
+unsigned int total_size = get_totalsize(&mc->hdr);
+unsigned int data_size = get_datasize(&mc->hdr);
+unsigned int i, ext_size;
+uint32_t sum, *ptr;
+
+/*
+ * Total size must be a multiple of 1024 bytes. Data size and the header
+ * must fit within it.
+ */
+if ( (total_size & 1023) ||
+ data_size > (total_size - MC_HEADER_SIZE) )
return -EINVAL;
-}
-if ( (mc_header->ldrver != 1) || (mc_header->hdrver != 1) )
-{
-printk(KERN_ERR "microcode: error! "
- "Unknown microcode update format\n");
+/* Checksum the main header and data. */
+for ( sum = 0, ptr = (uint32_t *)mc;
+ ptr < (uint32_t *)&mc->data[data_size]; ++ptr )
+sum += *ptr;
+
+if ( sum != 0 )
return -EINVAL;
-}
-ext_table_size = total_size - (MC_HEADER_SIZE + data_size);
-if ( ext_table_size )
-{
-if ( (ext_table_size < EXT_HEADER_SIZE) ||
- ((ext_table_size - EXT_HEADER_SIZE) % EXT_SIGNATURE_SIZE) )
-{
-printk(KERN_ERR "microcode: error! "
- "Small exttable size in microcode data file\n");
-return -EINVAL;
-}
-ext_header = (void *)mc + MC_HEADER_SIZE + data_size;
-if ( ext_table_size != exttable_size(ext_header) )
-{
-printk(KERN_ERR "microcode: error! "
- "Bad exttable size in microcode data file\n");
-return -EFAULT;
-}
-ext_sigcount = ext_header->count;
-}
-/* check extended table checksum */
-if ( ext_table_size )
-{
-uint32_t ext_table_sum = 0;
-uint32_t *ext_tablep = (uint32_t *)ext_header;
+/* Look to see if there is an extended signature table. */
+ext_size = total_size - data_size - MC_HEADER_SIZE;
-i = ext_table_size / DWSIZE;
-while ( i-- )
-ext_table_sum += ext_tablep[i];
-if ( ext_table_sum )
-{
-printk(KERN_WARNING "microcode: aborting, "
- "bad extended signature table checks
Re: [Xen-devel] [PATCH 17/17] xen: Switch parameter in get_page_from_gfn to use typesafe gfn
Hi Paul,
On 23/03/2020 08:37, Paul Durrant wrote:
-Original Message-
From: jul...@xen.org
Sent: 22 March 2020 16:14
To: xen-devel@lists.xenproject.org
Cc: jul...@xen.org; Julien Grall ; Stefano Stabellini
;
Volodymyr Babchuk ; Andrew Cooper
; George
Dunlap ; Ian Jackson ; Jan
Beulich
; Wei Liu ; Roger Pau Monné
; Paul Durrant
; Jun Nakajima ; Kevin Tian
; Tim Deegan
Subject: [PATCH 17/17] xen: Switch parameter in get_page_from_gfn to use
typesafe gfn
From: Julien Grall
No functional change intended.
Only reasonable clean-ups are done in this patch. The rest will use _gfn
for the time being.
Signed-off-by: Julien Grall
Definitely an improvement so...
Reviewed-by: Paul Durrant
But a couple of things I noticed...
[snip]
diff --git a/xen/arch/x86/hvm/domain.c b/xen/arch/x86/hvm/domain.c
index 5d5a746a25..3c29ff86be 100644
--- a/xen/arch/x86/hvm/domain.c
+++ b/xen/arch/x86/hvm/domain.c
@@ -296,8 +296,10 @@ int arch_set_info_hvm_guest(struct vcpu *v, const
vcpu_hvm_context_t *ctx)
if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
{
/* Shadow-mode CR3 change. Check PDBR and update refcounts. */
-struct page_info *page = get_page_from_gfn(v->domain,
- v->arch.hvm.guest_cr[3] >> PAGE_SHIFT,
+struct page_info *page;
+
+page = get_page_from_gfn(v->domain,
+ gaddr_to_gfn(v->arch.hvm.guest_cr[3]),
Should this be cr3_to_gfn?
Definitely yes. I thought I spotted all the use when introducing the new
helper but it looks like not. I will update the patch in the new version
to use cr3_to_gfn() everywhere you suggested.
Thanks.
Cheers,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-5.4 test] 148851: regressions - trouble: fail/pass/starved
flight 148851 linux-5.4 real [real] http://logs.test-lab.xenproject.org/osstest/logs/148851/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 146121 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-rtds 16 guest-localmigrate fail pass in 148814 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail pass in 148814 Regressions which are regarded as allowable (not blocking): test-amd64-amd64-xl-rtds 18 guest-localmigrate/x10 fail in 148814 REGR. vs. 146121 Tests which did not succeed, but are not blocking: test-amd64-amd64-dom0pvh-xl-intel 15 guest-saverestore fail baseline untested 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-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-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-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail 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-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-libvirt-xsm 13 migrate-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-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail 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-i386-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stop fail 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-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 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 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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail 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-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-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-dom0pvh-xl-amd 2 hosts-allocate starved n/a version targeted for testing: lin
Re: [Xen-devel] [PATCH v6 09/12] xen: add runtime parameter access support to hypfs
Hi Juergen & Jan, On 26/02/2020 12:47, Juergen Gross wrote: diff --git a/docs/misc/hypfs-paths.pandoc b/docs/misc/hypfs-paths.pandoc index 1faebcccbc..b4a5b6086e 100644 --- a/docs/misc/hypfs-paths.pandoc +++ b/docs/misc/hypfs-paths.pandoc @@ -152,3 +152,12 @@ The major version of Xen. /buildinfo/version/minor = INTEGER The minor version of Xen. + + /params/ + +A directory of runtime parameters. + + /params/* + +The individual parameters. The description of the different parameters can be +found in `docs/misc/xen-command-line.pandoc`. diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index a497f6a48d..0061a8dfea 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -89,6 +89,11 @@ SECTIONS __start_schedulers_array = .; *(.data.schedulers) __end_schedulers_array = .; + + . = ALIGN(8); Apologies for the late answer. I noticed that Jan asked the following question on v5: "Do you really need 8-byte alignment even on 32-bit Arm?" We forbid unaligned access on 32-bit Arm (and unaligned access should be avoided on 64-bit), so if the structure contains a 64-bit type, then we definitely need the data to be 8-byte aligned. What is the expected alignment of the structure? Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] xen/mm: Introduce PGC_state_uninitialised
On Mon, 2020-03-23 at 08:49 +, Paul Durrant wrote: > Ok, so deferring the call to free_heap_pages() (and consequently > init_heap_pages()) is safe to defer until the guest is torn down? (Or > is this only safe if the page is being assigned to the initial > domain?) It's intended to be safe in all cases, including pages which are allocated from the boot allocator to be used as page tables (cf. the early-vmap patches), etc. We kind of have to assume that it's safe to use the page for whatever purpose it was allocated for, for the lifetime of that usage. If *that* isn't true, we have bigger problems. The PGC_state_uninitialised thing is only about recycling it into the heap *later*, once the lifetime of that initial usage has ended. smime.p7s Description: S/MIME cryptographic signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 1/2] docs/designs: Add a design document for non-cooperative live migration
Hi, On 18/03/2020 11:11, Paul Durrant wrote: From: Paul Durrant It has become apparent to some large cloud providers that the current model of cooperative migration of guests under Xen is not usable as it relies on software running inside the guest, which is likely beyond the provider's control. This patch introduces a proposal for non-cooperative live migration, designed not to rely on any guest-side software. Signed-off-by: Paul Durrant --- Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Wei Liu My comments on v6 [1] don't seem to be addressed or explained why they were not addressed. Can you have a look? Cheers, [1] <17eb8b5e-1419-3a7b-f796-d014f937e...@xen.org> -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/2] xen/mm: Introduce PGC_state_uninitialised
On Mon, 2020-03-23 at 09:34 +, Julien Grall wrote: > For liveupdate, we will need a way to initialize a page but mark it as > already inuse (i.e in the same state as they would be if allocated > normally). I am unconvinced of the veracity of this claim. We don't want to turn specific details of the current Xen buddy allocator part into of the implicit ABI of live update. That goes for the power-of-two zone boundaries, amongst other things. What if Xen receives LU state in which *all* pages in a given zone are marked as already in use? That's one of the cases in which we *really* want to pass through init_heap_pages() instead of just free_heap_pages(), in order to allocate the zone data structures for the first pages that get freed into that zone. What if Xen starts to exclude more pages, like the exclusion at zero? What if new Xen wants to exclude an additional page due to a hardware erratum? It can't take it away from existing domains (especially if there are assigned PCI devices) but it could be part of the vetting in init_heap_pages(), for example. My intent for PGC_state_uninitialised was to mark pages that haven't been through init_heap_pages(), whatever init_heap_pages() does in the current version of Xen. The pages which are "already in use" because they're inherited through LU state should be in PGC_state_uninitialised, shouldn't they? Perhaps if there's a need for a helper, it could be a companion function to init_heap_pages() which would return a boolean saying, "nah, I didn't want to do anything to this page anyway", which could short-circuit it into the PGC_state_inuse state. But I'm not sure I see the need for such an optimisation. smime.p7s Description: S/MIME cryptographic signature ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 04/51] drm: Set final_kfree in drm_dev_alloc
On Sat, Mar 07, 2020 at 09:06:08AM +0100, Sam Ravnborg wrote: > On Mon, Mar 02, 2020 at 11:25:44PM +0100, Daniel Vetter wrote: > > I also did a full review of all callers, and only the xen driver > > forgot to call drm_dev_put in the failure path. Fix that up too. > > So ~40 callers - phew.. > > > > > v2: I noticed that xen has a drm_driver.release hook, and uses > > drm_dev_alloc(). We need to remove the kfree from > > xen_drm_drv_release(). > > > > bochs also has a release hook, but leaked the drm_device ever since > > > > commit 0a6659bdc5e8221da99eebb176fd9591435e38de > > Author: Gerd Hoffmann > > Date: Tue Dec 17 18:04:46 2013 +0100 > > > > drm/bochs: new driver > > > > This patch here fixes that leak. > > > > Same for virtio, started leaking with > > > > commit b1df3a2b24a917f8853d43fe9683c0e360d2c33a > > Author: Gerd Hoffmann > > Date: Tue Feb 11 14:58:04 2020 +0100 > > > > drm/virtio: add drm_driver.release callback. > > > > Cc: Gerd Hoffmann > > Cc: Oleksandr Andrushchenko > > Cc: xen-devel@lists.xenproject.org > > The above will be picked up by tools as regular Cc: lines. > But I guess it is fine. That was the idea, I've deleted the spurious blank line to make this less confusing. -Daniel > > > > > Reviewed-by: Oleksandr Andrushchenko > > Signed-off-by: Daniel Vetter > > Cc: Maarten Lankhorst > > Cc: Maxime Ripard > > Cc: Thomas Zimmermann > > Cc: David Airlie > > Cc: Daniel Vetter > > Cc: Oleksandr Andrushchenko > > Cc: xen-devel@lists.xenproject.org > > For the drivers I looked at everything looked fine. > > Acked-by: Sam Ravnborg > > > --- > > drivers/gpu/drm/drm_drv.c | 3 +++ > > drivers/gpu/drm/xen/xen_drm_front.c | 2 +- > > 2 files changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > index 153050fc926c..7b84ee8a5eb5 100644 > > --- a/drivers/gpu/drm/drm_drv.c > > +++ b/drivers/gpu/drm/drm_drv.c > > @@ -39,6 +39,7 @@ > > #include > > #include > > #include > > +#include > > #include > > #include > > > > @@ -819,6 +820,8 @@ struct drm_device *drm_dev_alloc(struct drm_driver > > *driver, > > return ERR_PTR(ret); > > } > > > > + drmm_add_final_kfree(dev, dev); > > + > > return dev; > > } > > EXPORT_SYMBOL(drm_dev_alloc); > > diff --git a/drivers/gpu/drm/xen/xen_drm_front.c > > b/drivers/gpu/drm/xen/xen_drm_front.c > > index 4be49c1aef51..d22b5da38935 100644 > > --- a/drivers/gpu/drm/xen/xen_drm_front.c > > +++ b/drivers/gpu/drm/xen/xen_drm_front.c > > @@ -461,7 +461,6 @@ static void xen_drm_drv_release(struct drm_device *dev) > > drm_mode_config_cleanup(dev); > > > > drm_dev_fini(dev); > > - kfree(dev); > > > > if (front_info->cfg.be_alloc) > > xenbus_switch_state(front_info->xb_dev, > > @@ -561,6 +560,7 @@ static int xen_drm_drv_init(struct xen_drm_front_info > > *front_info) > > fail_modeset: > > drm_kms_helper_poll_fini(drm_dev); > > drm_mode_config_cleanup(drm_dev); > > + drm_dev_put(drm_dev); > > fail: > > kfree(drm_info); > > return ret; > > -- > > 2.24.1 > > > > ___ > > dri-devel mailing list > > dri-de...@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 2/2] docs/designs: Add a design document for migration of xenstore data
Hi Paul, On 18/03/2020 11:11, Paul Durrant wrote: From: Paul Durrant This patch details proposes extra migration data and xenstore protocol extensions to support non-cooperative live migration of guests. NOTE: doc/misc/xenstore.txt is also amened to replace the term NIT: s/amened/amended/ for the INTRODUCE operation with the , since this is what it actually is. Signed-off-by: Paul Durrant --- Cc: Andrew Cooper Cc: George Dunlap Cc: Ian Jackson Cc: Jan Beulich Cc: Julien Grall Cc: Konrad Rzeszutek Wilk Cc: Stefano Stabellini Cc: Wei Liu v7: - Addressed further comments from Julien - Switched migration records to defined structures instead of tuples v6: - Addressed comments from Julien v5: - Add QUIESCE - Make semantics of in GET_DOMAIN_WATCHES more clear v4: - Drop the restrictions on special paths v3: - New in v3 --- docs/designs/xenstore-migration.md | 256 + docs/misc/xenstore.txt | 6 +- 2 files changed, 259 insertions(+), 3 deletions(-) create mode 100644 docs/designs/xenstore-migration.md diff --git a/docs/designs/xenstore-migration.md b/docs/designs/xenstore-migration.md new file mode 100644 index 00..e7e6593002 --- /dev/null +++ b/docs/designs/xenstore-migration.md @@ -0,0 +1,256 @@ +# Xenstore Migration + +## Background + +The design for *Non-Cooperative Migration of Guests*[1] explains that extra +save records are required in the migrations stream to allow a guest running +PV drivers to be migrated without its co-operation. Moreover the save +records must include details of registered xenstore watches as well as +content; information that cannot currently be recovered from `xenstored`, +and hence some extension to the xenstore protocol[2] will also be required. + +The *libxenlight Domain Image Format* specification[3] already defines a +record type `EMULATOR_XENSTORE_DATA` but this is not suitable for +transferring xenstore data pertaining to the domain directly as it is +specified such that keys are relative to the path +`/local/domain/$dm_domid/device-model/$domid`. Thus it is necessary to +define at least one new save record type. + +## Proposal + +### New Save Record + +A new mandatory record type should be defined within the libxenlight Domain +Image Format: + +`0x0007: DOMAIN_XENSTORE_DATA` + +An arbitrary number of these records may be present in the migration +stream and may appear in any order. The format of each record should be as +follows: + + +``` +0 1 2 3 4 5 6 7octet ++---+---+---+---+---+---+---+---+ +| type | record specific data | ++---+ | +... ++---+ +``` + +where type is one of the following values + + +| Field | Description | +||--| +| `type` | 0x: invalid | +|| 0x0001: NODE_DATA| +|| 0x0002: WATCH_DATA | +|| 0x0003: TRANSACTION_DATA | +|| 0x0004 - 0x: reserved for future use | + + +and data is one of the record data formats described in the following +sections. + + +NOTE: The record data does not contain an overall length because the +libxenlight record header specifies the length. + + +**NODE_DATA** + + +Each NODE_DATA record specifies a single node in xenstore and is formatted +as follows: + + +``` +0 1 2 3 octet ++---+---+---+---+ +| 1 | 0 | 0 | 0 | The stream may either be big-endian or little-endian. This will be specified in the header of the stream. But here, this suggests that the type will always be described in little-endian. I think it would be best to only describe the "record specific data". ++---+ +| path length | ++---+ +| path data | +... +| pad (0 to 3 octets) | ++---+ +| perm count (N)| ++---+ +| perm0 | ++---+ +... ++---+ +| permN | ++---+ +| value length | ++---+ +| value data| +... +| pad (0 to 3 octets) | ++---+ +``` + +where perm0..N are formatted as follows: + + +``` +0 1 2 3 octet ++---+---+---+---+ +| perm | pad | domid | ++---+ +``` + + +path length and value length are specified in octets (excludin
Re: [Xen-devel] [PATCH v7 1/2] docs/designs: Add a design document for non-cooperative live migration
> -Original Message- > From: Julien Grall > Sent: 23 March 2020 10:47 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Paul Durrant ; Andrew Cooper > ; George Dunlap > ; Ian Jackson ; Jan > Beulich > ; Konrad Rzeszutek Wilk ; Stefano > Stabellini > ; Wei Liu > Subject: Re: [PATCH v7 1/2] docs/designs: Add a design document for > non-cooperative live migration > > Hi, > > On 18/03/2020 11:11, Paul Durrant wrote: > > From: Paul Durrant > > > > It has become apparent to some large cloud providers that the current > > model of cooperative migration of guests under Xen is not usable as it > > relies on software running inside the guest, which is likely beyond the > > provider's control. > > This patch introduces a proposal for non-cooperative live migration, > > designed not to rely on any guest-side software. > > > > Signed-off-by: Paul Durrant > > --- > > Cc: Andrew Cooper > > Cc: George Dunlap > > Cc: Ian Jackson > > Cc: Jan Beulich > > Cc: Julien Grall > > Cc: Konrad Rzeszutek Wilk > > Cc: Stefano Stabellini > > Cc: Wei Liu > > My comments on v6 [1] don't seem to be addressed or explained why they > were not addressed. Can you have a look? > > Cheers, > > [1] <17eb8b5e-1419-3a7b-f796-d014f937e...@xen.org> Weird. I thought I'd fixed those. I think I must have lost some changes along the way. I'll fix and resend. Paul > > -- > Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v7 2/2] docs/designs: Add a design document for migration of xenstore data
> -Original Message- > From: Julien Grall > Sent: 23 March 2020 11:34 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Paul Durrant ; Andrew Cooper > ; George Dunlap > ; Ian Jackson ; Jan > Beulich > ; Konrad Rzeszutek Wilk ; Stefano > Stabellini > ; Wei Liu > Subject: Re: [PATCH v7 2/2] docs/designs: Add a design document for migration > of xenstore data > > Hi Paul, > > On 18/03/2020 11:11, Paul Durrant wrote: > > From: Paul Durrant > > > > This patch details proposes extra migration data and xenstore protocol > > extensions to support non-cooperative live migration of guests. > > > > NOTE: doc/misc/xenstore.txt is also amened to replace the term > > NIT: s/amened/amended/ > Yes. > >for the INTRODUCE operation with the , since this is what > >it actually is. > > > > Signed-off-by: Paul Durrant > > --- > > Cc: Andrew Cooper > > Cc: George Dunlap > > Cc: Ian Jackson > > Cc: Jan Beulich > > Cc: Julien Grall > > Cc: Konrad Rzeszutek Wilk > > Cc: Stefano Stabellini > > Cc: Wei Liu > > > > v7: > > - Addressed further comments from Julien > > - Switched migration records to defined structures instead of tuples > > > > v6: > > - Addressed comments from Julien > > > > v5: > > - Add QUIESCE > > - Make semantics of in GET_DOMAIN_WATCHES more clear > > > > v4: > > - Drop the restrictions on special paths > > > > v3: > > - New in v3 > > --- > > docs/designs/xenstore-migration.md | 256 + > > docs/misc/xenstore.txt | 6 +- > > 2 files changed, 259 insertions(+), 3 deletions(-) > > create mode 100644 docs/designs/xenstore-migration.md > > > > diff --git a/docs/designs/xenstore-migration.md > > b/docs/designs/xenstore-migration.md > > new file mode 100644 > > index 00..e7e6593002 > > --- /dev/null > > +++ b/docs/designs/xenstore-migration.md > > @@ -0,0 +1,256 @@ > > +# Xenstore Migration > > + > > +## Background > > + > > +The design for *Non-Cooperative Migration of Guests*[1] explains that extra > > +save records are required in the migrations stream to allow a guest running > > +PV drivers to be migrated without its co-operation. Moreover the save > > +records must include details of registered xenstore watches as well as > > +content; information that cannot currently be recovered from `xenstored`, > > +and hence some extension to the xenstore protocol[2] will also be required. > > + > > +The *libxenlight Domain Image Format* specification[3] already defines a > > +record type `EMULATOR_XENSTORE_DATA` but this is not suitable for > > +transferring xenstore data pertaining to the domain directly as it is > > +specified such that keys are relative to the path > > +`/local/domain/$dm_domid/device-model/$domid`. Thus it is necessary to > > +define at least one new save record type. > > + > > +## Proposal > > + > > +### New Save Record > > + > > +A new mandatory record type should be defined within the libxenlight Domain > > +Image Format: > > + > > +`0x0007: DOMAIN_XENSTORE_DATA` > > + > > +An arbitrary number of these records may be present in the migration > > +stream and may appear in any order. The format of each record should be as > > +follows: > > + > > + > > +``` > > +0 1 2 3 4 5 6 7octet > > ++---+---+---+---+---+---+---+---+ > > +| type | record specific data | > > ++---+ | > > +... > > ++---+ > > +``` > > + > > +where type is one of the following values > > + > > + > > +| Field | Description | > > +||--| > > +| `type` | 0x: invalid | > > +|| 0x0001: NODE_DATA| > > +|| 0x0002: WATCH_DATA | > > +|| 0x0003: TRANSACTION_DATA | > > +|| 0x0004 - 0x: reserved for future use | > > + > > + > > +and data is one of the record data formats described in the following > > +sections. > > + > > + > > +NOTE: The record data does not contain an overall length because the > > +libxenlight record header specifies the length. > > + > > + > > +**NODE_DATA** > > + > > + > > +Each NODE_DATA record specifies a single node in xenstore and is formatted > > +as follows: > > + > > + > > +``` > > +0 1 2 3 octet > > ++---+---+---+---+ > > +| 1 | 0 | 0 | 0 | > > > The stream may either be big-endian or little-endian. This will be > specified in the header of the stream. > > But here, this suggests that the type will always be described in > little-endian. > > I think it would be best to only describe the "record specific data". > True, I'll just say 'NODE_DATA'
Re: [Xen-devel] [XEN PATCH] mismatch between pyxc_methods flags and PyObject definitions
On Tue, Mar 17, 2020 at 11:01:43PM +, YOUNG, MICHAEL A. wrote: > pygrub in xen-4.13.0 with python 3.8.2 fails with the error > > Traceback (most recent call last): > File "/usr/libexec/xen/bin/pygrub", line 21, in > import xen.lowlevel.xc > SystemError: bad call flags > > This patch fixes mismatches in tools/python/xen/lowlevel/xc/xc.c > between the flag bits defined in pyxc_methods and the parameters passed > to the corresponding PyObject definitions. > > With this patch applied pygrub works as expected. > > Signed-off-by: Michael Young I briefly checked Python's documentation. This patch looks correctly to me. FWIW: Reviewed-by: Wei Liu I will wait for Marek's opinion. Wei. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] libx86/CPUID: fix (not just) leaf 7 processing
For one, subleaves within the respective union shouldn't live in
separate sub-structures. And then x86_cpuid_policy_fill_native() should,
as it did originally, iterate over all subleaves here as well as over
all main leaves. Switch to using a "<= MIN()"-based approach similar to
that used in x86_cpuid_copy_to_buffer(). Also follow this for the
extended main leaves then.
Fixes: 1bd2b750537b ("libx86: Fix 32bit stubdom build of
x86_cpuid_policy_fill_native()")
Fixes: 97e4ebdcd765 ("x86/CPUID: support leaf 7 subleaf 1 / AVX512_BF16")
Signed-off-by: Jan Beulich
--- a/xen/include/xen/lib/x86/cpuid.h
+++ b/xen/include/xen/lib/x86/cpuid.h
@@ -181,8 +181,7 @@ struct cpuid_policy
uint32_t _7d0;
struct { DECL_BITFIELD(7d0); };
};
-};
-struct {
+
/* Subleaf 1. */
union {
uint32_t _7a1;
--- a/xen/lib/x86/cpuid.c
+++ b/xen/lib/x86/cpuid.c
@@ -71,8 +71,8 @@ void x86_cpuid_policy_fill_native(struct
unsigned int i;
cpuid_leaf(0, &p->basic.raw[0]);
-for ( i = 1; i < min_t(unsigned int, ARRAY_SIZE(p->basic.raw),
- p->basic.max_leaf); ++i )
+for ( i = 1; i <= MIN(p->basic.max_leaf,
+ ARRAY_SIZE(p->basic.raw) - 1); ++i )
{
switch ( i )
{
@@ -116,8 +116,8 @@ void x86_cpuid_policy_fill_native(struct
{
cpuid_count_leaf(7, 0, &p->feat.raw[0]);
-for ( i = 1; i < min_t(unsigned int, ARRAY_SIZE(p->feat.raw),
- p->feat.max_subleaf); ++i )
+for ( i = 1; i <= MIN(p->feat.max_subleaf,
+ ARRAY_SIZE(p->feat.raw) - 1); ++i )
cpuid_count_leaf(7, i, &p->feat.raw[i]);
}
@@ -172,8 +172,8 @@ void x86_cpuid_policy_fill_native(struct
/* Extended leaves. */
cpuid_leaf(0x8000, &p->extd.raw[0]);
-for ( i = 1; i < min_t(unsigned int, ARRAY_SIZE(p->extd.raw),
- p->extd.max_leaf + 1 - 0x8000); ++i )
+for ( i = 1; i <= MIN(p->extd.max_leaf & 0xU,
+ ARRAY_SIZE(p->extd.raw) - 1); ++i )
cpuid_leaf(0x8000 + i, &p->extd.raw[i]);
x86_cpuid_policy_recalc_synth(p);
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 16/17] xen/mm: Convert {s, g}et_gpfn_from_mfn() to use typesafe MFN
On Sun, 2020-03-22 at 16:14 +, jul...@xen.org wrote:
> From: Julien Grall
>
> The first parameter of {s,g}et_gpfn_from_mfn() is an MFN, so it can
> be
> switched to use the typesafe.
>
> At the same time, replace gpfn with pfn in the helpers as they all
> deal
> with PFN and also turn the macros to static inline.
>
> Note that the return of the getter and the 2nd parameter of the
> setter
> have not been converted to use typesafe PFN because it was requiring
> more changes than expected.
>
> Signed-off-by: Julien Grall
>
> ---
> This was originally sent as part of "xen/arm: Properly disable
> M2P
> on Arm" [1].
>
> Changes since the original version:
> - mfn_to_gmfn() is still present for now so update it
> - Remove stray +
> - Avoid churn in set_pfn_from_mfn() by inverting mfn and mfn_
> - Remove tags
> - Fix build in mem_sharing
>
> [1] <20190603160350.29806-1-julien.gr...@arm.com>
> ---
> xen/arch/x86/cpu/mcheck/mcaction.c | 2 +-
> xen/arch/x86/mm.c | 14 +++
> xen/arch/x86/mm/mem_sharing.c | 20 -
> xen/arch/x86/mm/p2m-pod.c | 4 +-
> xen/arch/x86/mm/p2m-pt.c | 35
> xen/arch/x86/mm/p2m.c | 66 +++-
> --
> xen/arch/x86/mm/paging.c | 4 +-
> xen/arch/x86/pv/dom0_build.c | 6 +--
> xen/arch/x86/x86_64/traps.c| 8 ++--
> xen/common/page_alloc.c| 2 +-
> xen/include/asm-arm/mm.h | 2 +-
> xen/include/asm-x86/grant_table.h | 2 +-
> xen/include/asm-x86/mm.h | 12 --
> xen/include/asm-x86/p2m.h | 2 +-
> 14 files changed, 93 insertions(+), 86 deletions(-)
>
>
[...]
> diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index abf4cc23e4..11614f9107 100644
> --- a/xen/include/asm-arm/mm.h
> +++ b/xen/include/asm-arm/mm.h
> @@ -319,7 +319,7 @@ struct page_info *get_page_from_gva(struct vcpu
> *v, vaddr_t va,
> #define SHARED_M2P(_e) ((_e) == SHARED_M2P_ENTRY)
>
> /* Xen always owns P2M on ARM */
> -#define set_gpfn_from_mfn(mfn, pfn) do { (void) (mfn), (void)(pfn);
> } while (0)
> +static inline void set_pfn_from_mfn(mfn_t mfn, unsigned long pfn) {}
> #define mfn_to_gmfn(_d, mfn) (mfn)
I do not have a setup to compile and test code for Arm, but wouldn't
the compiler complain about unused arguments here? The marco version
explicitly silenced compiler complaints.
> diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-
> x86/grant_table.h
> index 5871238f6d..b6a09c4c6c 100644
> --- a/xen/include/asm-x86/grant_table.h
> +++ b/xen/include/asm-x86/grant_table.h
> @@ -41,7 +41,7 @@ static inline int
> replace_grant_host_mapping(uint64_t addr, mfn_t frame,
> #define gnttab_get_frame_gfn(gt, st, idx)
> ({ \
> mfn_t mfn_ = (st) ? gnttab_status_mfn(gt,
> idx) \
>: gnttab_shared_mfn(gt,
> idx); \
> -unsigned long gpfn_ =
> get_gpfn_from_mfn(mfn_x(mfn_));\
> +unsigned long gpfn_ =
> get_pfn_from_mfn(mfn_);\
> VALID_M2P(gpfn_) ? _gfn(gpfn_) :
> INVALID_GFN;\
> })
>
> diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
> index 53f2ed7c7d..2a4f42e78f 100644
> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -500,9 +500,10 @@ extern paddr_t mem_hotplug;
> */
> extern bool machine_to_phys_mapping_valid;
>
> -static inline void set_gpfn_from_mfn(unsigned long mfn, unsigned
> long pfn)
> +static inline void set_pfn_from_mfn(mfn_t mfn_, unsigned long pfn)
> {
> -const struct domain *d = page_get_owner(mfn_to_page(_mfn(mfn)));
> +const unsigned long mfn = mfn_x(mfn_);
> +const struct domain *d = page_get_owner(mfn_to_page(mfn_));
> unsigned long entry = (d && (d == dom_cow)) ? SHARED_M2P_ENTRY :
> pfn;
>
> if ( !machine_to_phys_mapping_valid )
> @@ -515,11 +516,14 @@ static inline void set_gpfn_from_mfn(unsigned
> long mfn, unsigned long pfn)
>
> extern struct rangeset *mmio_ro_ranges;
>
> -#define get_gpfn_from_mfn(mfn) (machine_to_phys_mapping[(mfn)])
> +static inline unsigned long get_pfn_from_mfn(mfn_t mfn)
> +{
> +return machine_to_phys_mapping[mfn_x(mfn)];
> +}
Any specific reason this (and some other macros) are turned into static
inline? I don't have a problem with them being inline functions but
just wondering if there is a reason to do so.
> #define mfn_to_gmfn(_d, mfn)\
> ( (paging_mode_translate(_d)) \
> - ? get_gpfn_from_mfn(mfn) \
> + ? get_pfn_from_mfn(_mfn(mfn)) \
>: (mfn) )
>
> #define compat_pfn_to_cr3(pfn) (((unsigned)(pfn) << 12) |
> ((unsigned)(pfn) >> 20))
> diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
> index a2
Re: [Xen-devel] [PATCH 16/17] xen/mm: Convert {s, g}et_gpfn_from_mfn() to use typesafe MFN
Hi,
On 23/03/2020 12:11, Hongyan Xia wrote:
On Sun, 2020-03-22 at 16:14 +, jul...@xen.org wrote:
From: Julien Grall
The first parameter of {s,g}et_gpfn_from_mfn() is an MFN, so it can
be
switched to use the typesafe.
At the same time, replace gpfn with pfn in the helpers as they all
deal
with PFN and also turn the macros to static inline.
Note that the return of the getter and the 2nd parameter of the
setter
have not been converted to use typesafe PFN because it was requiring
more changes than expected.
Signed-off-by: Julien Grall
---
This was originally sent as part of "xen/arm: Properly disable
M2P
on Arm" [1].
Changes since the original version:
- mfn_to_gmfn() is still present for now so update it
- Remove stray +
- Avoid churn in set_pfn_from_mfn() by inverting mfn and mfn_
- Remove tags
- Fix build in mem_sharing
[1] <20190603160350.29806-1-julien.gr...@arm.com>
---
xen/arch/x86/cpu/mcheck/mcaction.c | 2 +-
xen/arch/x86/mm.c | 14 +++
xen/arch/x86/mm/mem_sharing.c | 20 -
xen/arch/x86/mm/p2m-pod.c | 4 +-
xen/arch/x86/mm/p2m-pt.c | 35
xen/arch/x86/mm/p2m.c | 66 +++-
--
xen/arch/x86/mm/paging.c | 4 +-
xen/arch/x86/pv/dom0_build.c | 6 +--
xen/arch/x86/x86_64/traps.c| 8 ++--
xen/common/page_alloc.c| 2 +-
xen/include/asm-arm/mm.h | 2 +-
xen/include/asm-x86/grant_table.h | 2 +-
xen/include/asm-x86/mm.h | 12 --
xen/include/asm-x86/p2m.h | 2 +-
14 files changed, 93 insertions(+), 86 deletions(-)
[...]
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index abf4cc23e4..11614f9107 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -319,7 +319,7 @@ struct page_info *get_page_from_gva(struct vcpu
*v, vaddr_t va,
#define SHARED_M2P(_e) ((_e) == SHARED_M2P_ENTRY)
/* Xen always owns P2M on ARM */
-#define set_gpfn_from_mfn(mfn, pfn) do { (void) (mfn), (void)(pfn);
} while (0)
+static inline void set_pfn_from_mfn(mfn_t mfn, unsigned long pfn) {}
#define mfn_to_gmfn(_d, mfn) (mfn)
I do not have a setup to compile and test code for Arm, but wouldn't
the compiler complain about unused arguments here? The marco version
explicitly silenced compiler complaints.
The macro version does not use (void)(arg) for silencing unused
parameter. It is for evaluating (mfn) but ignore the result. A compiler
would warn without (void) because we build Xen with -Wall which include
-Wunused-value.
Xen is not used with -Wunused-parameter, so there is no concern about
unused parameters. If we ever decided to turn on -Wunused-parameter (or
-Wextra), then we will have quite a bit of code to modify (such as
callbacks not using all the parameters) to make it compile.
diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-
x86/grant_table.h
index 5871238f6d..b6a09c4c6c 100644
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -41,7 +41,7 @@ static inline int
replace_grant_host_mapping(uint64_t addr, mfn_t frame,
#define gnttab_get_frame_gfn(gt, st, idx)
({ \
mfn_t mfn_ = (st) ? gnttab_status_mfn(gt,
idx) \
: gnttab_shared_mfn(gt,
idx); \
-unsigned long gpfn_ =
get_gpfn_from_mfn(mfn_x(mfn_));\
+unsigned long gpfn_ =
get_pfn_from_mfn(mfn_);\
VALID_M2P(gpfn_) ? _gfn(gpfn_) :
INVALID_GFN;\
})
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index 53f2ed7c7d..2a4f42e78f 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -500,9 +500,10 @@ extern paddr_t mem_hotplug;
*/
extern bool machine_to_phys_mapping_valid;
-static inline void set_gpfn_from_mfn(unsigned long mfn, unsigned
long pfn)
+static inline void set_pfn_from_mfn(mfn_t mfn_, unsigned long pfn)
{
-const struct domain *d = page_get_owner(mfn_to_page(_mfn(mfn)));
+const unsigned long mfn = mfn_x(mfn_);
+const struct domain *d = page_get_owner(mfn_to_page(mfn_));
unsigned long entry = (d && (d == dom_cow)) ? SHARED_M2P_ENTRY :
pfn;
if ( !machine_to_phys_mapping_valid )
@@ -515,11 +516,14 @@ static inline void set_gpfn_from_mfn(unsigned
long mfn, unsigned long pfn)
extern struct rangeset *mmio_ro_ranges;
-#define get_gpfn_from_mfn(mfn) (machine_to_phys_mapping[(mfn)])
+static inline unsigned long get_pfn_from_mfn(mfn_t mfn)
+{
+return machine_to_phys_mapping[mfn_x(mfn)];
+}
Any specific reason this (and some other macros) are turned into static
inline? I don't have a problem with them being inline functions but
just wondering if there is a reason to do so.
static inline provides better safety check than mac
Re: [Xen-devel] [PATCH 1/7] x86/ucode: Document the behaviour of the microcode_ops hooks
On 23.03.2020 11:17, Andrew Cooper wrote:
> ... and struct cpu_signature for good measure.
>
> No comment is passed on the suitability of the behaviour...
>
> Signed-off-by: Andrew Cooper
> ---
> CC: Jan Beulich
> CC: Wei Liu
> CC: Roger Pau Monné
> ---
> xen/arch/x86/cpu/microcode/private.h | 46
>
> xen/include/asm-x86/microcode.h | 5
> 2 files changed, 51 insertions(+)
>
> diff --git a/xen/arch/x86/cpu/microcode/private.h
> b/xen/arch/x86/cpu/microcode/private.h
> index e64168a502..a2aec53047 100644
> --- a/xen/arch/x86/cpu/microcode/private.h
> +++ b/xen/arch/x86/cpu/microcode/private.h
> @@ -14,14 +14,60 @@ enum microcode_match_result {
> struct microcode_patch; /* Opaque */
>
> struct microcode_ops {
> +/*
> + * Parse a microcode container. Format is vendor-specific.
> + *
> + * Search within the container for the patch, suitable for the current
> + * CPU, which has the highest revision. (Note: May be a patch which is
> + * older that what is running in the CPU. This is a feature, to better
> + * cope with corner cases from buggy firmware.)
> + *
> + * If one is found, allocate and return a struct microcode_patch
> + * encapsulating the appropriate microcode patch. Does not alias the
> + * original buffer.
> + *
> + * If one is not found, (nothing matches the current CPU), return NULL.
> + * Also may return ERR_PTR(-err), e.g. bad container, out of memory.
> + */
> struct microcode_patch *(*cpu_request_microcode)(const void *buf,
> size_t size);
> +
> +/* Obtain microcode-relevant details for the current CPU. */
> int (*collect_cpu_info)(struct cpu_signature *csig);
> +
> +/*
> + * Attempt to load the provided patch into the CPU. Returns -EIO if
> + * anything didn't go as expected.
> + */
> int (*apply_microcode)(const struct microcode_patch *patch);
While at present -EIO may be the only error that may come back here, do
we want to risk the comment going stale when another error return gets
added? IOW - perhaps add "e.g." or some such?
> --- a/xen/include/asm-x86/microcode.h
> +++ b/xen/include/asm-x86/microcode.h
> @@ -7,8 +7,13 @@
> #include
>
> struct cpu_signature {
> +/* CPU signature (CPUID.1.EAX). Only written on Intel. */
> unsigned int sig;
> +
> +/* Platform Flags (only actually 1 bit). Only applicable to Intel. */
> unsigned int pf;
To me "only actually 1 bit" makes it an implication that this is the
lowest bit (like in a bool represented in a 32-bit memory location).
I didn't think this was the case though, so unless I'm wrong, could
you clarify this a little?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt test] 148887: regressions - FAIL
flight 148887 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/148887/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 146182 build-i386-libvirt6 libvirt-buildfail REGR. vs. 146182 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 146182 build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 146182 Tests which did not succeed, but are not blocking: test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-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-amd64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a version targeted for testing: libvirt ea903036fa8d2333edb74b617416416dd75be533 baseline version: libvirt a1cd25b919509be2645dbe6f952d5263e0d4e4e5 Last test of basis 146182 2020-01-17 06:00:23 Z 66 days Failing since146211 2020-01-18 04:18:52 Z 65 days 62 attempts Testing same since 148799 2020-03-21 04:19:42 Z2 days3 attempts People who touched revisions under test: Andrea Bolognani Arnaud Patard Boris Fiuczynski Christian Ehrhardt Collin Walling Daniel Henrique Barboza Daniel P. Berrangé Daniel Veillard Dario Faggioli Erik Skultety Gaurav Agrawal Han Han Jim Fehlig Jiri Denemark Jonathon Jongsma Julio Faracco Ján Tomko Laine Stump Lin Ma Marek Marczykowski-Górecki Michal Privoznik Nikolay Shirokovskiy Pavel Hrdina Pavel Mores Peter Krempa Pino Toscano Richard W.M. Jones Rikard Falkeborn Ryan Moeller Sahid Orentino Ferdjaoui Sebastian Mitterle Stefan Berger Stefan Berger Stefan Hajnoczi Thomas Huth Wu Qingliang Your Name Zhang Bo zhenwei pi Zhimin Feng 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 fail build-arm64-libvirt fail build-armhf-libvirt fail build-i386-libvirt fail build-amd64-pvopspass build-arm64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm blocked test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsmblocked test-amd64-amd64-libvirt-xsm blocked test-arm64-arm64-libvirt-xsm blocked test-amd64-i386-libvirt-xsm blocked test-amd64-amd64-libvirt blocked test-arm64-arm64-libvirt blocked test-armhf-armhf-libvirt blocked test-amd64-i386-libvirt blocked test-amd64-amd64-libvirt-pairblocked test-amd64-i386-libvirt-pair blocked test-arm64-arm64-libvirt-qcow2 blocked test-armhf-armhf-libvirt-raw blocked test-amd64-amd64-libvirt-vhd blocked sg-report-flight on osstest.tes
Re: [Xen-devel] [PATCH 1/7] x86/ucode: Document the behaviour of the microcode_ops hooks
On 23/03/2020 12:33, Jan Beulich wrote:
> On 23.03.2020 11:17, Andrew Cooper wrote:
>> ... and struct cpu_signature for good measure.
>>
>> No comment is passed on the suitability of the behaviour...
>>
>> Signed-off-by: Andrew Cooper
>> ---
>> CC: Jan Beulich
>> CC: Wei Liu
>> CC: Roger Pau Monné
>> ---
>> xen/arch/x86/cpu/microcode/private.h | 46
>>
>> xen/include/asm-x86/microcode.h | 5
>> 2 files changed, 51 insertions(+)
>>
>> diff --git a/xen/arch/x86/cpu/microcode/private.h
>> b/xen/arch/x86/cpu/microcode/private.h
>> index e64168a502..a2aec53047 100644
>> --- a/xen/arch/x86/cpu/microcode/private.h
>> +++ b/xen/arch/x86/cpu/microcode/private.h
>> @@ -14,14 +14,60 @@ enum microcode_match_result {
>> struct microcode_patch; /* Opaque */
>>
>> struct microcode_ops {
>> +/*
>> + * Parse a microcode container. Format is vendor-specific.
>> + *
>> + * Search within the container for the patch, suitable for the current
>> + * CPU, which has the highest revision. (Note: May be a patch which is
>> + * older that what is running in the CPU. This is a feature, to better
>> + * cope with corner cases from buggy firmware.)
>> + *
>> + * If one is found, allocate and return a struct microcode_patch
>> + * encapsulating the appropriate microcode patch. Does not alias the
>> + * original buffer.
>> + *
>> + * If one is not found, (nothing matches the current CPU), return NULL.
>> + * Also may return ERR_PTR(-err), e.g. bad container, out of memory.
>> + */
>> struct microcode_patch *(*cpu_request_microcode)(const void *buf,
>> size_t size);
>> +
>> +/* Obtain microcode-relevant details for the current CPU. */
>> int (*collect_cpu_info)(struct cpu_signature *csig);
>> +
>> +/*
>> + * Attempt to load the provided patch into the CPU. Returns -EIO if
>> + * anything didn't go as expected.
>> + */
>> int (*apply_microcode)(const struct microcode_patch *patch);
> While at present -EIO may be the only error that may come back here, do
> we want to risk the comment going stale when another error return gets
> added? IOW - perhaps add "e.g." or some such?
Can do.
>
>> --- a/xen/include/asm-x86/microcode.h
>> +++ b/xen/include/asm-x86/microcode.h
>> @@ -7,8 +7,13 @@
>> #include
>>
>> struct cpu_signature {
>> +/* CPU signature (CPUID.1.EAX). Only written on Intel. */
>> unsigned int sig;
>> +
>> +/* Platform Flags (only actually 1 bit). Only applicable to Intel. */
>> unsigned int pf;
> To me "only actually 1 bit" makes it an implication that this is the
> lowest bit (like in a bool represented in a 32-bit memory location).
> I didn't think this was the case though, so unless I'm wrong, could
> you clarify this a little?
There will be a single bit within the bottom 8 set (the 1 <<
MSR_PLATFORM_ID[52:50]), despite this field being called "Platform Flags".
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 1/2] xen: Use evtchn_type_t as a type for event channels
On 3/23/20 1:33 AM, Yan Yankovskyi wrote: > Make event channel functions pass event channel port using > evtchn_port_t type. It eliminates signed <-> unsigned conversion. > > Signed-off-by: Yan Yankovskyi Reviewed-by: Boris Ostrovsky > > @@ -1275,7 +1276,7 @@ void rebind_evtchn_irq(int evtchn, int irq) > > mutex_unlock(&irq_mapping_update_lock); > > -bind_evtchn_to_cpu(evtchn, info->cpu); > + bind_evtchn_to_cpu(evtchn, info->cpu); I'd leave this as is. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [libvirt bisection] complete build-i386-libvirt
branch xen-unstable xenbranch xen-unstable job build-i386-libvirt testid libvirt-build Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_keycodemapdb https://gitlab.com/keycodemap/keycodemapdb.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: libvirt git://libvirt.org/libvirt.git Bug introduced: 4d5f50d86b760864240c695adc341379fb47a796 Bug not present: a1a18c6ab55869d3b00cf8c32e0e2262a10c8ce7 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/148912/ commit 4d5f50d86b760864240c695adc341379fb47a796 Author: Pavel Hrdina Date: Wed Jan 8 22:54:31 2020 +0100 bootstrap.conf: stop creating AUTHORS file The existence of AUTHORS file is required for GNU projects but since commit <8bfb36db40f38e92823b657b5a342652064b5adc> we do not require these files to exist. Signed-off-by: Pavel Hrdina Reviewed-by: Daniel P. Berrangé For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/libvirt/build-i386-libvirt.libvirt-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/libvirt/build-i386-libvirt.libvirt-build --summary-out=tmp/148912.bisection-summary --basis-template=146182 --blessings=real,real-bisect libvirt build-i386-libvirt libvirt-build Searching for failure / basis pass: 148887 fail [host=italia0] / 146182 [host=pinot1] 146156 [host=huxelrebe0] 146103 [host=pinot1] 146061 ok. Failure / basis pass flights: 148887 / 146061 (tree with no url: minios) (tree in basispass but not in latest: libvirt_gnulib) Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_keycodemapdb https://gitlab.com/keycodemap/keycodemapdb.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git Latest ea903036fa8d2333edb74b617416416dd75be533 317d3eeb963a515e15a63fa356d8ebcda7041a51 0c8ea9fe1adbbee230ee0c68f28b68ca2b0534bc d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef 066a9956097b54530888b88ab9aa1ea02e42af5a d094e95fb7c61c5f46d8e446b4bdc028438dea1c Basis pass 7d608469621a3fda72dff2a89308e68cc9fb4c9a 317d3eeb963a515e15a63fa356d8ebcda7041a51 70911f1f4aee0366b6122f2b90d367ec0f066beb d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef f21b5a4aeb020f2a5e2c6503f906a9349dd2f069 03bfe526ecadc86f31eda433b91dc90be0563919 Generating revisions with ./adhoc-revtuple-generator git://libvirt.org/libvirt.git#7d608469621a3fda72dff2a89308e68cc9fb4c9a-ea903036fa8d2333edb74b617416416dd75be533 https://gitlab.com/keycodemap/keycodemapdb.git#317d3eeb963a515e15a63fa356d8ebcda7041a51-317d3eeb963a515e15a63fa356d8ebcda7041a51 git://xenbits.xen.org/osstest/ovmf.git#70911f1f4aee0366b6122f2b90d367ec0f066beb-0c8ea9fe1adbbee230ee0c68f28b68ca2b0534bc git://xenbits.xen.org/qemu-xen-traditional.git#d0d8ad39ecb51cd7497cd524484fe09f50876\ 798-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://xenbits.xen.org/qemu-xen.git#933ebad2470a169504799a1d95b8e410bd9847ef-933ebad2470a169504799a1d95b8e410bd9847ef git://xenbits.xen.org/osstest/seabios.git#f21b5a4aeb020f2a5e2c6503f906a9349dd2f069-066a9956097b54530888b88ab9aa1ea02e42af5a git://xenbits.xen.org/xen.git#03bfe526ecadc86f31eda433b91dc90be0563919-d094e95fb7c61c5f46d8e446b4bdc028438dea1c >From git://cache:9419/git://libvirt.org/libvirt ea903036fa..b66744e466 master -> origin/master Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. error: The last gc run reported the following. Please correct the root cause and remove gc.log. Automatic cleanup will not be performed until the file is removed. warning: There are too many unreachable loose objects; run 'git prune' to remove them. Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. error: The last gc run reported the following. Please correct the root cause and remove gc.log. Automatic cleanup will not be performed until the file is removed. warning: There are too many unreachable loose objects; run 'git prune' to remove them. Use of uninitialized value $parents in array dereference at ./adhoc-revtuple-generator line 465. Use of uninitialized value in concatenation (.) or string at ./adhoc-revtuple-generator line 465. Loaded 17544 nodes in revision graph Searching for test results: 146061 pass 7d608469621a3fda72dff2a89308e68c
[Xen-devel] [PATCH v4 00/17] Convert cpu_up/down to device_online/offline
=
Changes in v4
=
* Split arm and arm64 patches so that the change to use reboot_cpu goes
into its own separate patch (Russell)
* Collected new Acked-by
* Rebased on top of v5.6-rc6
* Trimmed the CC list on the cover letter as lists were rejecting it
git clone git://linux-arm.org/linux-qy.git -b cpu-hp-cleanup-v4
Older post can be found here
https://lore.kernel.org/lkml/20200223192942.18420-2-qais.you...@arm.com/
=
Test Coverage
=
All tests ran with LOCKDEP enabled.
Platform: Juno-r2: arm64
* Overnight rcutorture
* Overnight locktorture
* kexec -f Image --command="$(cat /proc/cmdline) reboot=s[0-5]"
* Hibernate to disk (using suspend option)
* Userspace hotplug via sysfs
* PSCI firemware checker
Notes:
* Couldn't convince Juno to hibernate using [reboot] or [shutdown]
options.
Platform: qemu (8 vCPUs) and VM (2 vCPUs): x86_64
-
* Overnight rcutorture
* Overnight locktorture
* Userspace hotplug via sysfs
* echo mmiotrace > /sys/kernel/debug/tracing/current_tracer &&
echo nop > /sys/kernel/debug/tracing/current_tracer
* Ran with CONFIG_DEBUG_HOTPLUG_CPU0 and CONFIG_BOOTPARAM_HOTPLUG_CPU0
Notes:
* qemu failed to bring cpu0 after offlining. Same behavior observed on
vanilla v5.6-rc6. Worked fine on the VM.
* mmiotrace successfully brought down all cpus when enabled,
then back online again when disabled. Including when cpu0 was
offline.
* My xen shenanigans are too 'humble' too create environment to test
the change in xen yet..
=
Original Cover Letter
=
Using cpu_up/down directly to bring cpus online/offline loses synchronization
with sysfs and could suffer from a race similar to what is described in
commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and serialization
during LPM").
cpu_up/down seem to be more of a internal implementation detail for the cpu
subsystem to use to boot up cpus, perform suspend/resume and low level hotplug
operations. Users outside of the cpu subsystem would be better using the device
core API to bring a cpu online/offline which is the interface used to hotplug
memory and other system devices.
Several users have already migrated to use the device core API, this series
converts the remaining users and hides cpu_up/down from internal users at the
end.
I noticed this problem while working on a hack to disable offlining
a particular CPU but noticed that setting the offline_disabled attribute in the
device struct isn't enough because users can easily bypass the device core.
While my hack isn't a valid use case but it did highlight the inconsistency in
the way cpus are being onlined/offlined and this attempt hopefully improves on
this.
The first patch introduces new API to {add,remove}_cpu() using device_{online,
offline}() with correct locks held and export it.
The following 10 patches fix arch users.
The remaining 6 patches fix generic code users. Particularly creating a new
special exported API for the device core to use instead of cpu_up/down.
The last patch removes cpu_up/down from cpu.h and unexport the functions.
In some cases where the use of cpu_up/down seemed legitimate, I encapsulated
the logic in a higher level - special purposed function; and converted the code
to use that instead.
CC: Thomas Gleixner
CC: Tony Luck
CC: Fenghua Yu
CC: Russell King
CC: Catalin Marinas
CC: Michael Ellerman
CC: "David S. Miller"
CC: Helge Deller
CC: Juergen Gross
CC: Mark Rutland
CC: Lorenzo Pieralisi
CC: "Paul E. McKenney"
CC: Greg Kroah-Hartman
CC: xen-devel@lists.xenproject.org
CC: linux-par...@vger.kernel.org
CC: sparcli...@vger.kernel.org
CC: linuxppc-...@lists.ozlabs.org
CC: x...@kernel.org
CC: linux-arm-ker...@lists.infradead.org
CC: linux-i...@vger.kernel.org
CC: linux-ker...@vger.kernel.org
Qais Yousef (17):
cpu: Add new {add,remove}_cpu() functions
smp: Create a new function to shutdown nonboot cpus
ia64: Replace cpu_down with smp_shutdown_nonboot_cpus()
arm: Don't use disable_nonboot_cpus()
arm: Use reboot_cpu instead of hardcoding it to 0
arm64: Don't use disable_nonboot_cpus()
arm64: Use reboot_cpu instead of hardconding it to 0
arm64: hibernate.c: Create a new function to handle cpu_up(sleep_cpu)
x86: Replace cpu_up/down with add/remove_cpu
powerpc: Replace cpu_up/down with add/remove_cpu
sparc: Replace cpu_up/down with add/remove_cpu
parisc: Replace cpu_up/down with add/remove_cpu
driver: xen: Replace cpu_up/down with device_online/offline
firmware: psci: Replace cpu_up/down with add/remove_cpu
torture: Replace cpu_up/down with add/remove_cpu
smp: Create a new function to
[Xen-devel] [PATCH v4 13/17] driver: xen: Replace cpu_up/down with device_online/offline
The core device API performs extra housekeeping bits that are missing
from directly calling cpu_up/down.
See commit a6717c01ddc2 ("powerpc/rtas: use device model APIs and
serialization during LPM") for an example description of what might go
wrong.
This also prepares to make cpu_up/down a private interface for anything
but the cpu subsystem.
Reviewed-by: Juergen Gross
Signed-off-by: Qais Yousef
CC: Boris Ostrovsky
CC: Juergen Gross
CC: Stefano Stabellini
CC: xen-devel@lists.xenproject.org
CC: linux-ker...@vger.kernel.org
---
drivers/xen/cpu_hotplug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index f192b6f42da9..ec975decb5de 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -94,7 +94,7 @@ static int setup_cpu_watcher(struct notifier_block *notifier,
for_each_possible_cpu(cpu) {
if (vcpu_online(cpu) == 0) {
- (void)cpu_down(cpu);
+ device_offline(get_cpu_device(cpu));
set_cpu_present(cpu, false);
}
}
--
2.17.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v4 01/17] cpu: Add new {add, remove}_cpu() functions
The new functions use device_{online,offline}() which are userspace
safe.
This is in preparation to move cpu_{up, down} kernel users to use
a safer interface that is not racy with userspace.
Suggested-by: "Paul E. McKenney"
Signed-off-by: Qais Yousef
CC: Thomas Gleixner
CC: "Paul E. McKenney"
CC: Helge Deller
CC: Michael Ellerman
CC: "David S. Miller"
CC: Juergen Gross
CC: Mark Rutland
CC: Lorenzo Pieralisi
CC: xen-devel@lists.xenproject.org
CC: linux-par...@vger.kernel.org
CC: sparcli...@vger.kernel.org
CC: linuxppc-...@lists.ozlabs.org
CC: linux-arm-ker...@lists.infradead.org
CC: x...@kernel.org
CC: linux-ker...@vger.kernel.org
---
include/linux/cpu.h | 2 ++
kernel/cpu.c| 24
2 files changed, 26 insertions(+)
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index 1ca2baf817ed..cf8cf38dca43 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -89,6 +89,7 @@ extern ssize_t arch_cpu_release(const char *, size_t);
#ifdef CONFIG_SMP
extern bool cpuhp_tasks_frozen;
int cpu_up(unsigned int cpu);
+int add_cpu(unsigned int cpu);
void notify_cpu_starting(unsigned int cpu);
extern void cpu_maps_update_begin(void);
extern void cpu_maps_update_done(void);
@@ -118,6 +119,7 @@ extern void cpu_hotplug_disable(void);
extern void cpu_hotplug_enable(void);
void clear_tasks_mm_cpumask(int cpu);
int cpu_down(unsigned int cpu);
+int remove_cpu(unsigned int cpu);
#else /* CONFIG_HOTPLUG_CPU */
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 9c706af713fb..069802f7010f 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -1057,6 +1057,18 @@ int cpu_down(unsigned int cpu)
}
EXPORT_SYMBOL(cpu_down);
+int remove_cpu(unsigned int cpu)
+{
+ int ret;
+
+ lock_device_hotplug();
+ ret = device_offline(get_cpu_device(cpu));
+ unlock_device_hotplug();
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(remove_cpu);
+
#else
#define takedown_cpu NULL
#endif /*CONFIG_HOTPLUG_CPU*/
@@ -1209,6 +1221,18 @@ int cpu_up(unsigned int cpu)
}
EXPORT_SYMBOL_GPL(cpu_up);
+int add_cpu(unsigned int cpu)
+{
+ int ret;
+
+ lock_device_hotplug();
+ ret = device_online(get_cpu_device(cpu));
+ unlock_device_hotplug();
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(add_cpu);
+
#ifdef CONFIG_PM_SLEEP_SMP
static cpumask_var_t frozen_cpus;
--
2.17.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH XTF benchmark v1 2/4] time: add stubs
To measure how long a certain interaction takes, we need time
primitives. This commit introduces these primitives, so that
future tests can use the gettimeofday function to retrieve the
current time.
Signed-off-by: Paul Semel
Signed-off-by: Norbert Manthey
---
build/files.mk | 1 +
common/time.c | 203 +
include/xtf/time.h | 66 +++
3 files changed, 270 insertions(+)
create mode 100644 common/time.c
create mode 100644 include/xtf/time.h
diff --git a/build/files.mk b/build/files.mk
--- a/build/files.mk
+++ b/build/files.mk
@@ -16,6 +16,7 @@ obj-perarch += $(ROOT)/common/libc/vsnprintf.o
obj-perarch += $(ROOT)/common/report.o
obj-perarch += $(ROOT)/common/setup.o
obj-perarch += $(ROOT)/common/xenbus.o
+obj-perarch += $(ROOT)/common/time.o
obj-perenv += $(ROOT)/arch/x86/decode.o
obj-perenv += $(ROOT)/arch/x86/desc.o
diff --git a/common/time.c b/common/time.c
new file mode 100644
--- /dev/null
+++ b/common/time.c
@@ -0,0 +1,203 @@
+#include
+#include
+#include
+#include
+
+/* This function was taken from mini-os source code [tag xen-RELEASE-4.11.1]
+
+ * (C) 2003 - Rolf Neugebauer - Intel Research Cambridge
+ * (C) 2002-2003 - Keir Fraser - University of Cambridge
+ * (C) 2005 - Grzegorz Milos - Intel Research Cambridge
+ * (C) 2006 - Robert Kaiser - FH Wiesbaden
+
+ *
+ *File: time.c
+ * Author: Rolf Neugebauer and Keir Fraser
+ * Changes: Grzegorz Milos
+ *
+ * Description: Simple time and timer functions
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+/* It returns ((delta << shift) * mul_frac) >> 32 */
+static inline uint64_t scale_delta(uint64_t delta, uint32_t mul_frac, int
shift)
+{
+uint64_t product;
+#ifdef __i386__
+uint32_t tmp1, tmp2;
+#endif
+
+if ( shift < 0 )
+delta >>= -shift;
+else
+delta <<= shift;
+
+#ifdef __i386__
+__asm__ (
+"mul %5 ; "
+"mov %4,%%eax ; "
+"mov %%edx,%4 ; "
+"mul %5 ; "
+"add %4,%%eax ; "
+"xor %5,%5; "
+"adc %5,%%edx ; "
+: "=A" (product), "=r" (tmp1), "=r" (tmp2)
+: "a" ((uint32_t)delta), "1" ((uint32_t)(delta >> 32)), "2"
(mul_frac) );
+#else
+__asm__ (
+"mul %%rdx ; shrd $32,%%rdx,%%rax"
+: "=a" (product) : "0" (delta), "d" ((uint64_t)mul_frac) );
+#endif
+
+return product;
+}
+
+
+#if defined(__i386__)
+uint32_t since_boot_time(void)
+#else
+uint64_t since_boot_time(void)
+#endif
+{
+unsigned long old_tsc, tsc;
+#if defined(__i386__)
+uint32_t system_time;
+#else
+uint64_t system_time;
+#endif
+uint32_t ver1, ver2;
+
+do {
+do {
+ver1 = shared_info.vcpu_info[0].time.version;
+smp_rmb();
+} while ( (ver1 & 1) == 1 );
+
+system_time = shared_info.vcpu_info[0].time.system_time;
+old_tsc = shared_info.vcpu_info[0].time.tsc_timestamp;
+smp_rmb();
+tsc = rdtscp();
+ver2 = ACCESS_ONCE(shared_info.vcpu_info[0].time.version);
+smp_rmb();
+} while ( ver1 != ver2 );
+
+system_time += scale_delta(tsc - old_tsc,
+ shared_info.vcpu_info[0].time.tsc_to_system_mul,
+ shared_info.vcpu_info[0].time.tsc_shift);
+
+return system_time;
+}
+
+/* This function return the epoch time (number of seconds elapsed
+ * since Juanary 1, 1970) */
+#if defined(__i386__)
+uint32_t current_time(void)
+#else
+uint64_t current_time(void)
+#endif
+{
+#if defined(__i386__)
+uint32_t seconds = shared_info.wc_sec;
+#else
+uint64_t seconds = ((uint64_t)shared_info.wc_sec_hi << 32) |
shared_info.wc_sec;
+#endif
+return seconds + (since_boo
[Xen-devel] [PATCH XTF benchmark v1 4/4] perf: measure MMUEXT_MARK_SUPER test
A first simple test is to call a hypercall in a tight loop. To measure
implementation aspects of the hypervisor, we picked a hypercall that
is not implemented and hence results in a no-op, namely the hypercall
mmuext_op with the command MMUEXT_MARK_SUPER.
The test calibrates the execution time for 1000 calls to the hypercall,
and next calculates the number of calls to take about 5 minutes.
Signed-off-by: Norbert Manthey
Reviewed-by: Bjoern Doebel
---
tests/perf-PV-MMUEXT_MARK_SUPER-noop/Makefile | 9 +++
tests/perf-PV-MMUEXT_MARK_SUPER-noop/main.c | 80 +++
2 files changed, 89 insertions(+)
create mode 100644 tests/perf-PV-MMUEXT_MARK_SUPER-noop/Makefile
create mode 100644 tests/perf-PV-MMUEXT_MARK_SUPER-noop/main.c
diff --git a/tests/perf-PV-MMUEXT_MARK_SUPER-noop/Makefile
b/tests/perf-PV-MMUEXT_MARK_SUPER-noop/Makefile
new file mode 100644
--- /dev/null
+++ b/tests/perf-PV-MMUEXT_MARK_SUPER-noop/Makefile
@@ -0,0 +1,9 @@
+include $(ROOT)/build/common.mk
+
+NAME := perf-PV-MMUEXT_MARK_SUPER-noop
+CATEGORY := benchmark
+TEST-ENVS := pv64
+
+obj-perenv += main.o
+
+include $(ROOT)/build/gen.mk
diff --git a/tests/perf-PV-MMUEXT_MARK_SUPER-noop/main.c
b/tests/perf-PV-MMUEXT_MARK_SUPER-noop/main.c
new file mode 100644
--- /dev/null
+++ b/tests/perf-PV-MMUEXT_MARK_SUPER-noop/main.c
@@ -0,0 +1,80 @@
+/**
+ * Copyright (C) Amazon.com, Inc. or its affiliates.
+ * Author: Norbert Manthey
+ *
+ * @file tests/perf-PV-MMUEXT_MARK_SUPER-noop/main.c
+ * @ref test-perf-PV-MMUEXT_MARK_SUPER-noop
+ *
+ * @page perf-PV-MMUEXT_MARK_SUPER-noop
+ *
+ * This test runs the hypercall mmuext_op with the command MMUEXT_MARK_SUPER in
+ * a tight loop, and measures how much time it takes for all loops. Finally,
the
+ * test prints this time.
+ *
+ * Since this is a performance test, the actual value is furthermore printed
+ * using the predefined pattern on a separate line. The reported value
+ * represents the time it takes to run the mmuext_op hypercall in nano seconds.
+ * The average is calculated by running the call for about 5 minutes.
+ *
+ * perf
+ *
+ * @see tests/perf-PV-MMUEXT_MARK_SUPER-noop/main.c
+ */
+
+/* To improve precision of the measurement, try to run the hypercall for this
+ amount of seconds. As the time per call can be different for test machines,
+ we measure the time for the below number of calls, and estimate the number
of
+ calls to perform accordingly. */
+#define MEASUREMENT_SECONDS 300
+
+/* This number of calls to the function under test will be used to estimate how
+ many times we need to call the function to measure for about 5 minutes. */
+#define CALIBRATION_CALLS 1000
+
+#include
+#include
+
+const char test_title[] = "Test perf-MMUEXT_MARK_SUPER";
+
+/* Use a global struct to avoid local variables in call_MMUEXT_MARK_SUPER */
+mmuext_op_t op =
+{
+.cmd = MMUEXT_MARK_SUPER,
+.arg1.mfn = 1,
+};
+
+/* Schedule a no-op hypercall */
+int call_MMUEXT_MARK_SUPER(void)
+{
+return hypercall_mmuext_op(&op, 1, NULL, DOMID_SELF);
+}
+
+void test_main(void)
+{
+int rc = 0;
+
+/* Test whether the hypercall is implemented as expected */
+rc = call_MMUEXT_MARK_SUPER();
+if(rc != -EOPNOTSUPP && rc != -EINVAL && rc != -ENOSYS)
+return xtf_error("Unexpected MMUEXT_MARK_SUPER, rc %d\n", rc);
+
+/* Measuring average execution time for given function, and print stats */
+measure_performance(test_title,
+"mmuext_op(MMUEXT_MARK_SUPER, ...)",
+call_MMUEXT_MARK_SUPER,
+MEASUREMENT_SECONDS,
+CALIBRATION_CALLS,
+1);
+
+return xtf_success("Success: performed MMUEXT_MARK_SUPER hypercall with
expected result\n");
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.17.1
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH XTF benchmark v1 1/4] categories: add benchmark
As XTF allows to write tests that interact with the hypervisor, we would like
to use this capability to implement micro benchmarks, so that we can measure
the performance impact of modifications to the hypervisor.
This change introduces a category benchmark, which can be used as
container for tests of this kind.
Signed-off-by: Norbert Manthey
---
build/common.mk | 2 +-
xtf-runner | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/build/common.mk b/build/common.mk
--- a/build/common.mk
+++ b/build/common.mk
@@ -1,4 +1,4 @@
-ALL_CATEGORIES := special functional xsa utility in-development
+ALL_CATEGORIES := special functional xsa utility in-development benchmark
ALL_ENVIRONMENTS := pv64 pv32pae hvm64 hvm32pae hvm32pse hvm32
diff --git a/xtf-runner b/xtf-runner
--- a/xtf-runner
+++ b/xtf-runner
@@ -43,7 +43,7 @@ def exit_code(state):
# All test categories
default_categories = set(("functional", "xsa"))
-non_default_categories = set(("special", "utility", "in-development"))
+non_default_categories = set(("special", "utility", "in-development",
"benchmark"))
all_categories = default_categories | non_default_categories
# All test environments
--
2.17.1
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH XTF benchmark v1 0/4] XTF: add micro benchmarks
Dear all, I added a benchmark category to XTF, and added functions to measure time in the guest. Finally, I added a first micro benchmark that measures the time to call a specified hypercall, and print the average time the hypercall takes. The added category should be useful to implement further micro benchmarks. I already implemented a few more that I will publish once the environment is agreed on. Best, Norbert Norbert Manthey (4): categories: add benchmark time: add stubs time: provide measurement template perf: measure MMUEXT_MARK_SUPER test build/common.mk | 2 +- build/files.mk| 1 + common/time.c | 279 ++ include/xtf/time.h| 81 + tests/perf-PV-MMUEXT_MARK_SUPER-noop/Makefile | 9 + tests/perf-PV-MMUEXT_MARK_SUPER-noop/main.c | 80 + xtf-runner| 2 +- 7 files changed, 452 insertions(+), 2 deletions(-) create mode 100644 common/time.c create mode 100644 include/xtf/time.h create mode 100644 tests/perf-PV-MMUEXT_MARK_SUPER-noop/Makefile create mode 100644 tests/perf-PV-MMUEXT_MARK_SUPER-noop/main.c -- 2.17.1 Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH XTF benchmark v1 3/4] time: provide measurement template
The added function measure_performance allows to measure the run time
of a function, by computing the average time it takes to call that
function a given number of retries. The measured total time is returned
in nano seconds. Furthermore, the value is printed via printk in a
fixed format, to allow processing the output further.
This format is, where average-time provides ns with ps granularity:
perf test_name ns
Signed-off-by: Norbert Manthey
---
common/time.c | 76 ++
include/xtf/time.h | 15 +
2 files changed, 91 insertions(+)
diff --git a/common/time.c b/common/time.c
--- a/common/time.c
+++ b/common/time.c
@@ -192,6 +192,82 @@ void msleep(uint64_t t)
mspin_sleep(t);
}
+unsigned long cycles2nsec(uint64_t cycles)
+{
+return scale_delta(cycles,
+shared_info.vcpu_info[0].time.tsc_to_system_mul,
+shared_info.vcpu_info[0].time.tsc_shift);
+}
+
+unsigned long measure_performance(const char* test_name,
+ const char* function_name,
+ function_under_test_t call,
+ unsigned long measure_seconds,
+ unsigned long callibration_calls,
+ int print_times)
+{
+unsigned long start, end; // time stamps for before and after the
execution
+unsigned long calibration_ns, total_measured_ns, avg_ns, avg_ps_fraction;
+unsigned long counter;
+unsigned long measure_calls;
+int rc = 0;
+
+
+/* Calibrate by measuring time for given amount of callibration calls*/
+printk("Calibrate %s by calling %lu times\n", function_name,
+ callibration_calls);
+start = rdtscp();
+for(counter = 0; counter < callibration_calls; ++ counter)
+{
+rc = call();
+}
+end = rdtscp();
+
+/* Calculate the total number in nano seconds */
+calibration_ns = cycles2nsec(end - start);
+
+/* Calculate number of calls for about measure_seconds based on
+ (callibration_calls / calibration_ns) equals
+ (measure_calls / (measure_seconds * 1000 * 1000 * 1000)) */
+measure_calls = ((measure_seconds * 1000UL * 1000UL * 1000UL)
+ / calibration_ns) * callibration_calls;
+
+printk("Measure %s by calling %lu times\n", function_name, measure_calls);
+
+/* Perform all calls, measure start and end time */
+start = rdtscp();
+for(counter = 0; counter < measure_calls; ++ counter)
+{
+rc = call();
+}
+end = rdtscp();
+
+/* Calculate the total number in nano seconds */
+total_measured_ns = cycles2nsec(end - start);
+avg_ns = total_measured_ns / measure_calls;
+avg_ps_fraction = (total_measured_ns / (measure_calls/1000)) % 1000;
+
+if(print_times)
+{
+/* Show the result of the last query */
+printk("%s's last return value: %d\n", function_name, rc);
+
+/* Print average time and total time */
+printk("Avg %s call time: avg: %ld.%s%ld ns total: %ld ns\n",
+ function_name, avg_ns,
+ avg_ps_fraction < 10 ? "00" : (avg_ps_fraction < 100 ? "0" :
""),
+ avg_ps_fraction, total_measured_ns);
+
+/* Print performance value */
+printk("perf %s %ld.%s%ld ns\n", test_name, avg_ns,
+ avg_ps_fraction < 10 ? "00" : (avg_ps_fraction < 100 ? "0" :
""),
+ avg_ps_fraction);
+}
+
+/* Return average run time in pico seconds */
+return avg_ns * 1000 + avg_ps_fraction;
+}
+
/*
* Local variables:
* mode: C
diff --git a/include/xtf/time.h b/include/xtf/time.h
--- a/include/xtf/time.h
+++ b/include/xtf/time.h
@@ -53,6 +53,21 @@ int gettimeofday(struct timeval *tp);
/* This returns the current epoch time */
#define NOW() current_time()
+unsigned long cycles2nsec(uint64_t cycles);
+
+/* Signature of a function to be called for measurement */
+typedef int (*function_under_test_t)(void);
+
+/* Measure the time it takes to call the passed function. Measure for a given
+ amount of time after calibrating for a given amount of calls. Returns the
+ average run time of the measure call in pico seconds. */
+unsigned long measure_performance(const char* test_name,
+ const char* function_name,
+ function_under_test_t call,
+ unsigned long measure_seconds,
+ unsigned long callibration_calls,
+ int print_times);
+
#endif /* XTF_TIME_H */
/*
--
2.17.1
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
___
Xen-devel mailing list
X
Re: [Xen-devel] [PATCH 1/7] x86/ucode: Document the behaviour of the microcode_ops hooks
On 23.03.2020 14:26, Andrew Cooper wrote:
> On 23/03/2020 12:33, Jan Beulich wrote:
>> On 23.03.2020 11:17, Andrew Cooper wrote:
>>> --- a/xen/include/asm-x86/microcode.h
>>> +++ b/xen/include/asm-x86/microcode.h
>>> @@ -7,8 +7,13 @@
>>> #include
>>>
>>> struct cpu_signature {
>>> +/* CPU signature (CPUID.1.EAX). Only written on Intel. */
>>> unsigned int sig;
>>> +
>>> +/* Platform Flags (only actually 1 bit). Only applicable to Intel. */
>>> unsigned int pf;
>> To me "only actually 1 bit" makes it an implication that this is the
>> lowest bit (like in a bool represented in a 32-bit memory location).
>> I didn't think this was the case though, so unless I'm wrong, could
>> you clarify this a little?
>
> There will be a single bit within the bottom 8 set (the 1 <<
> MSR_PLATFORM_ID[52:50]), despite this field being called "Platform Flags".
That's what I recalled. Could I talk you into
s/only actually 1 bit/only actually a single set bit/ or some such?
If so,
Acked-by: Jan Beulich
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] tools/xenstore: don't close connection in xs_talkv()
In case of some errors xs_talkv() will close the connection to
Xenstore. This is annoying as it is not clear to the caller in which
error case the connection is still available.
Drop that implicit closing to make the interface behave in a sane and
predictable way.
Signed-off-by: Juergen Gross
---
tools/xenstore/xs.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/tools/xenstore/xs.c b/tools/xenstore/xs.c
index aa1d24b8b9..bbf3e00bed 100644
--- a/tools/xenstore/xs.c
+++ b/tools/xenstore/xs.c
@@ -524,12 +524,14 @@ static void *xs_talkv(struct xs_handle *h,
xs_transaction_t t,
goto fail;
ret = read_reply(h, &msg.type, len);
- if (!ret)
- goto fail;
mutex_unlock(&h->request_mutex);
sigaction(SIGPIPE, &oldact, NULL);
+
+ if (!ret)
+ return NULL;
+
if (msg.type == XS_ERROR) {
saved_errno = get_error(ret);
free(ret);
@@ -539,19 +541,15 @@ static void *xs_talkv(struct xs_handle *h,
xs_transaction_t t,
if (msg.type != type) {
free(ret);
- saved_errno = EBADF;
- goto close_fd;
+ errno = EBADF;
+ return NULL;
}
return ret;
fail:
- /* We're in a bad state, so close fd. */
saved_errno = errno;
mutex_unlock(&h->request_mutex);
sigaction(SIGPIPE, &oldact, NULL);
-close_fd:
- close(h->fd);
- h->fd = -1;
errno = saved_errno;
return NULL;
}
--
2.16.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH OSSTEST] kernel-build: enable XEN_BALLOON_MEMORY_HOTPLUG
This allows a PVH/HVM domain to use unpopulated memory ranges to map foreign memory or grants, and is required for a PVH dom0 to function properly. Signed-off-by: Roger Pau Monné --- ts-kernel-build | 1 + 1 file changed, 1 insertion(+) diff --git a/ts-kernel-build b/ts-kernel-build index c976289e..89cdafcb 100755 --- a/ts-kernel-build +++ b/ts-kernel-build @@ -511,6 +511,7 @@ setopt CONFIG_XEN_KBDDEV_FRONTEND y setopt CONFIG_XEN_FBDEV_FRONTEND y setopt CONFIG_XEN_PCIDEV_FRONTEND y setopt CONFIG_XEN_BALLOON y +setopt CONFIG_XEN_BALLOON_MEMORY_HOTPLUG y setopt CONFIG_XEN_SCRUB_PAGES y setopt CONFIG_XEN_DEV_EVTCHN y setopt CONFIG_XEN_BACKEND y -- 2.25.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] Revert "x86/vvmx: fix virtual interrupt injection when Ack on exit control is used"
On Mon, Mar 23, 2020 at 09:09:59AM +0100, Jan Beulich wrote: > On 20.03.2020 20:07, Roger Pau Monne wrote: > > This reverts commit f96e1469ad06b61796c60193daaeb9f8a96d7458. > > > > The commit is wrong, as the whole point of nvmx_update_apicv is to > > update the guest interrupt status field when the Ack on exit VMEXIT > > control feature is enabled. > > > > Signed-off-by: Roger Pau Monné > > Before anyone gets to look at the other two patches, should this > be thrown in right away? I would like if possible get a confirmation from Kevin (or anyone else) that my understanding is correct. I find the nested code very confusing, and I've already made a mistake while trying to fix it. That being said, this was spotted by osstest as introducing a regression, so I guess it's safe to just toss it in now. FWIW patch 2/3 attempts to provide a description of my understanding of how nvmx_update_apicv works. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 04/51] drm: Set final_kfree in drm_dev_alloc
I also did a full review of all callers, and only the xen driver forgot to call drm_dev_put in the failure path. Fix that up too. v2: I noticed that xen has a drm_driver.release hook, and uses drm_dev_alloc(). We need to remove the kfree from xen_drm_drv_release(). bochs also has a release hook, but leaked the drm_device ever since commit 0a6659bdc5e8221da99eebb176fd9591435e38de Author: Gerd Hoffmann Date: Tue Dec 17 18:04:46 2013 +0100 drm/bochs: new driver This patch here fixes that leak. Same for virtio, started leaking with commit b1df3a2b24a917f8853d43fe9683c0e360d2c33a Author: Gerd Hoffmann Date: Tue Feb 11 14:58:04 2020 +0100 drm/virtio: add drm_driver.release callback. Acked-by: Gerd Hoffmann Acked-by: Thomas Zimmermann Acked-by: Sam Ravnborg Reviewed-by: Oleksandr Andrushchenko Cc: Gerd Hoffmann Cc: Oleksandr Andrushchenko Cc: xen-devel@lists.xenproject.org Signed-off-by: Daniel Vetter Cc: Maarten Lankhorst Cc: Maxime Ripard Cc: Thomas Zimmermann Cc: David Airlie Cc: Daniel Vetter Cc: Oleksandr Andrushchenko Cc: xen-devel@lists.xenproject.org --- drivers/gpu/drm/drm_drv.c | 3 +++ drivers/gpu/drm/xen/xen_drm_front.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 1b9be23b93b0..877ded348b6e 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include @@ -819,6 +820,8 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver, return ERR_PTR(ret); } + drmm_add_final_kfree(dev, dev); + return dev; } EXPORT_SYMBOL(drm_dev_alloc); diff --git a/drivers/gpu/drm/xen/xen_drm_front.c b/drivers/gpu/drm/xen/xen_drm_front.c index 4be49c1aef51..d22b5da38935 100644 --- a/drivers/gpu/drm/xen/xen_drm_front.c +++ b/drivers/gpu/drm/xen/xen_drm_front.c @@ -461,7 +461,6 @@ static void xen_drm_drv_release(struct drm_device *dev) drm_mode_config_cleanup(dev); drm_dev_fini(dev); - kfree(dev); if (front_info->cfg.be_alloc) xenbus_switch_state(front_info->xb_dev, @@ -561,6 +560,7 @@ static int xen_drm_drv_init(struct xen_drm_front_info *front_info) fail_modeset: drm_kms_helper_poll_fini(drm_dev); drm_mode_config_cleanup(drm_dev); + drm_dev_put(drm_dev); fail: kfree(drm_info); return ret; -- 2.25.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen-pciback: fix INTERRUPT_TYPE_* defines
On Fri, Mar 20, 2020 at 04:09:18AM +0100, Marek Marczykowski-Górecki wrote:
> xen_pcibk_get_interrupt_type() assumes INTERRUPT_TYPE_NONE being 0
> (initialize ret to 0 and return as INTERRUPT_TYPE_NONE).
> Fix the definition to make INTERRUPT_TYPE_NONE really 0, and also shift
> other values to not leave holes.
> But also, do not assume INTERRUPT_TYPE_NONE being 0 anymore to avoid
> similar confusions in the future.
>
> Fixes: 476878e4b2be ("xen-pciback: optionally allow interrupt enable flag
> writes")
> Signed-off-by: Marek Marczykowski-Górecki
Reviewed-by: Roger Pau Monné
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [XEN PATCH v3 15/23] xen/build: have the root Makefile generates the CFLAGS
On Thu, Mar 19, 2020 at 04:24:12PM +, Anthony PERARD wrote: > On Tue, Mar 17, 2020 at 06:05:24PM +, Anthony PERARD wrote: > > On Thu, Feb 27, 2020 at 12:05:04PM +0100, Roger Pau Monné wrote: > > > On Wed, Feb 26, 2020 at 11:33:47AM +, Anthony PERARD wrote: > > > > +ifneq ($(CONFIG_CC_IS_CLANG),y) > > > > +# Clang doesn't understand this command line argument, and doesn't > > > > appear to > > > > +# have an suitable alternative. The resulting compiled binary does > > > > function, > > > > +# but has an excessively large symbol table. > > > > +CFLAGS += -Wa,--strip-local-absolute > > > > > > This is not really related to clang, but to the assembler. If clang is > > > used with -no-integrated-as it's quite likely that the GNU assembler > > > will be used, and hence this option would be available. > > > > > > Can we use cc-option-add here in order to detect whether the build > > > toolchain support the option? > > > > That can be done, but I think I'll do that as a follow up of this patch, > > to avoid too many changes when moving the cflags around. > > > > > Ideally this should be done after the integrated assembler tests > > > performed in x86/Rules.mk. > > So, testing for the -Wa,--strip-local-absolute flags turns out to be > more complicated than I though it would be. > - cc-option-add doesn't work because it doesn't test with the current list >of CFLAGS. And if I add the CFLAGS, clang says the option is unused, >it doesn't matter if -no-integrated-as is present or not. Oh, that seems like completely bogus clang behavior. It's my understanding (from reading the manual) that whatever gets appended to -Wa, is just passed to the assembler, in which case the compiler has no idea whether it's used by it or not. Which version of clang did you use to test it? Isn't it fine to just attempt to test if it works, and if the compiler complains just don't append it? (regardless of whether -no-integrated-as is used or not) > - I tried to use as-option macro from Linux but that comes with issues >as well. I don't think that enough work as been done to make it work >well with clang. (I probably need to filter -Wall out of the CFLAGS >when testing, and they are probably other issues.) > > So I think I leave the enhancement for later. Having the flag depends > on GCC is good enough for now. Right, until clang adds support for it there's no pressure. Thanks, Roger.
Re: [PATCH v4 01/17] cpu: Add new {add,remove}_cpu() functions
On Mon, Mar 23, 2020 at 01:50:54PM +, Qais Yousef wrote:
> The new functions use device_{online,offline}() which are userspace
> safe.
>
> This is in preparation to move cpu_{up, down} kernel users to use
> a safer interface that is not racy with userspace.
>
> Suggested-by: "Paul E. McKenney"
> Signed-off-by: Qais Yousef
> CC: Thomas Gleixner
> CC: "Paul E. McKenney"
Reviewed-by: Paul E. McKenney
> CC: Helge Deller
> CC: Michael Ellerman
> CC: "David S. Miller"
> CC: Juergen Gross
> CC: Mark Rutland
> CC: Lorenzo Pieralisi
> CC: xen-devel@lists.xenproject.org
> CC: linux-par...@vger.kernel.org
> CC: sparcli...@vger.kernel.org
> CC: linuxppc-...@lists.ozlabs.org
> CC: linux-arm-ker...@lists.infradead.org
> CC: x...@kernel.org
> CC: linux-ker...@vger.kernel.org
> ---
> include/linux/cpu.h | 2 ++
> kernel/cpu.c| 24
> 2 files changed, 26 insertions(+)
>
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index 1ca2baf817ed..cf8cf38dca43 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -89,6 +89,7 @@ extern ssize_t arch_cpu_release(const char *, size_t);
> #ifdef CONFIG_SMP
> extern bool cpuhp_tasks_frozen;
> int cpu_up(unsigned int cpu);
> +int add_cpu(unsigned int cpu);
> void notify_cpu_starting(unsigned int cpu);
> extern void cpu_maps_update_begin(void);
> extern void cpu_maps_update_done(void);
> @@ -118,6 +119,7 @@ extern void cpu_hotplug_disable(void);
> extern void cpu_hotplug_enable(void);
> void clear_tasks_mm_cpumask(int cpu);
> int cpu_down(unsigned int cpu);
> +int remove_cpu(unsigned int cpu);
>
> #else /* CONFIG_HOTPLUG_CPU */
>
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 9c706af713fb..069802f7010f 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -1057,6 +1057,18 @@ int cpu_down(unsigned int cpu)
> }
> EXPORT_SYMBOL(cpu_down);
>
> +int remove_cpu(unsigned int cpu)
> +{
> + int ret;
> +
> + lock_device_hotplug();
> + ret = device_offline(get_cpu_device(cpu));
> + unlock_device_hotplug();
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(remove_cpu);
> +
> #else
> #define takedown_cpu NULL
> #endif /*CONFIG_HOTPLUG_CPU*/
> @@ -1209,6 +1221,18 @@ int cpu_up(unsigned int cpu)
> }
> EXPORT_SYMBOL_GPL(cpu_up);
>
> +int add_cpu(unsigned int cpu)
> +{
> + int ret;
> +
> + lock_device_hotplug();
> + ret = device_online(get_cpu_device(cpu));
> + unlock_device_hotplug();
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(add_cpu);
> +
> #ifdef CONFIG_PM_SLEEP_SMP
> static cpumask_var_t frozen_cpus;
>
> --
> 2.17.1
>
Re: [PATCH] libxl: create backend/ xenstore dir for driver domains
On Mon, Jan 06, 2020 at 05:03:40PM +0100, Marek Marczykowski-Górecki wrote:
> On Mon, Jan 06, 2020 at 03:40:22PM +, Ian Jackson wrote:
> > Adding Roger to the CC.
> >
> > Marek Marczykowski-Górecki writes ("Re: [PATCH] libxl: create backend/
> > xenstore dir for driver domains"):
> > > On Mon, Jan 06, 2020 at 02:20:46PM +, Ian Jackson wrote:
> > > > Marek Marczykowski-Górecki writes ("[PATCH] libxl: create backend/
> > > > xenstore dir for driver domains"):
> > > > > Cleaning up backend xenstore entries is a responsibility of the
> > > > > backend.
> > > > > When backend lives outside of dom0, the domain needs proper
> > > > > permissions
> > > > > to do it. Normally it is given permission to remove the device dir
> > > > > itself, but not the dir containing it (named after frontend ID).
> > > > > After a
> > > > > whole those empty leftover directories accumulate to the point
> > > > > xenstore
> > > > > returning E2BIG on listing them.
> > > > >
> > > > > Fix this by giving backend domain write access also to backend/
> > > > > directory itself when c_info->driver_domain option is set. The code
> > > > > removing relevant dir is already there (just lacked permissions to do
> > > > > so).
> > > > >
> > > > > Note this also allows the backend domain to create new entries,
> > > > > pretending to host backend devices it don't have. But since libxl uses
> > > > > /libxl/ xenstore dir for this information (still outside of backend
> > > > > domain control), this shouldn't be an issue.
> > > >
> > > > This seems quite hazardous to me. The reasoning you use to show that
> > > > this iws OK seems fragile, and in general it doesn't feel right to
> > > > give the particular backend such wide scope.
> > > >
> > > > Can we find another way to address this problem ? I think the
> > > > containing directory should be removed by the toolstack. Why is this
> > > > difficult ? (I presume there is a reason or you would have done it
> > > > that way...)
> > >
> > > It was done this way previously and caused issues, see this commit:
> > >
> > > commit 546678c6a60f64fb186640460dfa69a837c8fba5
> > > Author: Roger Pau Monne
> > > Date: Wed Sep 23 12:06:56 2015 +0200
> > >
> > > libxl: fix the cleanup of the backend path when using driver domains
> >
> > Thanks.
> >
> > > With the current libxl implementation the control domain will
> > > remove both the frontend and the backend xenstore paths of a
> > > device that's handled by a driver domain. This is incorrect,
> > > since the driver domain possibly needs to access the backend
> > > path in order to perform the disconnection and cleanup of the
> > > device.
> > >
> > > Fix this by making sure the control domain only cleans the
> > > frontend path, leaving the backend path to be cleaned by the
> > > driver domain. Note that if the device is not handled by a
> > > driver domain the control domain will perform the removal of
> > > both the frontend and the backend paths.
> >
> > Hmm. I see my Ack on that. Nevertheless maybe it is wrong.
> >
> > Looking at it afresh, I think maybe the right answer is:
> >
> > * If the driver domain is expected to be working properly, the
> >toolstack should wait for the driver domain to complete the device
> >shutdown, before removing the backend node. Indeed, the toolstack
> >ought to wait for this before actually destroying the guest in Xen,
> >by the usual logic for clean domain shutdown.
>
> I think that's not enough. .../state = 6 is set by the kernel, but
> xl devd in the driver domain may want to cleanup things (hotplug scripts
> etc). And indeed libxl__device_destroy() is called from
> device_hotplug_done(), not device_backend_callback().
>
> Alternatively, toolstack could wait for the actual backend node to be
> removed (by the driver domain), and then cleanup the parent directory (if
> empty).
I'm not sure you need to cleanup the parent directory, albeit it
wouldn't hurt. It needs to be done in a transaction though, so that
you don't race with new additions to it.
> I don't find it particularly appealing, as every contact with
> libxl async code reduce overall happiness...
>
> > * There needs to be a way to deal with a broken/unresponsive driver
> >domain. That will involve not waiting for the backend so must
> >involve simply deleting the backend from xenstore.
>
> It's already there: if driver domain fails to set .../state = 6 within
> a timeout, toolstack will forcibly remove the entry.
Would it work to change this and instead of monitor .../state = 6
monitor that the parent directory still exist?
> > Is the distinction here between "xl shutdown" and "xl destroy", on the
> > actual guest domain, good enough ? Hopefully if the driver domain
> > sees the backend directory simply vanish it can destructively tear
> > everything down ?
>
> In the past this lead to multiple issues, where hotplug script didn't
> k
Re: [Xen-devel] [PATCH] tools/xenstore: don't close connection in xs_talkv()
On 23/03/2020 14:29, Juergen Gross wrote: > In case of some errors xs_talkv() will close the connection to > Xenstore. This is annoying as it is not clear to the caller in which > error case the connection is still available. > > Drop that implicit closing to make the interface behave in a sane and > predictable way. > > Signed-off-by: Juergen Gross Reviewed-by: Andrew Cooper This definitely does improve the cascade failure cases.
Re: [Xen-devel] [PATCH] tools/xenstore: don't close connection in xs_talkv()
On 23/03/2020 15:44, Andrew Cooper wrote: > On 23/03/2020 14:29, Juergen Gross wrote: >> In case of some errors xs_talkv() will close the connection to >> Xenstore. This is annoying as it is not clear to the caller in which >> error case the connection is still available. >> >> Drop that implicit closing to make the interface behave in a sane and >> predictable way. >> >> Signed-off-by: Juergen Gross > Reviewed-by: Andrew Cooper > > This definitely does improve the cascade failure cases. > Actually, I spoke too soon. The EBADF goes, but the next read_message() ends up pulling junk out of the ring. I think something else in the middle needs teaching about hard errors, and not to pull more data out of the ring. ~Andrew
[Xen-devel] [PATCH v4 1/2] xen: Use evtchn_type_t as a type for event channels
Make event channel functions pass event channel port using
evtchn_port_t type. It eliminates signed <-> unsigned conversion.
Signed-off-by: Yan Yankovskyi
---
drivers/xen/events/events_2l.c| 16 ++---
drivers/xen/events/events_base.c | 93 ++-
drivers/xen/events/events_fifo.c | 22 +++
drivers/xen/events/events_internal.h | 30 -
drivers/xen/evtchn.c | 13 ++--
drivers/xen/gntdev-common.h | 3 +-
drivers/xen/gntdev.c | 2 +-
drivers/xen/pvcalls-back.c| 5 +-
drivers/xen/pvcalls-front.c | 15 +++--
drivers/xen/xen-pciback/xenbus.c | 7 +-
drivers/xen/xen-scsiback.c| 3 +-
drivers/xen/xenbus/xenbus_client.c| 6 +-
include/xen/events.h | 22 +++
include/xen/interface/event_channel.h | 2 +-
include/xen/xenbus.h | 5 +-
15 files changed, 128 insertions(+), 116 deletions(-)
diff --git a/drivers/xen/events/events_2l.c b/drivers/xen/events/events_2l.c
index 8edef51c92e5..64df919a2111 100644
--- a/drivers/xen/events/events_2l.c
+++ b/drivers/xen/events/events_2l.c
@@ -53,37 +53,37 @@ static void evtchn_2l_bind_to_cpu(struct irq_info *info,
unsigned cpu)
set_bit(info->evtchn, BM(per_cpu(cpu_evtchn_mask, cpu)));
}
-static void evtchn_2l_clear_pending(unsigned port)
+static void evtchn_2l_clear_pending(evtchn_port_t port)
{
struct shared_info *s = HYPERVISOR_shared_info;
sync_clear_bit(port, BM(&s->evtchn_pending[0]));
}
-static void evtchn_2l_set_pending(unsigned port)
+static void evtchn_2l_set_pending(evtchn_port_t port)
{
struct shared_info *s = HYPERVISOR_shared_info;
sync_set_bit(port, BM(&s->evtchn_pending[0]));
}
-static bool evtchn_2l_is_pending(unsigned port)
+static bool evtchn_2l_is_pending(evtchn_port_t port)
{
struct shared_info *s = HYPERVISOR_shared_info;
return sync_test_bit(port, BM(&s->evtchn_pending[0]));
}
-static bool evtchn_2l_test_and_set_mask(unsigned port)
+static bool evtchn_2l_test_and_set_mask(evtchn_port_t port)
{
struct shared_info *s = HYPERVISOR_shared_info;
return sync_test_and_set_bit(port, BM(&s->evtchn_mask[0]));
}
-static void evtchn_2l_mask(unsigned port)
+static void evtchn_2l_mask(evtchn_port_t port)
{
struct shared_info *s = HYPERVISOR_shared_info;
sync_set_bit(port, BM(&s->evtchn_mask[0]));
}
-static void evtchn_2l_unmask(unsigned port)
+static void evtchn_2l_unmask(evtchn_port_t port)
{
struct shared_info *s = HYPERVISOR_shared_info;
unsigned int cpu = get_cpu();
@@ -173,7 +173,7 @@ static void evtchn_2l_handle_events(unsigned cpu)
/* Timer interrupt has highest priority. */
irq = irq_from_virq(cpu, VIRQ_TIMER);
if (irq != -1) {
- unsigned int evtchn = evtchn_from_irq(irq);
+ evtchn_port_t evtchn = evtchn_from_irq(irq);
word_idx = evtchn / BITS_PER_LONG;
bit_idx = evtchn % BITS_PER_LONG;
if (active_evtchns(cpu, s, word_idx) & (1ULL << bit_idx))
@@ -228,7 +228,7 @@ static void evtchn_2l_handle_events(unsigned cpu)
do {
xen_ulong_t bits;
- int port;
+ evtchn_port_t port;
bits = MASK_LSBS(pending_bits, bit_idx);
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index 499eff7d3f65..3a791c8485d0 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -116,7 +116,7 @@ static void clear_evtchn_to_irq_all(void)
}
}
-static int set_evtchn_to_irq(unsigned evtchn, unsigned irq)
+static int set_evtchn_to_irq(evtchn_port_t evtchn, unsigned int irq)
{
unsigned row;
unsigned col;
@@ -143,7 +143,7 @@ static int set_evtchn_to_irq(unsigned evtchn, unsigned irq)
return 0;
}
-int get_evtchn_to_irq(unsigned evtchn)
+int get_evtchn_to_irq(evtchn_port_t evtchn)
{
if (evtchn >= xen_evtchn_max_channels())
return -1;
@@ -162,7 +162,7 @@ struct irq_info *info_for_irq(unsigned irq)
static int xen_irq_info_common_setup(struct irq_info *info,
unsigned irq,
enum xen_irq_type type,
-unsigned evtchn,
+evtchn_port_t evtchn,
unsigned short cpu)
{
int ret;
@@ -184,7 +184,7 @@ static int xen_irq_info_common_setup(struct irq_info *info,
}
static int xen_irq_info_evtchn_setup(unsigned irq,
-unsigned evtchn)
+evtchn_port_t evtchn)
{
struct irq_info *info = info_for_irq(irq);
@@ -193,7 +193,7 @@ static int xen_irq_info_evtchn_setup(unsigned irq,
static int xen_irq_info_ipi_setup(unsigned
Re: [Xen-devel] [PATCH] tools/xenstore: don't close connection in xs_talkv()
Sorry to come into this late.
Andrew Cooper writes ("Re: [Xen-devel] [PATCH] tools/xenstore: don't close
connection in xs_talkv()"):
> On 23/03/2020 15:44, Andrew Cooper wrote:
> > On 23/03/2020 14:29, Juergen Gross wrote:
> >> In case of some errors xs_talkv() will close the connection to
> >> Xenstore. This is annoying as it is not clear to the caller in which
> >> error case the connection is still available.
> >>
> >> Drop that implicit closing to make the interface behave in a sane and
> >> predictable way.
> >>
> >> Signed-off-by: Juergen Gross
> > Reviewed-by: Andrew Cooper
> >
> > This definitely does improve the cascade failure cases.
>
> Actually, I spoke too soon. The EBADF goes, but the next read_message()
> ends up pulling junk out of the ring.
I'm afraid this is predictable. For most of these errors there is
nothing else sensible that the client library could do. The
connection has been rendered unuseable. In principle I guess it could
reconnect to the socket, but this is a "should never happend" event.
EBADF is a worrying error because in many cases it means something has
messed up the process's fd space, which can easily lead to really bad
behaviours.
But having read the code in xenstore-ls, I see that as well as closing
the fd it sets the variable containing the fd number to -1. So all
future calls return EBADF.
I think this is correct.
Ian.
[Xen-devel] [RFC] hw/usb/xen-usb.c: Pass struct usbback_req* to usbback_packet_complete()
The function usbback_packet_complete() currently takes a USBPacket*,
which must be a pointer to the packet field within a struct
usbback_req; the function uses container_of() to get the struct
usbback_req* given the USBPacket*.
This is unnecessarily confusing (and in particular it confuses the
Coverity Scan analysis, resulting in the false positive CID 1421919
where it thinks that we write off the end of the structure). Since
both callsites already have the pointer to the struct usbback_req,
just pass that in directly.
Signed-off-by: Peter Maydell
---
This is an RFC because:
* I'm not very familiar with the Xen bits of QEMU
* the main rationale here is to change something that's
confusing Coverity -- the code as it stands isn't wrong
* the only testing I've done is "make check"
Still, the change seems like a good thing to me as a human reader...
PS: QEMU's MAINTAINERS file stanza for Xen doesn't pick up
that this file is Xen related, so it could use an extra F: line.
hw/usb/xen-usb.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
index 1fc2f32ce93..961190d0f78 100644
--- a/hw/usb/xen-usb.c
+++ b/hw/usb/xen-usb.c
@@ -347,13 +347,11 @@ static int32_t usbback_xlat_status(int status)
return -ESHUTDOWN;
}
-static void usbback_packet_complete(USBPacket *packet)
+static void usbback_packet_complete(struct usbback_req *usbback_req)
{
-struct usbback_req *usbback_req;
+USBPacket *packet = &usbback_req->packet;
int32_t status;
-usbback_req = container_of(packet, struct usbback_req, packet);
-
QTAILQ_REMOVE(&usbback_req->stub->submit_q, usbback_req, q);
status = usbback_xlat_status(packet->status);
@@ -566,7 +564,7 @@ static void usbback_dispatch(struct usbback_req
*usbback_req)
usb_handle_packet(usbback_req->stub->dev, &usbback_req->packet);
if (usbback_req->packet.status != USB_RET_ASYNC) {
-usbback_packet_complete(&usbback_req->packet);
+usbback_packet_complete(usbback_req);
}
return;
@@ -993,7 +991,7 @@ static void xen_bus_complete(USBPort *port, USBPacket
*packet)
usbif = usbback_req->usbif;
TR_REQ(&usbif->xendev, "\n");
-usbback_packet_complete(packet);
+usbback_packet_complete(usbback_req);
}
static USBPortOps xen_usb_port_ops = {
--
2.20.1
[Xen-devel] [PATCH v12 1/3] xen/mem_sharing: VM forking
VM forking is the process of creating a domain with an empty memory space and a
parent domain specified from which to populate the memory when necessary. For
the new domain to be functional the VM state is copied over as part of the fork
operation (HVM params, hap allocation, etc).
Signed-off-by: Tamas K Lengyel
---
v12: Minor style adjustments Jan pointed out
Convert mem_sharing_is_fork to inline function
v11: Fully copy vcpu_info pages
Setup vcpu_runstate for forks
Added TODO note for PV timers
Copy shared_info page
Add copy_settings function, to be shared with fork_reset in the next patch
---
xen/arch/x86/domain.c | 11 +
xen/arch/x86/hvm/hvm.c| 4 +-
xen/arch/x86/mm/hap/hap.c | 3 +-
xen/arch/x86/mm/mem_sharing.c | 368 ++
xen/arch/x86/mm/p2m.c | 9 +-
xen/common/domain.c | 3 +
xen/include/asm-x86/hap.h | 1 +
xen/include/asm-x86/hvm/hvm.h | 2 +
xen/include/asm-x86/mem_sharing.h | 18 ++
xen/include/public/memory.h | 5 +
xen/include/xen/sched.h | 5 +
11 files changed, 424 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index caf2ecad7e..11d3c2216e 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -2202,6 +2202,17 @@ int domain_relinquish_resources(struct domain *d)
ret = relinquish_shared_pages(d);
if ( ret )
return ret;
+
+/*
+ * If the domain is forked, decrement the parent's pause count
+ * and release the domain.
+ */
+if ( mem_sharing_is_fork(d) )
+{
+domain_unpause(d->parent);
+put_domain(d->parent);
+d->parent = NULL;
+}
}
#endif
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index a3d115b650..304b3d1562 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1917,7 +1917,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long
gla,
}
#endif
-/* Spurious fault? PoD and log-dirty also take this path. */
+/* Spurious fault? PoD, log-dirty and VM forking also take this path. */
if ( p2m_is_ram(p2mt) )
{
rc = 1;
@@ -4377,7 +4377,7 @@ static int hvm_allow_get_param(struct domain *d,
return rc;
}
-static int hvm_get_param(struct domain *d, uint32_t index, uint64_t *value)
+int hvm_get_param(struct domain *d, uint32_t index, uint64_t *value)
{
int rc;
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index a6d5e39b02..814d0c3253 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -321,8 +321,7 @@ static void hap_free_p2m_page(struct domain *d, struct
page_info *pg)
}
/* Return the size of the pool, rounded up to the nearest MB */
-static unsigned int
-hap_get_allocation(struct domain *d)
+unsigned int hap_get_allocation(struct domain *d)
{
unsigned int pg = d->arch.paging.hap.total_pages
+ d->arch.paging.hap.p2m_pages;
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 3835bc928f..23deeddff2 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -22,6 +22,7 @@
#include
#include
+#include
#include
#include
#include
@@ -36,6 +37,8 @@
#include
#include
#include
+#include
+#include
#include
#include "mm-locks.h"
@@ -1444,6 +1447,334 @@ static inline int mem_sharing_control(struct domain *d,
bool enable)
return 0;
}
+/*
+ * Forking a page only gets called when the VM faults due to no entry being
+ * in the EPT for the access. Depending on the type of access we either
+ * populate the physmap with a shared entry for read-only access or
+ * fork the page if its a write access.
+ *
+ * The client p2m is already locked so we only need to lock
+ * the parent's here.
+ */
+int mem_sharing_fork_page(struct domain *d, gfn_t gfn, bool unsharing)
+{
+int rc = -ENOENT;
+shr_handle_t handle;
+struct domain *parent = d->parent;
+struct p2m_domain *p2m;
+unsigned long gfn_l = gfn_x(gfn);
+mfn_t mfn, new_mfn;
+p2m_type_t p2mt;
+struct page_info *page;
+
+if ( !mem_sharing_is_fork(d) )
+return -ENOENT;
+
+if ( !unsharing )
+{
+/* For read-only accesses we just add a shared entry to the physmap */
+while ( parent )
+{
+if ( !(rc = nominate_page(parent, gfn, 0, &handle)) )
+break;
+
+parent = parent->parent;
+}
+
+if ( !rc )
+{
+/* The client's p2m is already locked */
+struct p2m_domain *pp2m = p2m_get_hostp2m(parent);
+
+p2m_lock(pp2m);
+rc = add_to_physmap(parent, gfn_l, handle, d, gfn_l, false);
+p2m_unlock(pp2m);
+
+if ( !rc )
+return 0;
+}
+}
+
+
[Xen-devel] [PATCH v12 3/3] xen/tools: VM forking toolstack side
Add necessary bits to implement "xl fork-vm" commands. The command allows the
user to specify how to launch the device model allowing for a late-launch model
in which the user can execute the fork without the device model and decide to
only later launch it.
Signed-off-by: Tamas K Lengyel
---
docs/man/xl.1.pod.in | 44 +
tools/libxc/include/xenctrl.h | 13 ++
tools/libxc/xc_memshr.c | 22 +++
tools/libxl/libxl.h | 11 ++
tools/libxl/libxl_create.c| 361 +++---
tools/libxl/libxl_dm.c| 2 +-
tools/libxl/libxl_dom.c | 43 +++-
tools/libxl/libxl_internal.h | 7 +
tools/libxl/libxl_types.idl | 1 +
tools/libxl/libxl_x86.c | 41
tools/xl/Makefile | 2 +-
tools/xl/xl.h | 5 +
tools/xl/xl_cmdtable.c| 15 ++
tools/xl/xl_forkvm.c | 147 ++
tools/xl/xl_vmcontrol.c | 14 ++
15 files changed, 562 insertions(+), 166 deletions(-)
create mode 100644 tools/xl/xl_forkvm.c
diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
index 09339282e6..59c03c6427 100644
--- a/docs/man/xl.1.pod.in
+++ b/docs/man/xl.1.pod.in
@@ -708,6 +708,50 @@ above).
=back
+=item B [I] I
+
+Create a fork of a running VM. The domain will be paused after the operation
+and remains paused while forks of it exist. Experimental and x86 only.
+Forks can only be made of domains with HAP enabled and on Intel hardware. The
+parent domain must be created with the xl toolstack and its configuration must
+not manually define max_grant_frames, max_maptrack_frames or
max_event_channels.
+
+B
+
+=over 4
+
+=item B<-p>
+
+Leave the fork paused after creating it.
+
+=item B<--launch-dm>
+
+Specify whether the device model (QEMU) should be launched for the fork. Late
+launch allows to start the device model for an already running fork.
+
+=item B<-C>
+
+The config file to use when launching the device model. Currently required
when
+launching the device model. Most config settings MUST match the parent domain
+exactly, only change VM name, disk path and network configurations.
+
+=item B<-Q>
+
+The path to the qemu save file to use when launching the device model.
Currently
+required when launching the device model.
+
+=item B<--fork-reset>
+
+Perform a reset operation of an already running fork. Note that resetting may
+be less performant then creating a new fork depending on how much memory the
+fork has deduplicated during its runtime.
+
+=item B<--max-vcpus>
+
+Specify the max-vcpus matching the parent domain when not launching the dm.
+
+=back
+
=item B [I]
Display the number of shared pages for a specified domain. If no domain is
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index fc6e57a1a0..00cb4cf1f7 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -2225,6 +2225,19 @@ int xc_memshr_range_share(xc_interface *xch,
uint64_t first_gfn,
uint64_t last_gfn);
+int xc_memshr_fork(xc_interface *xch,
+ uint32_t source_domain,
+ uint32_t client_domain);
+
+/*
+ * Note: this function is only intended to be used on short-lived forks that
+ * haven't yet aquired a lot of memory. In case the fork has a lot of memory
+ * it is likely more performant to create a new fork with xc_memshr_fork.
+ *
+ * With VMs that have a lot of memory this call may block for a long time.
+ */
+int xc_memshr_fork_reset(xc_interface *xch, uint32_t forked_domain);
+
/* Debug calls: return the number of pages referencing the shared frame backing
* the input argument. Should be one or greater.
*
diff --git a/tools/libxc/xc_memshr.c b/tools/libxc/xc_memshr.c
index 97e2e6a8d9..d0e4ee225b 100644
--- a/tools/libxc/xc_memshr.c
+++ b/tools/libxc/xc_memshr.c
@@ -239,6 +239,28 @@ int xc_memshr_debug_gref(xc_interface *xch,
return xc_memshr_memop(xch, domid, &mso);
}
+int xc_memshr_fork(xc_interface *xch, uint32_t pdomid, uint32_t domid)
+{
+xen_mem_sharing_op_t mso;
+
+memset(&mso, 0, sizeof(mso));
+
+mso.op = XENMEM_sharing_op_fork;
+mso.u.fork.parent_domain = pdomid;
+
+return xc_memshr_memop(xch, domid, &mso);
+}
+
+int xc_memshr_fork_reset(xc_interface *xch, uint32_t domid)
+{
+xen_mem_sharing_op_t mso;
+
+memset(&mso, 0, sizeof(mso));
+mso.op = XENMEM_sharing_op_fork_reset;
+
+return xc_memshr_memop(xch, domid, &mso);
+}
+
int xc_memshr_audit(xc_interface *xch)
{
xen_mem_sharing_op_t mso;
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 71709dc585..088e81c78b 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -2666,6 +2666,17 @@ int libxl_psr_get_hw_info(libxl_ctx *ctx,
libxl_psr_feat_type type,
unsigned int lvl, unsigned int *nr,
libxl_psr_hw_info **info);
void libxl_psr_hw_info_list_free(libxl_psr_hw_info *list, unsigned in
[Xen-devel] [PATCH v12 0/3] VM forking
The following series implements VM forking for Intel HVM guests to allow for
the fast creation of identical VMs without the assosciated high startup costs
of booting or restoring the VM from a savefile.
JIRA issue: https://xenproject.atlassian.net/browse/XEN-89
The fork operation is implemented as part of the "xl fork-vm" command:
xl fork-vm -C -Q -m
By default a fully functional fork is created. The user is in charge however to
create the appropriate config file for the fork and to generate the QEMU save
file before the fork-vm call is made. The config file needs to give the
fork a new name at minimum but other settings may also require changes. Certain
settings in the config file of both the parent and the fork have to be set to
default. Details are documented.
The interface also allows to split the forking into two steps:
xl fork-vm --launch-dm no \
-m \
-p
xl fork-vm --launch-dm late \
-C \
-Q \
The split creation model is useful when the VM needs to be created as fast as
possible. The forked VM can be unpaused without the device model being launched
to be monitored and accessed via VMI. Note however that without its device
model running (depending on what is executing in the VM) it is bound to
misbehave or even crash when its trying to access devices that would be
emulated by QEMU. We anticipate that for certain use-cases this would be an
acceptable situation, in case for example when fuzzing is performed of code
segments that don't access such devices.
Launching the device model requires the QEMU Xen savefile to be generated
manually from the parent VM. This can be accomplished simply by connecting to
its QMP socket and issuing the "xen-save-devices-state" command. For example
using the standard tool socat these commands can be used to generate the file:
socat - UNIX-CONNECT:/var/run/xen/qmp-libxl-
{ "execute": "qmp_capabilities" }
{ "execute": "xen-save-devices-state", \
"arguments": { "filename": "/path/to/save/qemu_state", \
"live": false} }
At runtime the forked VM starts running with an empty p2m which gets lazily
populated when the VM generates EPT faults, similar to how altp2m views are
populated. If the memory access is a read-only access, the p2m entry is
populated with a memory shared entry with its parent. For write memory accesses
or in case memory sharing wasn't possible (for example in case a reference is
held by a third party), a new page is allocated and the page contents are
copied over from the parent VM. Forks can be further forked if needed, thus
allowing for further memory savings.
A VM fork reset hypercall is also added that allows the fork to be reset to the
state it was just after a fork, also accessible via xl:
xl fork-vm --fork-reset -p
This is an optimization for cases where the forks are very short-lived and run
without a device model, so resetting saves some time compared to creating a
brand new fork provided the fork has not aquired a lot of memory. If the fork
has a lot of memory deduplicated it is likely going to be faster to create a
new fork from scratch and asynchronously destroying the old one.
The series has been tested with Windows VMs and functions as expected. Linux
VMs when forked from a running VM will have a frozen VNC screen. Linux VMs at
this time can only be forked with a working device model when the parent VM was
restored from a snapshot using "xl restore -p". This is a known limitation.
Also note that PVHVM/PVH Linux guests have not been tested. Forking most likely
works but PV devices and drivers would require additional wiring to set things
up properly since the guests are unaware of the forking taking place, unlike
the save/restore routine where the guest is made aware of the procedure.
Forking time has been measured to be 0.0007s, device model launch to be around
1s depending largely on the number of devices being emulated. Fork resets have
been measured to be 0.0001s under the optimal circumstances.
New in v12:
style cleanups & minor adjustments
removing contiuation for fork reset and add TODO comment
Patch 1 implements the VM fork
Patch 2 implements fork reset operation
Patch 3 adds the toolstack-side code implementing VM forking and reset
Tamas K Lengyel (3):
xen/mem_sharing: VM forking
x86/mem_sharing: reset a fork
xen/tools: VM forking toolstack side
docs/man/xl.1.pod.in | 44 +++
tools/libxc/include/xenctrl.h | 13 +
tools/libxc/xc_memshr.c | 22 ++
tools/libxl/libxl.h | 11 +
tools/libxl/libxl_create.c| 361 +---
tools/libxl/libxl_dm.c| 2 +-
tools/libxl/libxl_dom.c | 43 ++-
tools/libxl/libxl_internal.h | 7 +
tools/libxl/libxl_types.idl | 1 +
tools/libxl/libxl_x86.c | 41 +++
tools/xl/Makefile | 2 +-
tools/xl/xl.h
[Xen-devel] [PATCH v12 2/3] x86/mem_sharing: reset a fork
Implement hypercall that allows a fork to shed all memory that got allocated
for it during its execution and re-load its vCPU context from the parent VM.
This allows the forked VM to reset into the same state the parent VM is in a
faster way then creating a new fork would be. Measurements show about a 2x
speedup during normal fuzzing operations. Performance may vary depending how
much memory got allocated for the forked VM. If it has been completely
deduplicated from the parent VM then creating a new fork would likely be more
performant.
Signed-off-by: Tamas K Lengyel
---
v12: remove continuation & add comment back
address style issues pointed out by Jan
---
xen/arch/x86/mm/mem_sharing.c | 77 +++
xen/include/public/memory.h | 1 +
2 files changed, 78 insertions(+)
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 23deeddff2..930a5f58ef 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -1775,6 +1775,60 @@ static int fork(struct domain *cd, struct domain *d)
return rc;
}
+/*
+ * The fork reset operation is intended to be used on short-lived forks only.
+ * There is no hypercall continuation operation implemented for this reason.
+ * For forks that obtain a larger memory footprint it is likely going to be
+ * more performant to create a new fork instead of resetting an existing one.
+ *
+ * TODO: In case this hypercall would become useful on forks with larger memory
+ * footprints the hypercall continuation should be implemented (or if this
+ * feature needs to be become "stable").
+ */
+static int mem_sharing_fork_reset(struct domain *d, struct domain *pd)
+{
+int rc;
+struct p2m_domain *p2m = p2m_get_hostp2m(d);
+struct page_info *page, *tmp;
+
+spin_lock(&d->page_alloc_lock);
+domain_pause(d);
+
+page_list_for_each_safe(page, tmp, &d->page_list)
+{
+p2m_type_t p2mt;
+p2m_access_t p2ma;
+mfn_t mfn = page_to_mfn(page);
+gfn_t gfn = mfn_to_gfn(d, mfn);
+
+mfn = __get_gfn_type_access(p2m, gfn_x(gfn), &p2mt, &p2ma,
+0, NULL, false);
+
+/* only reset pages that are sharable */
+if ( !p2m_is_sharable(p2mt) )
+continue;
+
+/* take an extra reference or just skip if can't for whatever reason */
+if ( !get_page(page, d) )
+continue;
+
+/* forked memory is 4k, not splitting large pages so this must work */
+rc = p2m->set_entry(p2m, gfn, INVALID_MFN, PAGE_ORDER_4K,
+p2m_invalid, p2m_access_rwx, -1);
+ASSERT(!rc);
+
+put_page_alloc_ref(page);
+put_page(page);
+}
+
+rc = copy_settings(d, pd);
+
+domain_unpause(d);
+spin_unlock(&d->page_alloc_lock);
+
+return rc;
+}
+
int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
{
int rc;
@@ -2066,6 +2120,29 @@ int
mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg)
break;
}
+case XENMEM_sharing_op_fork_reset:
+{
+struct domain *pd;
+
+rc = -EINVAL;
+if ( mso.u.fork.pad[0] || mso.u.fork.pad[1] ||
+ mso.u.fork.pad[2] )
+goto out;
+
+rc = -ENOSYS;
+if ( !d->parent )
+goto out;
+
+rc = rcu_lock_live_remote_domain_by_id(d->parent->domain_id, &pd);
+if ( rc )
+goto out;
+
+rc = mem_sharing_fork_reset(d, pd);
+
+rcu_unlock_domain(pd);
+break;
+}
+
default:
rc = -ENOSYS;
break;
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index 5ee4e0da12..d36d64b8dc 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -483,6 +483,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_access_op_t);
#define XENMEM_sharing_op_audit 7
#define XENMEM_sharing_op_range_share 8
#define XENMEM_sharing_op_fork 9
+#define XENMEM_sharing_op_fork_reset10
#define XENMEM_SHARING_OP_S_HANDLE_INVALID (-10)
#define XENMEM_SHARING_OP_C_HANDLE_INVALID (-9)
--
2.20.1
[Xen-devel] [xen-unstable test] 148873: tolerable trouble: fail/pass/starved - PUSHED
flight 148873 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/148873/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail like 148611 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 148611 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 148611 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 148611 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 148611 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail like 148611 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 148611 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 148611 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 148611 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 148611 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 148611 test-arm64-arm64-xl-seattle 13 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 14 saverestore-support-checkfail 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-xl-pvshim12 guest-start fail never pass test-amd64-i386-libvirt 13 migrate-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-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail 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-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-credit1 13 migrate-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-credit1 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-amd64-amd64-libvirt-vhd 12 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-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-rtds 13 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 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-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-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 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-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-dom0pvh-xl-amd 2 hosts-allocate starved n/a version targeted for testing: xen 60d6ba1916dce0622a53b00dbae3c01d0761057e baseline version: xen d094e95fb7c61c5f46d8e446b4bdc028438dea1c Last test of basis 148611 2020-03-16 01:51:12 Z7 days Failing since148636 2020-03-16 18:36:29 Z6 days9 attempts Testing same since 148826 2020-03-22 02:13:50 Z1 days2 attempts People who touched r
[Xen-devel] Network performance issues on Qubes OS Server prototype
Hi all,
I'm currently working on a Qubes OS server version (example architecture can
been seen at
https://raw.githubusercontent.com/fepitre/qubes-mgmt-salt-qubes-server/devel-140320-extra/qubes-server.png).
I'm using this configuration since several months on Qubes R4.0 (xen-4.8) and
recently on Qubes R4.1 (xen-4.13). I'm writing to you because since the
beginning I'm having network performance issues that I never succeeded to solve.
This setup is done on a HP Gen8 DL360p with 2*CPUs, 160GB memory, 1TB RAID6 SAS.
On the picture I linked you, all the colored rectangles {zone}-* for zone in
(wan, dmz, lan, admin) are PVH VMs (Debian 10). There exist a VM not drawn in
the picture, called 'sys-net-interfaces' which holds four 1Gbits Ethernet
controllers of the server using PCI passthrough. It is a HVM with Linux-based
stubdomain.
All the inner links between VMs are NAT interfaces. All the outer links on
*-sys-net VMs are BRIDGE interfaces with backend 'sys-net-interfaces'. In VM
'sys-net-interfaces' a LACP bond0 is done with two Ethernet controllers, which
is a trunk, then several vlan interfaces are generated with parent device this
bond, and finally, bridges are created and associated to those vlans.
Here are my issues. Consider one computer named 'PC-LAN' in LAN network and
another 'PC-DMZ' in DMZ network. The considered network path is the following:
PC-LAN (1) <-- B --> lan-sys-net (2) <-- N --> lan-sys-firewall (3) <--
N --> dmz-sys-firewall (4) <-- N --> dmz-sys-net (5) <-- B --> PC-DMZ (6)
where B denotes bridge interface, N denotes NAT interface and numbers for
numbering machines. Up to 'wget', 'scp' (limited normally by ciphers), etc., I
ran multiple iperf3 tests over 20 seconds for having a clearer view of network
issues.
Example 1: Full path
From (1) to (6): 165 Mbits/s
From (2) to (6): 196 Mbits/s
From (3) to (6): 205 Mbits/s
From (4) to (6): 203 Mbits/s
From (5) to (6): 714 Mbits/s
Example 2: 'dmz-sys-net' as end node
From (1) to (5): 194 Mbits/s
From (2) to (5): 189 Mbits/s
From (3) to (5): 258 Mbits/s
From (4) to (5): 500 Mbits/s
Example 3: 'lan-sys-net' as end node
From (1) to (2): 830 Mbits/s
I've another HP Gen8 with almost the same physical configuration and network
configuration (LACP+vlan+bridges) running under Debian 10 as bare metal KVM,
and I obtain 1Gbits/s network workflows over bridges. The almost physical
configuration is due to the related mail I sent you in july 2019 '[Xen-devel]
Ethernet PCI passthrough problem'. The provided Ethernet card with 4 ports (HP
Ethernet 1Gb 4-port 331FLR Adapter) makes the driver tg3 crashing when
attaching those into a VM. So the Debian KVM has those HP Ethernet controllers
whereas on the Qubes server, it has a cheap PCI express 4 Ethernet Realtek 8169
card.
Of course physical connections on the switches have been changed, 'switched'
between the two servers for eliminating any hardware problem.
I had a look to
https://wiki.xen.org/wiki/Network_Throughput_and_Performance_Guide.
Unfortunately, trying some change of options with 'ethtool' in
'sys-net-interfaces', changing amount of RAM/VCPUs of it and other *-sys-net,
does not do that much.
I'm writing to you for having some clues into where I can dig and what I can
look in order to put in evidence the bottleneck. If it's purely dom0 side or
backend network VM side (sys-net-interfaces) or elsewhere.
I would like to thank you a lot in advance for any help on this problem.
Best regards,
Frédéric
signature.asc
Description: OpenPGP digital signature
[Xen-devel] [seabios bisection] complete test-amd64-amd64-qemuu-nested-intel
branch xen-unstable xenbranch xen-unstable job test-amd64-amd64-qemuu-nested-intel testid debian-hvm-install/l1/l2 Tree: linux git://xenbits.xen.org/linux-pvops.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios https://git.seabios.org/seabios.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: xen git://xenbits.xen.org/xen.git Bug introduced: 59e89cdabc71b5c3a956028ef1c439e6bae947f0 Bug not present: 6dacdcd439c1ddd32110d4a008de346e367409ec Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/148928/ commit 59e89cdabc71b5c3a956028ef1c439e6bae947f0 Author: Andrew Cooper Date: Thu Dec 20 17:25:29 2018 + x86/vtx: Disable executable EPT superpages to work around CVE-2018-12207 CVE-2018-12207 covers a set of errata on various Intel processors, whereby a machine check exception can be generated in a corner case when an executable mapping changes size or cacheability without TLB invalidation. HVM guest kernels can trigger this to DoS the host. To mitigate, in affected hardware, all EPT superpages are marked NX. When an instruction fetch violation is observed against the superpage, the superpage is shattered to 4k and has execute permissions restored. This prevents the guest kernel from being able to create the necessary preconditions in the iTLB to exploit the vulnerability. This does come with a workload-dependent performance overhead, caused by increased TLB pressure. Performance can be restored, if guest kernels are trusted not to mount an attack, by specifying ept=exec-sp on the command line. This is part of XSA-304 / CVE-2018-12207 Signed-off-by: Andrew Cooper Acked-by: George Dunlap Reviewed-by: Jan Beulich For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/seabios/test-amd64-amd64-qemuu-nested-intel.debian-hvm-install--l1--l2.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/seabios/test-amd64-amd64-qemuu-nested-intel.debian-hvm-install--l1--l2 --summary-out=tmp/148928.bisection-summary --basis-template=148666 --blessings=real,real-bisect seabios test-amd64-amd64-qemuu-nested-intel debian-hvm-install/l1/l2 Searching for failure / basis pass: 148853 fail [host=debina1] / 148690 [host=huxelrebe0] 148666 [host=godello1] 148646 [host=godello0] 148176 [host=italia0] 146357 [host=godello0] 146064 [host=godello1] 144665 [host=albana1] 144198 [host=elbling0] 144105 [host=godello0] 144081 [host=godello1] 143876 ok. Failure / basis pass flights: 148853 / 143876 (tree with no url: minios) Tree: linux git://xenbits.xen.org/linux-pvops.git Tree: linuxfirmware git://xenbits.xen.org/osstest/linux-firmware.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemu git://xenbits.xen.org/qemu-xen-traditional.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios https://git.seabios.org/seabios.git Tree: xen git://xenbits.xen.org/xen.git Latest c3038e718a19fc596f7b1baba0f83d5146dc7784 c530a75c1e6a472b0eb9558310b518f0dfcd8860 0c8ea9fe1adbbee230ee0c68f28b68ca2b0534bc d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef de88a9628426e82f1cee4b61b06e67e6787301b1 d094e95fb7c61c5f46d8e446b4bdc028438dea1c Basis pass b98aebd298246df37b472c52a2ee1023256d02e3 c530a75c1e6a472b0eb9558310b518f0dfcd8860 8d3f428109623096cb8845779cdf9dc44949b8e9 d0d8ad39ecb51cd7497cd524484fe09f50876798 933ebad2470a169504799a1d95b8e410bd9847ef 9caa19be0e534c687081fbdfcd301406e728c98c 518c935fac4d30b3ec35d4b6add82b17b7d7aca3 Generating revisions with ./adhoc-revtuple-generator git://xenbits.xen.org/linux-pvops.git#b98aebd298246df37b472c52a2ee1023256d02e3-c3038e718a19fc596f7b1baba0f83d5146dc7784 git://xenbits.xen.org/osstest/linux-firmware.git#c530a75c1e6a472b0eb9558310b518f0dfcd8860-c530a75c1e6a472b0eb9558310b518f0dfcd8860 git://xenbits.xen.org/osstest/ovmf.git#8d3f428109623096cb8845779cdf9dc44949b8e9-0c8ea9fe1adbbee230ee0c68f28b68ca2b0534bc git://xenbits.xen.org/qemu-xen-traditional.git#d0d8ad39ecb51cd7497cd524484\ fe09f50876798-d0d8ad39ecb51cd7497cd524484fe09f50876798 git://xenbits.xen.org/qemu-xen.git#933ebad2470a169504799a1d95b8e410bd9847ef-933ebad2470a169504799a1d95b8e410bd9847ef https://git.seabios.org/seabios.git#9caa19be0e534c687081fbdfcd301406e728c98c-de88a9628426e82f1cee4b61b06e67e6787301b1 git://xenbits.xen.org/xen.git#518c935fac4d30b3ec35d4b6add82b17b7d7aca3-d094e95fb7c61c5f46d8e446b4bdc028438dea1c adhoc-revtuple-generator: tree discontiguous: linux-pvops
[Xen-devel] PCIe IOMMU ACS support
Hi! I was going through how Xen support PCIe IOMMU ACS and all I could find is this: https://github.com/xen-project/xen/blob/master/xen/drivers/passthrough/pci.c#L608 which looks to me as an attempt of enabling ACS opportunistically, but still proceeding forward even if it fails. Am I missing something here? IOW, does Xen try to do any kind of fine grained ACS differentiation along the lines of what linux kernel is doing: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/quirks.c#n4299 Thanks, Roman.
Re: [Xen-devel] [PATCH v4 1/2] xen: Use evtchn_type_t as a type for event channels
On 3/23/20 12:15 PM, Yan Yankovskyi wrote: > Make event channel functions pass event channel port using > evtchn_port_t type. It eliminates signed <-> unsigned conversion. > > Signed-off-by: Yan Yankovskyi If the difference is only the whitespace fix then Reviewed-by: Boris Ostrovsky and I am sorry, I should have explicitly said that you don't need to resend.
[Xen-devel] [qemu-mainline test] 148879: regressions - trouble: fail/pass/starved
flight 148879 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/148879/ 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. 144861 test-amd64-amd64-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 144861 test-amd64-amd64-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-i386-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-amd64-qemuu-nested-amd 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-i386-freebsd10-i386 11 guest-startfail REGR. vs. 144861 test-amd64-i386-qemuu-rhel6hvm-intel 10 redhat-install fail REGR. vs. 144861 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-amd64-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 144861 test-amd64-amd64-xl-qemuu-debianhvm-amd64 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-i386-qemuu-rhel6hvm-amd 10 redhat-install fail REGR. vs. 144861 test-amd64-i386-xl-qemuu-ws16-amd64 10 windows-install fail REGR. vs. 144861 test-amd64-amd64-libvirt 12 guest-start fail REGR. vs. 144861 test-amd64-i386-xl-qemuu-win7-amd64 10 windows-install fail REGR. vs. 144861 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-amd64-qemuu-nested-intel 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-i386-xl-qemuu-ovmf-amd64 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 10 debian-hvm-install fail REGR. vs. 144861 test-amd64-amd64-libvirt-xsm 12 guest-start fail REGR. vs. 144861 test-amd64-i386-freebsd10-amd64 11 guest-start fail REGR. vs. 144861 test-amd64-i386-libvirt-pair 21 guest-start/debian fail REGR. vs. 144861 test-amd64-amd64-libvirt-pair 21 guest-start/debian fail REGR. vs. 144861 test-amd64-i386-libvirt-xsm 12 guest-start fail REGR. vs. 144861 test-amd64-i386-libvirt 12 guest-start fail REGR. vs. 144861 test-arm64-arm64-libvirt-xsm 12 guest-start fail REGR. vs. 144861 test-armhf-armhf-libvirt 12 guest-start fail REGR. vs. 144861 test-amd64-amd64-libvirt-vhd 10 debian-di-installfail REGR. vs. 144861 test-amd64-amd64-xl-qcow210 debian-di-installfail REGR. vs. 144861 test-armhf-armhf-libvirt-raw 10 debian-di-installfail REGR. vs. 144861 test-armhf-armhf-xl-vhd 10 debian-di-installfail REGR. vs. 144861 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds 15 guest-stop fail REGR. vs. 144861 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-xsm 13 migrate-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 13 migrate-support-checkfail never pass test-arm64-arm64-xl 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-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-thunderx 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 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 13 migrate-support-checkfail never pass test-armhf-armhf-xl 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-rtds 13 migrate-support-checkf
[Xen-devel] [examine test] 148921: tolerable trouble: pass/starved
flight 148921 examine real [real] http://logs.test-lab.xenproject.org/osstest/logs/148921/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: examine-fiano12 hosts-allocate starved n/a examine-huxelrebe12 hosts-allocate starved n/a examine-elbling0 2 hosts-allocate starved n/a examine-debina1 2 hosts-allocate starved n/a examine-godello1 2 hosts-allocate starved n/a examine-rimava1 2 hosts-allocate starved n/a examine-huxelrebe02 hosts-allocate starved n/a examine-italia0 2 hosts-allocate starved n/a examine-debina0 2 hosts-allocate starved n/a examine-fiano02 hosts-allocate starved n/a examine-albana1 2 hosts-allocate starved n/a examine-albana0 2 hosts-allocate starved n/a examine-chardonnay1 2 hosts-allocate starved n/a examine-chardonnay0 2 hosts-allocate starved n/a examine-godello0 2 hosts-allocate starved n/a examine-elbling1 2 hosts-allocate starved n/a baseline version: flight 147499 jobs: examine-albana0 starved examine-albana1 starved examine-arndale-bluewaterpass examine-cubietruck-braquepass examine-chardonnay0 starved examine-chardonnay1 starved examine-debina0 starved examine-debina1 starved examine-elbling0 starved examine-elbling1 starved examine-fiano0 starved examine-fiano1 starved examine-cubietruck-gleizes pass examine-godello0 starved examine-godello1 starved examine-huxelrebe0 starved examine-huxelrebe1 starved examine-italia0 starved examine-arndale-lakeside pass examine-laxton0 pass examine-laxton1 pass examine-arndale-metrocentre pass examine-cubietruck-metzinger pass examine-cubietruck-picasso pass examine-rimava1 starved examine-rochester0 pass examine-rochester1 pass examine-arndale-westfieldpass 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 Push not applicable.
Re: [Xen-devel] [XEN PATCH] mismatch between pyxc_methods flags and PyObject definitions
On Tue, Mar 17, 2020 at 11:01:43PM +, YOUNG, MICHAEL A. wrote:
> pygrub in xen-4.13.0 with python 3.8.2 fails with the error
>
> Traceback (most recent call last):
> File "/usr/libexec/xen/bin/pygrub", line 21, in
> import xen.lowlevel.xc
> SystemError: bad call flags
>
> This patch fixes mismatches in tools/python/xen/lowlevel/xc/xc.c
> between the flag bits defined in pyxc_methods and the parameters passed
> to the corresponding PyObject definitions.
>
> With this patch applied pygrub works as expected.
>
> Signed-off-by: Michael Young
This looks like a change in Python 3.7 (according to the documentation,
might not be enforced there yet). Python <= 3.6 allowed METH_KEYWORDS
used alone. Fortunately, all the versions supports METH_VARARGS |
METH_KEYWORDS, which looks to be equivalent to old METH_KEYWORDS alone.
Acked-by: Marek Marczykowski-Górecki
> ---
> tools/python/xen/lowlevel/xc/xc.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/tools/python/xen/lowlevel/xc/xc.c
> b/tools/python/xen/lowlevel/xc/xc.c
> index ac0e26a742..8fde5f311f 100644
> --- a/tools/python/xen/lowlevel/xc/xc.c
> +++ b/tools/python/xen/lowlevel/xc/xc.c
> @@ -2028,7 +2028,7 @@ static PyMethodDef pyxc_methods[] = {
>
> { "gnttab_hvm_seed",
>(PyCFunction)pyxc_gnttab_hvm_seed,
> - METH_KEYWORDS, "\n"
> + METH_VARARGS | METH_KEYWORDS, "\n"
>"Initialise HVM guest grant table.\n"
>" dom [int]: Identifier of domain to build into.\n"
>" console_gmfn [int]: \n"
> @@ -2097,7 +2097,7 @@ static PyMethodDef pyxc_methods[] = {
>
> { "sched_credit_domain_set",
>(PyCFunction)pyxc_sched_credit_domain_set,
> - METH_KEYWORDS, "\n"
> + METH_VARARGS | METH_KEYWORDS, "\n"
>"Set the scheduling parameters for a domain when running with the\n"
>"SMP credit scheduler.\n"
>" domid [int]: domain id to set\n"
> @@ -2115,7 +2115,7 @@ static PyMethodDef pyxc_methods[] = {
>
> { "sched_credit2_domain_set",
>(PyCFunction)pyxc_sched_credit2_domain_set,
> - METH_KEYWORDS, "\n"
> + METH_VARARGS | METH_KEYWORDS, "\n"
>"Set the scheduling parameters for a domain when running with the\n"
>"SMP credit2 scheduler.\n"
>" domid [int]: domain id to set\n"
> @@ -2393,21 +2393,21 @@ static PyMethodDef pyxc_methods[] = {
>
> { "flask_context_to_sid",
>(PyCFunction)pyflask_context_to_sid,
> - METH_KEYWORDS, "\n"
> + METH_VARARGS | METH_KEYWORDS, "\n"
>"Convert a context string to a dynamic SID.\n"
>" context [str]: String specifying context to be converted\n"
>"Returns: [int]: Numeric SID on success; -1 on error.\n" },
>
> { "flask_sid_to_context",
>(PyCFunction)pyflask_sid_to_context,
> - METH_KEYWORDS, "\n"
> + METH_VARARGS | METH_KEYWORDS, "\n"
>"Convert a dynamic SID to context string.\n"
>" context [int]: SID to be converted\n"
>"Returns: [str]: Numeric SID on success; -1 on error.\n" },
>
> { "flask_load",
>(PyCFunction)pyflask_load,
> - METH_KEYWORDS, "\n"
> + METH_VARARGS | METH_KEYWORDS, "\n"
>"Loads a policy into the hypervisor.\n"
>" policy [str]: policy to be load\n"
>"Returns: [int]: 0 on success; -1 on failure.\n" },
> @@ -2420,14 +2420,14 @@ static PyMethodDef pyxc_methods[] = {
>
> { "flask_setenforce",
>(PyCFunction)pyflask_setenforce,
> - METH_KEYWORDS, "\n"
> + METH_VARARGS | METH_KEYWORDS, "\n"
>"Modifies the current mode for the Flask XSM module.\n"
>" mode [int]: mode to change to\n"
>"Returns: [int]: 0 on success; -1 on failure.\n" },
>
> { "flask_access",
>(PyCFunction)pyflask_access,
> - METH_KEYWORDS, "\n"
> + METH_VARARGS | METH_KEYWORDS, "\n"
>"Returns whether a source context has access to target context based
> on \
> class and permissions requested.\n"
>" scon [str]: source context\n"
--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
signature.asc
Description: PGP signature
Re: [Xen-devel] [PATCH] libxl: create backend/ xenstore dir for driver domains
On Mon, Mar 23, 2020 at 04:35:12PM +0100, Roger Pau Monné wrote: > On Mon, Jan 06, 2020 at 05:03:40PM +0100, Marek Marczykowski-Górecki wrote: > > Alternatively, toolstack could wait for the actual backend node to be > > removed (by the driver domain), and then cleanup the parent directory (if > > empty). > > I'm not sure you need to cleanup the parent directory, You do, that's why this is an issue. Otherwise empty directories will accumulate there, leading to various issues (inability to list, running out of watches for monitoring them etc). Example state: /local/domain/5/backend = "" /local/domain/5/backend/vif = "" /local/domain/5/backend/vif/6 = "" /local/domain/5/backend/vif/7 = "" /local/domain/5/backend/vif/7/0 = "" /local/domain/5/backend/vif/7/0/frontend = "/local/domain/7/device/vif/0" /local/domain/5/backend/vif/7/0/frontend-id = "7" /local/domain/5/backend/vif/7/0/online = "1" /local/domain/5/backend/vif/7/0/state = "4" /local/domain/5/backend/vif/7/0/script = "/etc/xen/scripts/vif-route-qubes" /local/domain/5/backend/vif/7/0/mac = "00:16:3e:5e:6c:00" /local/domain/5/backend/vif/7/0/ip = "10.137.0.49 fd09:24ef:4179::a89:31" /local/domain/5/backend/vif/7/0/bridge = "xenbr0" /local/domain/5/backend/vif/7/0/handle = "0" /local/domain/5/backend/vif/7/0/type = "vif" /local/domain/5/backend/vif/7/0/feature-sg = "1" /local/domain/5/backend/vif/7/0/feature-gso-tcpv4 = "1" /local/domain/5/backend/vif/7/0/feature-gso-tcpv6 = "1" /local/domain/5/backend/vif/7/0/feature-ipv6-csum-offload = "1" /local/domain/5/backend/vif/7/0/feature-rx-copy = "1" /local/domain/5/backend/vif/7/0/feature-rx-flip = "0" /local/domain/5/backend/vif/7/0/feature-multicast-control = "1" /local/domain/5/backend/vif/7/0/feature-dynamic-multicast-control = "1" /local/domain/5/backend/vif/7/0/feature-split-event-channels = "1" /local/domain/5/backend/vif/7/0/multi-queue-max-queues = "2" /local/domain/5/backend/vif/7/0/feature-ctrl-ring = "1" /local/domain/5/backend/vif/7/0/hotplug-status = "connected" /local/domain/5/backend/vif/8 = "" /local/domain/5/backend/vif/11 = "" /local/domain/5/backend/vif/12 = "" /local/domain/5/backend/vif/17 = "" /local/domain/5/backend/vif/20 = "" /local/domain/5/backend/vif/23 = "" /local/domain/5/backend/vif/26 = "" /local/domain/5/backend/vif/28 = "" /local/domain/5/backend/vif/29 = "" /local/domain/5/backend/vif/30 = "" /local/domain/5/backend/vif/33 = "" /local/domain/5/backend/vif/34 = "" (...) /local/domain/5/backend/vif/416 = "" > albeit it > wouldn't hurt. It needs to be done in a transaction though, so that > you don't race with new additions to it. Good point. > > I don't find it particularly appealing, as every contact with > > libxl async code reduce overall happiness... > > > > > * There needs to be a way to deal with a broken/unresponsive driver > > >domain. That will involve not waiting for the backend so must > > >involve simply deleting the backend from xenstore. > > > > It's already there: if driver domain fails to set .../state = 6 within > > a timeout, toolstack will forcibly remove the entry. > > Would it work to change this and instead of monitor .../state = 6 > monitor that the parent directory still exist? That could be a good idea, to avoid introducing yet another (set of) callback. I'll look into it, it may require different handling of dom0/non-dom0 backend. > > > Is the distinction here between "xl shutdown" and "xl destroy", on the > > > actual guest domain, good enough ? Hopefully if the driver domain > > > sees the backend directory simply vanish it can destructively tear > > > everything down ? > > > > In the past this lead to multiple issues, where hotplug script didn't > > know which device actually was removed. In some cases I needed to > > workaround this by saving xenstore dump into a file in an "online" > > hotplug script, but it is very ugly solution. > > Removing the whole directory without giving time to the driver domain > to execute it's hotplug scripts can indeed lead to issues, as there's > no guarantee that the hotplug script won't use data in xenstore in > order to perform the cleanup IIRC. Yes, that's what 546678c6a60f64fb186640460dfa69a837c8fba5 fixed, but not removing it too early. > My preferred option would be to wait for the backend directory to be > removed by the driver domain, as I think it's the cleanest and likely > safest approach. > > Thanks, Roger. > -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature
[Xen-devel] [linux-linus test] 148890: regressions - trouble: fail/pass/starved
flight 148890 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/148890/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 133580 Regressions which are regarded as allowable (not blocking): test-armhf-armhf-xl-rtds16 guest-start/debian.repeat fail REGR. vs. 133580 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 133580 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 133580 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 133580 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 133580 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 133580 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 133580 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 133580 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 133580 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-dom0pvh-xl-intel 15 guest-saverestore fail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-arm64-arm64-xl-thunderx 13 migrate-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-credit1 13 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 14 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 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-libvirt-xsm 13 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 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-amd64-amd64-libvirt-vhd 12 migrate-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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 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-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 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-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-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-i386-xl-qemuu-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-dom0pvh-xl-amd 2 hosts-allocate starved n/a version targeted for testing: linux16fbf79b0f83bc752cee8589279f1ebfe57b3b6e baseline version: linux736706bee3298208343a76096370e4f6a5c55915 Last test of basis 133580 2019-03-04 19:53:09 Z 385 days Failing since
[Xen-devel] [seabios test] 148901: regressions - FAIL
flight 148901 seabios real [real] http://logs.test-lab.xenproject.org/osstest/logs/148901/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-nested-intel 17 debian-hvm-install/l1/l2 fail REGR. vs. 148666 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 12 guest-start/debianhvm.repeat fail pass in 148853 Tests which did not succeed, but are not blocking: test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 148666 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 148666 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 148666 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail starved in 148666 version targeted for testing: seabios de88a9628426e82f1cee4b61b06e67e6787301b1 baseline version: seabios 066a9956097b54530888b88ab9aa1ea02e42af5a Last test of basis 148666 2020-03-17 13:39:45 Z6 days Failing since148690 2020-03-18 06:43:59 Z5 days8 attempts Testing same since 148794 2020-03-20 23:39:57 Z3 days4 attempts People who touched revisions under test: Gerd Hoffmann Matt DeVillier Paul Menzel jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-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-xl-qemuu-debianhvm-i386-xsm pass test-amd64-i386-xl-qemuu-debianhvm-i386-xsm pass test-amd64-amd64-qemuu-nested-amdfail test-amd64-i386-qemuu-rhel6hvm-amd pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-i386-xl-qemuu-debianhvm-amd64 pass test-amd64-amd64-xl-qemuu-win7-amd64 fail test-amd64-i386-xl-qemuu-win7-amd64 fail test-amd64-amd64-xl-qemuu-ws16-amd64 fail test-amd64-i386-xl-qemuu-ws16-amd64 fail test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrictfail test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict pass test-amd64-amd64-qemuu-nested-intel fail test-amd64-i386-qemuu-rhel6hvm-intel pass test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow pass test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 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 de88a9628426e82f1cee4b61b06e67e6787301b1 Author: Paul Menzel Date: Wed Mar 4 14:51:27 2020 +0100 std/tcg: Replace zero-length array with flexible-array member GCC 10 gives the warnings below: In file included from out/ccode32flat.o.tmp.c:54: ./src/tcgbios.c: In function 'tpm20_write_EfiSpecIdEventStruct': ./src/tcgbios.c:290:30: warning: array subscript '() + 4294967295' is outside the bounds of an interior zero-length array 'struct TCG_EfiSpecIdEventAlgorithmSize[0]' [-Wzero-length-bounds] 290 | event.hdr.digestSizes[count].algorithmId = be16_to_cpu(sel->hashAlg); | ~^~~ In file included from ./src/tcgbios.c:22, from out/ccode32flat.o.tmp.c:54: ./src/std/tcg.h:527:7: note: while referencing 'digestSi
[Xen-devel] [PATCH] SVM: Add union intstat_t for offset 68h in vmcb struct
According to chapter "Appendix B Layout of VMCB" in the new version
(v3.32) AMD64 APM[1], bit 1 of the VMCB offset 68h is defined as
GUEST_INTERRUPT_MASK.
In current xen codes, it use whole u64 interrupt_shadow to setup
interrupt shadow, which will misuse other bit in VMCB offset 68h
as part of interrupt_shadow.
Add union intstat_t for VMCB offset 68h and fix codes to only use
bit 0 as intr_shadow according to the new APM description.
Reference:
[1] https://www.amd.com/system/files/TechDocs/24593.pdf
Signed-off-by: Pu Wen
---
xen/arch/x86/hvm/svm/nestedsvm.c | 4 ++--
xen/arch/x86/hvm/svm/svm.c | 8
xen/arch/x86/hvm/svm/svmdebug.c| 4 ++--
xen/include/asm-x86/hvm/svm/vmcb.h | 13 -
4 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 3bd2a119d3..595cbb1d81 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -508,7 +508,7 @@ static int nsvm_vmcb_prepare4vmrun(struct vcpu *v, struct
cpu_user_regs *regs)
}
/* Shadow Mode */
-n2vmcb->interrupt_shadow = ns_vmcb->interrupt_shadow;
+n2vmcb->int_stat.intr_shadow = ns_vmcb->int_stat.intr_shadow;
/* Exit codes */
n2vmcb->exitcode = ns_vmcb->exitcode;
@@ -1058,7 +1058,7 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct
cpu_user_regs *regs)
ns_vmcb->_vintr.fields.intr_masking = 0;
/* Shadow mode */
-ns_vmcb->interrupt_shadow = n2vmcb->interrupt_shadow;
+ns_vmcb->int_stat.intr_shadow = n2vmcb->int_stat.intr_shadow;
/* Exit codes */
ns_vmcb->exitcode = n2vmcb->exitcode;
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 32d8d847f2..888f504a94 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -116,7 +116,7 @@ void __update_guest_eip(struct cpu_user_regs *regs,
unsigned int inst_len)
regs->rip += inst_len;
regs->eflags &= ~X86_EFLAGS_RF;
-curr->arch.hvm.svm.vmcb->interrupt_shadow = 0;
+curr->arch.hvm.svm.vmcb->int_stat.intr_shadow = 0;
if ( regs->eflags & X86_EFLAGS_TF )
hvm_inject_hw_exception(TRAP_debug, X86_EVENT_NO_EC);
@@ -432,7 +432,7 @@ static unsigned int svm_get_interrupt_shadow(struct vcpu *v)
struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
unsigned int intr_shadow = 0;
-if ( vmcb->interrupt_shadow )
+if ( vmcb->int_stat.intr_shadow )
intr_shadow |= HVM_INTR_SHADOW_MOV_SS | HVM_INTR_SHADOW_STI;
if ( vmcb_get_general1_intercepts(vmcb) & GENERAL1_INTERCEPT_IRET )
@@ -446,7 +446,7 @@ static void svm_set_interrupt_shadow(struct vcpu *v,
unsigned int intr_shadow)
struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb);
-vmcb->interrupt_shadow =
+vmcb->int_stat.intr_shadow =
!!(intr_shadow & (HVM_INTR_SHADOW_MOV_SS|HVM_INTR_SHADOW_STI));
general1_intercepts &= ~GENERAL1_INTERCEPT_IRET;
@@ -2945,7 +2945,7 @@ void svm_vmexit_handler(struct cpu_user_regs *regs)
* retired.
*/
general1_intercepts &= ~GENERAL1_INTERCEPT_IRET;
-vmcb->interrupt_shadow = 1;
+vmcb->int_stat.intr_shadow = 1;
vmcb_set_general1_intercepts(vmcb, general1_intercepts);
break;
diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
index 366a003f21..5547baa497 100644
--- a/xen/arch/x86/hvm/svm/svmdebug.c
+++ b/xen/arch/x86/hvm/svm/svmdebug.c
@@ -51,9 +51,9 @@ void svm_vmcb_dump(const char *from, const struct vmcb_struct
*vmcb)
printk("iopm_base_pa = %#"PRIx64" msrpm_base_pa = %#"PRIx64" tsc_offset =
%#"PRIx64"\n",
vmcb_get_iopm_base_pa(vmcb), vmcb_get_msrpm_base_pa(vmcb),
vmcb_get_tsc_offset(vmcb));
-printk("tlb_control = %#x vintr = %#"PRIx64" interrupt_shadow =
%#"PRIx64"\n",
+printk("tlb_control = %#x vintr = %#"PRIx64" interrupt_shadow = %#x\n",
vmcb->tlb_control, vmcb_get_vintr(vmcb).bytes,
- vmcb->interrupt_shadow);
+ vmcb->int_stat.intr_shadow);
printk("event_inj %016"PRIx64", valid? %d, ec? %d, type %u, vector %#x\n",
vmcb->event_inj.raw, vmcb->event_inj.v,
vmcb->event_inj.ev, vmcb->event_inj.type,
diff --git a/xen/include/asm-x86/hvm/svm/vmcb.h
b/xen/include/asm-x86/hvm/svm/vmcb.h
index b9e389d481..d8a3285752 100644
--- a/xen/include/asm-x86/hvm/svm/vmcb.h
+++ b/xen/include/asm-x86/hvm/svm/vmcb.h
@@ -316,6 +316,17 @@ typedef union
uint64_t raw;
} intinfo_t;
+typedef union
+{
+struct
+{
+u64 intr_shadow:1;
+u64 guest_intr_mask:1;
+u64 resvd: 62;
+};
+uint64_t raw;
+} intstat_t;
+
typedef union
{
u64 bytes;
@@ -414,7 +425,7 @@ struct vmcb_struct {
u8 tlb_control;/* offset 0x5C */
u8 res07[3];
vintr_t _vintr; /* offset 0x60 - cleanbit 3 */
-u64 interrupt_s
[Xen-devel] [PATCH] x86/mce: Correct the machine check vendor for Hygon
Currently the xl dmesg output on Hygon platforms will be
"(XEN) CPU0: AMD Fam18h machine check reporting enabled",
which is misleading as AMD does not have family 18h (Hygon
negotiated with AMD to confirm that only Hygon has family 18h).
To correct this, add Hygon machine check type and vendor string.
Signed-off-by: Pu Wen
---
xen/arch/x86/cpu/mcheck/mce.c | 4 +++-
xen/arch/x86/cpu/mcheck/mce.h | 3 ++-
xen/arch/x86/cpu/mcheck/mce_amd.c | 3 ++-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index e5bd4f542c..fe9667e0c3 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -610,7 +610,8 @@ int show_mca_info(int inited, struct cpuinfo_x86 *c)
static const char *const type_str[] = {
[mcheck_amd_famXX] = "AMD",
[mcheck_amd_k8] = "AMD K8",
-[mcheck_intel] = "Intel"
+[mcheck_intel] = "Intel",
+[mcheck_hygon] = "Hygon"
};
snprintf(prefix, ARRAY_SIZE(prefix), "%sCPU%u: ",
@@ -625,6 +626,7 @@ int show_mca_info(int inited, struct cpuinfo_x86 *c)
break;
case mcheck_amd_famXX:
+case mcheck_hygon:
printk("%s%s Fam%xh machine check reporting enabled\n",
prefix, type_str[inited], c->x86);
break;
diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h
index 7137c2f763..1953626919 100644
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -36,7 +36,8 @@ enum mcheck_type {
mcheck_none,
mcheck_amd_famXX,
mcheck_amd_k8,
-mcheck_intel
+mcheck_intel,
+mcheck_hygon
};
extern uint8_t cmci_apic_vector;
diff --git a/xen/arch/x86/cpu/mcheck/mce_amd.c
b/xen/arch/x86/cpu/mcheck/mce_amd.c
index 787ce961b6..279a8e6f12 100644
--- a/xen/arch/x86/cpu/mcheck/mce_amd.c
+++ b/xen/arch/x86/cpu/mcheck/mce_amd.c
@@ -339,5 +339,6 @@ amd_mcheck_init(struct cpuinfo_x86 *ci)
mce_recoverable_register(mc_amd_recoverable_scan);
mce_register_addrcheck(mc_amd_addrcheck);
-return mcheck_amd_famXX;
+return ci->x86_vendor == X86_VENDOR_HYGON ?
+mcheck_hygon : mcheck_amd_famXX;
}
--
2.23.0
Re: [Xen-devel] [PATCH 1/3] Revert "x86/vvmx: fix virtual interrupt injection when Ack on exit control is used"
> From: Roger Pau Monné > Sent: Monday, March 23, 2020 10:49 PM > > On Mon, Mar 23, 2020 at 09:09:59AM +0100, Jan Beulich wrote: > > On 20.03.2020 20:07, Roger Pau Monne wrote: > > > This reverts commit f96e1469ad06b61796c60193daaeb9f8a96d7458. > > > > > > The commit is wrong, as the whole point of nvmx_update_apicv is to > > > update the guest interrupt status field when the Ack on exit VMEXIT > > > control feature is enabled. > > > > > > Signed-off-by: Roger Pau Monné > > > > Before anyone gets to look at the other two patches, should this > > be thrown in right away? > > I would like if possible get a confirmation from Kevin (or anyone > else) that my understanding is correct. I find the nested code very > confusing, and I've already made a mistake while trying to fix it. > That being said, this was spotted by osstest as introducing a > regression, so I guess it's safe to just toss it in now. > > FWIW patch 2/3 attempts to provide a description of my understanding > of how nvmx_update_apicv works. > I feel it is not good to take this patch alone, as it was introduced to fix another problem. W/o understanding whether the whole series can fix both old and new problems, we may risk putting nested interrupt logic in an even worse state... Thanks Kevin
[Xen-devel] [libvirt bisection] complete build-arm64-libvirt
branch xen-unstable xenbranch xen-unstable job build-arm64-libvirt testid libvirt-build Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_keycodemapdb https://gitlab.com/keycodemap/keycodemapdb.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: libvirt git://libvirt.org/libvirt.git Bug introduced: 4d5f50d86b760864240c695adc341379fb47a796 Bug not present: a1a18c6ab55869d3b00cf8c32e0e2262a10c8ce7 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/148958/ commit 4d5f50d86b760864240c695adc341379fb47a796 Author: Pavel Hrdina Date: Wed Jan 8 22:54:31 2020 +0100 bootstrap.conf: stop creating AUTHORS file The existence of AUTHORS file is required for GNU projects but since commit <8bfb36db40f38e92823b657b5a342652064b5adc> we do not require these files to exist. Signed-off-by: Pavel Hrdina Reviewed-by: Daniel P. Berrangé For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/libvirt/build-arm64-libvirt.libvirt-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/libvirt/build-arm64-libvirt.libvirt-build --summary-out=tmp/148958.bisection-summary --basis-template=146182 --blessings=real,real-bisect libvirt build-arm64-libvirt libvirt-build Searching for failure / basis pass: 148887 fail [host=rochester0] / 146182 [host=laxton0] 146156 ok. Failure / basis pass flights: 148887 / 146156 (tree in basispass but not in latest: libvirt_gnulib) Tree: libvirt git://libvirt.org/libvirt.git Tree: libvirt_keycodemapdb https://gitlab.com/keycodemap/keycodemapdb.git Tree: ovmf git://xenbits.xen.org/osstest/ovmf.git Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: seabios git://xenbits.xen.org/osstest/seabios.git Tree: xen git://xenbits.xen.org/xen.git Latest ea903036fa8d2333edb74b617416416dd75be533 317d3eeb963a515e15a63fa356d8ebcda7041a51 0c8ea9fe1adbbee230ee0c68f28b68ca2b0534bc 933ebad2470a169504799a1d95b8e410bd9847ef 066a9956097b54530888b88ab9aa1ea02e42af5a d094e95fb7c61c5f46d8e446b4bdc028438dea1c Basis pass 0f814c0fed420209ccb881325b854beaa7c70fcf 317d3eeb963a515e15a63fa356d8ebcda7041a51 70911f1f4aee0366b6122f2b90d367ec0f066beb 933ebad2470a169504799a1d95b8e410bd9847ef 2f4d068645c211e309812372cd0ac58c9024e93b 03bfe526ecadc86f31eda433b91dc90be0563919 Generating revisions with ./adhoc-revtuple-generator git://libvirt.org/libvirt.git#0f814c0fed420209ccb881325b854beaa7c70fcf-ea903036fa8d2333edb74b617416416dd75be533 https://gitlab.com/keycodemap/keycodemapdb.git#317d3eeb963a515e15a63fa356d8ebcda7041a51-317d3eeb963a515e15a63fa356d8ebcda7041a51 git://xenbits.xen.org/osstest/ovmf.git#70911f1f4aee0366b6122f2b90d367ec0f066beb-0c8ea9fe1adbbee230ee0c68f28b68ca2b0534bc git://xenbits.xen.org/qemu-xen.git#933ebad2470a169504799a1d95b8e410bd9847ef-933ebad2\ 470a169504799a1d95b8e410bd9847ef git://xenbits.xen.org/osstest/seabios.git#2f4d068645c211e309812372cd0ac58c9024e93b-066a9956097b54530888b88ab9aa1ea02e42af5a git://xenbits.xen.org/xen.git#03bfe526ecadc86f31eda433b91dc90be0563919-d094e95fb7c61c5f46d8e446b4bdc028438dea1c Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. error: The last gc run reported the following. Please correct the root cause and remove gc.log. Automatic cleanup will not be performed until the file is removed. warning: There are too many unreachable loose objects; run 'git prune' to remove them. Auto packing the repository in background for optimum performance. See "git help gc" for manual housekeeping. error: The last gc run reported the following. Please correct the root cause and remove gc.log. Automatic cleanup will not be performed until the file is removed. warning: There are too many unreachable loose objects; run 'git prune' to remove them. Use of uninitialized value $parents in array dereference at ./adhoc-revtuple-generator line 465. Use of uninitialized value in concatenation (.) or string at ./adhoc-revtuple-generator line 465. Loaded 8271 nodes in revision graph Searching for test results: 146182 [host=laxton0] 146156 pass 0f814c0fed420209ccb881325b854beaa7c70fcf 317d3eeb963a515e15a63fa356d8ebcda7041a51 70911f1f4aee0366b6122f2b90d367ec0f066beb 933ebad2470a169504799a1d95b8e410bd9847ef 2f4d068645c211e309812372cd0ac58c9024e93b 03bfe526ecadc86f31eda433b91dc90be0563919 146240 [host=rochester1] 146211 [host=rochester1] 146298 [host=rochester1] 146314 [host=rochester1] 146293 [host=rochester1] 146303 [host=rochester1] 146305 [host=rochester1] 146301 [host=rochester1] 146299 [host=rochester1] 146304 [host=rochester1]
Re: [Xen-devel] [PATCH 2/3] x86/nvmx: clarify and fix usage of nvmx_update_apicv
> From: Roger Pau Monne
> Sent: Saturday, March 21, 2020 3:08 AM
>
> The current usage of nvmx_update_apicv is not clear: it is deeply
> intertwined with the Ack interrupt on exit VMEXIT control.
>
> The code in nvmx_update_apicv should update the SVI (in service interrupt)
> field of the guest interrupt status only when the Ack interrupt on
> exit is set, as it is used to record that the interrupt being
> serviced is signaled in a vmcs field, and hence hasn't been injected
> as on native. It's important to record the current in service
> interrupt on the guest interrupt status field, or else further
> interrupts won't respect the priority of the in service one.
>
> While clarifying the usage make sure that the SVI is only updated when
> Ack on exit is set and the nested vmcs interrupt info field is valid. Or
> else a guest not using the Ack on exit feature would loose interrupts as
> they would be signaled as being in service on the guest interrupt
> status field but won't actually be recorded on the interrupt info vmcs
> field, neither injected in any other way.
It is insufficient. You also need to update RVI to enable virtual injection
when Ack on exit is cleared.
>
> Signed-off-by: Roger Pau Monné
> ---
> xen/arch/x86/hvm/vmx/vvmx.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 1b8461ba30..180d01e385 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1383,7 +1383,7 @@ static void nvmx_update_apicv(struct vcpu *v)
> {
> struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
> unsigned long reason = get_vvmcs(v, VM_EXIT_REASON);
> -uint32_t intr_info = nvmx->intr.intr_info;
> +unsigned long intr_info = get_vvmcs(v, VM_EXIT_INTR_INFO);
well, above reminds me an interesting question. Combining last and this
patch, we'd see essentially that it goes back to the state before f96e1469
(at least when Ack on exit is true). iirc, that commit was introduced to enable
nested x2apic with apicv, and your very first version even just removed
the whole nvmx_update_apicv. Then now with the new reverted logic,
are you still suffering x2apic problem? If not, does it imply the real fix
is actually coming from patch 3/3 for eoi bitmap update?
>
> if ( reason == EXIT_REASON_EXTERNAL_INTERRUPT &&
> nvmx->intr.source == hvm_intsrc_lapic &&
> @@ -1399,6 +1399,15 @@ static void nvmx_update_apicv(struct vcpu *v)
> ppr = vlapic_set_ppr(vlapic);
> WARN_ON((ppr & 0xf0) != (vector & 0xf0));
>
> +/*
> + * SVI must be updated when the interrupt has been signaled using the
> + * Ack on exit feature, or else the currently in-service interrupt
> + * won't be respected.
> + *
> + * Note that this is specific to the fact that when doing a VMEXIT an
> + * interrupt might get delivered using the interrupt info vmcs field
> + * instead of being injected normally.
> + */
I'm not sure mentioning SVI specifically here is necessary, since all steps
here are required - updating iir, isr, rvi, svi, ppr, etc. It is just an
emulation
of updating virtual APIC state as if a virtual interrupt delivery happens.
> status = vector << VMX_GUEST_INTR_STATUS_SVI_OFFSET;
> rvi = vlapic_has_pending_irq(v);
> if ( rvi != -1 )
> --
> 2.25.0
Re: [Xen-devel] [PATCH 3/3] x86/nvmx: update exit bitmap on vmexit
> From: Roger Pau Monne
> Sent: Saturday, March 21, 2020 3:08 AM
>
> Current code in nvmx_update_apicv set the guest interrupt status field
> but doesn't update the exit bitmap, which can cause issues of lost
> interrupts on the L1 hypervisor if vmx_intr_assist gets
> short-circuited by nvmx_intr_intercept returning true.
Above is not accurate. Currently Xen didn't choose to update the EOI
exit bitmap every time when there is a change. Instead, it chose to
batch the update before resuming to the guest. sort of optimization.
So it is not related to whether SVI is changed. We should always do the
bitmap update in nvmx_update_apicv, regardless of the setting of
Ack-on-exit ...
Thanks
Kevin
>
> Extract the code to update the exit bitmap from vmx_intr_assist into a
> helper and use it in nvmx_update_apicv when updating the guest
> interrupt status field.
>
> Signed-off-by: Roger Pau Monné
> ---
> xen/arch/x86/hvm/vmx/intr.c | 21 +
> xen/arch/x86/hvm/vmx/vvmx.c | 1 +
> xen/include/asm-x86/hvm/vmx/vmx.h | 2 ++
> 3 files changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
> index 49a1295f09..000e14af49 100644
> --- a/xen/arch/x86/hvm/vmx/intr.c
> +++ b/xen/arch/x86/hvm/vmx/intr.c
> @@ -224,6 +224,18 @@ static int nvmx_intr_intercept(struct vcpu *v, struct
> hvm_intack intack)
> return 0;
> }
>
> +void vmx_sync_exit_bitmap(struct vcpu *v)
> +{
> +const unsigned int n = ARRAY_SIZE(v->arch.hvm.vmx.eoi_exit_bitmap);
> +unsigned int i;
> +
> +while ( (i = find_first_bit(&v->arch.hvm.vmx.eoi_exitmap_changed, n)) <
> n )
> +{
> +clear_bit(i, &v->arch.hvm.vmx.eoi_exitmap_changed);
> +__vmwrite(EOI_EXIT_BITMAP(i), v->arch.hvm.vmx.eoi_exit_bitmap[i]);
> +}
> +}
> +
> void vmx_intr_assist(void)
> {
> struct hvm_intack intack;
> @@ -318,7 +330,6 @@ void vmx_intr_assist(void)
>intack.source != hvm_intsrc_vector )
> {
> unsigned long status;
> -unsigned int i, n;
>
> /*
> * intack.vector is the highest priority vector. So we set
> eoi_exit_bitmap
> @@ -379,13 +390,7 @@ void vmx_intr_assist(void)
> intack.vector;
> __vmwrite(GUEST_INTR_STATUS, status);
>
> -n = ARRAY_SIZE(v->arch.hvm.vmx.eoi_exit_bitmap);
> -while ( (i = find_first_bit(&v->arch.hvm.vmx.eoi_exitmap_changed,
> -n)) < n )
> -{
> -clear_bit(i, &v->arch.hvm.vmx.eoi_exitmap_changed);
> -__vmwrite(EOI_EXIT_BITMAP(i),
> v->arch.hvm.vmx.eoi_exit_bitmap[i]);
> -}
> +vmx_sync_exit_bitmap(v);
>
> pt_intr_post(v, intack);
> }
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 180d01e385..e041ecc115 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1414,6 +1414,7 @@ static void nvmx_update_apicv(struct vcpu *v)
> status |= rvi & VMX_GUEST_INTR_STATUS_SUBFIELD_BITMASK;
>
> __vmwrite(GUEST_INTR_STATUS, status);
> +vmx_sync_exit_bitmap(v);
> }
> }
>
> diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h b/xen/include/asm-
> x86/hvm/vmx/vmx.h
> index b334e1ec94..111ccd7e61 100644
> --- a/xen/include/asm-x86/hvm/vmx/vmx.h
> +++ b/xen/include/asm-x86/hvm/vmx/vmx.h
> @@ -610,6 +610,8 @@ void update_guest_eip(void);
> void vmx_pi_per_cpu_init(unsigned int cpu);
> void vmx_pi_desc_fixup(unsigned int cpu);
>
> +void vmx_sync_exit_bitmap(struct vcpu *v);
> +
> #ifdef CONFIG_HVM
> void vmx_pi_hooks_assign(struct domain *d);
> void vmx_pi_hooks_deassign(struct domain *d);
> --
> 2.25.0
