Re: [Xen-devel] [PATCH 3/3] x86/spec-ctrl: Add support for modifying SSBD AMD VIA LS_CFG MSR
>>> On 06.08.18 at 21:07, wrote: > On Thu, Aug 02, 2018 at 01:09:06AM -0600, Jan Beulich wrote: >> >>> On 02.08.18 at 00:20, wrote: >> > On Tue, Jul 31, 2018 at 05:25:27AM -0600, Jan Beulich wrote: >> >> Code structure wise this looks to undo a fair part of what patch >> >> 1 has done. It would be nice to limit code churn. >> > >> > Patch 1 stand alone just to improve reporting the capabilities of the >> > processor. Currently Xen doesn't mention anything if SSBD is actually >> > enable on AMD processors. Patch 2-3 add it where Xen can it >> > dynamically. >> >> Which is all fine, but couldn't patch 1 be written in a way that the >> next one(s) don't have to turn code structure all over again. > > Rather than change 1, I changed patch 3 (well now patch 2). > >> >> Why can't this be called from init_speculation_mitigations()? >> > >> > IIRC I was having memory init problems with. It was moved to later in >> > the boot so that xmalloc would work correctly. I haven't touched this >> > code in over month. If you want a 100% tested answer I can retest >> > putting it in init_speculation_mitigations(). >> >> Would be nice if that could be arranged for, as the rather specialized >> call looks pretty odd in (iirc __start_xen(); iirc because you've dropped >> too much of the context in your reply, and I'm too lazy to dig out the >> original patch again). > > It was __start_xen(). Well, IIRC xalloc didn't work (well writing to > the address given) until after arch_init_memory(). For it to work in > init_speculation_mitigations(), I'd assume you'd need to call > arch_init_memory() before init_speculation_mitigations(). I don't think that's a viable option, but it could certainly be explored. I wonder though whether you can't get away without allocation at this early point. >> >> Still I wonder whether less code duplication wouldn't be better. >> > >> > current_cpu_data isn't available when ssbd_amd_ls_cfg_init is called. >> >> By "isn't available" you mean "hasn't been populated yet"? Else >> I don't understand. > > It hasn't been populated yet. Not even boot_cpu_data? >> >> I find such a hard-coded upper bound quite concerning. Is nr_sockets >> >> not correct in the AMD case? If so, that would want fixing, such that >> >> you can use the variable here. >> > >> > It's been a while since I wrote this but IIRC, it has to do with >> > nr_sockets either being off or not available when the code is run. >> > For Family 17h which the patches are for, there's a max of two sockets. >> >> I've implied that from how you wrote the patches, but such hard coding >> will only make for more code churn later on. Try to be as generic as >> possible. > > Well, nr_sockets gets set in smp_prepare_cpus, which gets called after > init_speculation_mitigations() and ssbd_amd_ls_cfg_init(). It could be > put later on in the boot, but it needs to be run at least before other > cpus are brought online. I'm welcome to any suggestions about how to > restructure things though. Did you consider using a presmp-initcall? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 02/15] iommu: introduce the concept of BFN...
> -Original Message- > From: Tian, Kevin [mailto:kevin.t...@intel.com] > Sent: 07 August 2018 03:38 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Suravee Suthikulpanit ; Stefano > Stabellini ; Julien Grall ; Jan > Beulich > Subject: RE: [PATCH v5 02/15] iommu: introduce the concept of BFN... > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > Sent: Saturday, August 4, 2018 1:22 AM > > > > ...meaning 'bus frame number' i.e. a frame number mapped in the IOMMU > > rather than the MMU. > > > > This patch is a largely cosmetic change that substitutes the terms 'gfn' > > and 'gaddr' for 'bfn' and 'baddr' in all the places where the frame number > > or address relate to the IOMMU rather than the MMU. > > > > The parts that are not purely cosmetic are: > > > > - the introduction of a type-safe declaration of bfn_t and definition of > >INVALID_BFN to make the substitution of gfn_x(INVALID_GFN) > mechanical. > > - the introduction of __bfn_to_baddr and __baddr_to_bfn (and type-safe > >variants without the leading __) with some use of the former. > > > > Subsequent patches will convert code to make use of type-safe BFNs. > > > > Signed-off-by: Paul Durrant > > Reviewed-by: Wei Liu > > [...] > > > diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h > > index 24654e8e22..4ebf91f17e 100644 > > --- a/xen/include/xen/mm.h > > +++ b/xen/include/xen/mm.h > > @@ -14,8 +14,9 @@ > > * should be adhered to. > > * > > * mfn: Machine Frame Number > > - * The values Xen puts into its own pagetables. This is the host > > physical > > - * memory address space with RAM, MMIO etc. > > + * The values Xen puts into its own pagetables, 2nd stage pagetables > > (where > > + * hardware assisted 2nd stage translation is used) or IOMMU page > > tables. > > + * This is the host physical memory address space with RAM, MMIO etc. > > there is also shadow page table... not sure whether it is really helpful > to list all of them out. "Xen-owned pagetables" should be sufficient to cover. > > > * > > * gfn: Guest Frame Number > > * The values a guest puts in its own pagetables. For an auto-translated > > @@ -26,6 +27,11 @@ > > * A linear idea of a guest physical address space. For an > > auto-translated > > * guest, pfn == gfn while for a non-translated guest, pfn != gfn. > > * > > + * bfn: Bus Frame Number (definitions in include/xen/iommu.h) > > + * The linear frame numbers of IOMMU address space. All initiators for a > > + * given domain share a single IOMMU address space and, by default, > > Xen will > > + * ensure bfn == pfn. > > + * > > Above description is a bit confusing: > > - for 'domain', are you talking about VM or IOMMU domain? If the former > then each initiator could have its own IOMMU address space when a virtual > IOMMU (either pvIOMMU or emulated IOMMU) is exposed. > I mean VM and, yes, while each initiator *could* have its own IOMMU address space, that is not what we do in (or have ever done) in Xen. I can replace the word 'domain' with the abbreviation 'VM' if you feel it makes things clearer. > - since you talk about 'by default', better also talk about non-default case, > i.e. when virtual IOMMU is exposed then bfn is a separate address space > managed by guest That's putting the cart before the horse really. There is no PV-IOMMU at this stage in the series. I guess I could modify the comment in the later patch where I add the hypercall to enable PV-IOMMU. Paul > > > * WARNING: Some of these terms have changed over time while others > > have been > > * used inconsistently, meaning that a lot of existing code does not match > > the > > * definitions above. New code should use these terms as described here, > > and > > -- > > 2.11.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 05/15] iommu: don't domain_crash() inside iommu_map/unmap_page()
> -Original Message-
> From: Tian, Kevin [mailto:kevin.t...@intel.com]
> Sent: 07 August 2018 03:56
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Julien Grall
> ; Andrew Cooper ;
> Ian Jackson ; Jan Beulich ;
> Konrad Rzeszutek Wilk ; Tim (Xen.org)
> ; Nakajima, Jun ; George Dunlap
>
> Subject: RE: [PATCH v5 05/15] iommu: don't domain_crash() inside
> iommu_map/unmap_page()
>
> > From: Paul Durrant [mailto:paul.durr...@citrix.com]
> > Sent: Saturday, August 4, 2018 1:22 AM
> >
> > Turn iommu_map/unmap_page() into straightforward wrappers that check
> > the
>
> iommu_iotlb_flush is also changed.
Indeed it is. I'll call it out.
>
> > existence of the relevant iommu_op and call through to it. This makes
> them
> > usable by PV IOMMU code to be delivered in future patches.
> > Leave the decision on whether to invoke domain_crash() up to the caller.
> > This has the added benefit that the (module/line number) message that
> > domain_crash() spits out will be more indicative of where the problem lies.
> >
> > NOTE: This patch includes one bit of clean-up in set_identity_p2m_entry()
> > replacing use of p2m->domain with the domain pointer passed into the
> > function.
> >
> > Signed-off-by: Paul Durrant
> > Reviewed-by: George Dunlap
> > Reviewed-by: Wei Liu
>
> Reviewed-by: Kevin Tian , with one small comment:
Thanks.
>
> >
> > if ( need_iommu(p2m->domain) &&
> > (lpae_valid(orig_pte) || lpae_valid(*entry)) )
> > +{
> > rc = iommu_iotlb_flush(p2m->domain, _bfn(gfn_x(sgfn)),
> > 1UL << page_order);
> > +if ( unlikely(rc) && !is_hardware_domain(p2m->domain) )
> > +domain_crash(p2m->domain);
> > +}
>
> to reduce duplication, does it make sense to introduce a wrapper
> like domain_crash_nd ('nd' indicate !is_hardware_domain, and
> becomes a nop for hardware domain)? Then it becomes:
>
> if ( unlikely(rc) )
> domain_crash_nd(p2m->domain);
That's a bigger change and I'd like to defer to the other maintainers as to
whether they think it is a good idea. I'm happy to change this in v6 if anyone
gives me a +1.
Paul
>
> Thanks
> Kevin
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 06/15] public / x86: introduce __HYPERCALL_iommu_op
> -Original Message- > From: Tian, Kevin [mailto:kevin.t...@intel.com] > Sent: 07 August 2018 04:00 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Stefano Stabellini ; Wei Liu > ; George Dunlap ; > Andrew Cooper ; Ian Jackson > ; Tim (Xen.org) ; Jan Beulich > ; Daniel De Graaf > Subject: RE: [Xen-devel] [PATCH v5 06/15] public / x86: introduce > __HYPERCALL_iommu_op > > > From: Paul Durrant > > Sent: Saturday, August 4, 2018 1:22 AM > > > > This patch introduces the boilerplate for a new hypercall to allow a > > domain to control IOMMU mappings for its own pages. > > Whilst there is duplication of code between the native and compat entry > > points which appears ripe for some form of combination, I think it is > > better to maintain the separation as-is because the compat entry point > > will necessarily gain complexity in subsequent patches. > > > > NOTE: This hypercall is only implemented for x86 and is currently > > restricted by XSM to dom0. Its scope can be expanded in future > > if need be. > > > > Signed-off-by: Paul Durrant > [...] > > > diff --git a/xen/arch/x86/iommu_op.c b/xen/arch/x86/iommu_op.c > > new file mode 100644 > > index 00..744c0fce27 > > --- /dev/null > > +++ b/xen/arch/x86/iommu_op.c > > @@ -0,0 +1,184 @@ > > > +/* > *** > > ** > > + * x86/iommu_op.c > > + * > > + * Paravirtualised IOMMU functionality > > although only x86 is supported, logic in this file is vendor agnostic. Better > move to a common place now... > Personally I don't like to expand scope into architectures which I have no facility to test. IIUC ARM also very much relies on an identity GFN:BFN map so this functionality is unlikely to get used by anything other than x86 for the foreseeable future. Julien, would you prefer that this be in common code? Paul > Thanks > Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 07/15] iommu: track reserved ranges using a rangeset
> -Original Message-
> From: Tian, Kevin [mailto:kevin.t...@intel.com]
> Sent: 07 August 2018 04:04
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Jan Beulich
> Subject: RE: [PATCH v5 07/15] iommu: track reserved ranges using a rangeset
>
> > From: Paul Durrant [mailto:paul.durr...@citrix.com]
> > Sent: Saturday, August 4, 2018 1:22 AM
> >
> > Ranges that should be considered reserved in the IOMMU are not
> > necessarily
> > limited to RMRRs. If iommu_inclusive_mapping is set then any frame
> > number
> > falling within an E820 reserved region should also be considered as
> > reserved in the IOMMU.
>
> I don't think it is a good extension. RMRR by definition requires
> identity mapping in IOMMU page table, thus must be also reserved
> in bfn address space (to avoid being used for other purpose).
> Normal E820 reserved regions have no such requirement. Guest
> can use same bfn location for any purpose. I'm not sure why we
> want to add more limitations here.
Have you tried booting an average Intel based server without identity mapped
E820 reserved regions? Good luck with that, because I think about half the
servers on the planet will wedge up if you only give them RMRRs. My Dell R730
certainly will and I'd hazard a guess that any other Dell box we have with an
iDRAC will too.
Note the behaviour is predicated on 'iommu_inclusive_mapping' being set so if
you feel lucky then you can turn it off :-)
Paul
>
> > This patch adds a rangeset to the domain_iommu structure that is then
> > used
> > to track all reserved ranges. This will be needed by a subsequent patch
> > to test whether it is safe to modify a particular IOMMU entry.
> >
> > Signed-off-by: Paul Durrant
> > Reviewed-by: George Dunlap
> > Reviewed-by: Wei Liu
> > ---
> > Cc: Jan Beulich
> > Cc: Kevin Tian
> >
> > v2:
> > - New in v2.
> > ---
> > xen/drivers/passthrough/iommu.c | 10 +-
> > xen/drivers/passthrough/vtd/iommu.c | 20 +---
> > xen/drivers/passthrough/vtd/x86/vtd.c | 17 -
> > xen/include/xen/iommu.h | 6 ++
> > 4 files changed, 44 insertions(+), 9 deletions(-)
> >
> > diff --git a/xen/drivers/passthrough/iommu.c
> > b/xen/drivers/passthrough/iommu.c
> > index 21e6886a3f..b10a37e5d7 100644
> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -147,6 +147,10 @@ int iommu_domain_init(struct domain *d)
> > if ( !iommu_enabled )
> > return 0;
> >
> > +hd->reserved_ranges = rangeset_new(d, NULL, 0);
> > +if ( !hd->reserved_ranges )
> > +return -ENOMEM;
> > +
> > hd->platform_ops = iommu_get_ops();
> > return hd->platform_ops->init(d);
> > }
> > @@ -248,12 +252,16 @@ int iommu_construct(struct domain *d)
> >
> > void iommu_domain_destroy(struct domain *d)
> > {
> > -if ( !iommu_enabled || !dom_iommu(d)->platform_ops )
> > +const struct domain_iommu *hd = dom_iommu(d);
> > +
> > +if ( !iommu_enabled || !hd->platform_ops )
> > return;
> >
> > iommu_teardown(d);
> >
> > arch_iommu_domain_destroy(d);
> > +
> > +rangeset_destroy(hd->reserved_ranges);
> > }
> >
> > int iommu_map_page(struct domain *d, bfn_t bfn, mfn_t mfn,
> > diff --git a/xen/drivers/passthrough/vtd/iommu.c
> > b/xen/drivers/passthrough/vtd/iommu.c
> > index c9f50f04ad..282e227414 100644
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -1910,6 +1910,7 @@ static int rmrr_identity_mapping(struct domain
> > *d, bool_t map,
> > unsigned long end_pfn = PAGE_ALIGN_4K(rmrr->end_address) >>
> > PAGE_SHIFT_4K;
> > struct mapped_rmrr *mrmrr;
> > struct domain_iommu *hd = dom_iommu(d);
> > +int err = 0;
> >
> > ASSERT(pcidevs_locked());
> > ASSERT(rmrr->base_address < rmrr->end_address);
> > @@ -1923,8 +1924,6 @@ static int rmrr_identity_mapping(struct domain
> > *d, bool_t map,
> > if ( mrmrr->base == rmrr->base_address &&
> > mrmrr->end == rmrr->end_address )
> > {
> > -int ret = 0;
> > -
> > if ( map )
> > {
> > ++mrmrr->count;
> > @@ -1934,28 +1933,35 @@ static int rmrr_identity_mapping(struct
> > domain *d, bool_t map,
> > if ( --mrmrr->count )
> > return 0;
> >
> > -while ( base_pfn < end_pfn )
> > +err = rangeset_remove_range(hd->reserved_ranges,
> > +base_pfn, end_pfn);
> > +while ( !err && base_pfn < end_pfn )
> > {
> > if ( clear_identity_p2m_entry(d, base_pfn) )
> > -ret = -ENXIO;
> > +err = -ENXIO;
> > +
> > base_pfn++;
> > }
> >
> > list_del(&mrmrr->list);
> > xfree(mrmrr);
> > -return ret;
> > +return err;
> > }
> > }
> >
> > if
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
> -Original Message-
> From: Tian, Kevin [mailto:kevin.t...@intel.com]
> Sent: 07 August 2018 04:25
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Jan Beulich ; George Dunlap
>
> Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
>
> > From: Paul Durrant [mailto:paul.durr...@citrix.com]
> > Sent: Saturday, August 4, 2018 1:22 AM
> >
> > This patch adds a new method to the VT-d IOMMU implementation to find
> > the
> > MFN currently mapped by the specified BFN along with a wrapper function
> > in
> > generic IOMMU code to call the implementation if it exists.
> >
> > This functionality will be used by a subsequent patch.
> >
> > Signed-off-by: Paul Durrant
> > Reviewed-by: Wei Liu
> > ---
> > Cc: Kevin Tian
> > Cc: Jan Beulich
> > Cc: George Dunlap
> >
> > v3:
> > - Addressed comments from George.
> >
> > v2:
> > - Addressed some comments from Jan.
> > ---
> > xen/drivers/passthrough/iommu.c | 11 +++
> > xen/drivers/passthrough/vtd/iommu.c | 34
> > ++
> > xen/drivers/passthrough/vtd/iommu.h | 3 +++
> > xen/include/xen/iommu.h | 4
> > 4 files changed, 52 insertions(+)
> >
> > diff --git a/xen/drivers/passthrough/iommu.c
> > b/xen/drivers/passthrough/iommu.c
> > index b10a37e5d7..9b7baca93f 100644
> > --- a/xen/drivers/passthrough/iommu.c
> > +++ b/xen/drivers/passthrough/iommu.c
> > @@ -305,6 +305,17 @@ int iommu_unmap_page(struct domain *d, bfn_t
> > bfn)
> > return rc;
> > }
> >
> > +int iommu_lookup_page(struct domain *d, bfn_t bfn, mfn_t *mfn,
> > + unsigned int *flags)
> > +{
> > +const struct domain_iommu *hd = dom_iommu(d);
> > +
> > +if ( !iommu_enabled || !hd->platform_ops )
> > +return -EOPNOTSUPP;
> > +
> > +return hd->platform_ops->lookup_page(d, bfn, mfn, flags);
> > +}
> > +
> > static void iommu_free_pagetables(unsigned long unused)
> > {
> > do {
> > diff --git a/xen/drivers/passthrough/vtd/iommu.c
> > b/xen/drivers/passthrough/vtd/iommu.c
> > index 282e227414..8cd3b59aa0 100644
> > --- a/xen/drivers/passthrough/vtd/iommu.c
> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > @@ -1830,6 +1830,39 @@ static int __must_check
> > intel_iommu_unmap_page(struct domain *d,
> > return dma_pte_clear_one(d, bfn_to_baddr(bfn));
> > }
> >
> > +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn, mfn_t
> > *mfn,
> > + unsigned int *flags)
>
> Not looking at later patches yet... but in concept bfn address
> space is per device instead of per domain.
Not in this case. Xen has always maintained a single IOMMU address per virtual
machine. That is what BFN refers to.
Paul
> In default situation
> (w/o pvIOMMU exposed), all devices assigned to dom0 share
> the same address space (bfn=pfn) which is currently linked
> from domain structure. Then with pvIOMMU exposed, dom0
> starts to manage individual pfn address space (called IOVA
> address space within dom0) per assigned device. In that case
> lookup should accept a bdf number and then find the right
> page table...
>
No. That is over-complicating things and would probably involve re-writing most
of the IOMMU code in Xen AFAICT.
> > +{
> > +struct domain_iommu *hd = dom_iommu(d);
> > +struct dma_pte *page = NULL, *pte = NULL, val;
> > +u64 pg_maddr;
> > +
> > +spin_lock(&hd->arch.mapping_lock);
> > +
> > +pg_maddr = addr_to_dma_page_maddr(d, bfn_to_baddr(bfn), 0);
> > +if ( pg_maddr == 0 )
> > +{
> > +spin_unlock(&hd->arch.mapping_lock);
> > +return -ENOMEM;
> > +}
> > +
> > +page = map_vtd_domain_page(pg_maddr);
> > +pte = page + (bfn_x(bfn) & LEVEL_MASK);
> > +val = *pte;
> > +
> > +unmap_vtd_domain_page(page);
> > +spin_unlock(&hd->arch.mapping_lock);
> > +
> > +if ( !dma_pte_present(val) )
> > +return -ENOENT;
> > +
> > +*mfn = maddr_to_mfn(dma_pte_addr(val));
> > +*flags = dma_pte_read(val) ? IOMMUF_readable : 0;
> > +*flags |= dma_pte_write(val) ? IOMMUF_writable : 0;
> > +
> > +return 0;
> > +}
> > +
> > int iommu_pte_flush(struct domain *d, uint64_t bfn, uint64_t *pte,
> > int order, int present)
> > {
> > @@ -2661,6 +2694,7 @@ const struct iommu_ops intel_iommu_ops = {
> > .teardown = iommu_domain_teardown,
> > .map_page = intel_iommu_map_page,
> > .unmap_page = intel_iommu_unmap_page,
> > +.lookup_page = intel_iommu_lookup_page,
> > .free_page_table = iommu_free_page_table,
> > .reassign_device = reassign_device_ownership,
> > .get_device_group_id = intel_iommu_group_id,
> > diff --git a/xen/drivers/passthrough/vtd/iommu.h
> > b/xen/drivers/passthrough/vtd/iommu.h
> > index 72c1a2e3cd..47bdfcb5ea 100644
> > --- a/xen/drivers/passthrough/vtd/iommu.h
> > +++ b/xen/drivers/passthrough/vtd/iommu.h
> > @@ -272,6 +272,9 @@ struct dma_pte {
> > #define dma_set_pte_prot(p, prot) do {
[Xen-devel] [xen-unstable-smoke test] 125780: trouble: blocked/broken/pass
flight 125780 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/125780/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken Regressions which are regarded as allowable (not blocking): build-arm64-xsm 2 hosts-allocate broken REGR. vs. 125729 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64-xsm 3 capture-logs broken blocked in 125729 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen e752f28409678ce3ade49986b39309178fb2a6d6 baseline version: xen ed5f8d9ca47e69e30221c37ec812f2b38af19d83 Last test of basis 125729 2018-08-01 11:00:39 Z5 days Failing since125741 2018-08-02 10:01:09 Z4 days8 attempts Testing same since 125772 2018-08-06 16:00:37 Z0 days6 attempts People who touched revisions under test: Alexandru Isaila Andrew Cooper Doug Goldstein George Dunlap Jan Beulich Julien Grall Kevin Tian Razvan Cojocaru Roger Pau Monné Stefano Stabellini jobs: build-arm64-xsm broken build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary broken-job build-arm64-xsm broken broken-step build-arm64-xsm hosts-allocate broken-step build-arm64-xsm capture-logs Not pushing. (No revision log; it would be 480 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 05/15] iommu: don't domain_crash() inside iommu_map/unmap_page()
>>> On 07.08.18 at 10:05, wrote:
>> From: Tian, Kevin [mailto:kevin.t...@intel.com]
>> Sent: 07 August 2018 03:56
>> > From: Paul Durrant [mailto:paul.durr...@citrix.com]
>> > Sent: Saturday, August 4, 2018 1:22 AM
>> > if ( need_iommu(p2m->domain) &&
>> > (lpae_valid(orig_pte) || lpae_valid(*entry)) )
>> > +{
>> > rc = iommu_iotlb_flush(p2m->domain, _bfn(gfn_x(sgfn)),
>> > 1UL << page_order);
>> > +if ( unlikely(rc) && !is_hardware_domain(p2m->domain) )
>> > +domain_crash(p2m->domain);
>> > +}
>>
>> to reduce duplication, does it make sense to introduce a wrapper
>> like domain_crash_nd ('nd' indicate !is_hardware_domain, and
>> becomes a nop for hardware domain)? Then it becomes:
>>
>> if ( unlikely(rc) )
>> domain_crash_nd(p2m->domain);
>
> That's a bigger change and I'd like to defer to the other maintainers as to
> whether they think it is a good idea. I'm happy to change this in v6 if
> anyone gives me a +1.
Well, if this is a common idiom, then yes, I think a helper would
be desirable. However - what is the _nd suffix supposed to
stand for? I'm inclined to suggest domu_crash().
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 07/15] iommu: track reserved ranges using a rangeset
> From: Paul Durrant [mailto:paul.durr...@citrix.com] > Sent: Tuesday, August 7, 2018 4:16 PM > > > -Original Message- > > From: Tian, Kevin [mailto:kevin.t...@intel.com] > > Sent: 07 August 2018 04:04 > > To: Paul Durrant ; xen- > de...@lists.xenproject.org > > Cc: Jan Beulich > > Subject: RE: [PATCH v5 07/15] iommu: track reserved ranges using a > rangeset > > > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > > Sent: Saturday, August 4, 2018 1:22 AM > > > > > > Ranges that should be considered reserved in the IOMMU are not > > > necessarily > > > limited to RMRRs. If iommu_inclusive_mapping is set then any frame > > > number > > > falling within an E820 reserved region should also be considered as > > > reserved in the IOMMU. > > > > I don't think it is a good extension. RMRR by definition requires > > identity mapping in IOMMU page table, thus must be also reserved > > in bfn address space (to avoid being used for other purpose). > > Normal E820 reserved regions have no such requirement. Guest > > can use same bfn location for any purpose. I'm not sure why we > > want to add more limitations here. > > Have you tried booting an average Intel based server without identity > mapped E820 reserved regions? Good luck with that, because I think about > half the servers on the planet will wedge up if you only give them RMRRs. > My Dell R730 certainly will and I'd hazard a guess that any other Dell box > we have with an iDRAC will too. > Note the behaviour is predicated on 'iommu_inclusive_mapping' being set > so if you feel lucky then you can turn it off :-) > I know that fact... looks I misunderstood the patch - I thought you blindly add all reserved regions in this patch bypassing the existing control from inclusive_mapping. Now re-read the description and code you are just giving background :-) Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 11/15] mm / iommu: split need_iommu() into has_iommu_pt() and sync_iommu_pt()
> -Original Message- > From: Tian, Kevin [mailto:kevin.t...@intel.com] > Sent: 07 August 2018 04:41 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Stefano Stabellini ; Julien Grall > ; Andrew Cooper ; > George Dunlap ; Ian Jackson > ; Jan Beulich ; Konrad > Rzeszutek Wilk ; Tim (Xen.org) ; > Wei Liu ; Tamas K Lengyel ; > George Dunlap ; Nakajima, Jun > ; Razvan Cojocaru ; > Suravee Suthikulpanit ; Brian Woods > > Subject: RE: [PATCH v5 11/15] mm / iommu: split need_iommu() into > has_iommu_pt() and sync_iommu_pt() > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > Sent: Saturday, August 4, 2018 1:22 AM > > > > The name 'need_iommu' is a little confusing as it suggests a domain needs > > to use the IOMMU but something might not be set up yet, when in fact it > > doesn't become true until IOMMU mappings for the domain have been > > created. > > > > Two different meanings are also inferred from it in various places in the > > code: > > > > - Some callers want to test whether a domain has IOMMU mappings > > - Some callers want to test whether they need to synchronize the domain's > > P2M and IOMMU mappings > > > > This patch therefore creates two new boolean flags, 'has_iommu_pt' and > > 'sync_iommu_pt' to convey those meanings separately and creates the > > corresponding macros. All callers of need_iommu() are then modified to > > use the macro appropriate to what they are trying to test. > > sync_iommu_pt sounds like an operation. what about need_sync_iommu? > > and for has_iommu_pt, what about iommu_enabled where 'enabled' > implies page table created? I could do 'need_iommu_pt_sync', perhaps, but 'iommu_enabled' will clash with the global host flag which is why I went with 'has_iommu_pt'. Paul ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 06/15] public / x86: introduce __HYPERCALL_iommu_op
>>> On 07.08.18 at 10:10, wrote: >> From: Tian, Kevin [mailto:kevin.t...@intel.com] >> Sent: 07 August 2018 04:00 >> >> > From: Paul Durrant >> > Sent: Saturday, August 4, 2018 1:22 AM >> > >> > diff --git a/xen/arch/x86/iommu_op.c b/xen/arch/x86/iommu_op.c >> > new file mode 100644 >> > index 00..744c0fce27 >> > --- /dev/null >> > +++ b/xen/arch/x86/iommu_op.c >> > @@ -0,0 +1,184 @@ >> > >> +/* >> *** >> > ** >> > + * x86/iommu_op.c >> > + * >> > + * Paravirtualised IOMMU functionality >> >> although only x86 is supported, logic in this file is vendor agnostic. Better >> move to a common place now... >> > > Personally I don't like to expand scope into architectures which I have no > facility to test. IIUC ARM also very much relies on an identity GFN:BFN map > so this functionality is unlikely to get used by anything other than x86 for > the foreseeable future. Well - moving to a common place doesn't mean it needs to be built for ARM. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 02/15] iommu: introduce the concept of BFN...
> From: Paul Durrant [mailto:paul.durr...@citrix.com] > Sent: Tuesday, August 7, 2018 4:00 PM > > > > * > > > * gfn: Guest Frame Number > > > * The values a guest puts in its own pagetables. For an auto- > translated > > > @@ -26,6 +27,11 @@ > > > * A linear idea of a guest physical address space. For an auto- > translated > > > * guest, pfn == gfn while for a non-translated guest, pfn != gfn. > > > * > > > + * bfn: Bus Frame Number (definitions in include/xen/iommu.h) > > > + * The linear frame numbers of IOMMU address space. All initiators > for a > > > + * given domain share a single IOMMU address space and, by default, > > > Xen will > > > + * ensure bfn == pfn. > > > + * > > > > Above description is a bit confusing: > > > > - for 'domain', are you talking about VM or IOMMU domain? If the > former > > then each initiator could have its own IOMMU address space when a > virtual > > IOMMU (either pvIOMMU or emulated IOMMU) is exposed. > > > > I mean VM and, yes, while each initiator *could* have its own IOMMU > address space, that is not what we do in (or have ever done) in Xen. I can > replace the word 'domain' with the abbreviation 'VM' if you feel it makes > things clearer. > domain is OK here. just want to confirm. > > - since you talk about 'by default', better also talk about non-default > > case, > > i.e. when virtual IOMMU is exposed then bfn is a separate address space > > managed by guest > > That's putting the cart before the horse really. There is no PV-IOMMU at > this stage in the series. I guess I could modify the comment in the later > patch where I add the hypercall to enable PV-IOMMU. > it's fine as long as finally the information is complete. Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
>>> On 07.08.18 at 10:21, wrote: >> From: Tian, Kevin [mailto:kevin.t...@intel.com] >> Sent: 07 August 2018 04:25 >> >> > From: Paul Durrant [mailto:paul.durr...@citrix.com] >> > Sent: Saturday, August 4, 2018 1:22 AM >> > >> > --- a/xen/drivers/passthrough/vtd/iommu.c >> > +++ b/xen/drivers/passthrough/vtd/iommu.c >> > @@ -1830,6 +1830,39 @@ static int __must_check >> > intel_iommu_unmap_page(struct domain *d, >> > return dma_pte_clear_one(d, bfn_to_baddr(bfn)); >> > } >> > >> > +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn, mfn_t >> > *mfn, >> > + unsigned int *flags) >> >> Not looking at later patches yet... but in concept bfn address >> space is per device instead of per domain. > > Not in this case. Xen has always maintained a single IOMMU address per > virtual machine. That is what BFN refers to. Nut is that a model we can maintain mid and long term? In particular on ARM, where Julien has told me a single system could have multiple _different_ IOMMUs, I could easily see the address spaces diverging. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
> From: Paul Durrant [mailto:paul.durr...@citrix.com]
> Sent: Tuesday, August 7, 2018 4:22 PM
>
> > -Original Message-
> > From: Tian, Kevin [mailto:kevin.t...@intel.com]
> > Sent: 07 August 2018 04:25
> > To: Paul Durrant ; xen-
> de...@lists.xenproject.org
> > Cc: Jan Beulich ; George Dunlap
> >
> > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to
> iommu_ops
> >
> > > From: Paul Durrant [mailto:paul.durr...@citrix.com]
> > > Sent: Saturday, August 4, 2018 1:22 AM
> > >
> > > This patch adds a new method to the VT-d IOMMU implementation to
> find
> > > the
> > > MFN currently mapped by the specified BFN along with a wrapper
> function
> > > in
> > > generic IOMMU code to call the implementation if it exists.
> > >
> > > This functionality will be used by a subsequent patch.
> > >
> > > Signed-off-by: Paul Durrant
> > > Reviewed-by: Wei Liu
> > > ---
> > > Cc: Kevin Tian
> > > Cc: Jan Beulich
> > > Cc: George Dunlap
> > >
> > > v3:
> > > - Addressed comments from George.
> > >
> > > v2:
> > > - Addressed some comments from Jan.
> > > ---
> > > xen/drivers/passthrough/iommu.c | 11 +++
> > > xen/drivers/passthrough/vtd/iommu.c | 34
> > > ++
> > > xen/drivers/passthrough/vtd/iommu.h | 3 +++
> > > xen/include/xen/iommu.h | 4
> > > 4 files changed, 52 insertions(+)
> > >
> > > diff --git a/xen/drivers/passthrough/iommu.c
> > > b/xen/drivers/passthrough/iommu.c
> > > index b10a37e5d7..9b7baca93f 100644
> > > --- a/xen/drivers/passthrough/iommu.c
> > > +++ b/xen/drivers/passthrough/iommu.c
> > > @@ -305,6 +305,17 @@ int iommu_unmap_page(struct domain *d,
> bfn_t
> > > bfn)
> > > return rc;
> > > }
> > >
> > > +int iommu_lookup_page(struct domain *d, bfn_t bfn, mfn_t *mfn,
> > > + unsigned int *flags)
> > > +{
> > > +const struct domain_iommu *hd = dom_iommu(d);
> > > +
> > > +if ( !iommu_enabled || !hd->platform_ops )
> > > +return -EOPNOTSUPP;
> > > +
> > > +return hd->platform_ops->lookup_page(d, bfn, mfn, flags);
> > > +}
> > > +
> > > static void iommu_free_pagetables(unsigned long unused)
> > > {
> > > do {
> > > diff --git a/xen/drivers/passthrough/vtd/iommu.c
> > > b/xen/drivers/passthrough/vtd/iommu.c
> > > index 282e227414..8cd3b59aa0 100644
> > > --- a/xen/drivers/passthrough/vtd/iommu.c
> > > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > > @@ -1830,6 +1830,39 @@ static int __must_check
> > > intel_iommu_unmap_page(struct domain *d,
> > > return dma_pte_clear_one(d, bfn_to_baddr(bfn));
> > > }
> > >
> > > +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn,
> mfn_t
> > > *mfn,
> > > + unsigned int *flags)
> >
> > Not looking at later patches yet... but in concept bfn address
> > space is per device instead of per domain.
>
> Not in this case. Xen has always maintained a single IOMMU address per
> virtual machine. That is what BFN refers to.
>
> Paul
>
> > In default situation
> > (w/o pvIOMMU exposed), all devices assigned to dom0 share
> > the same address space (bfn=pfn) which is currently linked
> > from domain structure. Then with pvIOMMU exposed, dom0
> > starts to manage individual pfn address space (called IOVA
> > address space within dom0) per assigned device. In that case
> > lookup should accept a bdf number and then find the right
> > page table...
> >
>
> No. That is over-complicating things and would probably involve re-writing
> most of the IOMMU code in Xen AFAICT.
>
it's not over-complicating. it's about correctness.
think about the role of IOMMU. Now you expose it to dom0. Then
dom0 wants to use it to isolate devices. What is the matter having
an IOMMU which supports only one global address space cross all
devices, from dom0's p.o.v? If that is the case, why does dom0 ever
want to change bfn globally on all devices?
Thanks
Kevin
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable modification of IOMMU mappings
> -Original Message-
> From: Tian, Kevin [mailto:kevin.t...@intel.com]
> Sent: 07 August 2018 05:08
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Wei Liu
> ; George Dunlap ;
> Andrew Cooper ; Ian Jackson
> ; Tim (Xen.org) ; Julien Grall
> ; Jan Beulich
> Subject: RE: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable
> modification of IOMMU mappings
>
> > From: Paul Durrant
> > Sent: Saturday, August 4, 2018 1:22 AM
> >
> > This patch adds an iommu_op which checks whether it is possible or
> > safe for a domain to modify its own IOMMU mappings and, if so, creates
> > a rangeset to track modifications.
>
> Have to say that there might be a concept mismatch between us,
> so I will stop review here until we get aligned on the basic
> understanding.
>
> What an IOMMU does is to provide DMA isolation between devices.
> Each device can be hooked with a different translation structure
> (representing a different bfn address space). Linux kernel uses this
> mechanism to harden kernel drivers (through dma APIs). Multiple
> devices can be also attached to the same address space, used by
> hypervisor when devices are assigned to the same VM.
>
Indeed.
> Now with pvIOMMU exposed to dom0, , dom0 could use it to harden
> kernel drivers too. Then there will be multiple bfn address spaces:
>
> - A default bfn address space created by Xen, where bfn = pfn
> - multiple per-bdf bfn address spaces created by Dom0, where
> bfn is completely irrelevant to pfn.
>
> the default space should not be changed by Dom0. It is attached
> to devices which dom0 doesn't enable pviommu mapping.
No that's not the point here. I'm not trying to re-architect Xen's IOMMU
handling. All the IOMMU code in Xen AFAICT is built around the assumption there
is one set of page tables per-VM and all devices assigned to the VM get the
same page tables. I suspect trying to change that will be a huge can of worms
and I have no need to go there for my purposes.
>
> per-bdf address spaces can be changed by Dom0, attached to
> devices which dom0 enables pviommu mapping. then pviommu ops
> should accept a bdf parameter. and internally Xen needs to maintain
> multiple page tables under dom0, and find a right page table according
> to specified bdf to complete the operation.
>
> Now your series look assuming always just one bfn address space
> cross all assigned devices per domain... I'm not sure how it works.
>
It does make that assumption because that assumption is baked into Xen's IOMMU
support.
> Did I misunderstand anything?
Only perhaps that moving away from per-VM IOMMU pagetables will be something
that is something I could do without making very invasive and lengthy changes
to Xen's IOMMU code.
Paul
>
> >
> > NOTE: The actual map and unmap operations are introduced by
> > subsequent
> > patches.
> >
> > Signed-off-by: Paul Durrant
> > ---
> > Cc: Jan Beulich
> > Cc: Andrew Cooper
> > Cc: George Dunlap
> > Cc: Ian Jackson
> > Cc: Julien Grall
> > Cc: Konrad Rzeszutek Wilk
> > Cc: Stefano Stabellini
> > Cc: Tim Deegan
> > Cc: Wei Liu
> >
> > v4:
> > - Set sync_iommu_pt to false instead of need_iommu.
> >
> > v2:
> > - New in v2.
> > ---
> > xen/arch/x86/iommu_op.c | 42
> > +
> > xen/drivers/passthrough/iommu.c | 2 +-
> > xen/drivers/passthrough/pci.c | 4 ++--
> > xen/include/public/iommu_op.h | 6 ++
> > xen/include/xen/iommu.h | 3 +++
> > 5 files changed, 54 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/x86/iommu_op.c b/xen/arch/x86/iommu_op.c
> > index bcfcd49102..b29547bffd 100644
> > --- a/xen/arch/x86/iommu_op.c
> > +++ b/xen/arch/x86/iommu_op.c
> > @@ -78,6 +78,42 @@ static int iommu_op_query_reserved(struct
> > xen_iommu_op_query_reserved *op)
> > return 0;
> > }
> >
> > +static int iommu_op_enable_modification(void)
> > +{
> > +struct domain *currd = current->domain;
> > +struct domain_iommu *iommu = dom_iommu(currd);
> > +const struct iommu_ops *ops = iommu->platform_ops;
> > +
> > +/* Has modification already been enabled? */
> > +if ( iommu->iommu_op_ranges )
> > +return 0;
> > +
> > +/*
> > + * The IOMMU mappings cannot be modified if:
> > + * - the IOMMU is not enabled or,
> > + * - the current domain is dom0 and tranlsation is disabled or,
> > + * - HAP is enabled and the IOMMU shares the mappings.
> > + */
> > +if ( !iommu_enabled ||
> > + (is_hardware_domain(currd) && iommu_passthrough) ||
> > + iommu_use_hap_pt(currd) )
> > +return -EACCES;
> > +
> > +/*
> > + * The IOMMU implementation must provide the lookup method if
> > + * modification of the mappings is to be supported.
> > + */
> > +if ( !ops->lookup_page )
> > +return -EOPNOTSUPP;
> > +
> > +iommu->iommu_op_ranges = rangeset_new(currd, NULL, 0);
> > +if ( !iommu->iommu_op_ranges )
> > +
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
> From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: Tuesday, August 7, 2018 4:30 PM > > >>> On 07.08.18 at 10:21, wrote: > >> From: Tian, Kevin [mailto:kevin.t...@intel.com] > >> Sent: 07 August 2018 04:25 > >> > >> > From: Paul Durrant [mailto:paul.durr...@citrix.com] > >> > Sent: Saturday, August 4, 2018 1:22 AM > >> > > >> > --- a/xen/drivers/passthrough/vtd/iommu.c > >> > +++ b/xen/drivers/passthrough/vtd/iommu.c > >> > @@ -1830,6 +1830,39 @@ static int __must_check > >> > intel_iommu_unmap_page(struct domain *d, > >> > return dma_pte_clear_one(d, bfn_to_baddr(bfn)); > >> > } > >> > > >> > +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn, > mfn_t > >> > *mfn, > >> > + unsigned int *flags) > >> > >> Not looking at later patches yet... but in concept bfn address > >> space is per device instead of per domain. > > > > Not in this case. Xen has always maintained a single IOMMU address per > > virtual machine. That is what BFN refers to. > > Nut is that a model we can maintain mid and long term? In particular > on ARM, where Julien has told me a single system could have multiple > _different_ IOMMUs, I could easily see the address spaces diverging. > multiple IOMMUs is another thing. what I questioned is that even one IOMMU needs to support mulitiple address spaces. That is the point of an IOMMU... Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
> -Original Message-
> From: Tian, Kevin [mailto:kevin.t...@intel.com]
> Sent: 07 August 2018 09:31
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Jan Beulich ; George Dunlap
>
> Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
>
> > From: Paul Durrant [mailto:paul.durr...@citrix.com]
> > Sent: Tuesday, August 7, 2018 4:22 PM
> >
> > > -Original Message-
> > > From: Tian, Kevin [mailto:kevin.t...@intel.com]
> > > Sent: 07 August 2018 04:25
> > > To: Paul Durrant ; xen-
> > de...@lists.xenproject.org
> > > Cc: Jan Beulich ; George Dunlap
> > >
> > > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to
> > iommu_ops
> > >
> > > > From: Paul Durrant [mailto:paul.durr...@citrix.com]
> > > > Sent: Saturday, August 4, 2018 1:22 AM
> > > >
> > > > This patch adds a new method to the VT-d IOMMU implementation to
> > find
> > > > the
> > > > MFN currently mapped by the specified BFN along with a wrapper
> > function
> > > > in
> > > > generic IOMMU code to call the implementation if it exists.
> > > >
> > > > This functionality will be used by a subsequent patch.
> > > >
> > > > Signed-off-by: Paul Durrant
> > > > Reviewed-by: Wei Liu
> > > > ---
> > > > Cc: Kevin Tian
> > > > Cc: Jan Beulich
> > > > Cc: George Dunlap
> > > >
> > > > v3:
> > > > - Addressed comments from George.
> > > >
> > > > v2:
> > > > - Addressed some comments from Jan.
> > > > ---
> > > > xen/drivers/passthrough/iommu.c | 11 +++
> > > > xen/drivers/passthrough/vtd/iommu.c | 34
> > > > ++
> > > > xen/drivers/passthrough/vtd/iommu.h | 3 +++
> > > > xen/include/xen/iommu.h | 4
> > > > 4 files changed, 52 insertions(+)
> > > >
> > > > diff --git a/xen/drivers/passthrough/iommu.c
> > > > b/xen/drivers/passthrough/iommu.c
> > > > index b10a37e5d7..9b7baca93f 100644
> > > > --- a/xen/drivers/passthrough/iommu.c
> > > > +++ b/xen/drivers/passthrough/iommu.c
> > > > @@ -305,6 +305,17 @@ int iommu_unmap_page(struct domain *d,
> > bfn_t
> > > > bfn)
> > > > return rc;
> > > > }
> > > >
> > > > +int iommu_lookup_page(struct domain *d, bfn_t bfn, mfn_t *mfn,
> > > > + unsigned int *flags)
> > > > +{
> > > > +const struct domain_iommu *hd = dom_iommu(d);
> > > > +
> > > > +if ( !iommu_enabled || !hd->platform_ops )
> > > > +return -EOPNOTSUPP;
> > > > +
> > > > +return hd->platform_ops->lookup_page(d, bfn, mfn, flags);
> > > > +}
> > > > +
> > > > static void iommu_free_pagetables(unsigned long unused)
> > > > {
> > > > do {
> > > > diff --git a/xen/drivers/passthrough/vtd/iommu.c
> > > > b/xen/drivers/passthrough/vtd/iommu.c
> > > > index 282e227414..8cd3b59aa0 100644
> > > > --- a/xen/drivers/passthrough/vtd/iommu.c
> > > > +++ b/xen/drivers/passthrough/vtd/iommu.c
> > > > @@ -1830,6 +1830,39 @@ static int __must_check
> > > > intel_iommu_unmap_page(struct domain *d,
> > > > return dma_pte_clear_one(d, bfn_to_baddr(bfn));
> > > > }
> > > >
> > > > +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn,
> > mfn_t
> > > > *mfn,
> > > > + unsigned int *flags)
> > >
> > > Not looking at later patches yet... but in concept bfn address
> > > space is per device instead of per domain.
> >
> > Not in this case. Xen has always maintained a single IOMMU address per
> > virtual machine. That is what BFN refers to.
> >
> > Paul
> >
> > > In default situation
> > > (w/o pvIOMMU exposed), all devices assigned to dom0 share
> > > the same address space (bfn=pfn) which is currently linked
> > > from domain structure. Then with pvIOMMU exposed, dom0
> > > starts to manage individual pfn address space (called IOVA
> > > address space within dom0) per assigned device. In that case
> > > lookup should accept a bdf number and then find the right
> > > page table...
> > >
> >
> > No. That is over-complicating things and would probably involve re-writing
> > most of the IOMMU code in Xen AFAICT.
> >
>
> it's not over-complicating. it's about correctness.
>
> think about the role of IOMMU. Now you expose it to dom0. Then
> dom0 wants to use it to isolate devices. What is the matter having
> an IOMMU which supports only one global address space cross all
> devices, from dom0's p.o.v? If that is the case, why does dom0 ever
> want to change bfn globally on all devices?
>
Because even a PV dom0 would be beter off with a BFN:GFN map rather than a
BFN:MFN map, since then the xen_swiotlb code and be avoided and lots of things
that are contiguous in GFN space become contiguous in BFN space and hence
become DMA-able without bouncing.
Paul
> Thanks
> Kevin
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
> -Original Message- > From: Tian, Kevin [mailto:kevin.t...@intel.com] > Sent: 07 August 2018 09:33 > To: Jan Beulich ; Paul Durrant > > Cc: George Dunlap ; xen-devel de...@lists.xenproject.org> > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops > > > From: Jan Beulich [mailto:jbeul...@suse.com] > > Sent: Tuesday, August 7, 2018 4:30 PM > > > > >>> On 07.08.18 at 10:21, wrote: > > >> From: Tian, Kevin [mailto:kevin.t...@intel.com] > > >> Sent: 07 August 2018 04:25 > > >> > > >> > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > >> > Sent: Saturday, August 4, 2018 1:22 AM > > >> > > > >> > --- a/xen/drivers/passthrough/vtd/iommu.c > > >> > +++ b/xen/drivers/passthrough/vtd/iommu.c > > >> > @@ -1830,6 +1830,39 @@ static int __must_check > > >> > intel_iommu_unmap_page(struct domain *d, > > >> > return dma_pte_clear_one(d, bfn_to_baddr(bfn)); > > >> > } > > >> > > > >> > +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn, > > mfn_t > > >> > *mfn, > > >> > + unsigned int *flags) > > >> > > >> Not looking at later patches yet... but in concept bfn address > > >> space is per device instead of per domain. > > > > > > Not in this case. Xen has always maintained a single IOMMU address per > > > virtual machine. That is what BFN refers to. > > > > Nut is that a model we can maintain mid and long term? In particular > > on ARM, where Julien has told me a single system could have multiple > > _different_ IOMMUs, I could easily see the address spaces diverging. > > > > multiple IOMMUs is another thing. > > what I questioned is that even one IOMMU needs to support mulitiple > address spaces. That is the point of an IOMMU... Indeed and that is why we use it to enforce domain separation. I see no need, as yet, to enforce separation within a domain. That need may arise later and the code can be modified at that point. Paul > > Thanks > Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable modification of IOMMU mappings
> From: Paul Durrant [mailto:paul.durr...@citrix.com] > Sent: Tuesday, August 7, 2018 4:33 PM > > > > > > From: Paul Durrant > > > Sent: Saturday, August 4, 2018 1:22 AM > > > > > > This patch adds an iommu_op which checks whether it is possible or > > > safe for a domain to modify its own IOMMU mappings and, if so, > creates > > > a rangeset to track modifications. > > > > Have to say that there might be a concept mismatch between us, > > so I will stop review here until we get aligned on the basic > > understanding. > > > > What an IOMMU does is to provide DMA isolation between devices. > > Each device can be hooked with a different translation structure > > (representing a different bfn address space). Linux kernel uses this > > mechanism to harden kernel drivers (through dma APIs). Multiple > > devices can be also attached to the same address space, used by > > hypervisor when devices are assigned to the same VM. > > > > Indeed. > > > Now with pvIOMMU exposed to dom0, , dom0 could use it to harden > > kernel drivers too. Then there will be multiple bfn address spaces: > > > > - A default bfn address space created by Xen, where bfn = pfn > > - multiple per-bdf bfn address spaces created by Dom0, where > > bfn is completely irrelevant to pfn. > > > > the default space should not be changed by Dom0. It is attached > > to devices which dom0 doesn't enable pviommu mapping. > > No that's not the point here. I'm not trying to re-architect Xen's IOMMU > handling. All the IOMMU code in Xen AFAICT is built around the assumption > there is one set of page tables per-VM and all devices assigned to the VM > get the same page tables. I suspect trying to change that will be a huge can > of worms and I have no need to go there for my purposes. don't just think from Xen side. think about what Dom0 feels about this IOMMU. ideally pviommu driver is a new vendor driver attached to iommu core within dom0. it needs to provide iommu dma ops to support dma_alloc/map operations from different device drivers. iommu core maintains a separate iova space for each device, so device drivers can be isolated from each other. Now dom0 got only one global space. then why does dom0 need to enable pviommu at all? > > > > > per-bdf address spaces can be changed by Dom0, attached to > > devices which dom0 enables pviommu mapping. then pviommu ops > > should accept a bdf parameter. and internally Xen needs to maintain > > multiple page tables under dom0, and find a right page table according > > to specified bdf to complete the operation. > > > > Now your series look assuming always just one bfn address space > > cross all assigned devices per domain... I'm not sure how it works. > > > > It does make that assumption because that assumption is baked into Xen's > IOMMU support. > > > Did I misunderstand anything? > > Only perhaps that moving away from per-VM IOMMU pagetables will be > something that is something I could do without making very invasive and > lengthy changes to Xen's IOMMU code. > it's a must imo. Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable modification of IOMMU mappings
> -Original Message- > From: Tian, Kevin [mailto:kevin.t...@intel.com] > Sent: 07 August 2018 09:38 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Stefano Stabellini ; Wei Liu > ; George Dunlap ; > Andrew Cooper ; Ian Jackson > ; Tim (Xen.org) ; Julien Grall > ; Jan Beulich > Subject: RE: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable > modification of IOMMU mappings > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > Sent: Tuesday, August 7, 2018 4:33 PM > > > > > > > > > From: Paul Durrant > > > > Sent: Saturday, August 4, 2018 1:22 AM > > > > > > > > This patch adds an iommu_op which checks whether it is possible or > > > > safe for a domain to modify its own IOMMU mappings and, if so, > > creates > > > > a rangeset to track modifications. > > > > > > Have to say that there might be a concept mismatch between us, > > > so I will stop review here until we get aligned on the basic > > > understanding. > > > > > > What an IOMMU does is to provide DMA isolation between devices. > > > Each device can be hooked with a different translation structure > > > (representing a different bfn address space). Linux kernel uses this > > > mechanism to harden kernel drivers (through dma APIs). Multiple > > > devices can be also attached to the same address space, used by > > > hypervisor when devices are assigned to the same VM. > > > > > > > Indeed. > > > > > Now with pvIOMMU exposed to dom0, , dom0 could use it to harden > > > kernel drivers too. Then there will be multiple bfn address spaces: > > > > > > - A default bfn address space created by Xen, where bfn = pfn > > > - multiple per-bdf bfn address spaces created by Dom0, where > > > bfn is completely irrelevant to pfn. > > > > > > the default space should not be changed by Dom0. It is attached > > > to devices which dom0 doesn't enable pviommu mapping. > > > > No that's not the point here. I'm not trying to re-architect Xen's IOMMU > > handling. All the IOMMU code in Xen AFAICT is built around the assumption > > there is one set of page tables per-VM and all devices assigned to the VM > > get the same page tables. I suspect trying to change that will be a huge can > > of worms and I have no need to go there for my purposes. > > don't just think from Xen side. think about what Dom0 feels about > this IOMMU. > > ideally pviommu driver is a new vendor driver attached to iommu > core within dom0. it needs to provide iommu dma ops to support > dma_alloc/map operations from different device drivers. iommu > core maintains a separate iova space for each device, so device > drivers can be isolated from each other. But there is nothing that means that the IOVA space cannot be global, and that is good enough for a PV dom0. > > Now dom0 got only one global space. then why does dom0 need > to enable pviommu at all? As I explained in another reply, it is primarily to allow a PV dom0 to have a BFN:GFN map. Since a PV domain maintains its own P2M then it is the domain that maintains the mapping. That is all I need to do. > > > > > > > > > per-bdf address spaces can be changed by Dom0, attached to > > > devices which dom0 enables pviommu mapping. then pviommu ops > > > should accept a bdf parameter. and internally Xen needs to maintain > > > multiple page tables under dom0, and find a right page table according > > > to specified bdf to complete the operation. > > > > > > Now your series look assuming always just one bfn address space > > > cross all assigned devices per domain... I'm not sure how it works. > > > > > > > It does make that assumption because that assumption is baked into Xen's > > IOMMU support. > > > > > Did I misunderstand anything? > > > > Only perhaps that moving away from per-VM IOMMU pagetables will be > > something that is something I could do without making very invasive and > > lengthy changes to Xen's IOMMU code. > > > > it's a must imo. I may was well give up then. That's a mammoth task akin to e.g. moving to per-vcpu rather than per-domain P2Ms. I don't have a spare year to do the work. Paul > > Thanks > Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
> From: Paul Durrant [mailto:paul.durr...@citrix.com] > Sent: Tuesday, August 7, 2018 4:36 PM > > > > > > +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn, > > > mfn_t > > > > > *mfn, > > > > > + unsigned int *flags) > > > > > > > > Not looking at later patches yet... but in concept bfn address > > > > space is per device instead of per domain. > > > > > > Not in this case. Xen has always maintained a single IOMMU address > per > > > virtual machine. That is what BFN refers to. > > > > > > Paul > > > > > > > In default situation > > > > (w/o pvIOMMU exposed), all devices assigned to dom0 share > > > > the same address space (bfn=pfn) which is currently linked > > > > from domain structure. Then with pvIOMMU exposed, dom0 > > > > starts to manage individual pfn address space (called IOVA > > > > address space within dom0) per assigned device. In that case > > > > lookup should accept a bdf number and then find the right > > > > page table... > > > > > > > > > > No. That is over-complicating things and would probably involve re- > writing > > > most of the IOMMU code in Xen AFAICT. > > > > > > > it's not over-complicating. it's about correctness. > > > > think about the role of IOMMU. Now you expose it to dom0. Then > > dom0 wants to use it to isolate devices. What is the matter having > > an IOMMU which supports only one global address space cross all > > devices, from dom0's p.o.v? If that is the case, why does dom0 ever > > want to change bfn globally on all devices? > > > > Because even a PV dom0 would be beter off with a BFN:GFN map rather > than a BFN:MFN map, since then the xen_swiotlb code and be avoided and > lots of things that are contiguous in GFN space become contiguous in BFN > space and hence become DMA-able without bouncing. > so this pvIOMMU really loses most functionalities which a physical IOMMU provides... maybe not call it pvIOMMU. sort of pvDMA engine. Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
> From: Paul Durrant [mailto:paul.durr...@citrix.com] > Sent: Tuesday, August 7, 2018 4:37 PM > > > -Original Message- > > From: Tian, Kevin [mailto:kevin.t...@intel.com] > > Sent: 07 August 2018 09:33 > > To: Jan Beulich ; Paul Durrant > > > > Cc: George Dunlap ; xen-devel > de...@lists.xenproject.org> > > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to > iommu_ops > > > > > From: Jan Beulich [mailto:jbeul...@suse.com] > > > Sent: Tuesday, August 7, 2018 4:30 PM > > > > > > >>> On 07.08.18 at 10:21, wrote: > > > >> From: Tian, Kevin [mailto:kevin.t...@intel.com] > > > >> Sent: 07 August 2018 04:25 > > > >> > > > >> > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > > >> > Sent: Saturday, August 4, 2018 1:22 AM > > > >> > > > > >> > --- a/xen/drivers/passthrough/vtd/iommu.c > > > >> > +++ b/xen/drivers/passthrough/vtd/iommu.c > > > >> > @@ -1830,6 +1830,39 @@ static int __must_check > > > >> > intel_iommu_unmap_page(struct domain *d, > > > >> > return dma_pte_clear_one(d, bfn_to_baddr(bfn)); > > > >> > } > > > >> > > > > >> > +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn, > > > mfn_t > > > >> > *mfn, > > > >> > + unsigned int *flags) > > > >> > > > >> Not looking at later patches yet... but in concept bfn address > > > >> space is per device instead of per domain. > > > > > > > > Not in this case. Xen has always maintained a single IOMMU address > per > > > > virtual machine. That is what BFN refers to. > > > > > > Nut is that a model we can maintain mid and long term? In particular > > > on ARM, where Julien has told me a single system could have multiple > > > _different_ IOMMUs, I could easily see the address spaces diverging. > > > > > > > multiple IOMMUs is another thing. > > > > what I questioned is that even one IOMMU needs to support mulitiple > > address spaces. That is the point of an IOMMU... > > Indeed and that is why we use it to enforce domain separation. I see no > need, as yet, to enforce separation within a domain. That need may arise > later and the code can be modified at that point. > but then you need completely different set of APIs at that time... Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.9 test] 125764: trouble: blocked/broken/fail/pass
flight 125764 linux-4.9 real [real] http://logs.test-lab.xenproject.org/osstest/logs/125764/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-pvopsbroken build-arm64 broken build-arm64-xsm broken Regressions which are regarded as allowable (not blocking): build-arm64 2 hosts-allocate broken REGR. vs. 125183 build-arm64-xsm 2 hosts-allocate broken REGR. vs. 125183 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 125183 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-examine 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a build-arm64 3 capture-logs broken blocked in 125183 build-arm64-xsm 3 capture-logs broken blocked in 125183 build-arm64-pvops 3 capture-logs broken blocked in 125183 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125183 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125183 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 125183 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125183 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125183 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-xl-pvhv2-intel 12 guest-start fail never pass test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-pvhv2-amd 12 guest-start fail 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-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-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-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 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-libvirt 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-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-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-ws16-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-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass test-amd64-i386-xl-qemut-win10-i386 10 windows-install fail never pass version targeted for testing: linuxe01202b36f03f9284ffb
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
> -Original Message- > From: Tian, Kevin [mailto:kevin.t...@intel.com] > Sent: 07 August 2018 09:48 > To: Paul Durrant ; Jan Beulich > > Cc: George Dunlap ; xen-devel de...@lists.xenproject.org> > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > Sent: Tuesday, August 7, 2018 4:37 PM > > > > > -Original Message- > > > From: Tian, Kevin [mailto:kevin.t...@intel.com] > > > Sent: 07 August 2018 09:33 > > > To: Jan Beulich ; Paul Durrant > > > > > > Cc: George Dunlap ; xen-devel > > de...@lists.xenproject.org> > > > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to > > iommu_ops > > > > > > > From: Jan Beulich [mailto:jbeul...@suse.com] > > > > Sent: Tuesday, August 7, 2018 4:30 PM > > > > > > > > >>> On 07.08.18 at 10:21, wrote: > > > > >> From: Tian, Kevin [mailto:kevin.t...@intel.com] > > > > >> Sent: 07 August 2018 04:25 > > > > >> > > > > >> > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > > > >> > Sent: Saturday, August 4, 2018 1:22 AM > > > > >> > > > > > >> > --- a/xen/drivers/passthrough/vtd/iommu.c > > > > >> > +++ b/xen/drivers/passthrough/vtd/iommu.c > > > > >> > @@ -1830,6 +1830,39 @@ static int __must_check > > > > >> > intel_iommu_unmap_page(struct domain *d, > > > > >> > return dma_pte_clear_one(d, bfn_to_baddr(bfn)); > > > > >> > } > > > > >> > > > > > >> > +static int intel_iommu_lookup_page(struct domain *d, bfn_t bfn, > > > > mfn_t > > > > >> > *mfn, > > > > >> > + unsigned int *flags) > > > > >> > > > > >> Not looking at later patches yet... but in concept bfn address > > > > >> space is per device instead of per domain. > > > > > > > > > > Not in this case. Xen has always maintained a single IOMMU address > > per > > > > > virtual machine. That is what BFN refers to. > > > > > > > > Nut is that a model we can maintain mid and long term? In particular > > > > on ARM, where Julien has told me a single system could have multiple > > > > _different_ IOMMUs, I could easily see the address spaces diverging. > > > > > > > > > > multiple IOMMUs is another thing. > > > > > > what I questioned is that even one IOMMU needs to support mulitiple > > > address spaces. That is the point of an IOMMU... > > > > Indeed and that is why we use it to enforce domain separation. I see no > > need, as yet, to enforce separation within a domain. That need may arise > > later and the code can be modified at that point. > > > > but then you need completely different set of APIs at that time... Ok. Would you be happy if I add the option to supply a an SBDF in the map and unmap hypercalls but ignore the value for now? Or even have a 'global' flag but return -EOPNOTSUPP if it is not specified. That would avoid the need to add new hypercalls for per-device mapping. Paul > > Thanks > Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable modification of IOMMU mappings
> From: Paul Durrant > Sent: Tuesday, August 7, 2018 4:44 PM > > > -Original Message- > > From: Tian, Kevin [mailto:kevin.t...@intel.com] > > Sent: 07 August 2018 09:38 > > To: Paul Durrant ; xen- > de...@lists.xenproject.org > > Cc: Stefano Stabellini ; Wei Liu > > ; George Dunlap ; > > Andrew Cooper ; Ian Jackson > > ; Tim (Xen.org) ; Julien Grall > > ; Jan Beulich > > Subject: RE: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable > > modification of IOMMU mappings > > > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > > Sent: Tuesday, August 7, 2018 4:33 PM > > > > > > > > > > > > From: Paul Durrant > > > > > Sent: Saturday, August 4, 2018 1:22 AM > > > > > > > > > > This patch adds an iommu_op which checks whether it is possible or > > > > > safe for a domain to modify its own IOMMU mappings and, if so, > > > creates > > > > > a rangeset to track modifications. > > > > > > > > Have to say that there might be a concept mismatch between us, > > > > so I will stop review here until we get aligned on the basic > > > > understanding. > > > > > > > > What an IOMMU does is to provide DMA isolation between devices. > > > > Each device can be hooked with a different translation structure > > > > (representing a different bfn address space). Linux kernel uses this > > > > mechanism to harden kernel drivers (through dma APIs). Multiple > > > > devices can be also attached to the same address space, used by > > > > hypervisor when devices are assigned to the same VM. > > > > > > > > > > Indeed. > > > > > > > Now with pvIOMMU exposed to dom0, , dom0 could use it to harden > > > > kernel drivers too. Then there will be multiple bfn address spaces: > > > > > > > > - A default bfn address space created by Xen, where bfn = pfn > > > > - multiple per-bdf bfn address spaces created by Dom0, where > > > > bfn is completely irrelevant to pfn. > > > > > > > > the default space should not be changed by Dom0. It is attached > > > > to devices which dom0 doesn't enable pviommu mapping. > > > > > > No that's not the point here. I'm not trying to re-architect Xen's IOMMU > > > handling. All the IOMMU code in Xen AFAICT is built around the > assumption > > > there is one set of page tables per-VM and all devices assigned to the > VM > > > get the same page tables. I suspect trying to change that will be a huge > can > > > of worms and I have no need to go there for my purposes. > > > > don't just think from Xen side. think about what Dom0 feels about > > this IOMMU. > > > > ideally pviommu driver is a new vendor driver attached to iommu > > core within dom0. it needs to provide iommu dma ops to support > > dma_alloc/map operations from different device drivers. iommu > > core maintains a separate iova space for each device, so device > > drivers can be isolated from each other. > > But there is nothing that means that the IOVA space cannot be global, and > that is good enough for a PV dom0. You are right! Although my concept is all built around physical IOMMU capability, I did a check that Linux doesn't state that IOVA cannot be global. It's purely vendor IOMMU driver to decide. So here current version pvIOMMU only provides global address space, though unlike any existing IOMMU. maybe we should explicitly call out this fact in some capability field for future extension. > > > > > Now dom0 got only one global space. then why does dom0 need > > to enable pviommu at all? > > As I explained in another reply, it is primarily to allow a PV dom0 to have a > BFN:GFN map. Since a PV domain maintains its own P2M then it is the > domain that maintains the mapping. That is all I need to do. yes, I got this one. Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
> From: Paul Durrant [mailto:paul.durr...@citrix.com] > Sent: Tuesday, August 7, 2018 4:56 PM > > > -Original Message- > > From: Tian, Kevin [mailto:kevin.t...@intel.com] > > Sent: 07 August 2018 09:48 > > To: Paul Durrant ; Jan Beulich > > > > Cc: George Dunlap ; xen-devel > de...@lists.xenproject.org> > > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to > iommu_ops > > > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > > Sent: Tuesday, August 7, 2018 4:37 PM > > > > > > > -Original Message- > > > > From: Tian, Kevin [mailto:kevin.t...@intel.com] > > > > Sent: 07 August 2018 09:33 > > > > To: Jan Beulich ; Paul Durrant > > > > > > > > Cc: George Dunlap ; xen-devel > > > de...@lists.xenproject.org> > > > > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to > > > iommu_ops > > > > > > > > > From: Jan Beulich [mailto:jbeul...@suse.com] > > > > > Sent: Tuesday, August 7, 2018 4:30 PM > > > > > > > > > > >>> On 07.08.18 at 10:21, wrote: > > > > > >> From: Tian, Kevin [mailto:kevin.t...@intel.com] > > > > > >> Sent: 07 August 2018 04:25 > > > > > >> > > > > > >> > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > > > > >> > Sent: Saturday, August 4, 2018 1:22 AM > > > > > >> > > > > > > >> > --- a/xen/drivers/passthrough/vtd/iommu.c > > > > > >> > +++ b/xen/drivers/passthrough/vtd/iommu.c > > > > > >> > @@ -1830,6 +1830,39 @@ static int __must_check > > > > > >> > intel_iommu_unmap_page(struct domain *d, > > > > > >> > return dma_pte_clear_one(d, bfn_to_baddr(bfn)); > > > > > >> > } > > > > > >> > > > > > > >> > +static int intel_iommu_lookup_page(struct domain *d, bfn_t > bfn, > > > > > mfn_t > > > > > >> > *mfn, > > > > > >> > + unsigned int *flags) > > > > > >> > > > > > >> Not looking at later patches yet... but in concept bfn address > > > > > >> space is per device instead of per domain. > > > > > > > > > > > > Not in this case. Xen has always maintained a single IOMMU > address > > > per > > > > > > virtual machine. That is what BFN refers to. > > > > > > > > > > Nut is that a model we can maintain mid and long term? In > particular > > > > > on ARM, where Julien has told me a single system could have > multiple > > > > > _different_ IOMMUs, I could easily see the address spaces diverging. > > > > > > > > > > > > > multiple IOMMUs is another thing. > > > > > > > > what I questioned is that even one IOMMU needs to support mulitiple > > > > address spaces. That is the point of an IOMMU... > > > > > > Indeed and that is why we use it to enforce domain separation. I see no > > > need, as yet, to enforce separation within a domain. That need may > arise > > > later and the code can be modified at that point. > > > > > > > but then you need completely different set of APIs at that time... > > Ok. Would you be happy if I add the option to supply a an SBDF in the map > and unmap hypercalls but ignore the value for now? Or even have a 'global' > flag but return -EOPNOTSUPP if it is not specified. That would avoid the > need to add new hypercalls for per-device mapping. > that would be better! maybe also worthy of a capability query interface to tell whether global or per-bdf mapping is supported? Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops
> -Original Message- > From: Tian, Kevin [mailto:kevin.t...@intel.com] > Sent: 07 August 2018 10:03 > To: Paul Durrant ; Jan Beulich > > Cc: George Dunlap ; xen-devel de...@lists.xenproject.org> > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to iommu_ops > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > Sent: Tuesday, August 7, 2018 4:56 PM > > > > > -Original Message- > > > From: Tian, Kevin [mailto:kevin.t...@intel.com] > > > Sent: 07 August 2018 09:48 > > > To: Paul Durrant ; Jan Beulich > > > > > > Cc: George Dunlap ; xen-devel > > de...@lists.xenproject.org> > > > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to > > iommu_ops > > > > > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > > > Sent: Tuesday, August 7, 2018 4:37 PM > > > > > > > > > -Original Message- > > > > > From: Tian, Kevin [mailto:kevin.t...@intel.com] > > > > > Sent: 07 August 2018 09:33 > > > > > To: Jan Beulich ; Paul Durrant > > > > > > > > > > Cc: George Dunlap ; xen-devel > > > > de...@lists.xenproject.org> > > > > > Subject: RE: [PATCH v5 09/15] vtd: add lookup_page method to > > > > iommu_ops > > > > > > > > > > > From: Jan Beulich [mailto:jbeul...@suse.com] > > > > > > Sent: Tuesday, August 7, 2018 4:30 PM > > > > > > > > > > > > >>> On 07.08.18 at 10:21, wrote: > > > > > > >> From: Tian, Kevin [mailto:kevin.t...@intel.com] > > > > > > >> Sent: 07 August 2018 04:25 > > > > > > >> > > > > > > >> > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > > > > > >> > Sent: Saturday, August 4, 2018 1:22 AM > > > > > > >> > > > > > > > >> > --- a/xen/drivers/passthrough/vtd/iommu.c > > > > > > >> > +++ b/xen/drivers/passthrough/vtd/iommu.c > > > > > > >> > @@ -1830,6 +1830,39 @@ static int __must_check > > > > > > >> > intel_iommu_unmap_page(struct domain *d, > > > > > > >> > return dma_pte_clear_one(d, bfn_to_baddr(bfn)); > > > > > > >> > } > > > > > > >> > > > > > > > >> > +static int intel_iommu_lookup_page(struct domain *d, bfn_t > > bfn, > > > > > > mfn_t > > > > > > >> > *mfn, > > > > > > >> > + unsigned int *flags) > > > > > > >> > > > > > > >> Not looking at later patches yet... but in concept bfn address > > > > > > >> space is per device instead of per domain. > > > > > > > > > > > > > > Not in this case. Xen has always maintained a single IOMMU > > address > > > > per > > > > > > > virtual machine. That is what BFN refers to. > > > > > > > > > > > > Nut is that a model we can maintain mid and long term? In > > particular > > > > > > on ARM, where Julien has told me a single system could have > > multiple > > > > > > _different_ IOMMUs, I could easily see the address spaces > diverging. > > > > > > > > > > > > > > > > multiple IOMMUs is another thing. > > > > > > > > > > what I questioned is that even one IOMMU needs to support > mulitiple > > > > > address spaces. That is the point of an IOMMU... > > > > > > > > Indeed and that is why we use it to enforce domain separation. I see no > > > > need, as yet, to enforce separation within a domain. That need may > > arise > > > > later and the code can be modified at that point. > > > > > > > > > > but then you need completely different set of APIs at that time... > > > > Ok. Would you be happy if I add the option to supply a an SBDF in the map > > and unmap hypercalls but ignore the value for now? Or even have a 'global' > > flag but return -EOPNOTSUPP if it is not specified. That would avoid the > > need to add new hypercalls for per-device mapping. > > > > that would be better! maybe also worthy of a capability query interface > to tell whether global or per-bdf mapping is supported? I could make it an option to the 'enable' hypercall. A caller could try to enable per-device mappings first and, if that fails, fall back to global. Paul > > Thanks > Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable modification of IOMMU mappings
> -Original Message- > From: Tian, Kevin [mailto:kevin.t...@intel.com] > Sent: 07 August 2018 10:01 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Stefano Stabellini ; Wei Liu > ; Andrew Cooper ; Tim > (Xen.org) ; George Dunlap ; > Julien Grall ; Jan Beulich ; Ian > Jackson > Subject: RE: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable > modification of IOMMU mappings > > > From: Paul Durrant > > Sent: Tuesday, August 7, 2018 4:44 PM > > > > > -Original Message- > > > From: Tian, Kevin [mailto:kevin.t...@intel.com] > > > Sent: 07 August 2018 09:38 > > > To: Paul Durrant ; xen- > > de...@lists.xenproject.org > > > Cc: Stefano Stabellini ; Wei Liu > > > ; George Dunlap ; > > > Andrew Cooper ; Ian Jackson > > > ; Tim (Xen.org) ; Julien Grall > > > ; Jan Beulich > > > Subject: RE: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable > > > modification of IOMMU mappings > > > > > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > > > Sent: Tuesday, August 7, 2018 4:33 PM > > > > > > > > > > > > > > > From: Paul Durrant > > > > > > Sent: Saturday, August 4, 2018 1:22 AM > > > > > > > > > > > > This patch adds an iommu_op which checks whether it is possible or > > > > > > safe for a domain to modify its own IOMMU mappings and, if so, > > > > creates > > > > > > a rangeset to track modifications. > > > > > > > > > > Have to say that there might be a concept mismatch between us, > > > > > so I will stop review here until we get aligned on the basic > > > > > understanding. > > > > > > > > > > What an IOMMU does is to provide DMA isolation between devices. > > > > > Each device can be hooked with a different translation structure > > > > > (representing a different bfn address space). Linux kernel uses this > > > > > mechanism to harden kernel drivers (through dma APIs). Multiple > > > > > devices can be also attached to the same address space, used by > > > > > hypervisor when devices are assigned to the same VM. > > > > > > > > > > > > > Indeed. > > > > > > > > > Now with pvIOMMU exposed to dom0, , dom0 could use it to harden > > > > > kernel drivers too. Then there will be multiple bfn address spaces: > > > > > > > > > > - A default bfn address space created by Xen, where bfn = pfn > > > > > - multiple per-bdf bfn address spaces created by Dom0, where > > > > > bfn is completely irrelevant to pfn. > > > > > > > > > > the default space should not be changed by Dom0. It is attached > > > > > to devices which dom0 doesn't enable pviommu mapping. > > > > > > > > No that's not the point here. I'm not trying to re-architect Xen's IOMMU > > > > handling. All the IOMMU code in Xen AFAICT is built around the > > assumption > > > > there is one set of page tables per-VM and all devices assigned to the > > VM > > > > get the same page tables. I suspect trying to change that will be a huge > > can > > > > of worms and I have no need to go there for my purposes. > > > > > > don't just think from Xen side. think about what Dom0 feels about > > > this IOMMU. > > > > > > ideally pviommu driver is a new vendor driver attached to iommu > > > core within dom0. it needs to provide iommu dma ops to support > > > dma_alloc/map operations from different device drivers. iommu > > > core maintains a separate iova space for each device, so device > > > drivers can be isolated from each other. > > > > But there is nothing that means that the IOVA space cannot be global, and > > that is good enough for a PV dom0. > > You are right! Although my concept is all built around physical IOMMU > capability, I did a check that Linux doesn't state that IOVA cannot be > global. It's purely vendor IOMMU driver to decide. > Yes, that's my understanding. So Xen would be the vendor in this case and the strictly global IOVA space is not a problem. > So here current version pvIOMMU only provides global address space, > though unlike any existing IOMMU. maybe we should explicitly call out > this fact in some capability field for future extension. > Does Linux expose such a thing through the dma_ops? I didn't see one, but if there is one then we can of course set it correctly according to the limitations of the underying hypervisor. Thus, if Xen is modified in future to support per-device mappings, this could be exposed through to the drivers using DMA. > > > > > > > > Now dom0 got only one global space. then why does dom0 need > > > to enable pviommu at all? > > > > As I explained in another reply, it is primarily to allow a PV dom0 to have > > a > > BFN:GFN map. Since a PV domain maintains its own P2M then it is the > > domain that maintains the mapping. That is all I need to do. > > yes, I got this one. Ok, cool. Paul > > Thanks > Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable modification of IOMMU mappings
> From: Paul Durrant [mailto:paul.durr...@citrix.com] > Sent: Tuesday, August 7, 2018 5:12 PM > > > > So here current version pvIOMMU only provides global address space, > > though unlike any existing IOMMU. maybe we should explicitly call out > > this fact in some capability field for future extension. > > > > Does Linux expose such a thing through the dma_ops? I didn't see one, but > if there is one then we can of course set it correctly according to the > limitations of the underying hypervisor. Thus, if Xen is modified in future to > support per-device mappings, this could be exposed through to the drivers > using DMA. > not about Linux internal exposing. it's about hypervisor exposing the capability to pvIOMMU driver, so the latter knows whether to enable per-device mapping. But as you commented in another mail, it could be achieved through enabling sequence. btw could you put clarification according to our discussions in your next version coverletter to set the tone clear from start? I don't think I'll be the only one interpreting wrong goal for this work. :-) Thanks Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable modification of IOMMU mappings
> -Original Message- > From: Tian, Kevin [mailto:kevin.t...@intel.com] > Sent: 07 August 2018 10:20 > To: Paul Durrant ; xen-devel@lists.xenproject.org > Cc: Stefano Stabellini ; Wei Liu > ; Andrew Cooper ; Tim > (Xen.org) ; George Dunlap ; > Julien Grall ; Jan Beulich ; Ian > Jackson > Subject: RE: [Xen-devel] [PATCH v5 12/15] x86: add iommu_op to enable > modification of IOMMU mappings > > > From: Paul Durrant [mailto:paul.durr...@citrix.com] > > Sent: Tuesday, August 7, 2018 5:12 PM > > > > > > > So here current version pvIOMMU only provides global address space, > > > though unlike any existing IOMMU. maybe we should explicitly call out > > > this fact in some capability field for future extension. > > > > > > > Does Linux expose such a thing through the dma_ops? I didn't see one, but > > if there is one then we can of course set it correctly according to the > > limitations of the underying hypervisor. Thus, if Xen is modified in future > > to > > support per-device mappings, this could be exposed through to the drivers > > using DMA. > > > > not about Linux internal exposing. it's about hypervisor exposing > the capability to pvIOMMU driver, so the latter knows whether to > enable per-device mapping. But as you commented in another > mail, it could be achieved through enabling sequence. > Ok. Let's go with that then. > btw could you put clarification according to our discussions in your > next version coverletter to set the tone clear from start? I don't > think I'll be the only one interpreting wrong goal for this work. :-) > I can certainly do that. I'll put an explanation of the XenServer use-case in as a concrete example too. Paul > Thanks > Kevin ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] x86 Community Call - Wed Aug 15, 14:00 - 15:00 UTC - Call for agenda items
Dear community members, please send me agenda items for next week’s community call by this Friday. Note that this a week later than normal, because I had community calls in my diary as the week before Advisory Board meetings (not the 2nd Wed of each month) and I forgot to send out the agenda. I CC'ed attendees of the Arm call as sometimes there are cross-over issues which could be handled either in the Arm or x86 call. That way you get to see agenda, minutes, etc. and have a chance to decide on whether you want to join or not. Last month’s minutes are at https://docs.google.com/document/d/1fWQMuiblTmiNkWGGNbz20AQtbbnvtohxnUjhX2pA2jU/edit#heading=h.mz1wjb9vekjn Main topics and loose/ends: * Release Cadence for Xen 4.12 => resolved * Project Management stuff to keep the Momentum going => we had no Intel attendance at the last meeting and thus the status of various issues is unknown. Some ACTIONS were resolved Open ACTIONS/status unknown: * ACTION: George will update and re-submit the NVDIMM doc (he didn’t take any notes during the discussion - we are going to have to reconstruct some of the discussion) * ACTION: Rich to follow up with committ...@xenproject.org Resolution by committers@ off list: Allow anyone from THE REST to issue a "Maintainer-is-absent-override" ack for the XSM code while Daniel is away; the expectation, of course, that "the rest" will be very cautious and try not to do anything Daniel might object Best Regards Lars ## Future Community Call schedule Aug 15 (one week later than normal), Sept 12, Oct 10, Nov 14, Dec 12 ## Meeting time 14:00 - 15:00 UTC 10:00 - 11:00 EDT (New York) 15:00 - 16:00 BST (London) 16:00 - 17:00 CEST (Berlin) 22:00 - 23:00 CST (Beijing) Further International meeting times: https://www.timeanddate.com/worldclock/meetingdetails.html?year=2018&month=8&day=15&hour=14&min=0&sec=0&p1=224&p2=24&p3=179&p4=136&p5=37&p6=33 ## Dial in details Web: https://www.gotomeet.me/larskurth You can also dial in using your phone. Access Code: 906-886-965 China (Toll Free): 4008 811084 Germany: +49 692 5736 7317 Poland (Toll Free): 00 800 1124759 United Kingdom: +44 330 221 0088 United States: +1 (571) 317-3129 More phone numbers Australia: +61 2 9087 3604 Austria: +43 7 2081 5427 Argentina (Toll Free): 0 800 444 3375 Bahrain (Toll Free): 800 81 111 Belarus (Toll Free): 8 820 0011 0400 Belgium: +32 28 93 7018 Brazil (Toll Free): 0 800 047 4906 Bulgaria (Toll Free): 00800 120 4417 Canada: +1 (647) 497-9391 Chile (Toll Free): 800 395 150 Colombia (Toll Free): 01 800 518 4483 Czech Republic (Toll Free): 800 500448 Denmark: +45 32 72 03 82 Finland: +358 923 17 0568 France: +33 170 950 594 Greece (Toll Free): 00 800 4414 3838 Hong Kong (Toll Free): 30713169 Hungary (Toll Free): (06) 80 986 255 Iceland (Toll Free): 800 7204 India (Toll Free): 18002669272 Indonesia (Toll Free): 007 803 020 5375 Ireland: +353 15 360 728 Israel (Toll Free): 1 809 454 830 Italy: +39 0 247 92 13 01 Japan (Toll Free): 0 120 663 800 Korea, Republic of (Toll Free): 00798 14 207 4914 Luxembourg (Toll Free): 800 85158 Malaysia (Toll Free): 1 800 81 6854 Mexico (Toll Free): 01 800 522 1133 Netherlands: +31 207 941 377 New Zealand: +64 9 280 6302 Norway: +47 21 93 37 51 Panama (Toll Free): 00 800 226 7928 Peru (Toll Free): 0 800 77023 Philippines (Toll Free): 1 800 1110 1661 Portugal (Toll Free): 800 819 575 Romania (Toll Free): 0 800 410 029 Russian Federation (Toll Free): 8 800 100 6203 Saudi Arabia (Toll Free): 800 844 3633 Singapore (Toll Free): 18007231323 South Africa (Toll Free): 0 800 555 447 Spain: +34 932 75 2004 Sweden: +46 853 527 827 Switzerland: +41 225 4599 78 Taiwan (Toll Free): 0 800 666 854 Thailand (Toll Free): 001 800 011 023 Turkey (Toll Free): 00 800 4488 23683 Ukraine (Toll Free): 0 800 50 1733 United Arab Emirates (Toll Free): 800 044 40439 Uruguay (Toll Free): 0004 019 1018 Viet Nam (Toll Free): 122 80 481 First GoToMeeting? Let's do a quick system check: https://link.gotomeeting.com/system-check ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v2] xen/pv: Call get_cpu_address_sizes to set x86_virt/phys_bits
* Boris Ostrovsky wrote: > x86 maintainers, this needs your ack please. LGTM: Acked-by: Ingo Molnar Thanks, Ingo ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.9-testing test] 125766: regressions - trouble: blocked/broken/fail/pass
flight 125766 xen-4.9-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/125766/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64 broken build-arm64-pvopsbroken build-arm64-xsm broken build-arm644 host-install(4) broken in 125742 REGR. vs. 124328 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stopfail REGR. vs. 124248 test-amd64-amd64-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 124328 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-arndale 6 xen-installfail pass in 125742 test-armhf-armhf-xl-rtds 12 guest-startfail pass in 125742 Regressions which are regarded as allowable (not blocking): build-arm64-xsm 2 hosts-allocate broken REGR. vs. 124328 build-arm64 2 hosts-allocate broken REGR. vs. 124328 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 124328 Tests which did not succeed, but are not blocking: build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64-xsm 3 capture-logs broken blocked in 124328 build-arm64-pvops 3 capture-logs broken blocked in 124328 build-arm64 3 capture-logs broken blocked in 124328 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail blocked in 124328 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail in 125742 blocked in 124328 test-amd64-amd64-xl-qemut-ws16-amd64 18 guest-start/win.repeat fail in 125742 blocked in 124328 test-amd64-amd64-xl-qemuu-ws16-amd64 16 guest-localmigrate/x10 fail in 125742 like 124328 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeat fail in 125742 like 124328 test-amd64-i386-xl-qemut-ws16-amd64 16 guest-localmigrate/x10 fail in 125742 like 124328 test-armhf-armhf-xl-arndale 13 migrate-support-check fail in 125742 never pass test-armhf-armhf-xl-arndale 14 saverestore-support-check fail in 125742 never pass test-armhf-armhf-xl-rtds13 migrate-support-check fail in 125742 never pass test-armhf-armhf-xl-rtds 14 saverestore-support-check fail in 125742 never pass test-amd64-i386-libvirt-pair 22 guest-migrate/src_host/dst_host fail like 124248 test-amd64-amd64-xl-qemuu-ws16-amd64 14 guest-localmigratefail like 124248 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 124248 test-amd64-amd64-xl-qemut-win7-amd64 16 guest-localmigrate/x10 fail like 124328 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 124328 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 124328 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 124328 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-i386-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-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-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-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-libvirt 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-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-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfai
[Xen-devel] [PATCH 8/9] x86/time: only HVM has RTC
PV doesn't have RTC. Enclose relevant code in CONFIG_HVM.
Signed-off-by: Wei Liu
---
xen/arch/x86/time.c | 4
1 file changed, 4 insertions(+)
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 536449b..8d74210 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1144,6 +1144,7 @@ void force_update_vcpu_system_time(struct vcpu *v)
static void update_domain_rtc(void)
{
+#if CONFIG_HVM
struct domain *d;
rcu_read_lock(&domlist_read_lock);
@@ -1153,13 +1154,16 @@ static void update_domain_rtc(void)
rtc_update_clock(d);
rcu_read_unlock(&domlist_read_lock);
+#endif
}
void domain_set_time_offset(struct domain *d, int64_t time_offset_seconds)
{
d->time_offset_seconds = time_offset_seconds;
+#if CONFIG_HVM
if ( is_hvm_domain(d) )
rtc_update_clock(d);
+#endif
update_domain_wallclock_time(d);
}
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 6/9] x86: move memory_type_changed to mm.c
This function is common to both PV and HVM. Move it to x86 common
code.
Signed-off-by: Wei Liu
---
xen/arch/x86/hvm/mtrr.c | 9 -
xen/arch/x86/mm.c | 9 +
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
index c502dda..f33d3b5 100644
--- a/xen/arch/x86/hvm/mtrr.c
+++ b/xen/arch/x86/hvm/mtrr.c
@@ -822,15 +822,6 @@ static int hvm_load_mtrr_msr(struct domain *d,
hvm_domain_context_t *h)
HVM_REGISTER_SAVE_RESTORE(MTRR, hvm_save_mtrr_msr, hvm_load_mtrr_msr,
1, HVMSR_PER_VCPU);
-void memory_type_changed(struct domain *d)
-{
-if ( need_iommu(d) && d->vcpu && d->vcpu[0] )
-{
-p2m_memory_type_changed(d);
-flush_all(FLUSH_CACHE);
-}
-}
-
int epte_get_entry_emt(struct domain *d, unsigned long gfn, mfn_t mfn,
unsigned int order, uint8_t *ipat, bool_t direct_mmio)
{
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index a1a1f5f..f251bb3 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5761,6 +5761,15 @@ unsigned long get_upper_mfn_bound(void)
return min(max_mfn, 1UL << (paddr_bits - PAGE_SHIFT)) - 1;
}
+void memory_type_changed(struct domain *d)
+{
+if ( need_iommu(d) && d->vcpu && d->vcpu[0] )
+{
+p2m_memory_type_changed(d);
+flush_all(FLUSH_CACHE);
+}
+}
+
/*
* Local variables:
* mode: C
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 7/9] x86: move arch_evtchn_inject to x86 common code
It is not specific to HVM. It just so happens that PV doesn't need
special handling. Also enclose the code in CONFIG_HVM.
Signed-off-by: Wei Liu
---
xen/arch/x86/hvm/irq.c | 6 --
xen/arch/x86/irq.c | 8
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
index 8095c82..dfe8ed6 100644
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -577,12 +577,6 @@ int hvm_local_events_need_delivery(struct vcpu *v)
return !hvm_interrupt_blocked(v, intack);
}
-void arch_evtchn_inject(struct vcpu *v)
-{
-if ( is_hvm_vcpu(v) )
-hvm_assert_evtchn_irq(v);
-}
-
static void irq_dump(struct domain *d)
{
struct hvm_irq *hvm_irq = hvm_domain_irq(d);
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 7d0b19f..1aeb02f 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -2732,3 +2732,11 @@ int allocate_and_map_msi_pirq(struct domain *d, int
index, int *pirq_p,
return ret;
}
+
+void arch_evtchn_inject(struct vcpu *v)
+{
+#if CONFIG_HVM
+if ( is_hvm_vcpu(v) )
+hvm_assert_evtchn_irq(v);
+#endif
+}
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 4/9] x86/hvm: provide stubs for HVM guest accessors
Signed-off-by: Wei Liu
---
xen/include/asm-x86/hvm/guest_access.h | 24
1 file changed, 24 insertions(+)
diff --git a/xen/include/asm-x86/hvm/guest_access.h
b/xen/include/asm-x86/hvm/guest_access.h
index b92dbe9..c7e83dc 100644
--- a/xen/include/asm-x86/hvm/guest_access.h
+++ b/xen/include/asm-x86/hvm/guest_access.h
@@ -1,8 +1,32 @@
#ifndef __ASM_X86_HVM_GUEST_ACCESS_H__
#define __ASM_X86_HVM_GUEST_ACCESS_H__
+#if CONFIG_HVM
+
unsigned long copy_to_user_hvm(void *to, const void *from, unsigned len);
unsigned long clear_user_hvm(void *to, unsigned int len);
unsigned long copy_from_user_hvm(void *to, const void *from, unsigned len);
+#else
+
+static inline
+unsigned long copy_to_user_hvm(void *to, const void *from, unsigned len)
+{
+return len;
+}
+
+static inline
+unsigned long clear_user_hvm(void *to, unsigned int len)
+{
+return len;
+}
+
+static inline
+unsigned long copy_from_user_hvm(void *to, const void *from, unsigned len)
+{
+return len;
+}
+
+#endif
+
#endif /* __ASM_X86_HVM_GUEST_ACCESS_H__ */
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 3/9] x86: enclose hypercall page initialisation code in CONFIG_{HVM, PV}
Signed-off-by: Wei Liu
---
xen/arch/x86/x86_64/traps.c | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index ed02b78..87d5816 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -352,12 +352,19 @@ void subarch_percpu_traps_init(void)
void hypercall_page_initialise(struct domain *d, void *hypercall_page)
{
memset(hypercall_page, 0xCC, PAGE_SIZE);
+#if CONFIG_HVM
if ( is_hvm_domain(d) )
hvm_hypercall_page_initialise(d, hypercall_page);
-else if ( !is_pv_32bit_domain(d) )
-hypercall_page_initialise_ring3_kernel(hypercall_page);
-else
-hypercall_page_initialise_ring1_kernel(hypercall_page);
+#endif
+#if CONFIG_PV
+if ( is_pv_domain(d) )
+{
+if ( !is_pv_32bit_domain(d) )
+hypercall_page_initialise_ring3_kernel(hypercall_page);
+else
+hypercall_page_initialise_ring1_kernel(hypercall_page);
+}
+#endif
}
/*
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/9] x86: put compat.o and x86_64/compat.o under CONFIG_PV
They contain code for compat hypercall for PV guests. Signed-off-by: Wei Liu --- xen/arch/x86/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile index 17e7d3f..9b9b63a 100644 --- a/xen/arch/x86/Makefile +++ b/xen/arch/x86/Makefile @@ -17,7 +17,7 @@ obj-bin-y += bzimage.init.o obj-bin-y += clear_page.o obj-bin-y += copy_page.o obj-y += cpuid.o -obj-y += compat.o x86_64/compat.o +obj-$(CONFIG_PV) += compat.o x86_64/compat.o obj-$(CONFIG_KEXEC) += crash.o obj-y += debug.o obj-y += delay.o -- git-series 0.9.1 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 5/9] x86/hvm: put hvm_local_events_need_delivery into a header file
This allows us to provide a stub for it in that header file.
Signed-off-by: Wei Liu
---
xen/include/asm-x86/event.h | 3 ++-
xen/include/asm-x86/hvm/event.h | 14 ++
2 files changed, 16 insertions(+), 1 deletion(-)
create mode 100644 xen/include/asm-x86/hvm/event.h
diff --git a/xen/include/asm-x86/event.h b/xen/include/asm-x86/event.h
index 2f6ea54..b4472da 100644
--- a/xen/include/asm-x86/event.h
+++ b/xen/include/asm-x86/event.h
@@ -11,6 +11,8 @@
#include
+#include
+
void vcpu_kick(struct vcpu *v);
void vcpu_mark_events_pending(struct vcpu *v);
@@ -19,7 +21,6 @@ static inline int vcpu_event_delivery_is_enabled(struct vcpu
*v)
return !vcpu_info(v, evtchn_upcall_mask);
}
-int hvm_local_events_need_delivery(struct vcpu *v);
static inline int local_events_need_delivery(void)
{
struct vcpu *v = current;
diff --git a/xen/include/asm-x86/hvm/event.h b/xen/include/asm-x86/hvm/event.h
new file mode 100644
index 000..f4781a9
--- /dev/null
+++ b/xen/include/asm-x86/hvm/event.h
@@ -0,0 +1,14 @@
+#ifndef ASM_HVM_EVENT_H
+#define ASM_HVM_EVENT_H
+
+#if CONFIG_HVM
+
+int hvm_local_events_need_delivery(struct vcpu *v);
+
+#else
+
+static inline int hvm_local_events_need_delivery(struct vcpu *v) { return 0; }
+
+#endif
+
+#endif /* ASM_HVM_EVENT_H */
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 0/9] Miscellaneous CONFIG_HVM / CONFIG_PV clean up
Wei Liu (9):
x86: put compat.o and x86_64/compat.o under CONFIG_PV
x86: add missing "inline" keyword
x86: enclose hypercall page initialisation code in CONFIG_{HVM,PV}
x86/hvm: provide stubs for HVM guest accessors
x86/hvm: put hvm_local_events_need_delivery into a header file
x86: move memory_type_changed to mm.c
x86: move arch_evtchn_inject to x86 common code
x86/time: only HVM has RTC
x86: move declaration of arch_set_info_hvm_guest and provide stub
xen/arch/x86/Makefile | 2 +-
xen/arch/x86/hvm/irq.c | 6 --
xen/arch/x86/hvm/mtrr.c| 9 -
xen/arch/x86/irq.c | 8
xen/arch/x86/mm.c | 9 +
xen/arch/x86/time.c| 4
xen/arch/x86/x86_64/traps.c| 15 +++
xen/include/asm-x86/domain.h | 3 ---
xen/include/asm-x86/event.h| 3 ++-
xen/include/asm-x86/hvm/domain.h | 17 +
xen/include/asm-x86/hvm/event.h| 14 ++
xen/include/asm-x86/hvm/guest_access.h | 24
xen/include/asm-x86/pv/traps.h | 2 +-
13 files changed, 91 insertions(+), 25 deletions(-)
create mode 100644 xen/include/asm-x86/hvm/event.h
base-commit: e752f28409678ce3ade49986b39309178fb2a6d6
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 2/9] x86: add missing "inline" keyword
Signed-off-by: Wei Liu
---
xen/include/asm-x86/pv/traps.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/asm-x86/pv/traps.h b/xen/include/asm-x86/pv/traps.h
index 7279d16..89985d1 100644
--- a/xen/include/asm-x86/pv/traps.h
+++ b/xen/include/asm-x86/pv/traps.h
@@ -47,7 +47,7 @@ static inline bool pv_trap_callback_registered(const struct
vcpu *v,
static inline void pv_trap_init(void) {}
/* Deliver interrupt to PV guest. Return 0 on success. */
-static int pv_raise_interrupt(struct vcpu *v, uint8_t vector) { return
-EOPNOTSUPP; }
+static inline int pv_raise_interrupt(struct vcpu *v, uint8_t vector) { return
-EOPNOTSUPP; }
static inline int pv_emulate_privileged_op(struct cpu_user_regs *regs) {
return 0; }
static inline void pv_emulate_gate_op(struct cpu_user_regs *regs) {}
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 9/9] x86: move declaration of arch_set_info_hvm_guest and provide stub
While at it, remove declaration of vcpu_hvm_context and use the proper
header.
Signed-off-by: Wei Liu
---
xen/include/asm-x86/domain.h | 3 ---
xen/include/asm-x86/hvm/domain.h | 17 +
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index e2442f4..d3bc5dc 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -631,9 +631,6 @@ static inline void free_vcpu_guest_context(struct
vcpu_guest_context *vgc)
vfree(vgc);
}
-struct vcpu_hvm_context;
-int arch_set_info_hvm_guest(struct vcpu *v, const struct vcpu_hvm_context
*ctx);
-
void pv_inject_event(const struct x86_event *event);
static inline void pv_inject_hw_exception(unsigned int vector, int errcode)
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 5885950..231b424 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
struct hvm_ioreq_page {
gfn_t gfn;
@@ -204,6 +205,22 @@ struct hvm_domain {
#define hap_enabled(d) ((d)->arch.hvm_domain.hap_enabled)
+#if CONFIG_HVM
+
+int arch_set_info_hvm_guest(struct vcpu *v, const struct vcpu_hvm_context
*ctx);
+
+#else
+
+#include
+
+static inline
+int arch_set_info_hvm_guest(struct vcpu *v, const struct vcpu_hvm_context *ctx)
+{
+return -EINVAL;
+}
+
+#endif
+
#endif /* __ASM_X86_HVM_DOMAIN_H__ */
/*
--
git-series 0.9.1
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] x86/hvm: remove default ioreq server
My recent patch [1] to qemu-xen-traditional removes the last use of the
'default' ioreq server in Xen. (This is a catch-all ioreq server that is
used if no explicitly registered I/O range is targetted).
This patch can be applied once that patch is committed, to remove the
(>100 lines of) redundant code in Xen.
[1] https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg00270.html
Signed-off-by: Paul Durrant
---
Cc: Jan Beulich
Cc: Andrew Cooper
---
xen/arch/x86/hvm/dm.c| 2 +-
xen/arch/x86/hvm/hvm.c | 27 -
xen/arch/x86/hvm/ioreq.c | 123 +--
xen/include/asm-x86/hvm/domain.h | 1 -
xen/include/asm-x86/hvm/ioreq.h | 4 +-
5 files changed, 19 insertions(+), 138 deletions(-)
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index 6755f3fd96..6ace3dc7c1 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -411,7 +411,7 @@ static int dm_op(const struct dmop_args *op_args)
if ( data->pad[0] || data->pad[1] || data->pad[2] )
break;
-rc = hvm_create_ioreq_server(d, false, data->handle_bufioreq,
+rc = hvm_create_ioreq_server(d, data->handle_bufioreq,
&data->id);
break;
}
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 72c51faecb..227b5b3319 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4098,7 +4098,6 @@ static int hvm_allow_set_param(struct domain *d,
* since the domain may need to be paused.
*/
case HVM_PARAM_IDENT_PT:
-case HVM_PARAM_DM_DOMAIN:
case HVM_PARAM_ACPI_S_STATE:
/* The remaining parameters should not be set by the guest. */
default:
@@ -4373,13 +4372,6 @@ static int hvm_allow_get_param(struct domain *d,
case HVM_PARAM_ALTP2M:
case HVM_PARAM_X87_FIP_WIDTH:
break;
-/*
- * The following parameters must not be read by the guest
- * since the domain may need to be paused.
- */
-case HVM_PARAM_IOREQ_PFN:
-case HVM_PARAM_BUFIOREQ_PFN:
-case HVM_PARAM_BUFIOREQ_EVTCHN:
/* The remaining parameters should not be read by the guest. */
default:
if ( d == current->domain )
@@ -4433,25 +4425,6 @@ static int hvmop_get_param(
case HVM_PARAM_X87_FIP_WIDTH:
a.value = d->arch.x87_fip_width;
break;
-case HVM_PARAM_IOREQ_PFN:
-case HVM_PARAM_BUFIOREQ_PFN:
-case HVM_PARAM_BUFIOREQ_EVTCHN:
-/*
- * It may be necessary to create a default ioreq server here,
- * because legacy versions of QEMU are not aware of the new API for
- * explicit ioreq server creation. However, if the domain is not
- * under construction then it will not be QEMU querying the
- * parameters and thus the query should not have that side-effect.
- */
-if ( !d->creation_finished )
-{
-rc = hvm_create_ioreq_server(d, true,
- HVM_IOREQSRV_BUFIOREQ_LEGACY, NULL);
-if ( rc != 0 && rc != -EEXIST )
-goto out;
-}
-
-/*FALLTHRU*/
default:
a.value = d->arch.hvm_domain.params[a.index];
break;
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 7c515b3ef7..9cb3819c6b 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -55,9 +55,6 @@ static struct hvm_ioreq_server *get_ioreq_server(const struct
domain *d,
return GET_IOREQ_SERVER(d, id);
}
-#define IS_DEFAULT(s) \
-((s) && (s) == GET_IOREQ_SERVER((s)->target, DEFAULT_IOSERVID))
-
/*
* Iterate over all possible ioreq servers.
*
@@ -245,8 +242,6 @@ static gfn_t hvm_alloc_ioreq_gfn(struct hvm_ioreq_server *s)
struct domain *d = s->target;
unsigned int i;
-ASSERT(!IS_DEFAULT(s));
-
for ( i = 0; i < sizeof(d->arch.hvm_domain.ioreq_gfn.mask) * 8; i++ )
{
if ( test_and_clear_bit(i, &d->arch.hvm_domain.ioreq_gfn.mask) )
@@ -261,7 +256,6 @@ static void hvm_free_ioreq_gfn(struct hvm_ioreq_server *s,
gfn_t gfn)
struct domain *d = s->target;
unsigned int i = gfn_x(gfn) - d->arch.hvm_domain.ioreq_gfn.base;
-ASSERT(!IS_DEFAULT(s));
ASSERT(!gfn_eq(gfn, INVALID_GFN));
set_bit(i, &d->arch.hvm_domain.ioreq_gfn.mask);
@@ -277,9 +271,7 @@ static void hvm_unmap_ioreq_gfn(struct hvm_ioreq_server *s,
bool buf)
destroy_ring_for_helper(&iorp->va, iorp->page);
iorp->page = NULL;
-if ( !IS_DEFAULT(s) )
-hvm_free_ioreq_gfn(s, iorp->gfn);
-
+hvm_free_ioreq_gfn(s, iorp->gfn);
iorp->gfn = INVALID_GFN;
}
@@ -305,13 +297,7 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server *s,
bool buf)
if ( d->is_dying )
return -EINVAL;
-if ( IS_DEFAULT(s) )
-iorp->gfn = _gfn(buf ?
- d->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_PFN] :
- d->arch.hvm_domain.params[HVM_
Re: [Xen-devel] x86 Community Call - Wed Aug 15, 14:00 - 15:00 UTC - Call for agenda items
On 08/07/2018 10:26 AM, Lars Kurth wrote: > Dear community members, > > please send me agenda items for next week’s community call by this Friday. > Note that this a week later than normal, because I had community calls in > my diary as the week before Advisory Board meetings (not the 2nd Wed > of each month) and I forgot to send out the agenda. FYI I'll be on PTO on 15 August, so won't be able to make the call. -George ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/hvm: remove default ioreq server
On 07/08/18 11:03, Paul Durrant wrote: > My recent patch [1] to qemu-xen-traditional removes the last use of the > 'default' ioreq server in Xen. (This is a catch-all ioreq server that is > used if no explicitly registered I/O range is targetted). > > This patch can be applied once that patch is committed, to remove the > (>100 lines of) redundant code in Xen. > > [1] https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg00270.html > > Signed-off-by: Paul Durrant Acked-by: Andrew Cooper > --- > Cc: Jan Beulich > Cc: Andrew Cooper > --- > xen/arch/x86/hvm/dm.c| 2 +- > xen/arch/x86/hvm/hvm.c | 27 - > xen/arch/x86/hvm/ioreq.c | 123 > +-- > xen/include/asm-x86/hvm/domain.h | 1 - > xen/include/asm-x86/hvm/ioreq.h | 4 +- > 5 files changed, 19 insertions(+), 138 deletions(-) Wow - what a diffstat! ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-4.7-testing test] 125765: regressions - trouble: blocked/broken/fail/pass
flight 125765 xen-4.7-testing real [real] http://logs.test-lab.xenproject.org/osstest/logs/125765/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-pvopsbroken build-arm64-xsm broken build-arm64 broken test-amd64-i386-libvirt-pair 22 guest-migrate/src_host/dst_host fail REGR. vs. 125057 Tests which are failing intermittently (not blocking): test-armhf-armhf-xl-credit2 16 guest-start/debian.repeat fail in 125708 pass in 125765 test-xtf-amd64-amd64-5 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 125749 pass in 125765 test-amd64-amd64-libvirt-pair 22 guest-migrate/src_host/dst_host fail pass in 125708 test-xtf-amd64-amd64-3 50 xtf/test-hvm64-lbr-tsx-vmentry fail pass in 125749 Regressions which are regarded as allowable (not blocking): build-arm64 2 hosts-allocate broken REGR. vs. 125057 build-arm64-xsm 2 hosts-allocate broken REGR. vs. 125057 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 125057 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a build-arm64 3 capture-logs broken blocked in 125057 build-arm64-pvops 3 capture-logs broken blocked in 125057 build-arm64-xsm 3 capture-logs broken blocked in 125057 test-arm64-arm64-xl-credit2 13 migrate-support-check fail in 125708 never pass test-arm64-arm64-xl-credit2 14 saverestore-support-check fail in 125708 never pass test-arm64-arm64-xl-xsm 13 migrate-support-check fail in 125708 never pass test-arm64-arm64-xl 13 migrate-support-check fail in 125708 never pass test-arm64-arm64-xl-xsm 14 saverestore-support-check fail in 125708 never pass test-arm64-arm64-xl 14 saverestore-support-check fail in 125708 never pass test-arm64-arm64-libvirt-xsm 13 migrate-support-check fail in 125708 never pass test-arm64-arm64-libvirt-xsm 14 saverestore-support-check fail in 125708 never pass test-xtf-amd64-amd64-1 50 xtf/test-hvm64-lbr-tsx-vmentry fail in 125749 like 125057 test-xtf-amd64-amd64-2 50 xtf/test-hvm64-lbr-tsx-vmentry fail like 125057 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 125057 test-amd64-i386-xl-qemut-win7-amd64 17 guest-stop fail like 125057 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125057 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125057 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125057 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125057 test-amd64-i386-xl-qemut-ws16-amd64 17 guest-stop fail like 125057 test-amd64-i386-xl-qemuu-ws16-amd64 17 guest-stop fail like 125057 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125057 test-amd64-amd64-xl-rtds 10 debian-install fail like 125057 test-armhf-armhf-xl-rtds 16 guest-start/debian.repeatfail like 125057 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 125057 test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail 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-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-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-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-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass te
Re: [Xen-devel] [PATCH] x86/hvm: remove default ioreq server
>>> On 07.08.18 at 12:03, wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4098,7 +4098,6 @@ static int hvm_allow_set_param(struct domain *d,
> * since the domain may need to be paused.
> */
> case HVM_PARAM_IDENT_PT:
> -case HVM_PARAM_DM_DOMAIN:
> case HVM_PARAM_ACPI_S_STATE:
> /* The remaining parameters should not be set by the guest. */
> default:
How come you remove it here, but not from hvmop_set_param()?
And how is this related to qemu-trad _only_ anyway? Or wait - is
this just a leftover (and hence its removal not directly related to the
purpose of this patch)? If so, half a sentence in the description
would have helped recognizing this.
> @@ -4373,13 +4372,6 @@ static int hvm_allow_get_param(struct domain *d,
> case HVM_PARAM_ALTP2M:
> case HVM_PARAM_X87_FIP_WIDTH:
> break;
> -/*
> - * The following parameters must not be read by the guest
> - * since the domain may need to be paused.
> - */
> -case HVM_PARAM_IOREQ_PFN:
> -case HVM_PARAM_BUFIOREQ_PFN:
> -case HVM_PARAM_BUFIOREQ_EVTCHN:
> /* The remaining parameters should not be read by the guest. */
> default:
> if ( d == current->domain )
Shouldn't you also remove the (or at least some) uses of these
params from libxc? If all of them can go away, perhaps even
their definitions should be commented out in the public header
(or be put inside #ifdef __XEN__, as the values might want
using in hvm_allow_{g,s}et_param() to uniformly deny access),
considering that - as the comment you remove says - they
were not usable from guests themselves. (I don't think they
should be removed altogether, to keep a record of which
values they used, as I don't think we want to re-use the values
going forward.)
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 2/9] x86: add missing "inline" keyword
>>> On 07.08.18 at 12:00, wrote: > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/9] x86: put compat.o and x86_64/compat.o under CONFIG_PV
>>> On 07.08.18 at 12:00, wrote: > They contain code for compat hypercall for PV guests. > > Signed-off-by: Wei Liu Acked-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/9] x86: enclose hypercall page initialisation code in CONFIG_{HVM, PV}
>>> On 07.08.18 at 12:00, wrote:
> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -352,12 +352,19 @@ void subarch_percpu_traps_init(void)
> void hypercall_page_initialise(struct domain *d, void *hypercall_page)
> {
> memset(hypercall_page, 0xCC, PAGE_SIZE);
> +#if CONFIG_HVM
> if ( is_hvm_domain(d) )
> hvm_hypercall_page_initialise(d, hypercall_page);
> -else if ( !is_pv_32bit_domain(d) )
> -hypercall_page_initialise_ring3_kernel(hypercall_page);
> -else
> -hypercall_page_initialise_ring1_kernel(hypercall_page);
> +#endif
> +#if CONFIG_PV
> +if ( is_pv_domain(d) )
> +{
> +if ( !is_pv_32bit_domain(d) )
> +hypercall_page_initialise_ring3_kernel(hypercall_page);
> +else
> +hypercall_page_initialise_ring1_kernel(hypercall_page);
> +}
> +#endif
> }
I'm not convinced: This is uglier to read, and things like is_hvm_domain()
should evaluate to constant false anyway without the respective
CONFIG_* setting enabled.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/9] x86/hvm: provide stubs for HVM guest accessors
>>> On 07.08.18 at 12:00, wrote:
> --- a/xen/include/asm-x86/hvm/guest_access.h
> +++ b/xen/include/asm-x86/hvm/guest_access.h
> @@ -1,8 +1,32 @@
> #ifndef __ASM_X86_HVM_GUEST_ACCESS_H__
> #define __ASM_X86_HVM_GUEST_ACCESS_H__
>
> +#if CONFIG_HVM
> +
> unsigned long copy_to_user_hvm(void *to, const void *from, unsigned len);
> unsigned long clear_user_hvm(void *to, unsigned int len);
> unsigned long copy_from_user_hvm(void *to, const void *from, unsigned len);
>
> +#else
> +
> +static inline
> +unsigned long copy_to_user_hvm(void *to, const void *from, unsigned len)
> +{
> +return len;
> +}
> +
> +static inline
> +unsigned long clear_user_hvm(void *to, unsigned int len)
> +{
> +return len;
> +}
> +
> +static inline
> +unsigned long copy_from_user_hvm(void *to, const void *from, unsigned len)
> +{
> +return len;
> +}
> +
> +#endif
With BUG() or at least ASSERT_UNREACHABLE() put into all of them
Acked-by: Jan Beulich
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/hvm: remove default ioreq server
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 07 August 2018 11:37
> To: Paul Durrant
> Cc: Andrew Cooper ; xen-devel de...@lists.xenproject.org>
> Subject: Re: [PATCH] x86/hvm: remove default ioreq server
>
> >>> On 07.08.18 at 12:03, wrote:
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -4098,7 +4098,6 @@ static int hvm_allow_set_param(struct domain
> *d,
> > * since the domain may need to be paused.
> > */
> > case HVM_PARAM_IDENT_PT:
> > -case HVM_PARAM_DM_DOMAIN:
> > case HVM_PARAM_ACPI_S_STATE:
> > /* The remaining parameters should not be set by the guest. */
> > default:
>
> How come you remove it here, but not from hvmop_set_param()?
> And how is this related to qemu-trad _only_ anyway? Or wait - is
> this just a leftover (and hence its removal not directly related to the
> purpose of this patch)? If so, half a sentence in the description
> would have helped recognizing this.
I remove it here because it no longer has the semantic of pausing the domain.
In fact that disappeared a while ago and this should have been cleaned up then.
So, yes, I should have called it out in the commit comment. Sorry about that.
>
> > @@ -4373,13 +4372,6 @@ static int hvm_allow_get_param(struct domain
> *d,
> > case HVM_PARAM_ALTP2M:
> > case HVM_PARAM_X87_FIP_WIDTH:
> > break;
> > -/*
> > - * The following parameters must not be read by the guest
> > - * since the domain may need to be paused.
> > - */
> > -case HVM_PARAM_IOREQ_PFN:
> > -case HVM_PARAM_BUFIOREQ_PFN:
> > -case HVM_PARAM_BUFIOREQ_EVTCHN:
> > /* The remaining parameters should not be read by the guest. */
> > default:
> > if ( d == current->domain )
>
> Shouldn't you also remove the (or at least some) uses of these
> params from libxc? If all of them can go away, perhaps even
> their definitions should be commented out in the public header
> (or be put inside #ifdef __XEN__, as the values might want
> using in hvm_allow_{g,s}et_param() to uniformly deny access),
> considering that - as the comment you remove says - they
> were not usable from guests themselves. (I don't think they
> should be removed altogether, to keep a record of which
> values they used, as I don't think we want to re-use the values
> going forward.)
Note that the PFN params have to stay because, prior to direct resource
mapping, upstream QEMU makes use of the PFNs in question and so the
save/restore code in Xen has to preserve their values. The EVTCHN param can
probably go away totally though... I don't think the save/restore code touches
that. I guess the param definition should stay in the header but a comment be
added that it is deprecated. I can also completely disallow get/set of that
parameter too (with EOPNOTSUPP or EINVAL?) if you think that is a reasonable
thing to do.
Paul
>
> Jan
>
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 3/9] x86: enclose hypercall page initialisation code in CONFIG_{HVM, PV}
On 07/08/18 11:44, Jan Beulich wrote:
On 07.08.18 at 12:00, wrote:
>> --- a/xen/arch/x86/x86_64/traps.c
>> +++ b/xen/arch/x86/x86_64/traps.c
>> @@ -352,12 +352,19 @@ void subarch_percpu_traps_init(void)
>> void hypercall_page_initialise(struct domain *d, void *hypercall_page)
>> {
>> memset(hypercall_page, 0xCC, PAGE_SIZE);
>> +#if CONFIG_HVM
>> if ( is_hvm_domain(d) )
>> hvm_hypercall_page_initialise(d, hypercall_page);
>> -else if ( !is_pv_32bit_domain(d) )
>> -hypercall_page_initialise_ring3_kernel(hypercall_page);
>> -else
>> -hypercall_page_initialise_ring1_kernel(hypercall_page);
>> +#endif
>> +#if CONFIG_PV
>> +if ( is_pv_domain(d) )
>> +{
>> +if ( !is_pv_32bit_domain(d) )
>> +hypercall_page_initialise_ring3_kernel(hypercall_page);
>> +else
>> +hypercall_page_initialise_ring1_kernel(hypercall_page);
>> +}
>> +#endif
>> }
> I'm not convinced: This is uglier to read, and things like is_hvm_domain()
> should evaluate to constant false anyway without the respective
> CONFIG_* setting enabled.
I agree.
Each of the 4 initialisation functions could be static inlines, because
there is nothing complicated about them, and the static evaluation of
is_*_domain() would cause the properly unused ones to be omitted.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 5/9] x86/hvm: put hvm_local_events_need_delivery into a header file
>>> On 07.08.18 at 12:00, wrote:
> --- /dev/null
> +++ b/xen/include/asm-x86/hvm/event.h
> @@ -0,0 +1,14 @@
> +#ifndef ASM_HVM_EVENT_H
> +#define ASM_HVM_EVENT_H
> +
> +#if CONFIG_HVM
> +
> +int hvm_local_events_need_delivery(struct vcpu *v);
> +
> +#else
> +
> +static inline int hvm_local_events_need_delivery(struct vcpu *v) { return 0;
> }
> +
> +#endif
> +
> +#endif /* ASM_HVM_EVENT_H */
Are you expecting more stuff to go into this header? If not, I don't
think a separate header is really warranted here.
Did you consider taking the opportunity and switching the function
to have bool return value at the same time? (Seeing whether the
parameter could also be constified is probably more involved a
task, and hence not suitable here.)
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [qemu-mainline test] 125767: regressions - trouble: blocked/broken/fail/pass
flight 125767 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/125767/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-pvopsbroken build-arm64-xsm broken build-arm64 broken build-i386-libvirt6 libvirt-buildfail REGR. vs. 125640 test-armhf-armhf-xl 6 xen-install fail REGR. vs. 125640 Regressions which are regarded as allowable (not blocking): build-arm64-pvops 2 hosts-allocate broken REGR. vs. 125640 build-arm64 2 hosts-allocate broken REGR. vs. 125640 build-arm64-xsm 2 hosts-allocate broken REGR. vs. 125640 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64-xsm 3 capture-logs broken blocked in 125640 build-arm64-pvops 3 capture-logs broken blocked in 125640 build-arm64 3 capture-logs broken blocked in 125640 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125640 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 125640 test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail like 125640 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125640 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125640 test-amd64-i386-xl-pvshim12 guest-start fail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 fail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 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-multivcpu 13 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 13 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 14 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 12 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 13 saverestore-support-checkfail never pass test-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-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: qemuu1fb57da72ae0886eba1234a2d98ddd10e88a9efc baseline version: qemuu18a398f6a39df4b08ff86ac0d38384193ca5f4cc Last test of basis 125640 2018-07-27 22:16:44 Z 10 days Failing since125675 2018-07-30 09:36:58 Z8 days5 attempts Testing same since 125767 2018-08-06 15:00:21 Z0 days1 attempts People who touched revisions under test: Alex Bennée BALATON Zoltan Christian Borntraeger Cornelia Huck David Gibson Dou Liyang Dr. David Alan Gilbert Fam Zheng Geert Uytterhoeven Igor Mammedov Jay Zhou Kevin Wolf KONRAD Frederic Laurent Desnogues Laurent Vivier Leonid Bloch liujunjie Marc-André Lureau Markus Armbruster Michael S. Tsirkin Paolo Bonzini Pavel Dovgalyuk Peter Maydell Philippe Mathieu-Daudé Richard H
Re: [Xen-devel] [PATCH] x86/hvm: remove default ioreq server
>>> On 07.08.18 at 12:48, wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 07 August 2018 11:37
>>
>> >>> On 07.08.18 at 12:03, wrote:
>> > @@ -4373,13 +4372,6 @@ static int hvm_allow_get_param(struct domain
>> *d,
>> > case HVM_PARAM_ALTP2M:
>> > case HVM_PARAM_X87_FIP_WIDTH:
>> > break;
>> > -/*
>> > - * The following parameters must not be read by the guest
>> > - * since the domain may need to be paused.
>> > - */
>> > -case HVM_PARAM_IOREQ_PFN:
>> > -case HVM_PARAM_BUFIOREQ_PFN:
>> > -case HVM_PARAM_BUFIOREQ_EVTCHN:
>> > /* The remaining parameters should not be read by the guest. */
>> > default:
>> > if ( d == current->domain )
>>
>> Shouldn't you also remove the (or at least some) uses of these
>> params from libxc? If all of them can go away, perhaps even
>> their definitions should be commented out in the public header
>> (or be put inside #ifdef __XEN__, as the values might want
>> using in hvm_allow_{g,s}et_param() to uniformly deny access),
>> considering that - as the comment you remove says - they
>> were not usable from guests themselves. (I don't think they
>> should be removed altogether, to keep a record of which
>> values they used, as I don't think we want to re-use the values
>> going forward.)
>
> Note that the PFN params have to stay because, prior to direct resource
> mapping, upstream QEMU makes use of the PFNs in question and so the
> save/restore code in Xen has to preserve their values. The EVTCHN param can
> probably go away totally though... I don't think the save/restore code
> touches that. I guess the param definition should stay in the header but a
> comment be added that it is deprecated. I can also completely disallow
> get/set of that parameter too (with EOPNOTSUPP or EINVAL?) if you think that
> is a reasonable thing to do.
I indeed think so, yes. ENODATA, EPERM, or EOPNOTSUPP would
all seem fine to me (in the order of my personal preference).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 6/9] x86: move memory_type_changed to mm.c
>>> On 07.08.18 at 12:00, wrote: > This function is common to both PV and HVM. Move it to x86 common > code. I'm afraid that's the wrong way round: p2m_memory_type_changed() is HVM (in fact EPT) specific. The only uses of the function that aren't already HVM-specific are from domctl.c and from iommu_construct(), yet I doubt the flush_all(FLUSH_CACHE) has any relevance there in the PV case. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 7/9] x86: move arch_evtchn_inject to x86 common code
>>> On 07.08.18 at 12:00, wrote:
> --- a/xen/arch/x86/irq.c
> +++ b/xen/arch/x86/irq.c
> @@ -2732,3 +2732,11 @@ int allocate_and_map_msi_pirq(struct domain *d, int
> index, int *pirq_p,
>
> return ret;
> }
> +
> +void arch_evtchn_inject(struct vcpu *v)
> +{
> +#if CONFIG_HVM
> +if ( is_hvm_vcpu(v) )
> +hvm_assert_evtchn_irq(v);
> +#endif
> +}
Without the #ifdef
Acked-by: Jan Beulich
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 8/9] x86/time: only HVM has RTC
>>> On 07.08.18 at 12:00, wrote:
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -1144,6 +1144,7 @@ void force_update_vcpu_system_time(struct vcpu *v)
>
> static void update_domain_rtc(void)
> {
> +#if CONFIG_HVM
> struct domain *d;
>
> rcu_read_lock(&domlist_read_lock);
> @@ -1153,13 +1154,16 @@ static void update_domain_rtc(void)
> rtc_update_clock(d);
>
> rcu_read_unlock(&domlist_read_lock);
> +#endif
> }
I think the function should gain a hvm_ prefix and move to
hvm/rtc.c.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 9/9] x86: move declaration of arch_set_info_hvm_guest and provide stub
>>> On 07.08.18 at 12:00, wrote:
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -34,6 +34,7 @@
> #include
> #include
> #include
> +#include
Why? struct vcpu_hvm_context only needs a forward declaration, just
like was the case originally. Full structure/union definitions are needed
only if you de-reference the type, instantiate it, or apply sizeof() and
alike to it.
In fact I suspect we could reduce header dependencies quite a bit if
we followed that model more widely.
> @@ -204,6 +205,22 @@ struct hvm_domain {
>
> #define hap_enabled(d) ((d)->arch.hvm_domain.hap_enabled)
>
> +#if CONFIG_HVM
> +
> +int arch_set_info_hvm_guest(struct vcpu *v, const struct vcpu_hvm_context
> *ctx);
> +
> +#else
> +
> +#include
> +
> +static inline
> +int arch_set_info_hvm_guest(struct vcpu *v, const struct vcpu_hvm_context
> *ctx)
> +{
> +return -EINVAL;
> +}
> +
> +#endif
All uses sit either in HVM-specific code or inside is_hvm_...()
conditionals: Why do we need the inline stub? If the declaration
was visible independent of CONFIG_HVM, code would compile
fine, and references to the function would be removed by the
compiler, so linking would also succeed despite there not being
any definition of the function.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [distros-debian-snapshot test] 75051: regressions - FAIL
flight 75051 distros-debian-snapshot real [real] http://osstest.xensource.com/osstest/logs/75051/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf 6 xen-build fail REGR. vs. 75030 test-amd64-i386-amd64-daily-netboot-pygrub 10 debian-di-install fail REGR. vs. 75030 Tests which did not succeed, but are not blocking: test-armhf-armhf-armhf-daily-netboot-pygrub 1 build-check(1) blocked n/a test-amd64-i386-i386-daily-netboot-pvgrub 11 guest-start fail blocked in 75030 test-amd64-amd64-i386-daily-netboot-pygrub 11 guest-start fail like 75030 test-amd64-amd64-amd64-daily-netboot-pvgrub 11 guest-start fail like 75030 test-amd64-i386-amd64-current-netinst-pygrub 10 debian-di-install fail like 75030 test-amd64-i386-i386-weekly-netinst-pygrub 10 debian-di-install fail like 75030 test-amd64-i386-amd64-weekly-netinst-pygrub 10 debian-di-install fail like 75030 test-amd64-amd64-amd64-weekly-netinst-pygrub 10 debian-di-install fail like 75030 test-amd64-amd64-i386-weekly-netinst-pygrub 10 debian-di-install fail like 75030 test-amd64-amd64-amd64-current-netinst-pygrub 10 debian-di-install fail like 75030 test-amd64-amd64-i386-current-netinst-pygrub 10 debian-di-install fail like 75030 test-amd64-i386-i386-current-netinst-pygrub 10 debian-di-install fail like 75030 baseline version: flight 75030 jobs: build-amd64 pass build-armhf fail build-i386 pass build-amd64-pvopspass build-armhf-pvopspass build-i386-pvops pass test-amd64-amd64-amd64-daily-netboot-pvgrub fail test-amd64-i386-i386-daily-netboot-pvgrubfail test-amd64-i386-amd64-daily-netboot-pygrub fail test-armhf-armhf-armhf-daily-netboot-pygrub blocked test-amd64-amd64-i386-daily-netboot-pygrub fail test-amd64-amd64-amd64-current-netinst-pygrubfail test-amd64-i386-amd64-current-netinst-pygrub fail test-amd64-amd64-i386-current-netinst-pygrub fail test-amd64-i386-i386-current-netinst-pygrub fail test-amd64-amd64-amd64-weekly-netinst-pygrub fail test-amd64-i386-amd64-weekly-netinst-pygrub fail test-amd64-amd64-i386-weekly-netinst-pygrub fail test-amd64-i386-i386-weekly-netinst-pygrub fail sg-report-flight on osstest.xs.citrite.net logs: /home/osstest/logs images: /home/osstest/images Logs, config files, etc. are available at http://osstest.xensource.com/osstest/logs Test harness code can be found at http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary Push not applicable. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v15 06/14] x86/hvm: Introduce hvm_save_mtrr_msr_one func
>>> On 03.08.18 at 15:53, wrote:
> +for ( i = 0; i < MASK_EXTR(hw_mtrr.msr_mtrr_cap, MTRRcap_VCNT); i++ )
> +{
> +/* save physbase */
> +hw_mtrr.msr_mtrr_var[i * 2] = mtrr_state->var_ranges->base;
> +/* save physmask */
> +hw_mtrr.msr_mtrr_var[i * 2 + 1] = mtrr_state->var_ranges->mask;
> +}
One of the intended side effects of using structure field on the rhs
was to be able to drop the (now redundant) comments.
> -hvm_get_guest_pat(v, &hw_mtrr.msr_pat_cr);
> +memcpy(hw_mtrr.msr_mtrr_fixed, mtrr_state->fixed_ranges, NUM_FIXED_MSR);
You want to BUILD_BUG_ON() array sizes differing, and then use
sizeof() in the call to memcpy().
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [xen-unstable-smoke test] 125782: trouble: blocked/broken/pass
flight 125782 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/125782/ Failures and problems with tests :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken Regressions which are regarded as allowable (not blocking): build-arm64-xsm 2 hosts-allocate broken REGR. vs. 125729 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a build-arm64-xsm 3 capture-logs broken blocked in 125729 test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-armhf-armhf-xl 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass version targeted for testing: xen e752f28409678ce3ade49986b39309178fb2a6d6 baseline version: xen ed5f8d9ca47e69e30221c37ec812f2b38af19d83 Last test of basis 125729 2018-08-01 11:00:39 Z6 days Failing since125741 2018-08-02 10:01:09 Z5 days9 attempts Testing same since 125772 2018-08-06 16:00:37 Z0 days7 attempts People who touched revisions under test: Alexandru Isaila Andrew Cooper Doug Goldstein George Dunlap Jan Beulich Julien Grall Kevin Tian Razvan Cojocaru Roger Pau Monné Stefano Stabellini jobs: build-arm64-xsm broken build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-i386 pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary broken-job build-arm64-xsm broken broken-step build-arm64-xsm hosts-allocate broken-step build-arm64-xsm capture-logs Not pushing. (No revision log; it would be 480 lines long.) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v15 09/14] x86/hvm: Introduce lapic_save_regs_one func
>>> On 03.08.18 at 15:53, wrote:
> This is used to save data from a single instance.
>
> Signed-off-by: Alexandru Isaila
> ---
> xen/arch/x86/hvm/vlapic.c | 27 +++
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> index 0795161..d35810e 100644
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -1460,26 +1460,37 @@ static int lapic_save_hidden(struct domain *d,
> hvm_domain_context_t *h)
> return err;
> }
>
> +static int lapic_save_regs_one(struct vcpu *v, hvm_domain_context_t *h)
> +{
> +struct vlapic *s;
> +
> +if ( !has_vlapic(v->domain) )
> +return 0;
> +
> +if ( hvm_funcs.sync_pir_to_irr )
> +hvm_funcs.sync_pir_to_irr(v);
> +
> +s = vcpu_vlapic(v);
> +
> +return hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, s->regs);
> +}
Here as well as in patch 8 there's little point in having a local variable s
which is used just once. If you really think you want to retain them,
here it can be pointer to const (other than in patch 8 afaict), and like
in patch 8 it could have an initializer instead of later having a separate
assignment statement.
> static int lapic_save_regs(struct domain *d, hvm_domain_context_t *h)
> {
> struct vcpu *v;
> -struct vlapic *s;
> -int rc = 0;
> +int err = 0;
>
> if ( !has_vlapic(d) )
> return 0;
>
> for_each_vcpu ( d, v )
> {
> -if ( hvm_funcs.sync_pir_to_irr )
> -hvm_funcs.sync_pir_to_irr(v);
> -
> -s = vcpu_vlapic(v);
> -if ( (rc = hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, s->regs)) != 0 )
> +err = lapic_save_regs_one(v, h);
> +if ( err )
> break;
> }
>
> -return rc;
> +return err;
> }
Since the whole function is meant to go away anyway, it doesn't
matter much, but why did you see a need to replace "rc" by "err"?
This only increases code churn (even if just slightly). IOW: No
need to change this, but something to consider in the future.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v15 11/14] x86/domctl: Use hvm_save_vcpu_handler
>>> On 03.08.18 at 15:53, wrote:
> --- a/xen/arch/x86/hvm/save.c
> +++ b/xen/arch/x86/hvm/save.c
> @@ -196,7 +196,10 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h)
> struct hvm_save_header hdr;
> struct hvm_save_end end;
> hvm_save_handler handler;
> +hvm_save_vcpu_handler save_one_handler;
> unsigned int i;
> +int rc;
> +struct vcpu *v;
Please move the declarations you add into the scopes where they're
actually needed (but please avoid replicating rc). I realize pre-existing
code isn't in line with this, but please le's not widen the problem. In
fact I wouldn't mind at all if you moved handler down right away. But
as that's slated to go away, that's probably not very important.
> @@ -224,11 +227,32 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h)
> for ( i = 0; i <= HVM_SAVE_CODE_MAX; i++ )
> {
> handler = hvm_sr_handlers[i].save;
> -if ( handler != NULL )
> +save_one_handler = hvm_sr_handlers[i].save_one;
> +if ( save_one_handler != NULL )
Would you mind omitting the redundant "!= NULL" here and below?
> +{
> +for_each_vcpu ( d, v )
> +{
> +printk(XENLOG_G_INFO "HVM %pv save: %s\n",
> + v, hvm_sr_handlers[i].name);
> +rc = save_one_handler(v, h);
> +
> +if( rc != 0 )
Missing blank, and just like above "!= 0" is redundant and could be
omitted (same below).
> +{
> +printk(XENLOG_G_ERR
> + "HVM %pv save: failed to save type %"PRIu16"\n",
> + v, i);
> +return -EFAULT;
Why -EFAULT? The pre-existing bad use does not count as an excuse.
If the value of rc can't be used (perhaps because there may be positive
values or -1 coming back), pick something that at least comes a little
closer to representing the actual condition (EIO, ENODATA, EOPNOTSUPP
all come to mind, but much depends on what conditions actually exist).
I'd then encourage you to also change the pre-existing bad use.
> +}
> +}
> +}
> +else if ( handler != NULL )
> {
> printk(XENLOG_G_INFO "HVM%d save: %s\n",
> d->domain_id, hvm_sr_handlers[i].name);
> -if ( handler(d, h) != 0 )
> +
> +rc = handler(d, h);
> +
> +if( rc != 0 )
Please either omit the blank line ahead of the invocation of handler(),
or the one following it. First and foremost: Have this block be
consistent blank line wise with the one above.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v15 12/14] x86/hvm: Drop the use of save functions
>>> On 03.08.18 at 15:53, wrote: > This patch drops the use of save functions in hvm_save. But quite a few types still have this set to NULL? How do things work at this point of the series? Am I overlooking anything? I think this needs to be swapped with patch 13. Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v15 09/14] x86/hvm: Introduce lapic_save_regs_one func
On Ma, 2018-08-07 at 06:09 -0600, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 03.08.18 at 15:53, wrote:
> > This is used to save data from a single instance.
> >
> > Signed-off-by: Alexandru Isaila
> > ---
> > xen/arch/x86/hvm/vlapic.c | 27 +++
> > 1 file changed, 19 insertions(+), 8 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
> > index 0795161..d35810e 100644
> > --- a/xen/arch/x86/hvm/vlapic.c
> > +++ b/xen/arch/x86/hvm/vlapic.c
> > @@ -1460,26 +1460,37 @@ static int lapic_save_hidden(struct domain
> > *d,
> > hvm_domain_context_t *h)
> > return err;
> > }
> >
> > +static int lapic_save_regs_one(struct vcpu *v,
> > hvm_domain_context_t *h)
> > +{
> > +struct vlapic *s;
> > +
> > +if ( !has_vlapic(v->domain) )
> > +return 0;
> > +
> > +if ( hvm_funcs.sync_pir_to_irr )
> > +hvm_funcs.sync_pir_to_irr(v);
> > +
> > +s = vcpu_vlapic(v);
> > +
> > +return hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, s->regs);
> > +}
> Here as well as in patch 8 there's little point in having a local
> variable s
> which is used just once. If you really think you want to retain them,
> here it can be pointer to const (other than in patch 8 afaict), and
> like
> in patch 8 it could have an initializer instead of later having a
> separate
> assignment statement.
>
> >
> > static int lapic_save_regs(struct domain *d, hvm_domain_context_t
> > *h)
> > {
> > struct vcpu *v;
> > -struct vlapic *s;
> > -int rc = 0;
> > +int err = 0;
> >
> > if ( !has_vlapic(d) )
> > return 0;
> >
> > for_each_vcpu ( d, v )
> > {
> > -if ( hvm_funcs.sync_pir_to_irr )
> > -hvm_funcs.sync_pir_to_irr(v);
> > -
> > -s = vcpu_vlapic(v);
> > -if ( (rc = hvm_save_entry(LAPIC_REGS, v->vcpu_id, h, s-
> > >regs)) != 0 )
> > +err = lapic_save_regs_one(v, h);
> > +if ( err )
> > break;
> > }
> >
> > -return rc;
> > +return err;
> > }
> Since the whole function is meant to go away anyway, it doesn't
> matter much, but why did you see a need to replace "rc" by "err"?
> This only increases code churn (even if just slightly). IOW: No
> need to change this, but something to consider in the future.
>
Err was just to have all the functions work with the same variable name
so this was done just for consistency.
Alex
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v15 12/14] x86/hvm: Drop the use of save functions
>>> On 03.08.18 at 15:53, wrote:
> @@ -226,15 +225,14 @@ int hvm_save(struct domain *d, hvm_domain_context_t *h)
> /* Save all available kinds of state */
> for ( i = 0; i <= HVM_SAVE_CODE_MAX; i++ )
> {
> -handler = hvm_sr_handlers[i].save;
> -save_one_handler = hvm_sr_handlers[i].save_one;
> -if ( save_one_handler != NULL )
> +handler = hvm_sr_handlers[i].save_one;
> +if ( handler != NULL )
> {
> for_each_vcpu ( d, v )
> {
> printk(XENLOG_G_INFO "HVM %pv save: %s\n",
> v, hvm_sr_handlers[i].name);
> -rc = save_one_handler(v, h);
> +rc = handler(v, h);
As already said on v14: You must not invoke the handler once per
vCPU for HVMSR_PER_DOM type records.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v15 13/14] x86/hvm: Remove redundant save functions
>>> On 03.08.18 at 15:53, wrote:
> @@ -155,7 +152,7 @@ int hvm_save_one(struct domain *d, unsigned int typecode,
> unsigned int instance,
> if ( !ctxt.data )
> return -ENOMEM;
>
> -if ( (rv = hvm_sr_handlers[typecode].save(d, &ctxt)) != 0 )
> +if ( (rv = hvm_sr_handlers[typecode].save(d->vcpu[instance], &ctxt)) !=
> 0 )
There is no bounds check whatsoever before this use of instance as an
array index. You want to check against vCPU count for HVMSR_PER_VCPU
records, and pass vCPU0 for HVMSR_PER_DOM ones. I'm relatively sure
I've said so already on an earlier iteration of the series.
> @@ -107,7 +105,6 @@ typedef int (*hvm_load_handler) (struct domain *d,
> void hvm_register_savevm(uint16_t typecode,
> const char *name,
> hvm_save_handler save_state,
> - hvm_save_vcpu_handler save_one,
> hvm_load_handler load_state,
> size_t size, int kind);
>
> @@ -117,13 +114,12 @@ void hvm_register_savevm(uint16_t typecode,
>
> /* Syntactic sugar around that function: specify the max number of
> * saves, and this calculates the size of buffer needed */
> -#define HVM_REGISTER_SAVE_RESTORE(_x, _save, _save_one, _load, _num, _k) \
> +#define HVM_REGISTER_SAVE_RESTORE(_x, _save, _load, _num, _k) \
> static int __init __hvm_register_##_x##_save_and_restore(void)\
> { \
> hvm_register_savevm(HVM_SAVE_CODE(_x),\
> #_x, \
> &_save, \
> -_save_one,\
> &_load, \
> (_num) * (HVM_SAVE_LENGTH(_x) \
>+ sizeof (struct hvm_save_descriptor)), \
As to patch splitting: One option looks to be to fold 12 and 13, but
that would make for a pretty big patch doing at least two things at
the same time. Another option could be to simply store NULL here
into the now unused field in order to have what is now patch 12 in
the _next_ step remove that field (and its use in hvm_save()) and
do the renaming (i.e. the dropping of _one).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-4.14 test] 125771: regressions - trouble: blocked/broken/fail/pass
flight 125771 linux-4.14 real [real] http://logs.test-lab.xenproject.org/osstest/logs/125771/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64 broken build-arm64-pvopsbroken build-arm64-xsm broken test-amd64-amd64-pygrub 10 debian-di-installfail REGR. vs. 125175 Regressions which are regarded as allowable (not blocking): build-arm64 2 hosts-allocate broken REGR. vs. 125175 build-arm64-xsm 2 hosts-allocate broken REGR. vs. 125175 build-arm64-pvops 2 hosts-allocate broken REGR. vs. 125175 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-examine 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a build-arm64 3 capture-logs broken blocked in 125175 build-arm64-xsm 3 capture-logs broken blocked in 125175 build-arm64-pvops 3 capture-logs broken blocked in 125175 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail never pass test-amd64-i386-xl-pvshim12 guest-start fail 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 13 migrate-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 11 migrate-support-check fail 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 13 migrate-support-checkfail never pass test-armhf-armhf-xl 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 13 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 14 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 13 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 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-libvirt 14 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-vhd 12 migrate-support-checkfail never pass test-amd64-amd64-qemuu-nested-amd 17 debian-hvm-install/l1/l2 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-win7-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-amd64-amd64-xl-qemuu-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-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-i386-xl-qemuu-win7-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stop fail never pass test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stop fail 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-i386-xl-qemut-win10-i386 10 windows-install fail never pass test-amd64-amd64-xl-qemut-win10-i386 10 windows-installfail never pass test-amd64-amd64-xl-qemuu-win10-i386 10 windows-installfail never pass test-amd64-i386-xl-qemuu-win10-i386 10 windows-install fail never pass version targeted for testing: linux2ae6c0413b4768f9d8fc6f718a732f9dae014b67 baseline version: linux1e92e813554a93741666e9f378a83d70405b907
Re: [Xen-devel] [PATCH v15 14/14] x86/domctl: Don't pause the whole domain if only getting vcpu state
>>> On 03.08.18 at 15:53, wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -591,12 +591,12 @@ long arch_do_domctl(
> !is_hvm_domain(d) )
> break;
>
> -domain_pause(d);
> +vcpu_pause(d->vcpu[domctl->u.hvmcontext_partial.instance]);
> ret = hvm_save_one(d, domctl->u.hvmcontext_partial.type,
> domctl->u.hvmcontext_partial.instance,
> domctl->u.hvmcontext_partial.buffer,
> &domctl->u.hvmcontext_partial.bufsz);
> -domain_unpause(d);
> +vcpu_unpause(d->vcpu[domctl->u.hvmcontext_partial.instance]);
Same issue here - there's no bounds check of
domctl->u.hvmcontext_partial.instance before its use as array index.
I'm afraid you can't do the pausing here anymore, both for this reason
and because you still need to pause the whole domain for HVMSR_PER_DOM
type records. Yet you'll know the type only inside hvm_save_one().
> --- a/xen/arch/x86/hvm/save.c
> +++ b/xen/arch/x86/hvm/save.c
> @@ -138,6 +138,7 @@ int hvm_save_one(struct domain *d, unsigned int typecode,
> unsigned int instance,
> int rv;
> hvm_domain_context_t ctxt = { };
> const struct hvm_save_descriptor *desc;
> +uint32_t off = 0;
Why does this get moved here?
> @@ -157,29 +156,23 @@ int hvm_save_one(struct domain *d, unsigned int
> typecode, unsigned int instance,
> d->domain_id, typecode, rv);
> else if ( rv = -ENOENT, ctxt.cur >= sizeof(*desc) )
> {
> -uint32_t off;
> -
> -for ( off = 0; off <= (ctxt.cur - sizeof(*desc)); off +=
> desc->length )
> +desc = (void *)(ctxt.data + off);
> +/* Move past header */
> +off += sizeof(*desc);
> +if ( ctxt.cur < desc->length ||
> + off > ctxt.cur - desc->length )
> +rv = -EFAULT;
> +if ( instance == desc->instance )
> {
> -desc = (void *)(ctxt.data + off);
> -/* Move past header */
> -off += sizeof(*desc);
> -if ( ctxt.cur < desc->length ||
> - off > ctxt.cur - desc->length )
> -break;
> -if ( instance == desc->instance )
> -{
> -rv = 0;
> -if ( guest_handle_is_null(handle) )
> -*bufsz = desc->length;
> -else if ( *bufsz < desc->length )
> -rv = -ENOBUFS;
> -else if ( copy_to_guest(handle, ctxt.data + off,
> desc->length) )
> -rv = -EFAULT;
> -else
> -*bufsz = desc->length;
> -break;
> -}
You can't just delete this loop - it's still needed for multi-instance
records which aren't per-vCPU (PIC is the only example right now
iirc). Since the instance is going to be the correct one for
HVMSR_PER_VCPU type records, can't you simply leave the code
here alone?
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v15 00/14] x86/domctl: Save info for one vcpu instance
>>> On 03.08.18 at 15:53, wrote: > Alexandru Isaila (14): > > x86/cpu: Introduce vmce_save_vcpu_ctxt_one() func > x86/hvm: Introduce hvm_save_tsc_adjust_one() func > x86/hvm: Introduce hvm_save_cpu_ctxt_one func > x86/hvm: Introduce hvm_save_cpu_xsave_states_one > x86/hvm: Introduce hvm_save_cpu_msrs_one func > x86/hvm: Introduce hvm_save_mtrr_msr_one func > x86/hvm: Introduce viridian_save_vcpu_ctxt_one() > x86/hvm: Introduce lapic_save_hidden_one > x86/hvm: Introduce lapic_save_regs_one func > x86/hvm: Add handler for save_one funcs > x86/domctl: Use hvm_save_vcpu_handler > x86/hvm: Drop the use of save functions > x86/hvm: Remove redundant save functions > x86/domctl: Don't pause the whole domain if only Patches 1...5 and 10 Reviewed-by: Jan Beulich Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/4] xen: various cleanups
On 17/07/18 14:01, Juergen Gross wrote: > Some Xen related cleanups: > - move some pv-only code from CONFIG_XEN to CONFIG_XEN_PV > - use CONFIG_XEN_PVHVM in Makefile instead of #ifdef around a complete source > - add SPDX identifier where missing > > Juergen Gross (4): > xen: move pv irq related functions under CONFIG_XEN_PV umbrella > xen: move pv specific parts of arch/x86/xen/mmu.c to mmu_pv.c > xen: link platform-pci-unplug.o only if CONFIG_XEN_PVHVM > xen: add SPDX identifier in arch/x86/xen files > > arch/x86/entry/entry_32.S | 8 +- > arch/x86/entry/entry_64.S | 8 +- > arch/x86/xen/Makefile | 41 ++-- > arch/x86/xen/efi.c | 14 +-- > arch/x86/xen/enlighten.c | 2 + > arch/x86/xen/enlighten_hvm.c | 2 + > arch/x86/xen/grant-table.c | 25 + > arch/x86/xen/mmu.c | 188 > + > arch/x86/xen/mmu_pv.c | 140 +++ > arch/x86/xen/p2m.c | 2 + > arch/x86/xen/pci-swiotlb-xen.c | 2 + > arch/x86/xen/platform-pci-unplug.c | 18 +--- > arch/x86/xen/vdso.h| 2 + > arch/x86/xen/xen-pvh.S | 15 +-- > include/xen/interface/memory.h | 6 -- > include/xen/xen-ops.h | 133 +- > 16 files changed, 287 insertions(+), 319 deletions(-) > Boris, any objections? Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: don't use privcmd_call() from xen_mc_flush()
On 13/06/18 11:58, Juergen Gross wrote: > Using privcmd_call() for a singleton multicall seems to be wrong, as > privcmd_call() is using stac()/clac() to enable hypervisor access to > Linux user space. > > Add a new xen_single_call() function to be use for that purpose. > > Reported-by: Jan Beulich > Signed-off-by: Juergen Gross Boris, any objections? Juergen ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 1/4] xen/arm: drivers: scif: Remove unused #define-s
Hi Oleksandr, On 06/08/18 19:35, Oleksandr Tyshchenko wrote: From: Oleksandr Tyshchenko Signed-off-by: Oleksandr Tyshchenko CC: Stefano Stabellini CC: Julien Grall Acked-by: Julien Grall Cheers, --- xen/drivers/char/scif-uart.c| 4 xen/include/asm-arm/scif-uart.h | 11 --- 2 files changed, 15 deletions(-) diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c index 4a71bac..465fb34 100644 --- a/xen/drivers/char/scif-uart.c +++ b/xen/drivers/char/scif-uart.c @@ -29,10 +29,6 @@ #include #include -#define PARITY_NONE0 -#define PARITY_EVEN1 -#define PARITY_ODD 2 - #define scif_readb(uart, off) readb((uart)->regs + (off)) #define scif_writeb(uart, off, val)writeb((val), (uart)->regs + (off)) diff --git a/xen/include/asm-arm/scif-uart.h b/xen/include/asm-arm/scif-uart.h index d030b26..8137850 100644 --- a/xen/include/asm-arm/scif-uart.h +++ b/xen/include/asm-arm/scif-uart.h @@ -47,17 +47,6 @@ #define SCSCR_CKE1(1 << 1)/* Clock Enable 1 */ #define SCSCR_CKE0(1 << 0)/* Clock Enable 0 */ -#define SCSCR_CKE00(0) -#define SCSCR_CKE01(SCSCR_CKE0) -#define SCSCR_CKE10(SCSCR_CKE1) -#define SCSCR_CKE11(SCSCR_CKE1 | SCSCR_CKE0) - -/* Serial Mode Register (SCSMR) */ -#define SCSMR_CHR (1 << 6)/* 7-bit Character Length */ -#define SCSMR_PE (1 << 5)/* Parity Enable */ -#define SCSMR_ODD (1 << 4)/* Odd Parity */ -#define SCSMR_STOP(1 << 3)/* Stop Bit Length */ - /* Serial Status Register (SCFSR) */ #define SCFSR_ER (1 << 7)/* Receive Error */ #define SCFSR_TEND(1 << 6)/* Transmission End */ -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v20 1/2] common: add a new mappable resource type: XENMEM_resource_grant_table
>>> On 06.08.18 at 14:54, wrote:
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -3860,6 +3860,38 @@ int mem_sharing_gref_to_gfn(struct grant_table *gt,
> grant_ref_t ref,
> }
> #endif
>
> +/* caller must hold read or write lock */
> +static int gnttab_get_status_frame_mfn(struct domain *d,
> + unsigned long idx, mfn_t *mfn)
> +{
> +struct grant_table *gt = d->grant_table;
> +
> +if ( idx >= nr_status_frames(gt) )
> +return -EINVAL;
> +
> +*mfn = _mfn(virt_to_mfn(gt->status[idx]));
> +return 0;
> +}
> +
> +/* caller must hold write lock */
> +static int gnttab_get_shared_frame_mfn(struct domain *d,
> + unsigned long idx, mfn_t *mfn)
> +{
> +struct grant_table *gt = d->grant_table;
> +
> +if ( gt->gt_version == 0 )
> +gt->gt_version = 1;
> +
> +if ( (idx >= nr_grant_frames(gt)) && (idx < gt->max_grant_frames) )
> +gnttab_grow_table(d, idx + 1);
I guess I had commented on this before, but it has been a while:
While I realize that you're just moving code, I dislike the resulting
asymmetry between the two functions: Why would the former
not want to grow the grant table? And why would a version
adjustment be needed (only) here (I've put "only" in parentheses
because it's not obvious to me why this is needed)?
And this asymmetry would surely be coming as surprise at least
to callers of the new interface you add.
> +int gnttab_get_status_frame(struct domain *d, unsigned long idx,
> +mfn_t *mfn)
> +{
> +struct grant_table *gt = d->grant_table;
> +int rc;
> +
> +grant_read_lock(gt);
> +rc = gnttab_get_status_frame_mfn(d, idx, mfn);
> +grant_read_unlock(gt);
> +
> +return rc;
> +}
Along the lines of what gnttab_map_frame() does, I'd expect a
check for gt_version to be 2 here. Or well, maybe that's implicit
by there not being any status entries/pages for version 1 tables.
But it would become a requirement if gnttab_grow_table() was to
be called from gnttab_get_status_frame_mfn().
> @@ -982,6 +983,54 @@ static long xatp_permission_check(struct domain *d,
> unsigned int space)
> return xsm_add_to_physmap(XSM_TARGET, current->domain, d);
> }
>
> +static int acquire_grant_table(struct domain *d, unsigned int id,
> + unsigned long frame,
> + unsigned int nr_frames,
> + xen_pfn_t mfn_list[])
> +{
> +unsigned int i = nr_frames;
> +
> +/*
> + * FIXME: It is not currently safe to map grant status frames if they
> + *will be inserted into the caller's P2M, because these
> + *insertions are not yet properly reference counted.
> + *This restriction can be removed when appropriate reference
> + *counting is added.
> + */
> +if ( paging_mode_translate(current->domain) &&
> + (id == XENMEM_resource_grant_table_id_status) )
> +return -EOPNOTSUPP;
Would you mind reminding me why the P2M insertions are safe for
the "ordinary" (shared) grant frames? I'm puzzled because P2M
management in general lacks refcounting, yet I have the vague
feeling that we had discussed this before and there was a reason.
(If there is, perhaps slightly extending the comment above would
be helpful.)
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -611,16 +611,21 @@ struct xen_mem_acquire_resource {
> uint16_t type;
>
> #define XENMEM_resource_ioreq_server 0
> +#define XENMEM_resource_grant_table 1
>
> /*
> * IN - a type-specific resource identifier, which must be zero
> * unless stated otherwise.
> *
> * type == XENMEM_resource_ioreq_server -> id == ioreq server id
> + * type == XENMEM_resource_grant_table -> id defined below
> */
> uint32_t id;
> -/*
> - * IN/OUT - As an IN parameter number of frames of the resource
> +
> +#define XENMEM_resource_grant_table_id_shared 0
> +#define XENMEM_resource_grant_table_id_status 1
> +
> +/* IN/OUT - As an IN parameter number of frames of the resource
Please don't break previously correct comment style here.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [linux-linus test] 125769: regressions - trouble: blocked/broken/fail/pass
flight 125769 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/125769/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm broken build-arm64 broken build-arm64-pvopsbroken build-i386-pvops 6 kernel-build fail REGR. vs. 125702 Regressions which are regarded as allowable (not blocking): build-arm64-pvops 2 hosts-allocate broken REGR. vs. 125702 build-arm64 2 hosts-allocate broken REGR. vs. 125702 build-arm64-xsm 2 hosts-allocate broken REGR. vs. 125702 test-amd64-amd64-rumprun-amd64 17 rumprun-demo-xenstorels/xenstorels.repeat fail REGR. vs. 125702 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-xl-raw1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-freebsd10-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ovmf-amd64 1 build-check(1) blocked n/a test-arm64-arm64-xl-credit2 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a build-arm64-libvirt 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-pair 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 1 build-check(1) blocked n/a test-amd64-i386-xl-pvshim 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-ws16-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl1 build-check(1) blocked n/a test-amd64-i386-freebsd10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-shadow 1 build-check(1) blocked n/a test-amd64-i386-qemut-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-amd 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win7-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-win10-i386 1 build-check(1) blocked n/a test-amd64-i386-xl-xsm1 build-check(1) blocked n/a test-amd64-i386-rumprun-i386 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-arm64-arm64-xl 1 build-check(1) blocked n/a test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-arm64-arm64-examine 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 1 build-check(1) blocked n/a test-amd64-i386-xl-qemut-debianhvm-amd64 1 build-check(1) blocked n/a test-amd64-i386-examine 1 build-check(1) blocked n/a test-amd64-i386-qemuu-rhel6hvm-intel 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-xl-qemuu-ws16-amd64 1 build-check(1) blocked n/a build-arm64 3 capture-logs broken blocked in 125702 build-arm64-pvops 3 capture-logs broken blocked in 125702 build-arm64-xsm 3 capture-logs broken blocked in 125702 test-amd64-amd64-xl-qemut-win7-amd64 17 guest-stopfail like 125702 test-armhf-armhf-libvirt 14 saverestore-support-checkfail like 125702 test-amd64-amd64-xl-qemuu-win7-amd64 17 guest-stopfail like 125702 test-armhf-armhf-libvirt-raw 13 saverestore-support-checkfail like 125702 test-amd64-amd64-xl-qemut-ws16-amd64 17 guest-stopfail like 125702 test-amd64-amd64-xl-qemuu-ws16-amd64 17 guest-stopfail like 125702 test-amd64-amd64-xl-qemuu-dmrestrict-amd64-dmrestrict 10 debian-hvm-install fail never pass test-amd64-amd64-libvirt-xsm 13 migrate-support-checkfail never pass test-amd64-amd64-libvirt 13 migrate-support-checkfail 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 n
Re: [Xen-devel] x86 Community Call - Wed Aug 15, 14:00 - 15:00 UTC - Call for agenda items
On Tue, Aug 07, 2018 at 10:26:11AM +0100, Lars Kurth wrote: > Dear community members, > > please send me agenda items for next week’s community call by this Friday. > Note that this a week later than normal, because I had community calls in > my diary as the week before Advisory Board meetings (not the 2nd Wed > of each month) and I forgot to send out the agenda. > > I CC'ed attendees of the Arm call as sometimes there are cross-over > issues which could be handled either in the Arm or x86 call. That way you > get to see agenda, minutes, etc. and have a chance to decide on whether > you want to join or not. > > Last month’s minutes are at > https://docs.google.com/document/d/1fWQMuiblTmiNkWGGNbz20AQtbbnvtohxnUjhX2pA2jU/edit#heading=h.mz1wjb9vekjn > > > Main topics and loose/ends: > * Release Cadence for Xen 4.12 => resolved > * Project Management stuff to keep the Momentum going => we had no > Intel attendance at the last meeting and thus the status of various issues > is unknown. Some ACTIONS were resolved > > Open ACTIONS/status unknown: > * ACTION: George will update and re-submit the NVDIMM doc (he didn’t > take any notes during the discussion - we are going to have to reconstruct > some of the discussion) > * ACTION: Rich to follow up with committ...@xenproject.org > Resolution by committers@ off list: > Allow anyone from THE REST to issue a "Maintainer-is-absent-override" > ack for the XSM code while Daniel is away; the expectation, of course, > that "the rest" will be very cautious and try not to do anything Daniel > might object > > Best Regards > Lars > > ## Future Community Call schedule > Aug 15 (one week later than normal), Sept 12, Oct 10, Nov 14, Dec 12 The 15h of August is a public holiday here so I won't be able to attend. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v1 2/4] xen/arm: drivers: scif: Add support for SCIFA compatible UARTs
Hi Oleksandr,
On 06/08/18 19:35, Oleksandr Tyshchenko wrote:
From: Oleksandr Tyshchenko
Extend existing driver to be able to handle SCIFA interface as well.
SCIF and SCIFA have lot in common, though SCIFA has different
offsets and bits for some registers.
Use compatible string to recognize what interface is present
on a target board.
Signed-off-by: Oleksandr Tyshchenko
CC: Stefano Stabellini
CC: Julien Grall
---
xen/drivers/char/scif-uart.c| 129 +++-
xen/include/asm-arm/scif-uart.h | 44 --
2 files changed, 138 insertions(+), 35 deletions(-)
diff --git a/xen/drivers/char/scif-uart.c b/xen/drivers/char/scif-uart.c
index 465fb34..d1eb713 100644
--- a/xen/drivers/char/scif-uart.c
+++ b/xen/drivers/char/scif-uart.c
@@ -1,7 +1,7 @@
/*
* xen/drivers/char/scif-uart.c
*
- * Driver for SCIF (Serial communication interface with FIFO)
+ * Driver for SCIF(A) (Serial communication interface with FIFO (A))
* compatible UART.
*
* Oleksandr Tyshchenko
@@ -40,8 +40,57 @@ static struct scif_uart {
char __iomem *regs;
struct irqaction irqaction;
struct vuart_info vuart;
+const struct port_params *params;
} scif_com = {0};
+enum
Should not this enum has a name?
+{
+SCIF_PORT,
+SCIFA_PORT,
+NR_PORTS,
+};
+
+struct port_params
+{
+unsigned int status_reg;
+unsigned int tx_fifo_reg;
+unsigned int rx_fifo_reg;
+unsigned int overrun_reg;
+unsigned int overrun_mask;
+unsigned int error_mask;
+unsigned int irq_flags;
+unsigned int fifo_size;
+};
+
+static const struct port_params port_params[NR_PORTS] =
+{
+[SCIF_PORT] =
+{
+.status_reg = SCIF_SCFSR,
+.tx_fifo_reg = SCIF_SCFTDR,
+.rx_fifo_reg = SCIF_SCFRDR,
+.overrun_reg = SCIF_SCLSR,
+.overrun_mask = SCLSR_ORER,
+.error_mask = SCFSR_PER | SCFSR_FER | SCFSR_BRK | SCFSR_ER,
+.irq_flags= SCSCR_RIE | SCSCR_TIE | SCSCR_REIE,
+.fifo_size= 16,
+},
+
+[SCIFA_PORT] =
+{
+.status_reg = SCIFA_SCASSR,
+.tx_fifo_reg = SCIFA_SCAFTDR,
+.rx_fifo_reg = SCIFA_SCAFRDR,
+.overrun_reg = SCIFA_SCASSR,
+.overrun_mask = SCASSR_ORER,
+.error_mask = SCASSR_PER | SCASSR_FER | SCASSR_BRK | SCASSR_ER |
+SCASSR_ORER,
+.irq_flags= SCASCR_RIE | SCASCR_TIE | SCASCR_DRIE | SCASCR_ERIE |
+SCASCR_BRIE,
+.fifo_size= 64,
+},
+};
+
static void scif_uart_interrupt(int irq, void *data, struct cpu_user_regs
*regs)
{
struct serial_port *port = data;
@@ -49,7 +98,7 @@ static void scif_uart_interrupt(int irq, void *data, struct
cpu_user_regs *regs)
uint16_t status, ctrl;
ctrl = scif_readw(uart, SCIF_SCSCR);
-status = scif_readw(uart, SCIF_SCFSR) & ~SCFSR_TEND;
+status = scif_readw(uart, uart->params->status_reg) & ~SCFSR_TEND;
I would prefer if you introduce a local variable for the params. This
would avoid to write uart->params everywhere.
/* Ignore next flag if TX Interrupt is disabled */
if ( !(ctrl & SCSCR_TIE) )
status &= ~SCFSR_TDFE;
@@ -65,13 +114,19 @@ static void scif_uart_interrupt(int irq, void *data,
struct cpu_user_regs *regs)
serial_rx_interrupt(port, regs);
/* Error Interrupt */
-if ( status & SCIF_ERRORS )
-scif_writew(uart, SCIF_SCFSR, ~SCIF_ERRORS);
-if ( scif_readw(uart, SCIF_SCLSR) & SCLSR_ORER )
-scif_writew(uart, SCIF_SCLSR, 0);
+if ( status & uart->params->error_mask )
+scif_writew(uart, uart->params->status_reg,
+~uart->params->error_mask);
+if ( uart->params->overrun_reg != uart->params->status_reg )
+{
+if ( scif_readw(uart, uart->params->overrun_reg) &
+ uart->params->overrun_mask )
+scif_writew(uart, uart->params->overrun_reg,
+~uart->params->overrun_mask);
+}
ctrl = scif_readw(uart, SCIF_SCSCR);
-status = scif_readw(uart, SCIF_SCFSR) & ~SCFSR_TEND;
+status = scif_readw(uart, uart->params->status_reg) & ~SCFSR_TEND;
/* Ignore next flag if TX Interrupt is disabled */
if ( !(ctrl & SCSCR_TIE) )
status &= ~SCFSR_TDFE;
@@ -86,7 +141,7 @@ static void __init scif_uart_init_preirq(struct serial_port
*port)
* Wait until last bit has been transmitted. This is needed for a smooth
* transition when we come from early printk
*/
-while ( !(scif_readw(uart, SCIF_SCFSR) & SCFSR_TEND) );
+while ( !(scif_readw(uart, uart->params->status_reg) & SCFSR_TEND) );
/* Disable TX/RX parts and all interrupts */
scif_writew(uart, SCIF_SCSCR, 0);
@@ -95,10 +150,13 @@ static void __init scif_uart_init_preirq(struct
serial_port *port)
scif_writew(uart, SCIF_SCF
Re: [Xen-devel] [PATCH v1 3/4] xen/arm: Add SCIFA UART support for early printk
Hi, On 06/08/18 19:35, Oleksandr Tyshchenko wrote: From: Oleksandr Tyshchenko Add support for Renesas "Stout" development board based on R-Car H2 SoC which has SCIFA compatible UART. Actually existing SCIF UART support (debug-scif.inc) and newly added SCIFA UART support (debug-scifa.inc) differ only in registers offsets. In that case, could we just extend debug-scif.inc? Signed-off-by: Oleksandr Tyshchenko CC: Stefano Stabellini CC: Julien Grall --- docs/misc/arm/early-printk.txt | 3 ++- xen/arch/arm/Rules.mk | 1 + xen/arch/arm/arm32/debug-scifa.inc | 51 ++ 3 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 xen/arch/arm/arm32/debug-scifa.inc diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt index f765f59..f1b55d3 100644 --- a/docs/misc/arm/early-printk.txt +++ b/docs/misc/arm/early-printk.txt @@ -39,12 +39,13 @@ the name of the machine: - fastmodel: printk on ARM Fastmodel software emulators - hikey960: printk with pl011 with Hikey 960 - juno: printk with pl011 on Juno platform - - lager: printk with SCIF0 on Renesas R-Car H2 processors + - lager: printk with SCIF0 on Renesas Lager board (R-Car H2 processor) Why this change? - midway: printk with the pl011 on Calxeda Midway processors - mvebu: printk with the MVEBU for Marvell Armada 3700 SoCs - omap5432: printk with UART3 on TI OMAP5432 processors - rcar3: printk with SCIF2 on Renesas R-Car Gen3 processors - seattle: printk with pl011 for AMD Seattle processor + - stout: printk with SCIFA0 on Renesas Stout board (R-Car H2 processor) I have started to look at porting that to Kconfig ealyprintk and it is a massive pain. So I would tend to prefer if we avoid adding more convenience alias and instead document on the wiki page how to use earlyprintk for that. - sun6i: printk with 8250 on Allwinner A31 processors - sun7i: printk with 8250 on Allwinner A20 processors - thunderx: printk with pl011 for Cavium ThunderX processor diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk index f264592..e011f8b 100644 --- a/xen/arch/arm/Rules.mk +++ b/xen/arch/arm/Rules.mk @@ -40,6 +40,7 @@ EARLY_PRINTK_mvebu := mvebu,0xd0012000 EARLY_PRINTK_omap5432 := 8250,0x4802,2 EARLY_PRINTK_rcar3 := scif,0xe6e88000 EARLY_PRINTK_seattle:= pl011,0xe101 +EARLY_PRINTK_stout := scifa,0xe6c4 EARLY_PRINTK_sun6i := 8250,0x01c28000,2 EARLY_PRINTK_sun7i := 8250,0x01c28000,2 EARLY_PRINTK_thunderx := pl011,0x87e02400 diff --git a/xen/arch/arm/arm32/debug-scifa.inc b/xen/arch/arm/arm32/debug-scifa.inc new file mode 100644 index 000..b5e60db --- /dev/null +++ b/xen/arch/arm/arm32/debug-scifa.inc @@ -0,0 +1,51 @@ +/* + * xen/arch/arm/arm32/debug-scifa.inc + * + * SCIFA specific debug code + * + * Oleksandr Tyshchenko + * Copyright (C) 2018 EPAM Systems Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include + +/* + * SCIFA UART wait UART to be ready to transmit + * rb: register which contains the UART base address + * rc: scratch register + */ +.macro early_uart_ready rb rc +1: +ldrh \rc, [\rb, #SCIFA_SCASSR] /* <- SCASSR (status register) */ +tst\rc, #SCASSR_TDFE /* Check TDFE bit */ +beq1b /* Wait for the UART to be ready */ +.endm + +/* + * SCIFA UART transmit character + * rb: register which contains the UART base address + * rt: register which contains the character to transmit + */ +.macro early_uart_transmit rb rt +strb \rt, [\rb, #SCIFA_SCAFTDR] /* -> SCAFTDR (data register) */ +ldrh \rt, [\rb, #SCIFA_SCASSR] /* <- SCASSR (status register) */ +and\rt, \rt, #(~(SCASSR_TEND | SCASSR_TDFE)) /* Clear TEND and TDFE bits */ +strh \rt, [\rb, #SCIFA_SCASSR] /* -> SCASSR (status register) */ +.endm + +/* + * Local variables: + * mode: ASM + * indent-tabs-mode: nil + * End: + */ Cheers, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [RESEND] Spectre-v2 (IBPB/IBRS) and SSBD fixes for 4.4.y
On Fri, Aug 03, 2018 at 04:20:31PM -0700, Srivatsa S. Bhat wrote:
> On 8/2/18 3:22 PM, Kees Cook wrote:
> > On Thu, Aug 2, 2018 at 12:22 PM, Srivatsa S. Bhat
> > wrote:
> >> On 7/26/18 4:09 PM, Kees Cook wrote:
> >>> On Tue, Jul 24, 2018 at 3:02 PM, Jiri Kosina wrote:
> On Tue, 24 Jul 2018, Srivatsa S. Bhat wrote:
>
> > However, if you are proposing that you'd like to contribute the enhanced
> > PTI/Spectre (upstream) patches from the SLES 4.4 tree to 4.4 stable, and
> > have them merged instead of this patch series, then I would certainly
> > welcome it!
>
> I'd in principle love us to push everything back to 4.4, but there are a
> few reasons (*) why that's not happening shortly.
>
> Anyway, to point out explicitly what's really needed for those folks
> running 4.4-stable and relying on PTI providing The Real Thing(TM), it's
> either a 4.4-stable port of
>
>
> http://kernel.suse.com/cgit/kernel-source/plain/patches.suse/x86-entry-64-use-a-per-cpu-trampoline-stack.patch?id=3428a77b02b1ba03e45d8fc352ec350429f57fc7
>
> or making THREADINFO_GFP imply __GFP_ZERO.
> >>>
> >>> This is true in Linus's tree now. Should be trivial to backport:
> >>> https://git.kernel.org/linus/e01e80634ecdd
> >>>
> >>
> >> Hi Jiri, Kees,
> >>
> >> Thank you for suggesting the patch! I have attached the (locally
> >> tested) 4.4 and 4.9 backports of that patch with this mail. (The
> >> mainline commit applies cleanly on 4.14).
> >>
> >> Greg, could you please consider including them in stable 4.4, 4.9
> >> and 4.14?
> >
> > I don't think your v4.9 is sufficient: it leaves the vmapped stack
> > uncleared. v4.9 needs ca182551857 ("kmemleak: clear stale pointers
> > from task stacks") included in the backport (really, just adding the
> > memset()).
> >
>
> Ah, I see, thank you! I have attached the updated patchset for 4.9
> with this mail.
>
> > Otherwise, yup, looks good.
> >
> Thank you for reviewing the patches!
>
> Regards,
> Srivatsa
> VMware Photon OS
These work for 4.9, do you also have a set for 4.4?
thanks,
greg k-h
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 0/4] xen: various cleanups
On 08/07/2018 09:10 AM, Juergen Gross wrote: > On 17/07/18 14:01, Juergen Gross wrote: >> Some Xen related cleanups: >> - move some pv-only code from CONFIG_XEN to CONFIG_XEN_PV >> - use CONFIG_XEN_PVHVM in Makefile instead of #ifdef around a complete source >> - add SPDX identifier where missing >> >> Juergen Gross (4): >> xen: move pv irq related functions under CONFIG_XEN_PV umbrella >> xen: move pv specific parts of arch/x86/xen/mmu.c to mmu_pv.c >> xen: link platform-pci-unplug.o only if CONFIG_XEN_PVHVM >> xen: add SPDX identifier in arch/x86/xen files >> >> arch/x86/entry/entry_32.S | 8 +- >> arch/x86/entry/entry_64.S | 8 +- >> arch/x86/xen/Makefile | 41 ++-- >> arch/x86/xen/efi.c | 14 +-- >> arch/x86/xen/enlighten.c | 2 + >> arch/x86/xen/enlighten_hvm.c | 2 + >> arch/x86/xen/grant-table.c | 25 + >> arch/x86/xen/mmu.c | 188 >> + >> arch/x86/xen/mmu_pv.c | 140 +++ >> arch/x86/xen/p2m.c | 2 + >> arch/x86/xen/pci-swiotlb-xen.c | 2 + >> arch/x86/xen/platform-pci-unplug.c | 18 +--- >> arch/x86/xen/vdso.h| 2 + >> arch/x86/xen/xen-pvh.S | 15 +-- >> include/xen/interface/memory.h | 6 -- >> include/xen/xen-ops.h | 133 +- >> 16 files changed, 287 insertions(+), 319 deletions(-) >> > Boris, any objections? This series had my R-b, but I was waiting for x86 maintainers to ack the first patch. But now that I looked at this again, I wonder whether +#ifdef CONFIG_XEN BUILD_INTERRUPT3(xen_hvm_callback_vector, HYPERVISOR_CALLBACK_VECTOR, xen_evtchn_do_upcall) +#endif and +#ifdef CONFIG_XEN apicinterrupt3 HYPERVISOR_CALLBACK_VECTOR \ xen_hvm_callback_vector xen_evtchn_do_upcall +#endif should be under XEN_PVHVM -boris -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] xen: don't use privcmd_call() from xen_mc_flush()
On 08/07/2018 09:11 AM, Juergen Gross wrote: > On 13/06/18 11:58, Juergen Gross wrote: >> Using privcmd_call() for a singleton multicall seems to be wrong, as >> privcmd_call() is using stac()/clac() to enable hypervisor access to >> Linux user space. >> >> Add a new xen_single_call() function to be use for that purpose. >> >> Reported-by: Jan Beulich >> Signed-off-by: Juergen Gross > Boris, any objections? I think Jan wanted a change in commit message. I can commit with this: "Using privcmd_call() for a singleton multicall seems to be wrong, as privcmd_call() is using stac()/clac() to enable hypervisor access to Linux user space. Even if currently not a problem (pv domains can't use SMAP while HVM and PVH domains can't use multicalls) things might change when PVH dom0 support is added to the kernel." -boris ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v2] x86/hvm: remove default ioreq server
My recent patch [1] to qemu-xen-traditional removes the last use of the
'default' ioreq server in Xen. (This is a catch-all ioreq server that is
used if no explicitly registered I/O range is targetted).
This patch can be applied once that patch is committed, to remove the
(>100 lines of) redundant code in Xen.
NOTE: The removal of the special case for HVM_PARAM_DM_DOMAIN in
hvm_allow_set_param() is not directly related to removal of
default ioreq servers. It could have been cleaned up at any time
after commit 9a422c03 "x86/hvm: stop passing explicit domid to
hvm_create_ioreq_server()".
[1] https://lists.xenproject.org/archives/html/xen-devel/2018-08/msg00270.html
Signed-off-by: Paul Durrant
Acked-by: Andrew Cooper
---
Cc: Jan Beulich
v2:
- Disallow reads or writes of HVM_PARAM_BUFIOREQ_EVTCHN and mark it as
deprecated in the header.
- Updated commit comment w.r.t. HVM_PARAM_DM_DOMAIN change.
---
xen/arch/x86/hvm/dm.c| 2 +-
xen/arch/x86/hvm/hvm.c | 36 +++-
xen/arch/x86/hvm/ioreq.c | 123 +--
xen/include/asm-x86/hvm/domain.h | 1 -
xen/include/asm-x86/hvm/ioreq.h | 4 +-
xen/include/public/hvm/params.h | 5 +-
6 files changed, 30 insertions(+), 141 deletions(-)
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c
index 6755f3fd96..6ace3dc7c1 100644
--- a/xen/arch/x86/hvm/dm.c
+++ b/xen/arch/x86/hvm/dm.c
@@ -411,7 +411,7 @@ static int dm_op(const struct dmop_args *op_args)
if ( data->pad[0] || data->pad[1] || data->pad[2] )
break;
-rc = hvm_create_ioreq_server(d, false, data->handle_bufioreq,
+rc = hvm_create_ioreq_server(d, data->handle_bufioreq,
&data->id);
break;
}
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 72c51faecb..a797fc21c4 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4093,12 +4093,15 @@ static int hvm_allow_set_param(struct domain *d,
case HVM_PARAM_CONSOLE_EVTCHN:
case HVM_PARAM_X87_FIP_WIDTH:
break;
+/* The following parameters are deprecated. */
+case HVM_PARAM_BUFIOREQ_EVTCHN:
+rc = -EPERM;
+break;
/*
* The following parameters must not be set by the guest
* since the domain may need to be paused.
*/
case HVM_PARAM_IDENT_PT:
-case HVM_PARAM_DM_DOMAIN:
case HVM_PARAM_ACPI_S_STATE:
/* The remaining parameters should not be set by the guest. */
default:
@@ -4263,9 +4266,6 @@ static int hvmop_set_param(
d->arch.hvm_domain.params[HVM_PARAM_NESTEDHVM] )
rc = -EINVAL;
break;
-case HVM_PARAM_BUFIOREQ_EVTCHN:
-rc = -EINVAL;
-break;
case HVM_PARAM_TRIPLE_FAULT_REASON:
if ( a.value > SHUTDOWN_MAX )
rc = -EINVAL;
@@ -4373,13 +4373,10 @@ static int hvm_allow_get_param(struct domain *d,
case HVM_PARAM_ALTP2M:
case HVM_PARAM_X87_FIP_WIDTH:
break;
-/*
- * The following parameters must not be read by the guest
- * since the domain may need to be paused.
- */
-case HVM_PARAM_IOREQ_PFN:
-case HVM_PARAM_BUFIOREQ_PFN:
+/* The following parameters are deprecated. */
case HVM_PARAM_BUFIOREQ_EVTCHN:
+rc = -ENODATA;
+break;
/* The remaining parameters should not be read by the guest. */
default:
if ( d == current->domain )
@@ -4433,25 +4430,6 @@ static int hvmop_get_param(
case HVM_PARAM_X87_FIP_WIDTH:
a.value = d->arch.x87_fip_width;
break;
-case HVM_PARAM_IOREQ_PFN:
-case HVM_PARAM_BUFIOREQ_PFN:
-case HVM_PARAM_BUFIOREQ_EVTCHN:
-/*
- * It may be necessary to create a default ioreq server here,
- * because legacy versions of QEMU are not aware of the new API for
- * explicit ioreq server creation. However, if the domain is not
- * under construction then it will not be QEMU querying the
- * parameters and thus the query should not have that side-effect.
- */
-if ( !d->creation_finished )
-{
-rc = hvm_create_ioreq_server(d, true,
- HVM_IOREQSRV_BUFIOREQ_LEGACY, NULL);
-if ( rc != 0 && rc != -EEXIST )
-goto out;
-}
-
-/*FALLTHRU*/
default:
a.value = d->arch.hvm_domain.params[a.index];
break;
diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index 7c515b3ef7..9cb3819c6b 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -55,9 +55,6 @@ static struct hvm_ioreq_server *get_ioreq_server(const struct
domain *d,
return GET_IOREQ_SERVER(d, id);
}
-#define IS_DEFAULT(s) \
-((s) && (s) == GET_IOREQ_SERVER((s)->target, DEFAULT_IOSERVID))
-
/*
* Iterate over all possible ioreq servers.
*
@@ -245,8 +242,6 @@ static gfn_t hvm_alloc_ioreq_gfn(
[Xen-devel] [PATCH v3 4/4] x86/iommu: add reserved dom0-iommu option to map reserved memory ranges
Several people have reported hardware issues (malfunctioning USB
controllers) due to iommu page faults on Intel hardware. Those faults
are caused by missing RMRR (VTd) entries in the ACPI tables. Those can
be worked around on VTd hardware by manually adding RMRR entries on
the command line, this is however limited to Intel hardware and quite
cumbersome to do.
In order to solve those issues add a new dom0-iommu=reserved option
that identity maps all regions marked as reserved in the memory map.
Note that regions used by devices emulated by Xen (LAPIC, IO-APIC or
PCIe MCFG regions) are specifically avoided. Note that this option is
available to a PVH Dom0 (as opposed to the inclusive option which only
works for PV Dom0).
Signed-off-by: Roger Pau Monné
---
Changes since v2:
- Fix comment regarding dom0-strict.
- Change documentation style of xen command line.
- Rename iommu_map to hwdom_iommu_map.
- Move all the checks to hwdom_iommu_map.
Changes since v1:
- Introduce a new reserved option instead of abusing the inclusive
option.
- Use the same helper function for PV and PVH in order to decide if a
page should be added to the domain page tables.
- Use the data inside of the domain struct to detect overlaps with
emulated MMIO regions.
---
Cc: Andrew Cooper
Cc: George Dunlap
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Julien Grall
Cc: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini
Cc: Tim Deegan
Cc: Wei Liu
---
docs/misc/xen-command-line.markdown | 11 ++-
xen/arch/x86/hvm/io.c | 5 ++
xen/drivers/passthrough/amd/pci_amd_iommu.c | 3 +
xen/drivers/passthrough/iommu.c | 3 +
xen/drivers/passthrough/vtd/iommu.c | 3 +
xen/drivers/passthrough/x86/iommu.c | 86 ++---
xen/include/asm-x86/hvm/io.h| 3 +
xen/include/xen/iommu.h | 2 +-
8 files changed, 85 insertions(+), 31 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown
b/docs/misc/xen-command-line.markdown
index 90b32fe3f0..59ec2afc5d 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1205,7 +1205,7 @@ detection of systems known to misbehave upon accesses to
that port.
>> Enable IOMMU debugging code (implies `verbose`).
### dom0-iommu
-> `= List of [ none | strict | relaxed | inclusive ]`
+> `= List of [ none | strict | relaxed | inclusive | reserved ]`
* `none`: disables DMA remapping for Dom0.
@@ -1233,6 +1233,15 @@ meaning:
option is only applicable to a PV Dom0 and is enabled by default on Intel
hardware.
+* `reserved`: sets up DMA remapping for all the reserved regions in the memory
+ map for Dom0. Use this to work around firmware issues providing incorrect
+ RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU accesses
+ for Dom0, all memory regions marked as reserved in the memory map that don't
+ overlap with any MMIO region from emulated devices will be identity mapped.
+ This option maps a subset of the memory that would be mapped when using the
+ `inclusive` option. This option is available to a PVH Dom0 and is enabled by
+ default on Intel hardware.
+
### iommu\_dev\_iotlb\_timeout
> `= `
diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
index bf4d8748d3..5e01c33890 100644
--- a/xen/arch/x86/hvm/io.c
+++ b/xen/arch/x86/hvm/io.c
@@ -404,6 +404,11 @@ static const struct hvm_mmcfg *vpci_mmcfg_find(const
struct domain *d,
return NULL;
}
+bool vpci_mmcfg_address(const struct domain *d, paddr_t addr)
+{
+return vpci_mmcfg_find(d, addr);
+}
+
static unsigned int vpci_mmcfg_decode_addr(const struct hvm_mmcfg *mmcfg,
paddr_t addr, pci_sbdf_t *sbdf)
{
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 0e0c99c942..2c2867d088 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -256,6 +256,9 @@ static void __hwdom_init amd_iommu_hwdom_init(struct domain
*d)
/* Inclusive IOMMU mappings are disabled by default on AMD hardware. */
iommu_dom0_inclusive = iommu_dom0_inclusive == -1 ? false
: iommu_dom0_inclusive;
+/* Reserved IOMMU mappings are disabled by default on AMD hardware. */
+iommu_dom0_reserved = iommu_dom0_reserved == -1 ? false
+: iommu_dom0_reserved;
if ( allocate_domain_resources(dom_iommu(d)) )
BUG();
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index f15c94be42..9c991bd2cf 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -75,6 +75,7 @@ custom_param("dom0-iommu", parse_dom0_iommu_param);
bool __hwdom_initdata iommu_dom0_strict;
bool __read_mostly iommu_dom0_passthrough;
int8_t __hwdom_initdata iommu_dom0_inclusive = -1;
+int8_t __hwd
[Xen-devel] [PATCH v3 0/4] x86/iommu: PVH Dom0 workarounds for missing RMRR entries
Hello, The following series implement a workaround for missing RMRR entries for a PVH Dom0. It's based on the iommu_inclusive_mapping VTd option. The PVH workaround identity maps all regions marked as reserved in the memory map. Note that this workaround is enabled by default on Intel hardware. It's also available to AMD hardware, although it's disabled by default in that case. The series can be found at: git://xenbits.xen.org/people/royger/xen.git iommu_inclusive_v3 Thanks, Roger. Roger Pau Monne (4): iommu: introduce dom0-iommu option iommu: make iommu_inclusive_mapping a suboption of dom0-iommu dom0/pvh: change the order of the MMCFG initialization x86/iommu: add reserved dom0-iommu option to map reserved memory ranges docs/misc/xen-command-line.markdown | 47 +++ xen/arch/x86/hvm/dom0_build.c | 9 ++- xen/arch/x86/hvm/io.c | 5 ++ xen/arch/x86/x86_64/mm.c| 3 +- xen/drivers/passthrough/amd/iommu_init.c| 2 +- xen/drivers/passthrough/amd/pci_amd_iommu.c | 11 ++- xen/drivers/passthrough/arm/iommu.c | 4 + xen/drivers/passthrough/iommu.c | 62 +-- xen/drivers/passthrough/vtd/extern.h| 2 - xen/drivers/passthrough/vtd/iommu.c | 25 +++--- xen/drivers/passthrough/vtd/x86/vtd.c | 58 +- xen/drivers/passthrough/x86/iommu.c | 87 + xen/include/asm-x86/hvm/io.h| 3 + xen/include/xen/iommu.h | 8 +- 14 files changed, 240 insertions(+), 86 deletions(-) -- 2.18.0 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v3 2/4] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu
Introduce a new dom0-iommu=inclusive generic option that supersedes
iommu_inclusive_mapping. The previous behaviour is preserved and the
option should only be enabled by default on Intel hardware.
No functional change intended.
Signed-off-by: Roger Pau Monné
---
Changes since v2:
- Fix typo in commit message.
- Change style and text of the documentation in xen command line.
- Set the defaults in {intel/amd}_iommu_hwdom_init for inclusive.
- Re-add the iommu_dom0_passthrough || !is_pv_domain(d) check.
Changes since v1:
- Use dom0-iommu instead of the iommu option.
- Only enable by default on Intel hardware.
---
Cc: Andrew Cooper
Cc: George Dunlap
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Julien Grall
Cc: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini
Cc: Tim Deegan
Cc: Wei Liu
Cc: Kevin Tian
---
docs/misc/xen-command-line.markdown | 17 +-
xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 ++
xen/drivers/passthrough/arm/iommu.c | 4 ++
xen/drivers/passthrough/iommu.c | 23 ++--
xen/drivers/passthrough/vtd/extern.h| 2 -
xen/drivers/passthrough/vtd/iommu.c | 8 ++-
xen/drivers/passthrough/vtd/x86/vtd.c | 58 +---
xen/drivers/passthrough/x86/iommu.c | 59 +
xen/include/xen/iommu.h | 2 +
9 files changed, 109 insertions(+), 68 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown
b/docs/misc/xen-command-line.markdown
index ea451f088e..90b32fe3f0 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1205,7 +1205,7 @@ detection of systems known to misbehave upon accesses to
that port.
>> Enable IOMMU debugging code (implies `verbose`).
### dom0-iommu
-> `= List of [ none | strict | relaxed ]`
+> `= List of [ none | strict | relaxed | inclusive ]`
* `none`: disables DMA remapping for Dom0.
@@ -1221,6 +1221,18 @@ PV Dom0:
Note that all the above options are mutually exclusive. Specifying more than
one on the `dom0-iommu` command line will result in undefined behavior.
+The following options control whether non-RAM regions are added to the Dom0
+iommu tables. Note that they can be prefixed with `no-` to effect the inverse
+meaning:
+
+* `inclusive`: sets up DMA remapping for all the non-RAM memory below 4GB
+ except for unusable ranges. Use this to work around firmware issues providing
+ incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU
+ accesses for Dom0, with this option all pages up to 4GB, not marked as
+ unusable in the E820 table, will get a mapping established. Note that this
+ option is only applicable to a PV Dom0 and is enabled by default on Intel
+ hardware.
+
### iommu\_dev\_iotlb\_timeout
> `= `
@@ -1233,6 +1245,9 @@ wait descriptor timed out', try increasing this value.
### iommu\_inclusive\_mapping (VT-d)
> `= `
+**WARNING: This command line option is deprecated, and superseded by
+_dom0-iommu=inclusive_ - using both options in combination is undefined.**
+
> Default: `true`
Use this to work around firmware issues providing incorrect RMRR entries.
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index eeacf713e4..0e0c99c942 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -253,6 +253,10 @@ static void __hwdom_init amd_iommu_hwdom_init(struct
domain *d)
unsigned long i;
const struct amd_iommu *iommu;
+/* Inclusive IOMMU mappings are disabled by default on AMD hardware. */
+iommu_dom0_inclusive = iommu_dom0_inclusive == -1 ? false
+ : iommu_dom0_inclusive;
+
if ( allocate_domain_resources(dom_iommu(d)) )
BUG();
diff --git a/xen/drivers/passthrough/arm/iommu.c
b/xen/drivers/passthrough/arm/iommu.c
index 95b1abb972..325997b19f 100644
--- a/xen/drivers/passthrough/arm/iommu.c
+++ b/xen/drivers/passthrough/arm/iommu.c
@@ -73,3 +73,7 @@ int arch_iommu_populate_page_table(struct domain *d)
/* The IOMMU shares the p2m with the CPU */
return -ENOSYS;
}
+
+void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
+{
+}
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 830560bdcf..f15c94be42 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -74,6 +74,7 @@ bool_t __read_mostly amd_iommu_perdev_intremap = 1;
custom_param("dom0-iommu", parse_dom0_iommu_param);
bool __hwdom_initdata iommu_dom0_strict;
bool __read_mostly iommu_dom0_passthrough;
+int8_t __hwdom_initdata iommu_dom0_inclusive = -1;
DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
@@ -144,16 +145,23 @@ static int __init parse_dom0_iommu_param(const char *s)
int rc = 0;
do {
+bool val = !!strncmp(s, "no-", 3);
+
+if ( !val )
+s += 3;
+
ss = strchr(s, ',');
if ( !ss
[Xen-devel] [PATCH v3 1/4] iommu: introduce dom0-iommu option
To select the iommu configuration used by Dom0. This option supersedes
iommu=dom0-strict|dom0-passthrough.
No functional change.
Signed-off-by: Roger Pau Monné
Reviewed-by: Paul Durrant
---
Changes since v2:
- Change the style and text used in the xen command line document.
- Don't allow none, strict or relaxed to use the no- prefix.
Changes since v1:
- New in this version.
---
Cc: Andrew Cooper
Cc: George Dunlap
Cc: Ian Jackson
Cc: Jan Beulich
Cc: Julien Grall
Cc: Konrad Rzeszutek Wilk
Cc: Stefano Stabellini
Cc: Tim Deegan
Cc: Wei Liu
Cc: Suravee Suthikulpanit
Cc: Brian Woods
Cc: Kevin Tian
---
docs/misc/xen-command-line.markdown | 23 +++
xen/arch/x86/x86_64/mm.c| 3 +-
xen/drivers/passthrough/amd/iommu_init.c| 2 +-
xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 +-
xen/drivers/passthrough/iommu.c | 42 +
xen/drivers/passthrough/vtd/iommu.c | 16
xen/include/xen/iommu.h | 6 ++-
7 files changed, 75 insertions(+), 21 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown
b/docs/misc/xen-command-line.markdown
index 65b4754418..ea451f088e 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1150,12 +1150,18 @@ detection of systems known to misbehave upon accesses
to that port.
> `dom0-passthrough`
+> **WARNING: This command line option is deprecated, and superseded by
+> _dom0-iommu=none_ - using both options in combination is undefined.**
+
> Default: `false`
>> Control whether to disable DMA remapping for Dom0.
> `dom0-strict`
+> **WARNING: This command line option is deprecated, and superseded by
+> _dom0-iommu=strict_ - using both options in combination is undefined.**
+
> Default: `false`
>> Control whether to set up DMA remapping only for the memory Dom0 actually
@@ -1198,6 +1204,23 @@ detection of systems known to misbehave upon accesses to
that port.
>> Enable IOMMU debugging code (implies `verbose`).
+### dom0-iommu
+> `= List of [ none | strict | relaxed ]`
+
+* `none`: disables DMA remapping for Dom0.
+
+The following two options control how RAM regions are mapped in the iommu for
+PV Dom0:
+
+* `strict`: sets up DMA remapping only for the memory Dom0 actually got
+ assigned.
+
+* `relaxed`: sets DMA remapping for all the host RAM except regions in use by
+ Xen. This is the default iommu behaviour.
+
+Note that all the above options are mutually exclusive. Specifying more than
+one on the `dom0-iommu` command line will result in undefined behavior.
+
### iommu\_dev\_iotlb\_timeout
> `= `
diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
index cca4ae926e..84226b3326 100644
--- a/xen/arch/x86/x86_64/mm.c
+++ b/xen/arch/x86/x86_64/mm.c
@@ -1426,7 +1426,8 @@ int memory_add(unsigned long spfn, unsigned long epfn,
unsigned int pxm)
if ( ret )
goto destroy_m2p;
-if ( iommu_enabled && !iommu_passthrough && !need_iommu(hardware_domain) )
+if ( iommu_enabled && !iommu_dom0_passthrough &&
+ !need_iommu(hardware_domain) )
{
for ( i = spfn; i < epfn; i++ )
if ( iommu_map_page(hardware_domain, i, i,
IOMMUF_readable|IOMMUF_writable) )
diff --git a/xen/drivers/passthrough/amd/iommu_init.c
b/xen/drivers/passthrough/amd/iommu_init.c
index 474992a75a..ad8c48be1c 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -1062,7 +1062,7 @@ static void __init amd_iommu_init_cleanup(void)
radix_tree_destroy(&ivrs_maps, xfree);
iommu_enabled = 0;
-iommu_passthrough = 0;
+iommu_dom0_passthrough = false;
iommu_intremap = 0;
iommuv2_enabled = 0;
}
diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 12d2695b89..eeacf713e4 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -121,7 +121,7 @@ static void amd_iommu_setup_domain_device(
BUG_ON( !hd->arch.root_table || !hd->arch.paging_mode ||
!iommu->dev_table.buffer );
-if ( iommu_passthrough && is_hardware_domain(domain) )
+if ( iommu_dom0_passthrough && is_hardware_domain(domain) )
valid = 0;
if ( ats_enabled )
@@ -256,7 +256,7 @@ static void __hwdom_init amd_iommu_hwdom_init(struct domain
*d)
if ( allocate_domain_resources(dom_iommu(d)) )
BUG();
-if ( !iommu_passthrough && !need_iommu(d) )
+if ( !iommu_dom0_passthrough && !need_iommu(d) )
{
int rc = 0;
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 70d218f910..830560bdcf 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -22,6 +22,7 @@
#include
static int parse_iommu_param(const char *s);
+static int parse_dom0_iommu_param(const char *s);
static void iommu_dump_p2m_table(unsigned char
[Xen-devel] [PATCH v3 3/4] dom0/pvh: change the order of the MMCFG initialization
So it's done before the iommu is initialized. This is required in
order to be able to fetch the MMCFG regions from the domain struct.
No functional change.
Signed-off-by: Roger Pau Monné
---
Changes since v1:
- New in this version.
---
Cc: Jan Beulich
Cc: Andrew Cooper
---
xen/arch/x86/hvm/dom0_build.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index f0cd63b1ec..5065729106 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -1100,6 +1100,13 @@ int __init dom0_construct_pvh(struct domain *d, const
module_t *image,
return rc;
}
+/*
+ * NB: MMCFG initialization needs to be performed before iommu
+ * initialization so the iommu code can fetch the MMCFG regions used by the
+ * domain.
+ */
+pvh_setup_mmcfg(d);
+
iommu_hwdom_init(d);
rc = pvh_load_kernel(d, image, image_headroom, initrd,
bootstrap_map(image),
@@ -1124,8 +1131,6 @@ int __init dom0_construct_pvh(struct domain *d, const
module_t *image,
return rc;
}
-pvh_setup_mmcfg(d);
-
printk("WARNING: PVH is an experimental mode with limited
functionality\n");
return 0;
}
--
2.18.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v20 1/2] common: add a new mappable resource type: XENMEM_resource_grant_table
> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: 07 August 2018 14:38
> To: Paul Durrant
> Cc: Andrew Cooper ; Wei Liu
> ; George Dunlap ; Ian
> Jackson ; Stefano Stabellini
> ; xen-devel ;
> Konrad Rzeszutek Wilk ; Tim (Xen.org)
>
> Subject: Re: [PATCH v20 1/2] common: add a new mappable resource type:
> XENMEM_resource_grant_table
>
> >>> On 06.08.18 at 14:54, wrote:
> > --- a/xen/common/grant_table.c
> > +++ b/xen/common/grant_table.c
> > @@ -3860,6 +3860,38 @@ int mem_sharing_gref_to_gfn(struct
> grant_table *gt, grant_ref_t ref,
> > }
> > #endif
> >
> > +/* caller must hold read or write lock */
> > +static int gnttab_get_status_frame_mfn(struct domain *d,
> > + unsigned long idx, mfn_t *mfn)
> > +{
> > +struct grant_table *gt = d->grant_table;
> > +
> > +if ( idx >= nr_status_frames(gt) )
> > +return -EINVAL;
> > +
> > +*mfn = _mfn(virt_to_mfn(gt->status[idx]));
> > +return 0;
> > +}
> > +
> > +/* caller must hold write lock */
> > +static int gnttab_get_shared_frame_mfn(struct domain *d,
> > + unsigned long idx, mfn_t *mfn)
> > +{
> > +struct grant_table *gt = d->grant_table;
> > +
> > +if ( gt->gt_version == 0 )
> > +gt->gt_version = 1;
> > +
> > +if ( (idx >= nr_grant_frames(gt)) && (idx < gt->max_grant_frames) )
> > +gnttab_grow_table(d, idx + 1);
>
> I guess I had commented on this before, but it has been a while:
> While I realize that you're just moving code, I dislike the resulting
> asymmetry between the two functions: Why would the former
> not want to grow the grant table? And why would a version
> adjustment be needed (only) here (I've put "only" in parentheses
> because it's not obvious to me why this is needed)?
>
> And this asymmetry would surely be coming as surprise at least
> to callers of the new interface you add.
>
I'm happy to have get_status_frame() grow the table too but it does mean a
change in behaviour to callers of gnttab_map_frame(). If that's not a problem
then I'll make the change.
The version adjustment may be a hangover from the previous code base. Juergen's
per-domain gnttab adjustments have probably rendered this unnecessary so I'll
drop it as long as nothing breaks.
> > +int gnttab_get_status_frame(struct domain *d, unsigned long idx,
> > +mfn_t *mfn)
> > +{
> > +struct grant_table *gt = d->grant_table;
> > +int rc;
> > +
> > +grant_read_lock(gt);
> > +rc = gnttab_get_status_frame_mfn(d, idx, mfn);
> > +grant_read_unlock(gt);
> > +
> > +return rc;
> > +}
>
> Along the lines of what gnttab_map_frame() does, I'd expect a
> check for gt_version to be 2 here. Or well, maybe that's implicit
> by there not being any status entries/pages for version 1 tables.
Yes, that should be the case.
> But it would become a requirement if gnttab_grow_table() was to
> be called from gnttab_get_status_frame_mfn().
>
Yes, so making that change will certainly add complexity.
> > @@ -982,6 +983,54 @@ static long xatp_permission_check(struct domain
> *d, unsigned int space)
> > return xsm_add_to_physmap(XSM_TARGET, current->domain, d);
> > }
> >
> > +static int acquire_grant_table(struct domain *d, unsigned int id,
> > + unsigned long frame,
> > + unsigned int nr_frames,
> > + xen_pfn_t mfn_list[])
> > +{
> > +unsigned int i = nr_frames;
> > +
> > +/*
> > + * FIXME: It is not currently safe to map grant status frames if they
> > + *will be inserted into the caller's P2M, because these
> > + *insertions are not yet properly reference counted.
> > + *This restriction can be removed when appropriate reference
> > + *counting is added.
> > + */
> > +if ( paging_mode_translate(current->domain) &&
> > + (id == XENMEM_resource_grant_table_id_status) )
> > +return -EOPNOTSUPP;
>
> Would you mind reminding me why the P2M insertions are safe for
> the "ordinary" (shared) grant frames? I'm puzzled because P2M
> management in general lacks refcounting, yet I have the vague
> feeling that we had discussed this before and there was a reason.
> (If there is, perhaps slightly extending the comment above would
> be helpful.)
My memory is hazy too so I'll have to mine back down the email traffic. Without
having done that I can only assume it is because there is the possibility of
mapping the status frames and then setting the version back to 1 thus causing
the status frames to evaporate whilst still being present in the P2M. This
clearly won't happen to the shared frames.
>
> > --- a/xen/include/public/memory.h
> > +++ b/xen/include/public/memory.h
> > @@ -611,16 +611,21 @@ struct xen_mem_acquire_resource {
> > uint16_t type;
> >
> > #define XENMEM_resource_ioreq_server 0
> > +#define
Re: [Xen-devel] [PATCH 2/4] xen/blkfront: cleanup stale persistent grants
On Tue, Aug 07, 2018 at 08:31:31AM +0200, Juergen Gross wrote:
> On 06/08/18 18:16, Roger Pau Monné wrote:
> > On Mon, Aug 06, 2018 at 01:34:01PM +0200, Juergen Gross wrote:
> >> Add a periodic cleanup function to remove old persistent grants which
> >> are no longer in use on the backend side. This avoids starvation in
> >> case there are lots of persistent grants for a device which no longer
> >> is involved in I/O business.
> >>
> >> Signed-off-by: Juergen Gross
> >> ---
> >> drivers/block/xen-blkfront.c | 99
> >> ++--
> >> 1 file changed, 95 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> >> index b5cedccb5d7d..19feb8835fc4 100644
> >> --- a/drivers/block/xen-blkfront.c
> >> +++ b/drivers/block/xen-blkfront.c
> >> @@ -46,6 +46,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >>
> >> #include
> >> #include
> >> @@ -121,6 +122,9 @@ static inline struct blkif_req *blkif_req(struct
> >> request *rq)
> >>
> >> static DEFINE_MUTEX(blkfront_mutex);
> >> static const struct block_device_operations xlvbd_block_fops;
> >> +static struct delayed_work blkfront_work;
> >> +static LIST_HEAD(info_list);
> >> +static bool blkfront_work_active;
> >>
> >> /*
> >> * Maximum number of segments in indirect requests, the actual value used
> >> by
> >> @@ -216,6 +220,7 @@ struct blkfront_info
> >>/* Save uncomplete reqs and bios for migration. */
> >>struct list_head requests;
> >>struct bio_list bio_list;
> >> + struct list_head info_list;
> >> };
> >>
> >> static unsigned int nr_minors;
> >> @@ -1764,6 +1769,12 @@ static int write_per_ring_nodes(struct
> >> xenbus_transaction xbt,
> >>return err;
> >> }
> >>
> >> +static void free_info(struct blkfront_info *info)
> >> +{
> >> + list_del(&info->info_list);
> >> + kfree(info);
> >> +}
> >> +
> >> /* Common code used when first setting up, and when resuming. */
> >> static int talk_to_blkback(struct xenbus_device *dev,
> >> struct blkfront_info *info)
> >> @@ -1885,7 +1896,10 @@ static int talk_to_blkback(struct xenbus_device
> >> *dev,
> >> destroy_blkring:
> >>blkif_free(info, 0);
> >>
> >> - kfree(info);
> >> + mutex_lock(&blkfront_mutex);
> >> + free_info(info);
> >> + mutex_unlock(&blkfront_mutex);
> >> +
> >>dev_set_drvdata(&dev->dev, NULL);
> >>
> >>return err;
> >> @@ -1996,6 +2010,10 @@ static int blkfront_probe(struct xenbus_device *dev,
> >>info->handle = simple_strtoul(strrchr(dev->nodename, '/')+1, NULL, 0);
> >>dev_set_drvdata(&dev->dev, info);
> >>
> >> + mutex_lock(&blkfront_mutex);
> >> + list_add(&info->info_list, &info_list);
> >> + mutex_unlock(&blkfront_mutex);
> >> +
> >>return 0;
> >> }
> >>
> >> @@ -2306,6 +2324,15 @@ static void blkfront_gather_backend_features(struct
> >> blkfront_info *info)
> >>if (indirect_segments <= BLKIF_MAX_SEGMENTS_PER_REQUEST)
> >>indirect_segments = 0;
> >>info->max_indirect_segments = indirect_segments;
> >> +
> >> + if (info->feature_persistent) {
> >> + mutex_lock(&blkfront_mutex);
> >> + if (!blkfront_work_active) {
> >> + blkfront_work_active = true;
> >> + schedule_delayed_work(&blkfront_work, HZ * 10);
> >
> > Does it make sense to provide a module parameter to rune the schedule
> > of the cleanup routine?
>
> I don't think this is something anyone would like to tune.
>
> In case you think it should be tunable I can add a parameter, of course.
We can always add it later if required. I'm fine as-is now.
> >
> >> + }
> >> + mutex_unlock(&blkfront_mutex);
> >
> > Is it really necessary to have the blkfront_work_active boolean? What
> > happens if you queue the same delayed work more than once?
>
> In case there is already work queued later calls of
> schedule_delayed_work() will be ignored.
>
> So yes, I can drop the global boolean (I still need a local flag in
> blkfront_delay_work() for controlling the need to call
> schedule_delayed_work() again).
Can't you just call schedule_delayed_work if info->feature_persistent
is set, even if that means calling it multiple times if multiple
blkfront instances are using persistent grants?
Thanks, Roger.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v4 31/32] libxl_disk: Cut libxl_cdrom_insert into step
On Mon, Aug 06, 2018 at 06:20:50PM +0100, Anthony PERARD wrote: > On Thu, Aug 02, 2018 at 05:50:52PM +0200, Roger Pau Monné wrote: > > On Fri, Jul 27, 2018 at 03:06:13PM +0100, Anthony PERARD wrote: > > Can you provide a comment explaining how is this supposed to work? The > > current code is already quite convoluted IMO (maybe because I'm not > > familiar with it), so I think a comment would help reviewers. > > Have you read the CODING_STYLE file? I think you'll find that the > section "ASYNCHRONOUS/LONG-RUNNING OPERATIONS" will give you some answer > you are looking for. I'm not sure I'm following it perfectly, but at > least partially. Oh, I didn't mean comments about the async operations, but I would add a note to clarify that you first empty the CDROM and then you attach the specified image. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 2/4] iommu: make iommu_inclusive_mapping a suboption of dom0-iommu
> -Original Message-
> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> Of Roger Pau Monne
> Sent: 07 August 2018 15:03
> To: xen-devel@lists.xenproject.org
> Cc: Kevin Tian ; Stefano Stabellini
> ; Wei Liu ; George Dunlap
> ; Andrew Cooper
> ; Ian Jackson ; Tim
> (Xen.org) ; Julien Grall ; Jan Beulich
> ; Roger Pau Monne
> Subject: [Xen-devel] [PATCH v3 2/4] iommu: make
> iommu_inclusive_mapping a suboption of dom0-iommu
>
> Introduce a new dom0-iommu=inclusive generic option that supersedes
> iommu_inclusive_mapping. The previous behaviour is preserved and the
> option should only be enabled by default on Intel hardware.
>
> No functional change intended.
>
> Signed-off-by: Roger Pau Monné
Reviewed-by: Paul Durrant
> ---
> Changes since v2:
> - Fix typo in commit message.
> - Change style and text of the documentation in xen command line.
> - Set the defaults in {intel/amd}_iommu_hwdom_init for inclusive.
> - Re-add the iommu_dom0_passthrough || !is_pv_domain(d) check.
>
> Changes since v1:
> - Use dom0-iommu instead of the iommu option.
> - Only enable by default on Intel hardware.
> ---
> Cc: Andrew Cooper
> Cc: George Dunlap
> Cc: Ian Jackson
> Cc: Jan Beulich
> Cc: Julien Grall
> Cc: Konrad Rzeszutek Wilk
> Cc: Stefano Stabellini
> Cc: Tim Deegan
> Cc: Wei Liu
> Cc: Kevin Tian
> ---
> docs/misc/xen-command-line.markdown | 17 +-
> xen/drivers/passthrough/amd/pci_amd_iommu.c | 4 ++
> xen/drivers/passthrough/arm/iommu.c | 4 ++
> xen/drivers/passthrough/iommu.c | 23 ++--
> xen/drivers/passthrough/vtd/extern.h| 2 -
> xen/drivers/passthrough/vtd/iommu.c | 8 ++-
> xen/drivers/passthrough/vtd/x86/vtd.c | 58 +---
> xen/drivers/passthrough/x86/iommu.c | 59 +
> xen/include/xen/iommu.h | 2 +
> 9 files changed, 109 insertions(+), 68 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-
> command-line.markdown
> index ea451f088e..90b32fe3f0 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1205,7 +1205,7 @@ detection of systems known to misbehave upon
> accesses to that port.
> >> Enable IOMMU debugging code (implies `verbose`).
>
> ### dom0-iommu
> -> `= List of [ none | strict | relaxed ]`
> +> `= List of [ none | strict | relaxed | inclusive ]`
>
> * `none`: disables DMA remapping for Dom0.
>
> @@ -1221,6 +1221,18 @@ PV Dom0:
> Note that all the above options are mutually exclusive. Specifying more than
> one on the `dom0-iommu` command line will result in undefined behavior.
>
> +The following options control whether non-RAM regions are added to the
> Dom0
> +iommu tables. Note that they can be prefixed with `no-` to effect the
> inverse
> +meaning:
> +
> +* `inclusive`: sets up DMA remapping for all the non-RAM memory below
> 4GB
> + except for unusable ranges. Use this to work around firmware issues
> providing
> + incorrect RMRR/IVMD entries. Rather than only mapping RAM pages for
> IOMMU
> + accesses for Dom0, with this option all pages up to 4GB, not marked as
> + unusable in the E820 table, will get a mapping established. Note that this
> + option is only applicable to a PV Dom0 and is enabled by default on Intel
> + hardware.
> +
> ### iommu\_dev\_iotlb\_timeout
> > `= `
>
> @@ -1233,6 +1245,9 @@ wait descriptor timed out', try increasing this value.
> ### iommu\_inclusive\_mapping (VT-d)
> > `= `
>
> +**WARNING: This command line option is deprecated, and superseded by
> +_dom0-iommu=inclusive_ - using both options in combination is
> undefined.**
> +
> > Default: `true`
>
> Use this to work around firmware issues providing incorrect RMRR entries.
> diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> index eeacf713e4..0e0c99c942 100644
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -253,6 +253,10 @@ static void __hwdom_init
> amd_iommu_hwdom_init(struct domain *d)
> unsigned long i;
> const struct amd_iommu *iommu;
>
> +/* Inclusive IOMMU mappings are disabled by default on AMD hardware.
> */
> +iommu_dom0_inclusive = iommu_dom0_inclusive == -1 ? false
> + : iommu_dom0_inclusive;
> +
> if ( allocate_domain_resources(dom_iommu(d)) )
> BUG();
>
> diff --git a/xen/drivers/passthrough/arm/iommu.c
> b/xen/drivers/passthrough/arm/iommu.c
> index 95b1abb972..325997b19f 100644
> --- a/xen/drivers/passthrough/arm/iommu.c
> +++ b/xen/drivers/passthrough/arm/iommu.c
> @@ -73,3 +73,7 @@ int arch_iommu_populate_page_table(struct domain
> *d)
> /* The IOMMU shares the p2m with the CPU */
> return -ENOSYS;
> }
> +
> +void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
> +{
> +}
> diff --git a/xen/drivers
Re: [Xen-devel] [PATCH v1 3/4] xen/arm: Add SCIFA UART support for early printk
On Tue, Aug 7, 2018 at 4:48 PM, Julien Grall wrote: > Hi, Hi, Julien > > On 06/08/18 19:35, Oleksandr Tyshchenko wrote: >> >> From: Oleksandr Tyshchenko >> >> Add support for Renesas "Stout" development board based on >> R-Car H2 SoC which has SCIFA compatible UART. >> >> Actually existing SCIF UART support (debug-scif.inc) and >> newly added SCIFA UART support (debug-scifa.inc) differ only >> in registers offsets. > > In that case, could we just extend debug-scif.inc? I was thinking about that, but couldn't find suitable solution without adding extra config option. As I understand, we need to recognize in run-time somehow which interface is present to use proper register offsets, so in UART driver it is easy to recognize using device-tree compatible string, but what to do here in such an early code. > >> >> Signed-off-by: Oleksandr Tyshchenko >> CC: Stefano Stabellini >> CC: Julien Grall >> --- >> docs/misc/arm/early-printk.txt | 3 ++- >> xen/arch/arm/Rules.mk | 1 + >> xen/arch/arm/arm32/debug-scifa.inc | 51 >> ++ >> 3 files changed, 54 insertions(+), 1 deletion(-) >> create mode 100644 xen/arch/arm/arm32/debug-scifa.inc >> >> diff --git a/docs/misc/arm/early-printk.txt >> b/docs/misc/arm/early-printk.txt >> index f765f59..f1b55d3 100644 >> --- a/docs/misc/arm/early-printk.txt >> +++ b/docs/misc/arm/early-printk.txt >> @@ -39,12 +39,13 @@ the name of the machine: >> - fastmodel: printk on ARM Fastmodel software emulators >> - hikey960: printk with pl011 with Hikey 960 >> - juno: printk with pl011 on Juno platform >> - - lager: printk with SCIF0 on Renesas R-Car H2 processors >> + - lager: printk with SCIF0 on Renesas Lager board (R-Car H2 processor) > > > Why this change? This sentence was not entirely correct. Since SCIF0 interface is applicable for Lager board, but is not applicable for Stout board which also based on R-Car H2 processor. Shall I create a separate patch for this small correction? > >> - midway: printk with the pl011 on Calxeda Midway processors >> - mvebu: printk with the MVEBU for Marvell Armada 3700 SoCs >> - omap5432: printk with UART3 on TI OMAP5432 processors >> - rcar3: printk with SCIF2 on Renesas R-Car Gen3 processors >> - seattle: printk with pl011 for AMD Seattle processor >> + - stout: printk with SCIFA0 on Renesas Stout board (R-Car H2 processor) > > > I have started to look at porting that to Kconfig ealyprintk and it is a > massive pain. So I would tend to prefer if we avoid adding more convenience > alias and instead document on the wiki page how to use earlyprintk for that. I will update a wiki page. Shall I drop this string in early-printk.txt? > > >> - sun6i: printk with 8250 on Allwinner A31 processors >> - sun7i: printk with 8250 on Allwinner A20 processors >> - thunderx: printk with pl011 for Cavium ThunderX processor >> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk >> index f264592..e011f8b 100644 >> --- a/xen/arch/arm/Rules.mk >> +++ b/xen/arch/arm/Rules.mk >> @@ -40,6 +40,7 @@ EARLY_PRINTK_mvebu := mvebu,0xd0012000 >> EARLY_PRINTK_omap5432 := 8250,0x4802,2 >> EARLY_PRINTK_rcar3 := scif,0xe6e88000 >> EARLY_PRINTK_seattle:= pl011,0xe101 >> +EARLY_PRINTK_stout := scifa,0xe6c4 >> EARLY_PRINTK_sun6i := 8250,0x01c28000,2 >> EARLY_PRINTK_sun7i := 8250,0x01c28000,2 >> EARLY_PRINTK_thunderx := pl011,0x87e02400 >> diff --git a/xen/arch/arm/arm32/debug-scifa.inc >> b/xen/arch/arm/arm32/debug-scifa.inc >> new file mode 100644 >> index 000..b5e60db >> --- /dev/null >> +++ b/xen/arch/arm/arm32/debug-scifa.inc >> @@ -0,0 +1,51 @@ >> +/* >> + * xen/arch/arm/arm32/debug-scifa.inc >> + * >> + * SCIFA specific debug code >> + * >> + * Oleksandr Tyshchenko >> + * Copyright (C) 2018 EPAM Systems Inc. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License, or >> + * (at your option) any later version. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#include >> + >> +/* >> + * SCIFA UART wait UART to be ready to transmit >> + * rb: register which contains the UART base address >> + * rc: scratch register >> + */ >> +.macro early_uart_ready rb rc >> +1: >> +ldrh \rc, [\rb, #SCIFA_SCASSR] /* <- SCASSR (status register) >> */ >> +tst\rc, #SCASSR_TDFE /* Check TDFE bit */ >> +beq1b /* Wait for the UART to be >> ready */ >> +.endm >> + >> +/* >> + * SCIFA UART transmit character >> + * rb:
Re: [Xen-devel] [PATCH v3 0/4] x86/iommu: PVH Dom0 workarounds for missing RMRR entries
On Tue, Aug 7, 2018 at 8:04 AM Roger Pau Monne wrote: > > Hello, > > The following series implement a workaround for missing RMRR > entries for a PVH Dom0. It's based on the iommu_inclusive_mapping VTd > option. > > The PVH workaround identity maps all regions marked as reserved in the > memory map. > > Note that this workaround is enabled by default on Intel hardware. It's > also available to AMD hardware, although it's disabled by default in > that case. > > The series can be found at: > > git://xenbits.xen.org/people/royger/xen.git iommu_inclusive_v3 > > Thanks, Roger. > Roger Pau Monne (4): > iommu: introduce dom0-iommu option > iommu: make iommu_inclusive_mapping a suboption of dom0-iommu > dom0/pvh: change the order of the MMCFG initialization > x86/iommu: add reserved dom0-iommu option to map reserved memory > ranges > > docs/misc/xen-command-line.markdown | 47 +++ > xen/arch/x86/hvm/dom0_build.c | 9 ++- > xen/arch/x86/hvm/io.c | 5 ++ > xen/arch/x86/x86_64/mm.c| 3 +- > xen/drivers/passthrough/amd/iommu_init.c| 2 +- > xen/drivers/passthrough/amd/pci_amd_iommu.c | 11 ++- > xen/drivers/passthrough/arm/iommu.c | 4 + > xen/drivers/passthrough/iommu.c | 62 +-- > xen/drivers/passthrough/vtd/extern.h| 2 - > xen/drivers/passthrough/vtd/iommu.c | 25 +++--- > xen/drivers/passthrough/vtd/x86/vtd.c | 58 +- > xen/drivers/passthrough/x86/iommu.c | 87 + > xen/include/asm-x86/hvm/io.h| 3 + > xen/include/xen/iommu.h | 8 +- > 14 files changed, 240 insertions(+), 86 deletions(-) > > -- Hi Roger, I gave this branch a spin on a Dell XPS laptop booting UEFI with Linux 4.18-rc8. I was able to get dom0 to boot with PVH but the physical keyboard of the laptop stopped working, it works no problem with just Linux 4.18-rc8 or PV dom0, so I had to plug in a USB keyboard. After running for a minute or two the system starts to slow down to the point where it becomes unresponsive. The xl dmesg log is filled with this error: (XEN) [VT-D]iommu.c:919: iommu_fault_status: Fault Overflow (XEN) [VT-D]iommu.c:921: iommu_fault_status: Primary Pending Fault (XEN) [VT-D]DMAR:[DMA Read] Request device [:00:02.0] fault addr 4625f3a000, iommu reg = 82c00181c000 (XEN) [VT-D]DMAR: reason 06 - PTE Read access is not set (XEN) print_vtd_entries: iommu #0 dev :00:02.0 gmfn 4625f3a (XEN) root_entry[00] = 273a18001 (XEN) context[10] = 2_27ba35001 (XEN) l4[000] = 9c027ba34107 (XEN) l3[118] = 8000 (XEN) l3[118] not present The device in question is: 00:02.0 VGA compatible controller: Intel Corporation HD Graphics 620 (rev 02) (prog-if 00 [VGA controller]) Subsystem: Dell HD Graphics 620 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Capabilities: [70] Express (v2) Root Complex Integrated Endpoint, MSI 00 DevCap:MaxPayload 128 bytes, PhantFunc 0 ExtTag- RBE+ DevCtl:Report errors: Correctable- Non-Fatal- Fatal- Unsupported- RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop- MaxPayload 128 bytes, MaxReadReq 128 bytes DevSta:CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr- TransPend- DevCap2: Completion Timeout: Not Supported, TimeoutDis-, LTR-, OBFF Not Supported DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled Capabilities: [ac] MSI: Enable+ Count=1/1 Maskable- 64bit- Address: fee0 Data: 4028 Capabilities: [d0] Power Management version 2 Flags: PMEClk- DSI+ D1- D2- AuxCurrent=0mA PME(D0-,D1-,D2-,D3hot-,D3cold-) Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME- Capabilities: [100 v1] Process Address Space ID (PASID) PASIDCap: Exec- Priv-, Max PASID Width: 14 PASIDCtl: Enable- Exec- Priv- Capabilities: [200 v1] Address Translation Service (ATS) ATSCap:Invalidate Queue Depth: 00 ATSCtl:Enable-, Smallest Translation Unit: 00 Capabilities: [300 v1] Page Request Interface (PRI) PRICtl: Enable- Reset- PRISta: RF- UPRGI- Stopped+ Page Request Capacity: 8000, Page Request Allocation: Kernel driver in use: i915 Kernel modules: i915 The error is similar to one I reported in 2015 (https://lists.xenproject.org/archives/html/xen-devel/2015-08/msg02339.html) but that was due to AMT being enabled on the system, while this laptop doesn't have AMT enabled. The boot params I had for Xen: loglvl=all guest_loglvl=all dom0_mem=4096M,max:4096M dom0_max_vcpus=2 sched=null dom0=pvh iommu=required,debug dom0-iommu=relaxed console=vga Let us know if we can provide any additional information. Thanks, Tamas __
Re: [Xen-devel] [PATCH v2] x86/hvm: remove default ioreq server
>>> On 07.08.18 at 16:02, wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4093,12 +4093,15 @@ static int hvm_allow_set_param(struct domain *d,
> case HVM_PARAM_CONSOLE_EVTCHN:
> case HVM_PARAM_X87_FIP_WIDTH:
> break;
> +/* The following parameters are deprecated. */
> +case HVM_PARAM_BUFIOREQ_EVTCHN:
> +rc = -EPERM;
> +break;
> /*
> * The following parameters must not be set by the guest
> * since the domain may need to be paused.
> */
> case HVM_PARAM_IDENT_PT:
> -case HVM_PARAM_DM_DOMAIN:
> case HVM_PARAM_ACPI_S_STATE:
> /* The remaining parameters should not be set by the guest. */
> default:
So why again don't you add DM_DOMAIN alongside BUFIOREQ_EVTCHN
above (and remove it from hvm_set_param()), and then also to
hvm_allow_get_param()'s new deprecated group, plus respective
treatment in public/hvm/params.h? I can't find any use of this
except in the get/set functions here, and your qemu-trad patch
removes the sole use ("set" request) there.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 4/4] x86/iommu: add reserved dom0-iommu option to map reserved memory ranges
> -Original Message-
> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> Of Roger Pau Monne
> Sent: 07 August 2018 15:03
> To: xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Wei Liu
> ; George Dunlap ;
> Andrew Cooper ; Ian Jackson
> ; Tim (Xen.org) ; Julien Grall
> ; Jan Beulich ; Roger Pau
> Monne
> Subject: [Xen-devel] [PATCH v3 4/4] x86/iommu: add reserved dom0-iommu
> option to map reserved memory ranges
>
> Several people have reported hardware issues (malfunctioning USB
> controllers) due to iommu page faults on Intel hardware. Those faults
> are caused by missing RMRR (VTd) entries in the ACPI tables. Those can
> be worked around on VTd hardware by manually adding RMRR entries on
> the command line, this is however limited to Intel hardware and quite
> cumbersome to do.
>
> In order to solve those issues add a new dom0-iommu=reserved option
> that identity maps all regions marked as reserved in the memory map.
> Note that regions used by devices emulated by Xen (LAPIC, IO-APIC or
> PCIe MCFG regions) are specifically avoided. Note that this option is
> available to a PVH Dom0 (as opposed to the inclusive option which only
> works for PV Dom0).
>
> Signed-off-by: Roger Pau Monné
Reviewed-by: Paul Durrant
> ---
> Changes since v2:
> - Fix comment regarding dom0-strict.
> - Change documentation style of xen command line.
> - Rename iommu_map to hwdom_iommu_map.
> - Move all the checks to hwdom_iommu_map.
>
> Changes since v1:
> - Introduce a new reserved option instead of abusing the inclusive
>option.
> - Use the same helper function for PV and PVH in order to decide if a
>page should be added to the domain page tables.
> - Use the data inside of the domain struct to detect overlaps with
>emulated MMIO regions.
> ---
> Cc: Andrew Cooper
> Cc: George Dunlap
> Cc: Ian Jackson
> Cc: Jan Beulich
> Cc: Julien Grall
> Cc: Konrad Rzeszutek Wilk
> Cc: Stefano Stabellini
> Cc: Tim Deegan
> Cc: Wei Liu
> ---
> docs/misc/xen-command-line.markdown | 11 ++-
> xen/arch/x86/hvm/io.c | 5 ++
> xen/drivers/passthrough/amd/pci_amd_iommu.c | 3 +
> xen/drivers/passthrough/iommu.c | 3 +
> xen/drivers/passthrough/vtd/iommu.c | 3 +
> xen/drivers/passthrough/x86/iommu.c | 86 ++---
> xen/include/asm-x86/hvm/io.h| 3 +
> xen/include/xen/iommu.h | 2 +-
> 8 files changed, 85 insertions(+), 31 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-
> command-line.markdown
> index 90b32fe3f0..59ec2afc5d 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1205,7 +1205,7 @@ detection of systems known to misbehave upon
> accesses to that port.
> >> Enable IOMMU debugging code (implies `verbose`).
>
> ### dom0-iommu
> -> `= List of [ none | strict | relaxed | inclusive ]`
> +> `= List of [ none | strict | relaxed | inclusive | reserved ]`
>
> * `none`: disables DMA remapping for Dom0.
>
> @@ -1233,6 +1233,15 @@ meaning:
>option is only applicable to a PV Dom0 and is enabled by default on Intel
>hardware.
>
> +* `reserved`: sets up DMA remapping for all the reserved regions in the
> memory
> + map for Dom0. Use this to work around firmware issues providing incorrect
> + RMRR/IVMD entries. Rather than only mapping RAM pages for IOMMU
> accesses
> + for Dom0, all memory regions marked as reserved in the memory map that
> don't
> + overlap with any MMIO region from emulated devices will be identity
> mapped.
> + This option maps a subset of the memory that would be mapped when
> using the
> + `inclusive` option. This option is available to a PVH Dom0 and is enabled
> by
> + default on Intel hardware.
> +
> ### iommu\_dev\_iotlb\_timeout
> > `= `
>
> diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c
> index bf4d8748d3..5e01c33890 100644
> --- a/xen/arch/x86/hvm/io.c
> +++ b/xen/arch/x86/hvm/io.c
> @@ -404,6 +404,11 @@ static const struct hvm_mmcfg
> *vpci_mmcfg_find(const struct domain *d,
> return NULL;
> }
>
> +bool vpci_mmcfg_address(const struct domain *d, paddr_t addr)
> +{
> +return vpci_mmcfg_find(d, addr);
> +}
> +
> static unsigned int vpci_mmcfg_decode_addr(const struct hvm_mmcfg
> *mmcfg,
> paddr_t addr, pci_sbdf_t *sbdf)
> {
> diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> index 0e0c99c942..2c2867d088 100644
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -256,6 +256,9 @@ static void __hwdom_init
> amd_iommu_hwdom_init(struct domain *d)
> /* Inclusive IOMMU mappings are disabled by default on AMD hardware.
> */
> iommu_dom0_inclusive = iommu_dom0_inclusive == -1 ? false
>: iom
Re: [Xen-devel] [PATCH v20 1/2] common: add a new mappable resource type: XENMEM_resource_grant_table
>>> On 07.08.18 at 16:14, wrote:
>> From: Jan Beulich [mailto:jbeul...@suse.com]
>> Sent: 07 August 2018 14:38
>>
>> >>> On 06.08.18 at 14:54, wrote:
>> > --- a/xen/common/grant_table.c
>> > +++ b/xen/common/grant_table.c
>> > @@ -3860,6 +3860,38 @@ int mem_sharing_gref_to_gfn(struct
>> grant_table *gt, grant_ref_t ref,
>> > }
>> > #endif
>> >
>> > +/* caller must hold read or write lock */
>> > +static int gnttab_get_status_frame_mfn(struct domain *d,
>> > + unsigned long idx, mfn_t *mfn)
>> > +{
>> > +struct grant_table *gt = d->grant_table;
>> > +
>> > +if ( idx >= nr_status_frames(gt) )
>> > +return -EINVAL;
>> > +
>> > +*mfn = _mfn(virt_to_mfn(gt->status[idx]));
>> > +return 0;
>> > +}
>> > +
>> > +/* caller must hold write lock */
>> > +static int gnttab_get_shared_frame_mfn(struct domain *d,
>> > + unsigned long idx, mfn_t *mfn)
>> > +{
>> > +struct grant_table *gt = d->grant_table;
>> > +
>> > +if ( gt->gt_version == 0 )
>> > +gt->gt_version = 1;
>> > +
>> > +if ( (idx >= nr_grant_frames(gt)) && (idx < gt->max_grant_frames) )
>> > +gnttab_grow_table(d, idx + 1);
>>
>> I guess I had commented on this before, but it has been a while:
>> While I realize that you're just moving code, I dislike the resulting
>> asymmetry between the two functions: Why would the former
>> not want to grow the grant table? And why would a version
>> adjustment be needed (only) here (I've put "only" in parentheses
>> because it's not obvious to me why this is needed)?
>>
>> And this asymmetry would surely be coming as surprise at least
>> to callers of the new interface you add.
>
> I'm happy to have get_status_frame() grow the table too but it does mean a
> change in behaviour to callers of gnttab_map_frame(). If that's not a problem
> then I'll make the change.
I'd say it was a mistake there before. The (slight) difficulty (which
probably made whoever wrote this originally ignore this aspect) is
figuring out how much to grow the table.
>> > +int gnttab_get_status_frame(struct domain *d, unsigned long idx,
>> > +mfn_t *mfn)
>> > +{
>> > +struct grant_table *gt = d->grant_table;
>> > +int rc;
>> > +
>> > +grant_read_lock(gt);
>> > +rc = gnttab_get_status_frame_mfn(d, idx, mfn);
>> > +grant_read_unlock(gt);
>> > +
>> > +return rc;
>> > +}
>>
>> Along the lines of what gnttab_map_frame() does, I'd expect a
>> check for gt_version to be 2 here. Or well, maybe that's implicit
>> by there not being any status entries/pages for version 1 tables.
>
> Yes, that should be the case.
>
>> But it would become a requirement if gnttab_grow_table() was to
>> be called from gnttab_get_status_frame_mfn().
>>
>
> Yes, so making that change will certainly add complexity.
Hmm, yes - a single if().
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 0/4] x86/iommu: PVH Dom0 workarounds for missing RMRR entries
On Tue, Aug 07, 2018 at 08:29:49AM -0600, Tamas K Lengyel wrote: > On Tue, Aug 7, 2018 at 8:04 AM Roger Pau Monne wrote: > > > > Hello, > > > > The following series implement a workaround for missing RMRR > > entries for a PVH Dom0. It's based on the iommu_inclusive_mapping VTd > > option. > > > > The PVH workaround identity maps all regions marked as reserved in the > > memory map. > > > > Note that this workaround is enabled by default on Intel hardware. It's > > also available to AMD hardware, although it's disabled by default in > > that case. > > > > The series can be found at: > > > > git://xenbits.xen.org/people/royger/xen.git iommu_inclusive_v3 > > > > Thanks, Roger. > > Roger Pau Monne (4): > > iommu: introduce dom0-iommu option > > iommu: make iommu_inclusive_mapping a suboption of dom0-iommu > > dom0/pvh: change the order of the MMCFG initialization > > x86/iommu: add reserved dom0-iommu option to map reserved memory > > ranges > > > > docs/misc/xen-command-line.markdown | 47 +++ > > xen/arch/x86/hvm/dom0_build.c | 9 ++- > > xen/arch/x86/hvm/io.c | 5 ++ > > xen/arch/x86/x86_64/mm.c| 3 +- > > xen/drivers/passthrough/amd/iommu_init.c| 2 +- > > xen/drivers/passthrough/amd/pci_amd_iommu.c | 11 ++- > > xen/drivers/passthrough/arm/iommu.c | 4 + > > xen/drivers/passthrough/iommu.c | 62 +-- > > xen/drivers/passthrough/vtd/extern.h| 2 - > > xen/drivers/passthrough/vtd/iommu.c | 25 +++--- > > xen/drivers/passthrough/vtd/x86/vtd.c | 58 +- > > xen/drivers/passthrough/x86/iommu.c | 87 + > > xen/include/asm-x86/hvm/io.h| 3 + > > xen/include/xen/iommu.h | 8 +- > > 14 files changed, 240 insertions(+), 86 deletions(-) > > > > -- > > Hi Roger, > I gave this branch a spin on a Dell XPS laptop booting UEFI with Linux > 4.18-rc8. I was able to get dom0 to boot with PVH but the physical > keyboard of the laptop stopped working, it works no problem with just > Linux 4.18-rc8 or PV dom0, so I had to plug in a USB keyboard. After > running for a minute or two the system starts to slow down to the > point where it becomes unresponsive. The xl dmesg log is filled with > this error: > > (XEN) [VT-D]iommu.c:919: iommu_fault_status: Fault Overflow > (XEN) [VT-D]iommu.c:921: iommu_fault_status: Primary Pending Fault > (XEN) [VT-D]DMAR:[DMA Read] Request device [:00:02.0] fault addr > 4625f3a000, iommu reg = 82c00181c000 > (XEN) [VT-D]DMAR: reason 06 - PTE Read access is not set > (XEN) print_vtd_entries: iommu #0 dev :00:02.0 gmfn 4625f3a Is the gmfn always the same (0x4625f3a)? > (XEN) root_entry[00] = 273a18001 > (XEN) context[10] = 2_27ba35001 > (XEN) l4[000] = 9c027ba34107 > (XEN) l3[118] = 8000 > (XEN) l3[118] not present Can you also paste the full xl dmesg log? I'm specially interested in the memory map of the machine which is printed quite early during Xen boot. Thanks, Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
