RE: [PATCH 06/10] xen: replace order with nr_pfns in assign_pages for better compatibility
Hi Julien > -Original Message- > From: Julien Grall > Sent: Tuesday, May 18, 2021 6:21 PM > To: Penny Zheng ; xen-devel@lists.xenproject.org; > sstabell...@kernel.org > Cc: Bertrand Marquis ; Wei Chen > ; nd > Subject: Re: [PATCH 06/10] xen: replace order with nr_pfns in assign_pages > for better compatibility > > Hi Penny, > > On 18/05/2021 06:21, Penny Zheng wrote: > > Function parameter order in assign_pages is always used as 1ul << > > order, referring to 2@order pages. > > > > Now, for better compatibility with new static memory, order shall be > > replaced with nr_pfns pointing to page count with no constraint, like > > 250MB. > > We have similar requirements for LiveUpdate because are preserving the > memory with a number of pages (rather than a power-of-two). With the > current interface would be need to split the range in a power of 2 which is a > bit of pain. > > However, I think I would prefer if we introduce a new interface (maybe > assign_pages_nr()) rather than change the meaning of the field. This is for > two reasons: >1) We limit the risk to make mistake when backporting a patch touch > assign_pages(). >2) Adding (1UL << order) for pretty much all the caller is not nice. > Ok. I will create a new interface assign_pages_nr(), and let assign_pages to call it with 2@order. > Cheers, > > -- > Julien Grall Cheers Penny Zheng
RE: [PATCH 05/10] xen/arm: introduce alloc_staticmem_pages
Hi Julien
> -Original Message-
> From: Julien Grall
> Sent: Tuesday, May 18, 2021 6:15 PM
> To: Penny Zheng ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org
> Cc: Bertrand Marquis ; Wei Chen
> ; nd
> Subject: Re: [PATCH 05/10] xen/arm: introduce alloc_staticmem_pages
>
> Hi Penny,
>
> On 18/05/2021 06:21, Penny Zheng wrote:
> > alloc_staticmem_pages is designated to allocate nr_pfns contiguous
> > pages of static memory. And it is the equivalent of alloc_heap_pages
> > for static memory.
> > This commit only covers allocating at specified starting address.
> >
> > For each page, it shall check if the page is reserved
> > (PGC_reserved) and free. It shall also do a set of necessary
> > initialization, which are mostly the same ones in alloc_heap_pages,
> > like, following the same cache-coherency policy and turning page
> > status into PGC_state_used, etc.
> >
> > Signed-off-by: Penny Zheng
> > ---
> > xen/common/page_alloc.c | 64
> +
> > 1 file changed, 64 insertions(+)
> >
> > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index
> > 58b53c6ac2..adf2889e76 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -1068,6 +1068,70 @@ static struct page_info *alloc_heap_pages(
> > return pg;
> > }
> >
> > +/*
> > + * Allocate nr_pfns contiguous pages, starting at #start, of static memory.
> > + * It is the equivalent of alloc_heap_pages for static memory */
> > +static struct page_info *alloc_staticmem_pages(unsigned long nr_pfns,
>
> This wants to be nr_mfns.
>
> > +paddr_t start,
>
> I would prefer if this helper takes an mfn_t in parameter.
>
Sure, I will change both.
> > +unsigned int
> > +memflags) {
> > +bool need_tlbflush = false;
> > +uint32_t tlbflush_timestamp = 0;
> > +unsigned int i;
> > +struct page_info *pg;
> > +mfn_t s_mfn;
> > +
> > +/* For now, it only supports allocating at specified address. */
> > +s_mfn = maddr_to_mfn(start);
> > +pg = mfn_to_page(s_mfn);
>
> We should avoid to make the assumption the start address will be valid.
> So you want to call mfn_valid() first.
>
> At the same time, there is no guarantee that if the first page is valid, then
> the
> next nr_pfns will be. So the check should be performed for all of them.
>
Ok. I'll do validation check on both of them.
> > +if ( !pg )
> > +return NULL;
> > +
> > +for ( i = 0; i < nr_pfns; i++)
> > +{
> > +/*
> > + * Reference count must continuously be zero for free pages
> > + * of static memory(PGC_reserved).
> > + */
> > +ASSERT(pg[i].count_info & PGC_reserved);
> > +if ( (pg[i].count_info & ~PGC_reserved) != PGC_state_free )
> > +{
> > +printk(XENLOG_ERR
> > +"Reference count must continuously be zero for free
> > pages"
> > +"pg[%u] MFN %"PRI_mfn" c=%#lx t=%#x\n",
> > +i, mfn_x(page_to_mfn(pg + i)),
> > +pg[i].count_info, pg[i].tlbflush_timestamp);
> > +BUG();
>
> So we would crash Xen if the caller pass a wrong range. Is it what we want?
>
> Also, who is going to prevent concurrent access?
>
Sure, to fix concurrency issue, I may need to add one spinlock like
`static DEFINE_SPINLOCK(staticmem_lock);`
In current alloc_heap_pages, it will do similar check, that pages in free state
MUST have
zero reference count. I guess, if condition not met, there is no need to
proceed.
> > +}
> > +
> > +if ( !(memflags & MEMF_no_tlbflush) )
> > +accumulate_tlbflush(_tlbflush, [i],
> > +_timestamp);
> > +
> > +/*
> > + * Reserve flag PGC_reserved and change page state
> > + * to PGC_state_inuse.
> > + */
> > +pg[i].count_info = (pg[i].count_info & PGC_reserved) |
> PGC_state_inuse;
> > +/* Initialise fields which have other uses for free pages. */
> > +pg[i].u.inuse.type_info = 0;
> > +page_set_owner([i], NULL);
> > +
> > +/*
> > + * Ensure cache and RAM are consistent for platforms where the
> > + * guest can control its own visibility of/through the cache.
> > + */
> > +flush_page_to_ram(mfn_x(page_to_mfn([i])),
> > +!(memflags & MEMF_no_icache_flush));
> > +}
> > +
> > +if ( need_tlbflush )
> > +filtered_flush_tlb_mask(tlbflush_timestamp);
> > +
> > +return pg;
> > +}
> > +
> > /* Remove any offlined page in the buddy pointed to by head. */
> > static int reserve_offlined_page(struct page_info *head)
> > {
> >
>
> Cheers,
>
> --
> Julien Grall
Cheers,
Penny Zheng
[xen-unstable-smoke test] 162075: regressions - FAIL
flight 162075 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/162075/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm 6 xen-buildfail REGR. vs. 162023 build-armhf 6 xen-buildfail REGR. vs. 162023 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass version targeted for testing: xen 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 baseline version: xen caa9c4471d1d74b2d236467aaf7e63a806ac11a4 Last test of basis 162023 2021-05-18 13:00:27 Z0 days Testing same since 162036 2021-05-18 16:00:26 Z0 days7 attempts People who touched revisions under test: Julien Grall Wei Liu jobs: build-arm64-xsm fail build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 Author: Julien Grall Date: Tue May 18 14:51:48 2021 +0100 tools/xenmon: xenbaked: Mark const the field text in stat_map_t The field text in stat_map_t will point to string literals. So mark it as const to allow the compiler to catch any modified of the string. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 4b7702727a8d89fea0a239adcbeb18aa2c85ede0 Author: Julien Grall Date: Tue May 18 14:51:28 2021 +0100 tools/top: The string parameter in set_prompt() and set_delay() should be const Neither string parameter in set_prompt() and set_delay() are meant to be modified. In particular, new_prompt can point to a literal string. So mark the two parameters as const and propagate it. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 5605cfd49a18df41a21fb50cd81528312a39d7c9 Author: Julien Grall Date: Tue May 18 14:50:32 2021 +0100 tools/misc: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we we to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 89aae4ad8f495b647de33f2df5046b3ce68225f8 Author: Julien Grall Date: Tue May 18 14:35:07 2021 +0100 tools/libs: stat: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Author: Julien Grall Date: Tue May 18 14:34:22 2021 +0100 tools/libs: guest: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const *char rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu (qemu changes not included)
Re: [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled
On 17.5.2021 17.24, Connor Davis wrote: > > On 5/17/21 8:13 AM, Jan Beulich wrote: >> On 17.05.2021 15:48, Connor Davis wrote: >>> On 5/17/21 3:32 AM, Jan Beulich wrote: On 14.05.2021 02:56, Connor Davis wrote: > Check if the debug capability is enabled in early_xdbc_parse_parameter, > and if it is, return with an error. This avoids collisions with whatever > enabled the DbC prior to linux starting. Doesn't this go too far and prevent use even if firmware (perhaps mistakenly) left it enabled? >>> Yes, but how is one supposed to distinguish the broken firmware and >>> non-broken >>> >>> firmware cases? >> Well, a first step might be to only check if running virtualized. >> And then if your running virtualized, there might be a way to >> inquire the hypervisor? > > Right, but if it was enabled by something other than a hypervisor, > > or you're not running virtualized, how do you distinguish then? IMO > > the proper thing to do in any case is to simply not use the DbC in linux. > Sounds reasonable. We can address "broken firmware" during the xHC handoff from BIOS to OS. Only first OS that loads after BIOS should see the "HC BIOS owned semaphore" bit set in xHCI MMIO. If it's set then OS requests ownership, which clears BIOS owned bit. If DbC is running here we can stop it. -Mathias
[xen-unstable test] 162058: tolerable FAIL - PUSHED
flight 162058 xen-unstable real [real] http://logs.test-lab.xenproject.org/osstest/logs/162058/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 161990 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 161990 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 161990 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 161990 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 161990 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 161990 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 161990 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 161990 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 161990 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 161990 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 161990 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen caa9c4471d1d74b2d236467aaf7e63a806ac11a4 baseline version: xen 3ac8835a80b27fc4e7116dbde78d3eececc66fc9 Last test of basis 161990 2021-05-18 05:50:04 Z0 days Testing same since 162058 2021-05-18 18:36:47 Z0 days1 attempts People who touched revisions under test: Juergen Gross jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64-xtf
RE: [PATCH 04/10] xen/arm: static memory initialization
Hi Julien
> -Original Message-
> From: Julien Grall
> Sent: Tuesday, May 18, 2021 6:01 PM
> To: Penny Zheng ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org
> Cc: Bertrand Marquis ; Wei Chen
> ; nd
> Subject: Re: [PATCH 04/10] xen/arm: static memory initialization
>
> Hi Penny,
>
> On 18/05/2021 06:21, Penny Zheng wrote:
> > This patch introduces static memory initialization, during system RAM boot
> up.
> >
> > New func init_staticmem_pages is the equivalent of init_heap_pages,
> > responsible for static memory initialization.
> >
> > Helper func free_staticmem_pages is the equivalent of free_heap_pages,
> > to free nr_pfns pages of static memory.
> > For each page, it includes the following steps:
> > 1. change page state from in-use(also initialization state) to free
> > state and grant PGC_reserved.
> > 2. set its owner NULL and make sure this page is not a guest frame any
> > more 3. follow the same cache coherency policy in free_heap_pages 4.
> > scrub the page in need
> >
> > Signed-off-by: Penny Zheng
> > ---
> > xen/arch/arm/setup.c| 2 ++
> > xen/common/page_alloc.c | 70
> +
> > xen/include/xen/mm.h| 3 ++
> > 3 files changed, 75 insertions(+)
> >
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c index
> > 444dbbd676..f80162c478 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -818,6 +818,8 @@ static void __init setup_mm(void)
> >
> > setup_frametable_mappings(ram_start, ram_end);
> > max_page = PFN_DOWN(ram_end);
> > +
> > +init_staticmem_pages();
> > }
> > #endif
> >
> > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index
> > ace6333c18..58b53c6ac2 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -150,6 +150,9 @@
> > #define p2m_pod_offline_or_broken_hit(pg) 0
> > #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
> > #endif
> > +#ifdef CONFIG_ARM_64
> > +#include
> > +#endif
> >
> > /*
> >* Comma-separated list of hexadecimal page numbers containing bad
> bytes.
> > @@ -1512,6 +1515,49 @@ static void free_heap_pages(
> > spin_unlock(_lock);
> > }
> >
> > +/* Equivalent of free_heap_pages to free nr_pfns pages of static
> > +memory. */ static void free_staticmem_pages(struct page_info *pg,
> > +unsigned long nr_pfns,
>
> This function is dealing with MFNs, so the second parameter should be called
> nr_mfns.
>
Agreed, thx.
> > + bool need_scrub) {
> > +mfn_t mfn = page_to_mfn(pg);
> > +int i;
> > +
> > +for ( i = 0; i < nr_pfns; i++ )
> > +{
> > +switch ( pg[i].count_info & PGC_state )
> > +{
> > +case PGC_state_inuse:
> > +BUG_ON(pg[i].count_info & PGC_broken);
> > +/* Make it free and reserved. */
> > +pg[i].count_info = PGC_state_free | PGC_reserved;
> > +break;
> > +
> > +default:
> > +printk(XENLOG_ERR
> > + "Page state shall be only in PGC_state_inuse. "
> > + "pg[%u] MFN %"PRI_mfn" count_info=%#lx
> tlbflush_timestamp=%#x.\n",
> > + i, mfn_x(mfn) + i,
> > + pg[i].count_info,
> > + pg[i].tlbflush_timestamp);
> > +BUG();
> > +}
> > +
> > +/*
> > + * Follow the same cache coherence scheme in free_heap_pages.
> > + * If a page has no owner it will need no safety TLB flush.
> > + */
> > +pg[i].u.free.need_tlbflush = (page_get_owner([i]) != NULL);
> > +if ( pg[i].u.free.need_tlbflush )
> > +page_set_tlbflush_timestamp([i]);
> > +
> > +/* This page is not a guest frame any more. */
> > +page_set_owner([i], NULL);
> > +set_gpfn_from_mfn(mfn_x(mfn) + i, INVALID_M2P_ENTRY);
>
> The code looks quite similar to free_heap_pages(). Could we possibly create
> an helper which can be called from both?
>
Ok, I will extract the common code here and there.
> > +
> > +if ( need_scrub )
> > +scrub_one_page([i]);
>
> So the scrubbing will be synchronous. Is it what we want?
>
> You also seem to miss the flush the call to flush_page_to_ram().
>
Yeah, right now, it is synchronous.
I guess that it is not an optimal choice, only a workable one right now.
I'm trying to borrow some asynchronous scrubbing ideas from heap in the future.
Yes! If we are doing synchronous scrubbing, we need to flush_page_to_ram(). Thx.
> > +}
> > +}
> >
> > /*
> >* Following rules applied for page offline:
> > @@ -1828,6 +1874,30 @@ static void init_heap_pages(
> > }
> > }
> >
> > +/* Equivalent of init_heap_pages to do static memory initialization
> > +*/ void __init init_staticmem_pages(void) {
> > +int bank;
> > +
> > +/*
> > + * TODO: Considering NUMA-support scenario.
> > + */
> > +for ( bank = 0 ; bank <
[xen-unstable-smoke test] 162072: regressions - FAIL
flight 162072 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/162072/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm 6 xen-buildfail REGR. vs. 162023 build-armhf 6 xen-buildfail REGR. vs. 162023 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass version targeted for testing: xen 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 baseline version: xen caa9c4471d1d74b2d236467aaf7e63a806ac11a4 Last test of basis 162023 2021-05-18 13:00:27 Z0 days Testing same since 162036 2021-05-18 16:00:26 Z0 days6 attempts People who touched revisions under test: Julien Grall Wei Liu jobs: build-arm64-xsm fail build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 Author: Julien Grall Date: Tue May 18 14:51:48 2021 +0100 tools/xenmon: xenbaked: Mark const the field text in stat_map_t The field text in stat_map_t will point to string literals. So mark it as const to allow the compiler to catch any modified of the string. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 4b7702727a8d89fea0a239adcbeb18aa2c85ede0 Author: Julien Grall Date: Tue May 18 14:51:28 2021 +0100 tools/top: The string parameter in set_prompt() and set_delay() should be const Neither string parameter in set_prompt() and set_delay() are meant to be modified. In particular, new_prompt can point to a literal string. So mark the two parameters as const and propagate it. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 5605cfd49a18df41a21fb50cd81528312a39d7c9 Author: Julien Grall Date: Tue May 18 14:50:32 2021 +0100 tools/misc: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we we to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 89aae4ad8f495b647de33f2df5046b3ce68225f8 Author: Julien Grall Date: Tue May 18 14:35:07 2021 +0100 tools/libs: stat: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Author: Julien Grall Date: Tue May 18 14:34:22 2021 +0100 tools/libs: guest: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const *char rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu (qemu changes not included)
RE: [PATCH 03/10] xen/arm: introduce PGC_reserved
Hi Julien
> -Original Message-
> From: Julien Grall
> Sent: Tuesday, May 18, 2021 5:46 PM
> To: Penny Zheng ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org
> Cc: Bertrand Marquis ; Wei Chen
> ; nd
> Subject: Re: [PATCH 03/10] xen/arm: introduce PGC_reserved
>
>
>
> On 18/05/2021 06:21, Penny Zheng wrote:
> > In order to differentiate pages of static memory, from those allocated
> > from heap, this patch introduces a new page flag PGC_reserved to tell.
> >
> > New struct reserved in struct page_info is to describe reserved page
> > info, that is, which specific domain this page is reserved to. >
> > Helper page_get_reserved_owner and page_set_reserved_owner are
> > designated to get/set reserved page's owner.
> >
> > Struct domain is enlarged to more than PAGE_SIZE, due to
> > newly-imported struct reserved in struct page_info.
>
> struct domain may embed a pointer to a struct page_info but never directly
> embed the structure. So can you clarify what you mean?
>
> >
> > Signed-off-by: Penny Zheng
> > ---
> > xen/include/asm-arm/mm.h | 16 +++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
> index
> > 0b7de3102e..d8922fd5db 100644
> > --- a/xen/include/asm-arm/mm.h
> > +++ b/xen/include/asm-arm/mm.h
> > @@ -88,7 +88,15 @@ struct page_info
> >*/
> > u32 tlbflush_timestamp;
> > };
> > -u64 pad;
> > +
> > +/* Page is reserved. */
> > +struct {
> > +/*
> > + * Reserved Owner of this page,
> > + * if this page is reserved to a specific domain.
> > + */
> > +struct domain *domain;
> > +} reserved;
>
> The space in page_info is quite tight, so I would like to avoid introducing
> new
> fields unless we can't get away from it.
>
> In this case, it is not clear why we need to differentiate the "Owner"
> vs the "Reserved Owner". It might be clearer if this change is folded in the
> first user of the field.
>
> As an aside, for 32-bit Arm, you need to add a 4-byte padding.
>
Yeah, I may delete this change. I imported this change as considering the
functionality
of rebooting domain on static allocation.
A little more discussion on rebooting domain on static allocation.
Considering the major user cases for domain on static allocation
are system has a total pre-defined, static behavior all the time. No domain
allocation
on runtime, while there still exists domain rebooting.
And when rebooting domain on static allocation, all these reserved pages could
not go back to heap when freeing them. So I am considering to use one global
`struct page_info*[DOMID]` value to store.
As Jan suggested, when domain get rebooted, struct domain will not exist
anymore.
But I think DOMID info could last.
> > };
> >
> > #define PG_shift(idx) (BITS_PER_LONG - (idx))
> > @@ -108,6 +116,9 @@ struct page_info
> > /* Page is Xen heap? */
> > #define _PGC_xen_heap PG_shift(2)
> > #define PGC_xen_heap PG_mask(1, 2)
> > + /* Page is reserved, referring static memory */
>
> I would drop the second part of the sentence because the flag could be used
> for other purpose. One example is reserved memory when Live Updating.
>
Sure, I will drop it.
> > +#define _PGC_reserved PG_shift(3)
> > +#define PGC_reserved PG_mask(1, 3)
> > /* ... */
> > /* Page is broken? */
> > #define _PGC_broken PG_shift(7)
> > @@ -161,6 +172,9 @@ extern unsigned long xenheap_base_pdx;
> > #define page_get_owner(_p)(_p)->v.inuse.domain
> > #define page_set_owner(_p,_d) ((_p)->v.inuse.domain = (_d))
> >
> > +#define page_get_reserved_owner(_p)(_p)->reserved.domain
> > +#define page_set_reserved_owner(_p,_d) ((_p)->reserved.domain = (_d))
> > +
> > #define maddr_get_owner(ma) (page_get_owner(maddr_to_page((ma
> >
> > #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
> >
>
> Cheers,
>
> --
> Julien Grall
Cheers,
Penny Zheng
RE: [PATCH 01/10] xen/arm: introduce domain on Static Allocation
Hi Julien
> -Original Message-
> From: Julien Grall
> Sent: Tuesday, May 18, 2021 4:58 PM
> To: Penny Zheng ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org
> Cc: Bertrand Marquis ; Wei Chen
> ; nd
> Subject: Re: [PATCH 01/10] xen/arm: introduce domain on Static Allocation
>
> Hi Penny,
>
> On 18/05/2021 06:21, Penny Zheng wrote:
> > Static Allocation refers to system or sub-system(domains) for which
> > memory areas are pre-defined by configuration using physical address
> ranges.
> > Those pre-defined memory, -- Static Momery, as parts of RAM reserved
> > in the
>
> s/Momery/Memory/
Oh, thx!
>
> > beginning, shall never go to heap allocator or boot allocator for any use.
> >
> > Domains on Static Allocation is supported through device tree property
> > `xen,static-mem` specifying reserved RAM banks as this domain's guest
> RAM.
> > By default, they shall be mapped to the fixed guest RAM address
> > `GUEST_RAM0_BASE`, `GUEST_RAM1_BASE`.
> >
> > This patch introduces this new `xen,static-mem` property to define
> > static memory nodes in device tree file.
> > This patch also documents and parses this new attribute at boot time
> > and stores related info in static_mem for later initialization.
> >
> > Signed-off-by: Penny Zheng
> > ---
> > docs/misc/arm/device-tree/booting.txt | 33 +
> > xen/arch/arm/bootfdt.c| 52 +++
> > xen/include/asm-arm/setup.h | 2 ++
> > 3 files changed, 87 insertions(+)
> >
> > diff --git a/docs/misc/arm/device-tree/booting.txt
> > b/docs/misc/arm/device-tree/booting.txt
> > index 5243bc7fd3..d209149d71 100644
> > --- a/docs/misc/arm/device-tree/booting.txt
> > +++ b/docs/misc/arm/device-tree/booting.txt
> > @@ -268,3 +268,36 @@ The DTB fragment is loaded at 0xc00 in the
> example above. It should
> > follow the convention explained in docs/misc/arm/passthrough.txt. The
> > DTB fragment will be added to the guest device tree, so that the guest
> > kernel will be able to discover the device.
> > +
> > +
> > +Static Allocation
> > +=
> > +
> > +Static Allocation refers to system or sub-system(domains) for which
> > +memory areas are pre-defined by configuration using physical address
> ranges.
> > +Those pre-defined memory, -- Static Momery, as parts of RAM reserved
> > +in the
>
> s/Momery/Memory/
>
Oh, thx
> > +beginning, shall never go to heap allocator or boot allocator for any use.
> > +
> > +Domains on Static Allocation is supported through device tree
> > +property `xen,static-mem` specifying reserved RAM banks as this domain's
> guest RAM.
>
> I would suggest to use "physical RAM" when you refer to the host memory.
>
> > +By default, they shall be mapped to the fixed guest RAM address
> > +`GUEST_RAM0_BASE`, `GUEST_RAM1_BASE`.
>
> There are a few bits that needs to clarified or part of the description:
>1) "By default" suggests there is an alternative possibility.
> However, I don't see any.
>2) Will the first region of xen,static-mem be mapped to GUEST_RAM0_BASE
> and the second to GUEST_RAM1_BASE? What if a third region is specificed?
>3) We don't guarantee the base address and the size of the banks.
> Wouldn't it be better to let the admin select the region he/she wants?
>4) How do you determine the number of cells for the address and the size?
>
The specific implementation on this part could be traced to the last commit
https://patchew.org/Xen/20210518052113.725808-1-penny.zh...@arm.com/20210518052113.725808-11-penny.zh...@arm.com/
It will exhaust the GUEST_RAM0_SIZE, then seek to the GUEST_RAM1_BASE.
GUEST_RAM0 may take up several regions.
Yes, I may add the 1:1 direct-map scenario here to explain the alternative
possibility
For the third point, are you suggesting that we could provide an option that
let user
also define guest memory base address/size?
I'm confused on the fourth point, you mean the address cell and size cell for
xen,static-mem = <...>?
It will be consistent with the ones defined in the parent node, domUx.
> > +Static Allocation is only supported on AArch64 for now.
>
> The code doesn't seem to be AArch64 specific. So why can't this be used for
> 32-bit Arm?
>
True, we have plans to make it also workable in AArch32 in the future.
Because we considered XEN on cortex-R52.
> > +
> > +The dtb property should look like as follows:
> > +
> > +chosen {
> > +domU1 {
> > +compatible = "xen,domain";
> > +#address-cells = <0x2>;
> > +#size-cells = <0x2>;
> > +cpus = <2>;
> > +xen,static-mem = <0x0 0x3000 0x0 0x2000>;
> > +
> > +...
> > +};
> > +};
> > +
> > +DOMU1 on Static Allocation has reserved RAM bank at 0x3000 of
> > +512MB size
>
> Do you mean "DomU1 will have a static memory of 512MB reserved from the
> physical address..."?
>
Yes, yes. You phrase it
[qemu-mainline test] 162019: regressions - FAIL
flight 162019 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/162019/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-freebsd11-amd64 16 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-freebsd10-i386 16 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-freebsd10-amd64 16 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-qemuu-freebsd12-amd64 16 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-debianhvm-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 15 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-ovmf-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-debianhvm-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-ovmf-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-qemuu-nested-intel 20 debian-hvm-install/l1/l2 fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-win7-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-ws16-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-ws16-amd64 15 guest-saverestore fail REGR. vs. 152631 Tests which did not succeed, but are not blocking: test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152631 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 152631 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 152631 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass
[xen-unstable-smoke test] 162067: regressions - FAIL
flight 162067 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/162067/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm 6 xen-buildfail REGR. vs. 162023 build-armhf 6 xen-buildfail REGR. vs. 162023 Tests which are failing intermittently (not blocking): test-amd64-amd64-xl-qemuu-debianhvm-amd64 20 guest-start/debianhvm.repeat fail pass in 162065 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass version targeted for testing: xen 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 baseline version: xen caa9c4471d1d74b2d236467aaf7e63a806ac11a4 Last test of basis 162023 2021-05-18 13:00:27 Z0 days Testing same since 162036 2021-05-18 16:00:26 Z0 days5 attempts People who touched revisions under test: Julien Grall Wei Liu jobs: build-arm64-xsm fail build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-amd64fail test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 Author: Julien Grall Date: Tue May 18 14:51:48 2021 +0100 tools/xenmon: xenbaked: Mark const the field text in stat_map_t The field text in stat_map_t will point to string literals. So mark it as const to allow the compiler to catch any modified of the string. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 4b7702727a8d89fea0a239adcbeb18aa2c85ede0 Author: Julien Grall Date: Tue May 18 14:51:28 2021 +0100 tools/top: The string parameter in set_prompt() and set_delay() should be const Neither string parameter in set_prompt() and set_delay() are meant to be modified. In particular, new_prompt can point to a literal string. So mark the two parameters as const and propagate it. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 5605cfd49a18df41a21fb50cd81528312a39d7c9 Author: Julien Grall Date: Tue May 18 14:50:32 2021 +0100 tools/misc: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we we to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 89aae4ad8f495b647de33f2df5046b3ce68225f8 Author: Julien Grall Date: Tue May 18 14:35:07 2021 +0100 tools/libs: stat: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Author: Julien Grall Date: Tue May 18 14:34:22 2021 +0100 tools/libs: guest: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const *char rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu (qemu changes not included)
[xen-unstable-smoke test] 162065: regressions - FAIL
flight 162065 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/162065/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm 6 xen-buildfail REGR. vs. 162023 build-armhf 6 xen-buildfail REGR. vs. 162023 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass version targeted for testing: xen 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 baseline version: xen caa9c4471d1d74b2d236467aaf7e63a806ac11a4 Last test of basis 162023 2021-05-18 13:00:27 Z0 days Testing same since 162036 2021-05-18 16:00:26 Z0 days4 attempts People who touched revisions under test: Julien Grall Wei Liu jobs: build-arm64-xsm fail build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 Author: Julien Grall Date: Tue May 18 14:51:48 2021 +0100 tools/xenmon: xenbaked: Mark const the field text in stat_map_t The field text in stat_map_t will point to string literals. So mark it as const to allow the compiler to catch any modified of the string. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 4b7702727a8d89fea0a239adcbeb18aa2c85ede0 Author: Julien Grall Date: Tue May 18 14:51:28 2021 +0100 tools/top: The string parameter in set_prompt() and set_delay() should be const Neither string parameter in set_prompt() and set_delay() are meant to be modified. In particular, new_prompt can point to a literal string. So mark the two parameters as const and propagate it. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 5605cfd49a18df41a21fb50cd81528312a39d7c9 Author: Julien Grall Date: Tue May 18 14:50:32 2021 +0100 tools/misc: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we we to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 89aae4ad8f495b647de33f2df5046b3ce68225f8 Author: Julien Grall Date: Tue May 18 14:35:07 2021 +0100 tools/libs: stat: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Author: Julien Grall Date: Tue May 18 14:34:22 2021 +0100 tools/libs: guest: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const *char rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu (qemu changes not included)
[xen-unstable-smoke test] 162062: regressions - FAIL
flight 162062 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/162062/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm 6 xen-buildfail REGR. vs. 162023 build-armhf 6 xen-buildfail REGR. vs. 162023 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass version targeted for testing: xen 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 baseline version: xen caa9c4471d1d74b2d236467aaf7e63a806ac11a4 Last test of basis 162023 2021-05-18 13:00:27 Z0 days Testing same since 162036 2021-05-18 16:00:26 Z0 days3 attempts People who touched revisions under test: Julien Grall Wei Liu jobs: build-arm64-xsm fail build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 Author: Julien Grall Date: Tue May 18 14:51:48 2021 +0100 tools/xenmon: xenbaked: Mark const the field text in stat_map_t The field text in stat_map_t will point to string literals. So mark it as const to allow the compiler to catch any modified of the string. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 4b7702727a8d89fea0a239adcbeb18aa2c85ede0 Author: Julien Grall Date: Tue May 18 14:51:28 2021 +0100 tools/top: The string parameter in set_prompt() and set_delay() should be const Neither string parameter in set_prompt() and set_delay() are meant to be modified. In particular, new_prompt can point to a literal string. So mark the two parameters as const and propagate it. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 5605cfd49a18df41a21fb50cd81528312a39d7c9 Author: Julien Grall Date: Tue May 18 14:50:32 2021 +0100 tools/misc: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we we to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 89aae4ad8f495b647de33f2df5046b3ce68225f8 Author: Julien Grall Date: Tue May 18 14:35:07 2021 +0100 tools/libs: stat: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Author: Julien Grall Date: Tue May 18 14:34:22 2021 +0100 tools/libs: guest: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const *char rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu (qemu changes not included)
[linux-linus test] 161996: regressions - FAIL
flight 161996 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/161996/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-xsm7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-examine 6 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt 7 xen-install fail REGR. vs. 152332 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-libvirt-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-coresched-i386-xl 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-freebsd10-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-pvshim 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-raw7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-i386 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-xl-qemut-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-amd64-libvirt-vhd 13 guest-start fail REGR. vs. 152332 test-arm64-arm64-xl-thunderx 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl-xsm 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-libvirt-xsm 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl-credit1 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl-credit2 14 guest-start fail REGR. vs. 152332 test-amd64-amd64-amd64-pvgrub 20 guest-stop fail REGR. vs. 152332 test-amd64-amd64-i386-pvgrub 20 guest-stop fail REGR. vs. 152332 test-amd64-amd64-xl-qcow213 guest-start fail REGR. vs. 152332 test-armhf-armhf-xl-vhd 13 guest-start fail REGR. vs. 152332 Tests which did not succeed, but are not blocking: test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 152332 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 152332 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152332 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152332 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 152332 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16
[xen-unstable-smoke bisection] complete build-arm64-xsm
branch xen-unstable-smoke xenbranch xen-unstable-smoke job build-arm64-xsm testid xen-build Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git *** Found and reproduced problem changeset *** Bug is in tree: xen git://xenbits.xen.org/xen.git Bug introduced: 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Bug not present: caa9c4471d1d74b2d236467aaf7e63a806ac11a4 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/162060/ commit 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Author: Julien Grall Date: Tue May 18 14:34:22 2021 +0100 tools/libs: guest: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const *char rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu For bisection revision-tuple graph see: http://logs.test-lab.xenproject.org/osstest/results/bisect/xen-unstable-smoke/build-arm64-xsm.xen-build.html Revision IDs in each graph node refer, respectively, to the Trees above. Running cs-bisection-step --graph-out=/home/logs/results/bisect/xen-unstable-smoke/build-arm64-xsm.xen-build --summary-out=tmp/162060.bisection-summary --basis-template=162023 --blessings=real,real-bisect,real-retry xen-unstable-smoke build-arm64-xsm xen-build Searching for failure / basis pass: 162055 fail [host=laxton0] / 162023 ok. Failure / basis pass flights: 162055 / 162023 Tree: qemuu git://xenbits.xen.org/qemu-xen.git Tree: xen git://xenbits.xen.org/xen.git Latest 7ea428895af2840d85c524f0bd11a38aac308308 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 Basis pass 7ea428895af2840d85c524f0bd11a38aac308308 caa9c4471d1d74b2d236467aaf7e63a806ac11a4 Generating revisions with ./adhoc-revtuple-generator git://xenbits.xen.org/qemu-xen.git#7ea428895af2840d85c524f0bd11a38aac308308-7ea428895af2840d85c524f0bd11a38aac308308 git://xenbits.xen.org/xen.git#caa9c4471d1d74b2d236467aaf7e63a806ac11a4-01d84420fb4a9be2ec474a7c1910bb22c28b53c8 Loaded 5001 nodes in revision graph Searching for test results: 162023 pass 7ea428895af2840d85c524f0bd11a38aac308308 caa9c4471d1d74b2d236467aaf7e63a806ac11a4 162036 fail 7ea428895af2840d85c524f0bd11a38aac308308 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 162047 pass 7ea428895af2840d85c524f0bd11a38aac308308 caa9c4471d1d74b2d236467aaf7e63a806ac11a4 162050 fail 7ea428895af2840d85c524f0bd11a38aac308308 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 162052 fail 7ea428895af2840d85c524f0bd11a38aac308308 89aae4ad8f495b647de33f2df5046b3ce68225f8 162054 fail 7ea428895af2840d85c524f0bd11a38aac308308 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d 162056 pass 7ea428895af2840d85c524f0bd11a38aac308308 caa9c4471d1d74b2d236467aaf7e63a806ac11a4 162057 fail 7ea428895af2840d85c524f0bd11a38aac308308 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d 162055 fail 7ea428895af2840d85c524f0bd11a38aac308308 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 162059 pass 7ea428895af2840d85c524f0bd11a38aac308308 caa9c4471d1d74b2d236467aaf7e63a806ac11a4 162060 fail 7ea428895af2840d85c524f0bd11a38aac308308 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Searching for interesting versions Result found: flight 162023 (pass), for basis pass For basis failure, parent search stopping at 7ea428895af2840d85c524f0bd11a38aac308308 caa9c4471d1d74b2d236467aaf7e63a806ac11a4, results HASH(0x5592b2a64f98) HASH(0x5592b2a705d8) HASH(0x5592b2a78da0) HASH(0x5592b2a80608) Result found: flight 162036 (fail), for basis failure (at ancestor ~449) Repro found: flight 162047 (pass), for basis pass Repro found: flight 162050 (fail), for basis failure 0 revisions at 7ea428895af2840d85c524f0bd11a38aac308308 caa9c4471d1d74b2d236467aaf7e63a806ac11a4 No revisions left to test, checking graph state. Result found: flight 162023 (pass), for last pass Result found: flight 162054 (fail), for first failure Repro found: flight 162056 (pass), for last pass Repro found: flight 162057 (fail), for first failure Repro found: flight 162059 (pass), for last pass Repro found: flight 162060 (fail), for first failure *** Found and reproduced problem changeset *** Bug is in tree: xen git://xenbits.xen.org/xen.git Bug introduced: 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Bug not present: caa9c4471d1d74b2d236467aaf7e63a806ac11a4 Last fail repro: http://logs.test-lab.xenproject.org/osstest/logs/162060/ commit 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Author: Julien Grall Date: Tue May 18 14:34:22 2021 +0100 tools/libs: guest: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const *char rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu Revision graph left in
[ovmf test] 162046: all pass - PUSHED
flight 162046 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/162046/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 42ec0a315b8a2f445b7a7d74b8d78965f1dff8f6 baseline version: ovmf 29e300ff815283259e81822ed3cb926bb9ad6460 Last test of basis 162002 2021-05-18 08:10:25 Z0 days Testing same since 162046 2021-05-18 17:10:06 Z0 days1 attempts People who touched revisions under test: Zhiguang Liu jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 29e300ff81..42ec0a315b 42ec0a315b8a2f445b7a7d74b8d78965f1dff8f6 -> xen-tested-master
[xen-unstable-smoke test] 162055: regressions - FAIL
flight 162055 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/162055/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm 6 xen-buildfail REGR. vs. 162023 build-armhf 6 xen-buildfail REGR. vs. 162023 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass version targeted for testing: xen 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 baseline version: xen caa9c4471d1d74b2d236467aaf7e63a806ac11a4 Last test of basis 162023 2021-05-18 13:00:27 Z0 days Testing same since 162036 2021-05-18 16:00:26 Z0 days2 attempts People who touched revisions under test: Julien Grall Wei Liu jobs: build-arm64-xsm fail build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 Author: Julien Grall Date: Tue May 18 14:51:48 2021 +0100 tools/xenmon: xenbaked: Mark const the field text in stat_map_t The field text in stat_map_t will point to string literals. So mark it as const to allow the compiler to catch any modified of the string. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 4b7702727a8d89fea0a239adcbeb18aa2c85ede0 Author: Julien Grall Date: Tue May 18 14:51:28 2021 +0100 tools/top: The string parameter in set_prompt() and set_delay() should be const Neither string parameter in set_prompt() and set_delay() are meant to be modified. In particular, new_prompt can point to a literal string. So mark the two parameters as const and propagate it. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 5605cfd49a18df41a21fb50cd81528312a39d7c9 Author: Julien Grall Date: Tue May 18 14:50:32 2021 +0100 tools/misc: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we we to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 89aae4ad8f495b647de33f2df5046b3ce68225f8 Author: Julien Grall Date: Tue May 18 14:35:07 2021 +0100 tools/libs: stat: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Author: Julien Grall Date: Tue May 18 14:34:22 2021 +0100 tools/libs: guest: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const *char rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu (qemu changes not included)
Preserving transactions accross Xenstored Live-Update
Hi Juergen, I have started to look at preserving transaction accross Live-update in C Xenstored. So far, I managed to transfer transaction that read/write existing nodes. Now, I am running into trouble to transfer new/deleted node within a transaction with the existing migration format. C Xenstored will keep track of nodes accessed during the transaction but not the children (AFAICT for performance reason). Therefore we have the name of the children but not the content (i.e. permission, data...). I have been exploring a couple of approaches: 1) Introducing a flag to indicate there is a child but no content. Pros: * Close to the existing stream. * Fairly implementation agnostic. Cons: * Memory overhead as we need to transfer the full path (rather than the child name) * Checking for duplication (if the node was actually accessed) will introduce runtime overhead. 2) Extend XS_STATE_TYPE_NODE (or introduce a new record) to allow transferring the children name for transaction Pros: * The implementation is more straight forward Cons: * The stream becomes implementation specific Neither approach looks very appealing to me. So I would like to request some feedback for other proposals or preference between the two options. Note that I haven't looked into much detail how transactions works on OCaml Xenstored. Cheers, -- Julien Grall
[xen-unstable test] 161990: tolerable FAIL - PUSHED
flight 161990 xen-unstable real [real] flight 162045 xen-unstable real-retest [real] http://logs.test-lab.xenproject.org/osstest/logs/161990/ http://logs.test-lab.xenproject.org/osstest/logs/162045/ Failures :-/ but no regressions. Tests which are failing intermittently (not blocking): test-arm64-arm64-xl-credit2 8 xen-bootfail pass in 162045-retest test-amd64-amd64-examine 4 memdisk-try-append fail pass in 162045-retest Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-credit2 15 migrate-support-check fail in 162045 never pass test-arm64-arm64-xl-credit2 16 saverestore-support-check fail in 162045 never pass test-amd64-amd64-xl-qemut-debianhvm-i386-xsm 12 debian-hvm-install fail like 161939 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 161973 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 161973 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 161973 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 161973 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 161973 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 161973 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 161973 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 161973 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 161973 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 161973 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 161973 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit1 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail never pass version targeted for testing: xen 3ac8835a80b27fc4e7116dbde78d3eececc66fc9 baseline version: xen cb199cc7de987cfda4659fccf51059f210f6ad34 Last test of basis 161973 2021-05-17 01:52:45 Z1 days Failing since161983 2021-05-17 17:06:44 Z1 days
Re: [PATCH] Xen Keyboard: don't advertise every key known to man
Phillip Susi writes: > Dmitry Torokhov writes: > >> By doing this you are stopping delivery of all key events from this >> device. Hrm... I don't have very many "interesting" keys to test, but when I hit the menu key, I see KEY_COMPOSE, which is > KEY_MIN_INTERESTING. When I press the button to have my vnc client send a windows key, I see KEY_LEFTCTRL+KEY_ESC. I also see KEY_PAUSE when I hit that key and it is also "interesting". I get the same thing with or without this patch, so it does not appear to be breaking delivery of the keys that are no longer being advertised. Oddly though, libinput list-devices does not even show the Xen Virtual Keyboard. It's sysfs path is /sys/class/input/input1, but it also does not have a device node in /dev/input so I can't even ask libinput to only monitor that device. Ok... this is really odd.. it does show the device without this patch, and not with it. The input events I was seeing were coming through the "AT Translated Set 2 keyboard" and no events come though the Xen Virtual Keyboard ( even without this patch ). This makes me wonder why we have this device at all?
Re: [PATCH v3 2/5] xen/x86: manually build xen.mb.efi binary
On Mon, May 17, 2021 at 03:24:28PM +0200, Jan Beulich wrote: > On 17.05.2021 15:20, Daniel Kiper wrote: > > On Mon, May 17, 2021 at 08:48:32AM +0200, Jan Beulich wrote: > >> On 07.05.2021 22:26, Bob Eshleman wrote: > >>> What is your intuition WRT the idea that instead of trying add a PE/COFF > >>> hdr > >>> in front of Xen's mb2 bin, we instead go the route of introducing valid > >>> mb2 > >>> entry points into xen.efi? > >> > >> At the first glance I think this is going to be less intrusive, and hence > >> to be preferred. But of course I haven't experimented in any way ... > > > > When I worked on this a few years ago I tried that way. Sadly I failed > > because I was not able to produce "linear" PE image using binutils > > exiting that days. > > What is a "linear" PE image? The problem with Multiboot family protocols is that all code and data sections have to be glued together in the image and as such loaded into the memory (IIRC BSS is an exception but it has to live behind the image). So, you cannot use PE image which has different representation in file and memory. IIRC by default at least code and data sections in xen.efi have different sizes in PE file and in memory. I tried to fix that using linker script and objcopy but it did not work. Sadly I do not remember the details but there is pretty good chance you can find relevant emails in Xen-devel archive with me explaining what kind of problems I met. > > Maybe > > newer binutils are more flexible and will be able to produce a PE image > > with properties required by Multiboot2 protocol. > > Isn't all you need the MB2 header within the first so many bytes of the > (disk) image? Or was it the image as loaded into memory? Both should be > possible to arrange for. IIRC Multiboot2 protocol requires the header in first 32 kiB of an image. So, this is not a problem. Daniel
[xen-unstable-smoke test] 162036: regressions - FAIL
flight 162036 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/162036/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-arm64-xsm 6 xen-buildfail REGR. vs. 162023 build-armhf 6 xen-buildfail REGR. vs. 162023 Tests which did not succeed, but are not blocking: test-arm64-arm64-xl-xsm 1 build-check(1) blocked n/a test-armhf-armhf-xl 1 build-check(1) blocked n/a test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass version targeted for testing: xen 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 baseline version: xen caa9c4471d1d74b2d236467aaf7e63a806ac11a4 Last test of basis 162023 2021-05-18 13:00:27 Z0 days Testing same since 162036 2021-05-18 16:00:26 Z0 days1 attempts People who touched revisions under test: Julien Grall Wei Liu jobs: build-arm64-xsm fail build-amd64 pass build-armhf fail build-amd64-libvirt pass test-armhf-armhf-xl blocked test-arm64-arm64-xl-xsm blocked test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Not pushing. commit 01d84420fb4a9be2ec474a7c1910bb22c28b53c8 Author: Julien Grall Date: Tue May 18 14:51:48 2021 +0100 tools/xenmon: xenbaked: Mark const the field text in stat_map_t The field text in stat_map_t will point to string literals. So mark it as const to allow the compiler to catch any modified of the string. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 4b7702727a8d89fea0a239adcbeb18aa2c85ede0 Author: Julien Grall Date: Tue May 18 14:51:28 2021 +0100 tools/top: The string parameter in set_prompt() and set_delay() should be const Neither string parameter in set_prompt() and set_delay() are meant to be modified. In particular, new_prompt can point to a literal string. So mark the two parameters as const and propagate it. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 5605cfd49a18df41a21fb50cd81528312a39d7c9 Author: Julien Grall Date: Tue May 18 14:50:32 2021 +0100 tools/misc: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we we to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 89aae4ad8f495b647de33f2df5046b3ce68225f8 Author: Julien Grall Date: Tue May 18 14:35:07 2021 +0100 tools/libs: stat: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const char * rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu commit 8fc4916daf2aac34088ebd5ec3d6fd707ac4221d Author: Julien Grall Date: Tue May 18 14:34:22 2021 +0100 tools/libs: guest: Use const whenever we point to literal strings literal strings are not meant to be modified. So we should use const *char rather than char * when we want to store a pointer to them. Signed-off-by: Julien Grall Reviewed-by: Anthony PERARD Acked-by: Wei Liu (qemu changes not included)
Re: [PATCH] tools/libs: guest: Fix Arm build after 8fc4916daf2a
On 18/05/2021 18:03, Julien Grall wrote:
> From: Julien Grall
>
> Gitlab CI spotted an issue when building the tools Arm:
>
> xg_dom_arm.c: In function 'meminit':
> xg_dom_arm.c:401:50: error: passing argument 3 of 'set_mode' discards 'const'
> qualifier from pointer target type [-Werror=discarded-qualifiers]
> 401 | rc = set_mode(dom->xch, dom->guest_domid, dom->guest_type);
> | ~~~^~~~
>
> This is because the const was not propagated in the Arm code. Fix it
> by constifying the 3rd parameter of set_mode().
>
> Fixes: 8fc4916daf2a ("tools/libs: guest: Use const whenever we point to
> literal strings")
> Signed-off-by: Julien Grall
Acked-by: Andrew Cooper
[PATCH] tools/libs: guest: Fix Arm build after 8fc4916daf2a
From: Julien Grall
Gitlab CI spotted an issue when building the tools Arm:
xg_dom_arm.c: In function 'meminit':
xg_dom_arm.c:401:50: error: passing argument 3 of 'set_mode' discards 'const'
qualifier from pointer target type [-Werror=discarded-qualifiers]
401 | rc = set_mode(dom->xch, dom->guest_domid, dom->guest_type);
| ~~~^~~~
This is because the const was not propagated in the Arm code. Fix it
by constifying the 3rd parameter of set_mode().
Fixes: 8fc4916daf2a ("tools/libs: guest: Use const whenever we point to literal
strings")
Signed-off-by: Julien Grall
---
tools/libs/guest/xg_dom_arm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/libs/guest/xg_dom_arm.c b/tools/libs/guest/xg_dom_arm.c
index b4c24f15fb27..01e85e0ea9c7 100644
--- a/tools/libs/guest/xg_dom_arm.c
+++ b/tools/libs/guest/xg_dom_arm.c
@@ -195,7 +195,7 @@ static int vcpu_arm64(struct xc_dom_image *dom)
/* */
-static int set_mode(xc_interface *xch, uint32_t domid, char *guest_type)
+static int set_mode(xc_interface *xch, uint32_t domid, const char *guest_type)
{
static const struct {
char *guest;
--
2.17.1
[ovmf test] 162002: all pass - PUSHED
flight 162002 ovmf real [real] http://logs.test-lab.xenproject.org/osstest/logs/162002/ Perfect :-) All tests in this flight passed as required version targeted for testing: ovmf 29e300ff815283259e81822ed3cb926bb9ad6460 baseline version: ovmf 1fbf5e30ae8eb725f4e10984f7b0a208f78abbd0 Last test of basis 161987 2021-05-18 01:10:06 Z0 days Testing same since 162002 2021-05-18 08:10:25 Z0 days1 attempts People who touched revisions under test: Ray Ni jobs: build-amd64-xsm pass build-i386-xsm pass build-amd64 pass build-i386 pass build-amd64-libvirt pass build-i386-libvirt pass build-amd64-pvopspass build-i386-pvops pass test-amd64-amd64-xl-qemuu-ovmf-amd64 pass test-amd64-i386-xl-qemuu-ovmf-amd64 pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/osstest/ovmf.git 1fbf5e30ae..29e300ff81 29e300ff815283259e81822ed3cb926bb9ad6460 -> xen-tested-master
[PATCH v2 2/2] automation: fix dependencies on openSUSE Tumbleweed containers
Fix the build inside our openSUSE Tumbleweed container by using adding libzstd headers. While there, remove the explicit dependency for python and python3 as the respective -devel packages will pull them in anyway. Signed-off-by: Dario Faggioli --- Cc: Doug Goldstein Cc: Roger Pau Monne Cc: Andrew Cooper --- Changes from v1: - fix my email address in From: - don't request python38-devel explicitly, python3-devel is just fine and is more generic (and hence better!) --- .../build/suse/opensuse-tumbleweed.dockerfile |3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/automation/build/suse/opensuse-tumbleweed.dockerfile b/automation/build/suse/opensuse-tumbleweed.dockerfile index 8ff7b9b5ce..a33ab0d870 100644 --- a/automation/build/suse/opensuse-tumbleweed.dockerfile +++ b/automation/build/suse/opensuse-tumbleweed.dockerfile @@ -45,6 +45,7 @@ RUN zypper install -y --no-recommends \ libtasn1-devel \ libuuid-devel \ libyajl-devel \ +libzstd-devel \ lzo-devel \ make \ nasm \ @@ -56,9 +57,7 @@ RUN zypper install -y --no-recommends \ pandoc \ patch \ pkg-config \ -python \ python-devel \ -python3 \ python3-devel \ systemd-devel \ tar \
[PATCH v2 1/2] automation: use DOCKER_CMD for building containers too
Use DOCKER_CMD from the environment (if defined) in the containers' makefile too, so that, e.g., when doing `export DOCKED_CMD=podman` podman is used for building the containers too. Signed-off-by: Dario Faggioli Acked-by: Roger Pau Monné --- Cc: Doug Goldstein Cc: Andrew Cooper --- Changes from v1: - fix my email address in From: --- automation/build/Makefile |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/automation/build/Makefile b/automation/build/Makefile index 7c7612b1d9..a4b2b85178 100644 --- a/automation/build/Makefile +++ b/automation/build/Makefile @@ -2,6 +2,7 @@ # the base of where these containers will appear REGISTRY := registry.gitlab.com/xen-project/xen CONTAINERS = $(subst .dockerfile,,$(wildcard */*.dockerfile)) +DOCKER_CMD ?= docker help: @echo "Builds containers for building Xen based on different distros" @@ -10,9 +11,9 @@ help: @echo "To push container builds, set the env var PUSH" %: %.dockerfile ## Builds containers - docker build -t $(REGISTRY)/$(@D):$(@F) -f $< $(
[PATCH v2 0/2] automation: fix building in the openSUSE Tumbleweed
Fix the build in the openSUSE Tumbleweed container within our CI. There was a missing dependency (libzstd-devel), which needs to be added to the dockerfile. OTOH, python3-devel was in the dockerfile already, and hence it should be there in the image. Yet, build was failing due to that... Maybe we forgot to build and then push a new image after adding it? Well, whatever. If this change is accepted, I'm happy to push a new, updated, image to our registry (ISTR that I used to have the right to do that). While there, extend the generalization of the container runtime to use (we have that in containerize already, through the DOCKER_CMD variable) to the local building of the containers as well. Thanks and Regards --- Dario Faggioli (2): automation: use DOCKER_CMD for building containers too automation: fix dependencies on openSUSE Tumbleweed containers automation/build/suse/opensuse-tumbleweed.dockerfile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ --- <> (Raistlin Majere)
Re: [PATCH] Xen Keyboard: don't advertise every key known to man
Dmitry Torokhov writes: > By doing this you are stopping delivery of all key events from this > device. It does? How does the PS/2 keyboard driver work then? It has no way of knowning what keys the keyboard has other than waiting to see what scan codes are emitted. If the keys must be advertised in order to emit them at runtime, then I see no other possible fix than to remove the codes from the modalias string in the input subsystem. Or maybe allow certain drivers that don't know to set some sort of flag that allows them to emit all codes at runtime, but NOT advertise them so you end up with a mile long modalias.
[PATCH v2 2/2] xen-pciback: reconfigure also from backend watch handler
When multiple PCI devices get assigned to a guest right at boot, libxl
incrementally populates the backend tree. The writes for the first of
the devices trigger the backend watch. In turn xen_pcibk_setup_backend()
will set the XenBus state to Initialised, at which point no further
reconfigures would happen unless a device got hotplugged. Arrange for
reconfigure to also get triggered from the backend watch handler.
Signed-off-by: Jan Beulich
Cc: sta...@vger.kernel.org
---
v2: Also move comment. Add a comment.
---
I will admit that this isn't entirely race-free (with the guest actually
attaching in parallel), but from the looks of it such a race ought to be
benign (not the least thanks to the mutex). Ideally the tool stack
wouldn't write num_devs until all devices had their information
populated. I tried doing so in libxl, but xen_pcibk_setup_backend()
calling xenbus_dev_fatal() when not being able to read that node
prohibits such an approach (or else libxl and driver changes would need
to go into use in lock-step).
I wonder why what is being watched isn't just the num_devs node. Right
now the watch triggers quite frequently without anything relevant
actually having changed (I suppose in at least some cases in response
to writes by the backend itself).
--- a/drivers/xen/xen-pciback/xenbus.c
+++ b/drivers/xen/xen-pciback/xenbus.c
@@ -359,7 +359,8 @@ out:
return err;
}
-static int xen_pcibk_reconfigure(struct xen_pcibk_device *pdev)
+static int xen_pcibk_reconfigure(struct xen_pcibk_device *pdev,
+enum xenbus_state state)
{
int err = 0;
int num_devs;
@@ -373,9 +374,7 @@ static int xen_pcibk_reconfigure(struct
dev_dbg(>xdev->dev, "Reconfiguring device ...\n");
mutex_lock(>dev_lock);
- /* Make sure we only reconfigure once */
- if (xenbus_read_driver_state(pdev->xdev->nodename) !=
- XenbusStateReconfiguring)
+ if (xenbus_read_driver_state(pdev->xdev->nodename) != state)
goto out;
err = xenbus_scanf(XBT_NIL, pdev->xdev->nodename, "num_devs", "%d",
@@ -500,6 +499,10 @@ static int xen_pcibk_reconfigure(struct
}
}
+ if (state != XenbusStateReconfiguring)
+ /* Make sure we only reconfigure once. */
+ goto out;
+
err = xenbus_switch_state(pdev->xdev, XenbusStateReconfigured);
if (err) {
xenbus_dev_fatal(pdev->xdev, err,
@@ -525,7 +528,7 @@ static void xen_pcibk_frontend_changed(s
break;
case XenbusStateReconfiguring:
- xen_pcibk_reconfigure(pdev);
+ xen_pcibk_reconfigure(pdev, XenbusStateReconfiguring);
break;
case XenbusStateConnected:
@@ -664,6 +667,15 @@ static void xen_pcibk_be_watch(struct xe
xen_pcibk_setup_backend(pdev);
break;
+ case XenbusStateInitialised:
+ /*
+* We typically move to Initialised when the first device was
+* added. Hence subsequent devices getting added may need
+* reconfiguring.
+*/
+ xen_pcibk_reconfigure(pdev, XenbusStateInitialised);
+ break;
+
default:
break;
}
[PATCH v2 1/2] xen-pciback: redo VF placement in the virtual topology
The commit referenced below was incomplete: It merely affected what
would get written to the vdev- xenstore node. The guest would still
find the function at the original function number as long as
__xen_pcibk_get_pci_dev() wouldn't be in sync. The same goes for AER wrt
__xen_pcibk_get_pcifront_dev().
Undo overriding the function to zero and instead make sure that VFs at
function zero remain alone in their slot. This has the added benefit of
improving overall capacity, considering that there's only a total of 32
slots available right now (PCI segment and bus can both only ever be
zero at present).
Fixes: 8a5248fe10b1 ("xen PV passthru: assign SR-IOV virtual functions to
separate virtual slots")
Signed-off-by: Jan Beulich
Cc: sta...@vger.kernel.org
---
Like the original change this has the effect of changing where devices
would appear in the guest, when there are multiple of them. I don't see
an immediate problem with this, but if there is we may need to reduce
the effect of the change.
Taking into account, besides the described breakage, how xen-pcifront's
pcifront_scan_bus() works, I also wonder what problem it was in the
first place that needed fixing. It may therefore also be worth to
consider simply reverting the original change.
--- a/drivers/xen/xen-pciback/vpci.c
+++ b/drivers/xen/xen-pciback/vpci.c
@@ -70,7 +70,7 @@ static int __xen_pcibk_add_pci_dev(struc
struct pci_dev *dev, int devid,
publish_pci_dev_cb publish_cb)
{
- int err = 0, slot, func = -1;
+ int err = 0, slot, func = PCI_FUNC(dev->devfn);
struct pci_dev_entry *t, *dev_entry;
struct vpci_dev_data *vpci_dev = pdev->pci_dev_data;
@@ -95,22 +95,25 @@ static int __xen_pcibk_add_pci_dev(struc
/*
* Keep multi-function devices together on the virtual PCI bus, except
-* virtual functions.
+* that we want to keep virtual functions at func 0 on their own. They
+* aren't multi-function devices and hence their presence at func 0
+* may cause guests to not scan the other functions.
*/
- if (!dev->is_virtfn) {
+ if (!dev->is_virtfn || func) {
for (slot = 0; slot < PCI_SLOT_MAX; slot++) {
if (list_empty(_dev->dev_list[slot]))
continue;
t = list_entry(list_first(_dev->dev_list[slot]),
struct pci_dev_entry, list);
+ if (t->dev->is_virtfn && !PCI_FUNC(t->dev->devfn))
+ continue;
if (match_slot(dev, t->dev)) {
dev_info(>dev, "vpci: assign to virtual
slot %d func %d\n",
-slot, PCI_FUNC(dev->devfn));
+slot, func);
list_add_tail(_entry->list,
_dev->dev_list[slot]);
- func = PCI_FUNC(dev->devfn);
goto unlock;
}
}
@@ -123,7 +126,6 @@ static int __xen_pcibk_add_pci_dev(struc
slot);
list_add_tail(_entry->list,
_dev->dev_list[slot]);
- func = dev->is_virtfn ? 0 : PCI_FUNC(dev->devfn);
goto unlock;
}
}
Re: [PATCH] tools/xenstored: Remove unused parameter in check_domains()
> On 18 May 2021, at 16:21, Julien Grall wrote:
>
> From: Julien Grall
>
> The parameter of check_domains() is not used within the function. In fact,
> this was a left over of the original implementation as the version merged
> doesn't need to know whether we are restoring.
>
> So remove it.
>
> Signed-off-by: Julien Grall
> ---
> tools/xenstore/xenstored_control.c | 2 +-
> tools/xenstore/xenstored_domain.c | 4 ++--
> tools/xenstore/xenstored_domain.h | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/xenstore/xenstored_control.c
> b/tools/xenstore/xenstored_control.c
> index 8e470f2b2056..07458d7b48d0 100644
> --- a/tools/xenstore/xenstored_control.c
> +++ b/tools/xenstore/xenstored_control.c
> @@ -589,7 +589,7 @@ void lu_read_state(void)
>* have died while we were live-updating. So check all the domains are
>* still alive.
>*/
> - check_domains(true);
> + check_domains();
> }
>
> static const char *lu_activate_binary(const void *ctx)
> diff --git a/tools/xenstore/xenstored_domain.c
> b/tools/xenstore/xenstored_domain.c
> index 3d4d0649a243..0e4bae9a9dd6 100644
> --- a/tools/xenstore/xenstored_domain.c
> +++ b/tools/xenstore/xenstored_domain.c
> @@ -220,7 +220,7 @@ static bool get_domain_info(unsigned int domid,
> xc_dominfo_t *dominfo)
> dominfo->domid == domid;
> }
>
> -void check_domains(bool restore)
> +void check_domains(void)
> {
> xc_dominfo_t dominfo;
> struct domain *domain;
> @@ -277,7 +277,7 @@ void handle_event(void)
> barf_perror("Failed to read from event fd");
>
> if (port == virq_port)
> - check_domains(false);
> + check_domains();
>
> if (xenevtchn_unmask(xce_handle, port) == -1)
> barf_perror("Failed to write to event fd");
> diff --git a/tools/xenstore/xenstored_domain.h
> b/tools/xenstore/xenstored_domain.h
> index dc9759171317..cc5147d7e747 100644
> --- a/tools/xenstore/xenstored_domain.h
> +++ b/tools/xenstore/xenstored_domain.h
> @@ -21,7 +21,7 @@
>
> void handle_event(void);
>
> -void check_domains(bool restore);
> +void check_domains(void);
>
> /* domid, mfn, eventchn, path */
> int do_introduce(struct connection *conn, struct buffered_data *in);
> --
> 2.17.1
>
>
Reviewed-by: Luca Fancellu
Cheers,
Luca
[PATCH v2 0/2] xen-pciback: a fix and a workaround
The first change completes a several years old but still incomplete change. As mentioned there, reverting the original change may also be an option. The second change works around some odd libxl behavior, as described in [1]. As per a response to that mail addressing the issue in libxl may also be possible, but it's not clear to me who would get to doing so at which point in time. Hence the kernel side alternative is being proposed here. As to Konrad being on the Cc list: I find it puzzling that he's listed under "XEN PCI SUBSYSTEM", but pciback isn't considered part of this. 1: redo VF placement in the virtual topology 2: reconfigure also from backend watch handler Jan [1] https://lists.xen.org/archives/html/xen-devel/2021-03/msg00956.html
Re: [PATCH v2 2/2] tools/console: Use const whenever we point to literal strings
On Tue, May 18, 2021 at 03:01:34PM +0100, Julien Grall wrote: > From: Julien Grall > > Literal strings are not meant to be modified. So we should use const > char * rather than char * when we want to store a pointer to them. > > Take the opportunity to remove the cast (char *) in console_init(). It > is unnecessary and will remove the const. > > Signed-off-by: Julien Grall > Acked-by: Wei Liu > > --- > Changes in v2: > - Remove the cast (char *) in console_init() Reviewed-by: Anthony PERARD Thanks, -- Anthony PERARD
[xen-unstable-smoke test] 162023: tolerable all pass - PUSHED
flight 162023 xen-unstable-smoke real [real] http://logs.test-lab.xenproject.org/osstest/logs/162023/ Failures :-/ but no regressions. Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass version targeted for testing: xen caa9c4471d1d74b2d236467aaf7e63a806ac11a4 baseline version: xen 3ac8835a80b27fc4e7116dbde78d3eececc66fc9 Last test of basis 161985 2021-05-17 21:00:36 Z0 days Testing same since 162023 2021-05-18 13:00:27 Z0 days1 attempts People who touched revisions under test: Juergen Gross jobs: build-arm64-xsm pass build-amd64 pass build-armhf pass build-amd64-libvirt pass test-armhf-armhf-xl pass test-arm64-arm64-xl-xsm pass test-amd64-amd64-xl-qemuu-debianhvm-amd64pass test-amd64-amd64-libvirt pass sg-report-flight on osstest.test-lab.xenproject.org logs: /home/logs/logs images: /home/logs/images Logs, config files, etc. are available at http://logs.test-lab.xenproject.org/osstest/logs Explanation of these reports, and of osstest in general, is at http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master Test harness code can be found at http://xenbits.xen.org/gitweb?p=osstest.git;a=summary Pushing revision : To xenbits.xen.org:/home/xen/git/xen.git 3ac8835a80..caa9c4471d caa9c4471d1d74b2d236467aaf7e63a806ac11a4 -> smoke
Re: [PATCH] tools/xenstored: Remove unused parameter in check_domains()
On 18.05.21 17:21, Julien Grall wrote: From: Julien Grall The parameter of check_domains() is not used within the function. In fact, this was a left over of the original implementation as the version merged doesn't need to know whether we are restoring. So remove it. Signed-off-by: Julien Grall Reviewed-by: Juergen Gross Juergen OpenPGP_0xB0DE9DD628BF132F.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 2/2] automation: fix dependencies on openSUSE Tumbleweed containers
On Tue, 2021-05-18 at 16:33 +0200, Dario Faggioli wrote: > On Tue, 2021-05-18 at 15:20 +0200, Roger Pau Monné wrote: > > On Tue, May 18, 2021 at 02:04:13PM +0200, Dario Faggioli wrote: > > > From: Dario Faggioli > > > > Mmm... this email address does not really exist, and it's a mistake > that it ended up here. :-/ > > > > Fix the build inside our openSUSE Tumbleweed container by using > > > the proper python development packages (and adding zstd headers). > > > > > > Signed-off-by: Dario Faggioli > > > --- > > > Cc: Doug Goldstein > > > Cc: Roger Pau Monne > > > Cc: Andrew Cooper > > > --- > > > .../build/suse/opensuse-tumbleweed.dockerfile | 5 ++--- > > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > > > diff --git a/automation/build/suse/opensuse-tumbleweed.dockerfile > > > b/automation/build/suse/opensuse-tumbleweed.dockerfile > > > index 8ff7b9b5ce..5b99cb8a53 100644 > > > --- a/automation/build/suse/opensuse-tumbleweed.dockerfile > > > +++ b/automation/build/suse/opensuse-tumbleweed.dockerfile > > > > > > @@ -56,10 +57,8 @@ RUN zypper install -y --no-recommends \ > > > pandoc \ > > > patch \ > > > pkg-config \ > > > - python \ > > > python-devel \ > > > - python3 \ > > > - python3-devel \ > > > + python38-devel \ > > > > When I tested python3-devel was translated into python38-devel, > > > Oh, really? And when was it that you tested it, if I can ask? > > > which > > I think is better as we don't need to modify the docker file for > > every > > new Python version? > > > That would definitely be better. Right now, I don't see any > python3-devel package. If python3-devel can still be used (and it > somehow translates to the proper -devel package), then sure we should > use it. I'm not sure how that would happen, but maybe it's just me > being unaware of some packaging magic. > > Let me put "python3-devel" there and test locally again, so we know > if > it actually works. > Ok, indeed it works. And, on a second though, it's not obscure at all that it does. It's just that python38-devel _provides_ python3-devel, which makes a lot of sense, and it was silly of me to not think it does that and use just python3-devel in the first place: dario@4b10a592ca98:~> zypper if --provides python38-devel Information for package python38-devel: --- Repository : @System Name : python38-devel Version: 3.8.10-1.2 Arch : x86_64 Vendor : openSUSE Installed Size : 882.7 KiB Installed : Yes Status : up-to-date Source package : python38-core-3.8.10-1.2.src Summary: Include Files and Libraries Mandatory for Building Python Modules [...] Provides : [8] libpython3.so()(64bit) pkgconfig(python-3.8) = 3.8 pkgconfig(python-3.8-embed) = 3.8 pkgconfig(python3) = 3.8 pkgconfig(python3-embed) = 3.8 python3-devel = 3.8.10 python38-devel = 3.8.10-1.2 python38-devel(x86-64) = 3.8.10-1.2 What now puzzles me a little, though, is why the build was failing, as python3-devel was already there in the docker file. Maybe we "just" forgot to push the image? Well, there's a different issue (missing libzstd-devel), so I'll send a v2 of this series anyway. Regards -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ --- <> (Raistlin Majere) signature.asc Description: This is a digitally signed message part
QEMU backport necessary for building with "recent" toolchain (on openSUSE Tumbleweed)
Hello,
While trying to build Xen on openSUSE Tumbleweed, I run into this
error, when qemu-xen is being built:
ld: Error: unable to disambiguate: -no-pie (did you mean --no-pie ?)
make[1]: *** [Makefile:53: multiboot.img] Error 1
make: *** [Makefile:576: pc-bios/optionrom/all] Error 2
make: Leaving directory '/build/tools/qemu-xen-build'
make[3]: *** [Makefile:212: subdir-all-qemu-xen-dir] Error 2
make[3]: Leaving directory '/build/tools'
make[2]: *** [/build/tools/../tools/Rules.mk:156: subdirs-install] Error 2
make[2]: Leaving directory '/build/tools'
make[1]: *** [Makefile:66: install] Error 2
make[1]: Leaving directory '/build/tools'
make: *** [Makefile:140: install-tools] Error 2
Build tools versions are as follows:
dario@885e566747e1:~> gcc -v
gcc version 10.3.0 (SUSE Linux)
dario@885e566747e1:~> ld -v
GNU ld (GNU Binutils; openSUSE Tumbleweed) 2.36.1.20210326-3
I think we need the following commit in our QEMU: bbd2d5a812077
("build: -no-pie is no functional linker flag").
I have attempted a quick-&-dirty backport of it here:
https://xenbits.xen.org/gitweb/?p=people/dariof/qemu-xen.git;a=commit;h=85575b7b661cedb8e6f6e192d36199ca9fde5841
Feel free to use it as a base, or tell me if I can help more with it in
any other way with it.
Regards
--
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
---
<> (Raistlin Majere)
signature.asc
Description: This is a digitally signed message part
Re: [PATCH] tools/xenstored: Remove unused parameter in check_domains()
Hi,
Please ignore this patch. I forgot to CC the maintainers. I will resend it.
Sorry for the noise.
Cheers,
On 18/05/2021 16:21, Julien Grall wrote:
From: Julien Grall
The parameter of check_domains() is not used within the function. In fact,
this was a left over of the original implementation as the version merged
doesn't need to know whether we are restoring.
So remove it.
Signed-off-by: Julien Grall
---
tools/xenstore/xenstored_control.c | 2 +-
tools/xenstore/xenstored_domain.c | 4 ++--
tools/xenstore/xenstored_domain.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/xenstore/xenstored_control.c
b/tools/xenstore/xenstored_control.c
index 8e470f2b2056..07458d7b48d0 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -589,7 +589,7 @@ void lu_read_state(void)
* have died while we were live-updating. So check all the domains are
* still alive.
*/
- check_domains(true);
+ check_domains();
}
static const char *lu_activate_binary(const void *ctx)
diff --git a/tools/xenstore/xenstored_domain.c
b/tools/xenstore/xenstored_domain.c
index 3d4d0649a243..0e4bae9a9dd6 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -220,7 +220,7 @@ static bool get_domain_info(unsigned int domid,
xc_dominfo_t *dominfo)
dominfo->domid == domid;
}
-void check_domains(bool restore)
+void check_domains(void)
{
xc_dominfo_t dominfo;
struct domain *domain;
@@ -277,7 +277,7 @@ void handle_event(void)
barf_perror("Failed to read from event fd");
if (port == virq_port)
- check_domains(false);
+ check_domains();
if (xenevtchn_unmask(xce_handle, port) == -1)
barf_perror("Failed to write to event fd");
diff --git a/tools/xenstore/xenstored_domain.h
b/tools/xenstore/xenstored_domain.h
index dc9759171317..cc5147d7e747 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -21,7 +21,7 @@
void handle_event(void);
-void check_domains(bool restore);
+void check_domains(void);
/* domid, mfn, eventchn, path */
int do_introduce(struct connection *conn, struct buffered_data *in);
--
Julien Grall
[PATCH] tools/xenstored: Remove unused parameter in check_domains()
From: Julien Grall
The parameter of check_domains() is not used within the function. In fact,
this was a left over of the original implementation as the version merged
doesn't need to know whether we are restoring.
So remove it.
Signed-off-by: Julien Grall
---
tools/xenstore/xenstored_control.c | 2 +-
tools/xenstore/xenstored_domain.c | 4 ++--
tools/xenstore/xenstored_domain.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/xenstore/xenstored_control.c
b/tools/xenstore/xenstored_control.c
index 8e470f2b2056..07458d7b48d0 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -589,7 +589,7 @@ void lu_read_state(void)
* have died while we were live-updating. So check all the domains are
* still alive.
*/
- check_domains(true);
+ check_domains();
}
static const char *lu_activate_binary(const void *ctx)
diff --git a/tools/xenstore/xenstored_domain.c
b/tools/xenstore/xenstored_domain.c
index 3d4d0649a243..0e4bae9a9dd6 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -220,7 +220,7 @@ static bool get_domain_info(unsigned int domid,
xc_dominfo_t *dominfo)
dominfo->domid == domid;
}
-void check_domains(bool restore)
+void check_domains(void)
{
xc_dominfo_t dominfo;
struct domain *domain;
@@ -277,7 +277,7 @@ void handle_event(void)
barf_perror("Failed to read from event fd");
if (port == virq_port)
- check_domains(false);
+ check_domains();
if (xenevtchn_unmask(xce_handle, port) == -1)
barf_perror("Failed to write to event fd");
diff --git a/tools/xenstore/xenstored_domain.h
b/tools/xenstore/xenstored_domain.h
index dc9759171317..cc5147d7e747 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -21,7 +21,7 @@
void handle_event(void);
-void check_domains(bool restore);
+void check_domains(void);
/* domid, mfn, eventchn, path */
int do_introduce(struct connection *conn, struct buffered_data *in);
--
2.17.1
[PATCH] tools/xenstored: Remove unused parameter in check_domains()
From: Julien Grall
The parameter of check_domains() is not used within the function. In fact,
this was a left over of the original implementation as the version merged
doesn't need to know whether we are restoring.
So remove it.
Signed-off-by: Julien Grall
---
tools/xenstore/xenstored_control.c | 2 +-
tools/xenstore/xenstored_domain.c | 4 ++--
tools/xenstore/xenstored_domain.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/xenstore/xenstored_control.c
b/tools/xenstore/xenstored_control.c
index 8e470f2b2056..07458d7b48d0 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -589,7 +589,7 @@ void lu_read_state(void)
* have died while we were live-updating. So check all the domains are
* still alive.
*/
- check_domains(true);
+ check_domains();
}
static const char *lu_activate_binary(const void *ctx)
diff --git a/tools/xenstore/xenstored_domain.c
b/tools/xenstore/xenstored_domain.c
index 3d4d0649a243..0e4bae9a9dd6 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -220,7 +220,7 @@ static bool get_domain_info(unsigned int domid,
xc_dominfo_t *dominfo)
dominfo->domid == domid;
}
-void check_domains(bool restore)
+void check_domains(void)
{
xc_dominfo_t dominfo;
struct domain *domain;
@@ -277,7 +277,7 @@ void handle_event(void)
barf_perror("Failed to read from event fd");
if (port == virq_port)
- check_domains(false);
+ check_domains();
if (xenevtchn_unmask(xce_handle, port) == -1)
barf_perror("Failed to write to event fd");
diff --git a/tools/xenstore/xenstored_domain.h
b/tools/xenstore/xenstored_domain.h
index dc9759171317..cc5147d7e747 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -21,7 +21,7 @@
void handle_event(void);
-void check_domains(bool restore);
+void check_domains(void);
/* domid, mfn, eventchn, path */
int do_introduce(struct connection *conn, struct buffered_data *in);
--
2.17.1
Re: [PATCH v3 2/5] xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH
Hi Jan,
On 18/05/2021 16:13, Jan Beulich wrote:
On 18.05.2021 16:06, Julien Grall wrote:
On 18/05/2021 07:27, Jan Beulich wrote:
On 18.05.2021 06:11, Connor Davis wrote:
On 5/17/21 9:42 AM, Julien Grall wrote:
Hi Jan,
On 17/05/2021 12:16, Jan Beulich wrote:
On 14.05.2021 20:53, Connor Davis wrote:
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -294,7 +294,9 @@ int guest_remove_page(struct domain *d, unsigned
long gmfn)
p2m_type_t p2mt;
#endif
mfn_t mfn;
+#ifdef CONFIG_HAS_PASSTHROUGH
bool *dont_flush_p, dont_flush;
+#endif
int rc;
#ifdef CONFIG_X86
@@ -385,13 +387,17 @@ int guest_remove_page(struct domain *d,
unsigned long gmfn)
* Since we're likely to free the page below, we need to suspend
* xenmem_add_to_physmap()'s suppressing of IOMMU TLB flushes.
*/
+#ifdef CONFIG_HAS_PASSTHROUGH
dont_flush_p = _cpu(iommu_dont_flush_iotlb);
dont_flush = *dont_flush_p;
*dont_flush_p = false;
+#endif
rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
+#ifdef CONFIG_HAS_PASSTHROUGH
*dont_flush_p = dont_flush;
+#endif
/*
* With the lack of an IOMMU on some platforms, domains with
DMA-capable
@@ -839,11 +845,13 @@ int xenmem_add_to_physmap(struct domain *d,
struct xen_add_to_physmap *xatp,
xatp->gpfn += start;
xatp->size -= start;
+#ifdef CONFIG_HAS_PASSTHROUGH
if ( is_iommu_enabled(d) )
{
this_cpu(iommu_dont_flush_iotlb) = 1;
extra.ppage = [0];
}
+#endif
while ( xatp->size > done )
{
@@ -868,6 +876,7 @@ int xenmem_add_to_physmap(struct domain *d,
struct xen_add_to_physmap *xatp,
}
}
+#ifdef CONFIG_HAS_PASSTHROUGH
if ( is_iommu_enabled(d) )
{
int ret;
@@ -894,6 +903,7 @@ int xenmem_add_to_physmap(struct domain *d,
struct xen_add_to_physmap *xatp,
if ( unlikely(ret) && rc >= 0 )
rc = ret;
}
+#endif
return rc;
}
I wonder whether all of these wouldn't better become CONFIG_X86:
ISTR Julien indicating that he doesn't see the override getting used
on Arm. (Julien, please correct me if I'm misremembering.)
Right, so far, I haven't been in favor to introduce it because:
1) The P2M code may free some memory. So you can't always ignore
the flush (I think this is wrong for the upper layer to know when this
can happen).
2) It is unclear what happen if the IOMMU TLBs and the PT contains
different mappings (I received conflicted advice).
So it is better to always flush and as early as possible.
So keep it as is or switch to CONFIG_X86?
Please switch, unless anyone else voices a strong opinion towards
keeping as is.
I would like to avoid adding more #ifdef CONFIG_X86 in the common code.
Can we instead provide a wrapper for them?
Doable, sure, but I don't know whether Connor is up to going this
more extensive route.
That's a fair point. If that the case, then I prefer the #ifdef
CONFIG_HAS_PASSTHROUGH version.
I can add an item in my todo list to introduce some helpers.
Cheers,
--
Julien Grall
Re: [PATCH v3 2/5] xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH
On 18.05.2021 16:06, Julien Grall wrote:
>
>
> On 18/05/2021 07:27, Jan Beulich wrote:
>> On 18.05.2021 06:11, Connor Davis wrote:
>>>
>>> On 5/17/21 9:42 AM, Julien Grall wrote:
Hi Jan,
On 17/05/2021 12:16, Jan Beulich wrote:
> On 14.05.2021 20:53, Connor Davis wrote:
>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -294,7 +294,9 @@ int guest_remove_page(struct domain *d, unsigned
>> long gmfn)
>> p2m_type_t p2mt;
>> #endif
>> mfn_t mfn;
>> +#ifdef CONFIG_HAS_PASSTHROUGH
>> bool *dont_flush_p, dont_flush;
>> +#endif
>> int rc;
>> #ifdef CONFIG_X86
>> @@ -385,13 +387,17 @@ int guest_remove_page(struct domain *d,
>> unsigned long gmfn)
>> * Since we're likely to free the page below, we need to suspend
>> * xenmem_add_to_physmap()'s suppressing of IOMMU TLB flushes.
>> */
>> +#ifdef CONFIG_HAS_PASSTHROUGH
>> dont_flush_p = _cpu(iommu_dont_flush_iotlb);
>> dont_flush = *dont_flush_p;
>> *dont_flush_p = false;
>> +#endif
>> rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
>> +#ifdef CONFIG_HAS_PASSTHROUGH
>> *dont_flush_p = dont_flush;
>> +#endif
>> /*
>> * With the lack of an IOMMU on some platforms, domains with
>> DMA-capable
>> @@ -839,11 +845,13 @@ int xenmem_add_to_physmap(struct domain *d,
>> struct xen_add_to_physmap *xatp,
>> xatp->gpfn += start;
>> xatp->size -= start;
>> +#ifdef CONFIG_HAS_PASSTHROUGH
>> if ( is_iommu_enabled(d) )
>> {
>> this_cpu(iommu_dont_flush_iotlb) = 1;
>> extra.ppage = [0];
>> }
>> +#endif
>> while ( xatp->size > done )
>> {
>> @@ -868,6 +876,7 @@ int xenmem_add_to_physmap(struct domain *d,
>> struct xen_add_to_physmap *xatp,
>> }
>> }
>> +#ifdef CONFIG_HAS_PASSTHROUGH
>> if ( is_iommu_enabled(d) )
>> {
>> int ret;
>> @@ -894,6 +903,7 @@ int xenmem_add_to_physmap(struct domain *d,
>> struct xen_add_to_physmap *xatp,
>> if ( unlikely(ret) && rc >= 0 )
>> rc = ret;
>> }
>> +#endif
>> return rc;
>> }
>
> I wonder whether all of these wouldn't better become CONFIG_X86:
> ISTR Julien indicating that he doesn't see the override getting used
> on Arm. (Julien, please correct me if I'm misremembering.)
Right, so far, I haven't been in favor to introduce it because:
1) The P2M code may free some memory. So you can't always ignore
the flush (I think this is wrong for the upper layer to know when this
can happen).
2) It is unclear what happen if the IOMMU TLBs and the PT contains
different mappings (I received conflicted advice).
So it is better to always flush and as early as possible.
>>>
>>> So keep it as is or switch to CONFIG_X86?
>>
>> Please switch, unless anyone else voices a strong opinion towards
>> keeping as is.
>
> I would like to avoid adding more #ifdef CONFIG_X86 in the common code.
> Can we instead provide a wrapper for them?
Doable, sure, but I don't know whether Connor is up to going this
more extensive route.
Jan
Re: [PATCH 2/2] automation: fix dependencies on openSUSE Tumbleweed containers
On Tue, May 18, 2021 at 04:33:43PM +0200, Dario Faggioli wrote: > On Tue, 2021-05-18 at 15:20 +0200, Roger Pau Monné wrote: > > On Tue, May 18, 2021 at 02:04:13PM +0200, Dario Faggioli wrote: > > > From: Dario Faggioli > > > > Mmm... this email address does not really exist, and it's a mistake > that it ended up here. :-/ > > > > Fix the build inside our openSUSE Tumbleweed container by using > > > the proper python development packages (and adding zstd headers). > > > > > > Signed-off-by: Dario Faggioli > > > --- > > > Cc: Doug Goldstein > > > Cc: Roger Pau Monne > > > Cc: Andrew Cooper > > > --- > > > .../build/suse/opensuse-tumbleweed.dockerfile | 5 ++--- > > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > > > diff --git a/automation/build/suse/opensuse-tumbleweed.dockerfile > > > b/automation/build/suse/opensuse-tumbleweed.dockerfile > > > index 8ff7b9b5ce..5b99cb8a53 100644 > > > --- a/automation/build/suse/opensuse-tumbleweed.dockerfile > > > +++ b/automation/build/suse/opensuse-tumbleweed.dockerfile > > > > > > @@ -56,10 +57,8 @@ RUN zypper install -y --no-recommends \ > > > pandoc \ > > > patch \ > > > pkg-config \ > > > - python \ > > > python-devel \ > > > - python3 \ > > > - python3-devel \ > > > + python38-devel \ > > > > When I tested python3-devel was translated into python38-devel, > > > Oh, really? And when was it that you tested it, if I can ask? I've tried just now with the current docker file, and this is the (trimmed) output: Step 7/7 : RUN zypper install -y --no-recommends acpica bc bin86 bison bzip2 checkpolicy clang cmake dev86 discount flex gcc gcc-c++ gettext-tools git glib2-devel glibc-devel glibc-devel-32bit gzip hostname libSDL2-devel libaio-devel libbz2-devel libext2fs-devel libgnutls-devel libjpeg62-devel libnl3-devel libnuma-devel libpixman-1-0-devel libpng16-devel libssh2-devel libtasn1-devel libuuid-devel libyajl-devel lzo-devel make nasm ncurses-devel ocaml ocaml-findlib-devel ocaml-ocamlbuild ocaml-ocamldoc pandoc patch pkg-config python python-devel python3 python3-devel systemd-devel tar transfig valgrind-devel wget which xz-devel zlib-devel && zypper clean -a ---> Running in af6a184e482b Retrieving repository 'openSUSE-Tumbleweed-Non-Oss' metadata [..done] Building repository 'openSUSE-Tumbleweed-Non-Oss' cache [done] Retrieving repository 'openSUSE-Tumbleweed-Oss' metadata [...done] Building repository 'openSUSE-Tumbleweed-Oss' cache [done] Retrieving repository 'openSUSE-Tumbleweed-Update' metadata [.done] Building repository 'openSUSE-Tumbleweed-Update' cache [done] Loading repository data... Reading installed packages... 'python3' not found in package names. Trying capabilities. 'python3-devel' not found in package names. Trying capabilities. There's this message here ... 'pkg-config' not found in package names. Trying capabilities. Resolving package dependencies... The following 509 NEW packages are going to be installed: [...] python38 python38-base python38-devel [...] ... but it seems python3-devel gets translated into python38-devel, or maybe something else selects python38-devel? Not familiar with the system, so maybe it's indeed a dependency of some other package. > > which > > I think is better as we don't need to modify the docker file for > > every > > new Python version? > > > That would definitely be better. Right now, I don't see any > python3-devel package. If python3-devel can still be used (and it > somehow translates to the proper -devel package), then sure we should > use it. I'm not sure how that would happen, but maybe it's just me > being unaware of some packaging magic. > > Let me put "python3-devel" there and test locally again, so we know if > it actually works. It does seem to be picked up, whether that's because python3-devel gets translated into python38-devel, or something else pulls it in I don't certainly know. Thanks, Roger.
Re: [PATCH v2 1/2] xen/char: console: Use const whenever we point to literal strings
On 18.05.2021 16:01, Julien Grall wrote: > From: Julien Grall > > Literal strings are not meant to be modified. So we should use const > char * rather than char * when we want to store a pointer to them. > > The array should also not be modified at all and is only used by > xenlog_update_val(). So take the opportunity to add an extra const and > move the definition in the function. > > Signed-off-by: Julien Grall Reviewed-by: Jan Beulich
[PATCH v2] libelf: improve PVH elfnote parsing
Pass an hvm boolean parameter to the elf note parsing and checking
routines, so that better checking can be done in case libelf is
dealing with an hvm container.
elf_xen_note_check shouldn't return early unless PHYS32_ENTRY is set
and the container is of type HVM, or else the loader and version
checks would be avoided for kernels intended to be booted as PV but
that also have PHYS32_ENTRY set.
Adjust elf_xen_addr_calc_check so that the virtual addresses are
actually physical ones (by setting virt_base and elf_paddr_offset to
zero) when the container is of type HVM, as that container is always
started with paging disabled.
Signed-off-by: Roger Pau Monné
---
Changes since v1:
- Expand comments.
- Do not set virt_entry to phys_entry unless it's an HVM container.
---
tools/fuzz/libelf/libelf-fuzzer.c | 3 ++-
tools/libs/guest/xg_dom_elfloader.c | 6 --
tools/libs/guest/xg_dom_hvmloader.c | 2 +-
xen/arch/x86/hvm/dom0_build.c | 2 +-
xen/arch/x86/pv/dom0_build.c| 2 +-
xen/common/libelf/libelf-dominfo.c | 32 +++--
xen/include/xen/libelf.h| 2 +-
7 files changed, 31 insertions(+), 18 deletions(-)
diff --git a/tools/fuzz/libelf/libelf-fuzzer.c
b/tools/fuzz/libelf/libelf-fuzzer.c
index 1ba85717114..84fb84720fa 100644
--- a/tools/fuzz/libelf/libelf-fuzzer.c
+++ b/tools/fuzz/libelf/libelf-fuzzer.c
@@ -17,7 +17,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)
return -1;
elf_parse_binary(elf);
-elf_xen_parse(elf, );
+elf_xen_parse(elf, , false);
+elf_xen_parse(elf, , true);
return 0;
}
diff --git a/tools/libs/guest/xg_dom_elfloader.c
b/tools/libs/guest/xg_dom_elfloader.c
index 06e713fe111..ad71163dd92 100644
--- a/tools/libs/guest/xg_dom_elfloader.c
+++ b/tools/libs/guest/xg_dom_elfloader.c
@@ -135,7 +135,8 @@ static elf_negerrnoval xc_dom_probe_elf_kernel(struct
xc_dom_image *dom)
* or else we might be trying to load a plain ELF.
*/
elf_parse_binary();
-rc = elf_xen_parse(, dom->parms);
+rc = elf_xen_parse(, dom->parms,
+ dom->container_type == XC_DOM_HVM_CONTAINER);
if ( rc != 0 )
return rc;
@@ -166,7 +167,8 @@ static elf_negerrnoval xc_dom_parse_elf_kernel(struct
xc_dom_image *dom)
/* parse binary and get xen meta info */
elf_parse_binary(elf);
-if ( elf_xen_parse(elf, dom->parms) != 0 )
+if ( elf_xen_parse(elf, dom->parms,
+ dom->container_type == XC_DOM_HVM_CONTAINER) != 0 )
{
rc = -EINVAL;
goto out;
diff --git a/tools/libs/guest/xg_dom_hvmloader.c
b/tools/libs/guest/xg_dom_hvmloader.c
index ec6ebad7fd5..3a63b23ba39 100644
--- a/tools/libs/guest/xg_dom_hvmloader.c
+++ b/tools/libs/guest/xg_dom_hvmloader.c
@@ -73,7 +73,7 @@ static elf_negerrnoval xc_dom_probe_hvm_kernel(struct
xc_dom_image *dom)
* else we might be trying to load a PV kernel.
*/
elf_parse_binary();
-rc = elf_xen_parse(, dom->parms);
+rc = elf_xen_parse(, dom->parms, true);
if ( rc == 0 )
return -EINVAL;
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index 878dc1d808e..c24b9efdb0a 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -561,7 +561,7 @@ static int __init pvh_load_kernel(struct domain *d, const
module_t *image,
elf_set_verbose();
#endif
elf_parse_binary();
-if ( (rc = elf_xen_parse(, )) != 0 )
+if ( (rc = elf_xen_parse(, , true)) != 0 )
{
printk("Unable to parse kernel for ELFNOTES\n");
return rc;
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index e0801a9e6d1..af47615b226 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -353,7 +353,7 @@ int __init dom0_construct_pv(struct domain *d,
elf_set_verbose();
elf_parse_binary();
-if ( (rc = elf_xen_parse(, )) != 0 )
+if ( (rc = elf_xen_parse(, , false)) != 0 )
goto out;
/* compatibility check */
diff --git a/xen/common/libelf/libelf-dominfo.c
b/xen/common/libelf/libelf-dominfo.c
index 69c94b6f3bb..5ad2832aa75 100644
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -360,7 +360,7 @@ elf_errorstatus elf_xen_parse_guest_info(struct elf_binary
*elf,
/* sanity checks*/
static elf_errorstatus elf_xen_note_check(struct elf_binary *elf,
- struct elf_dom_parms *parms)
+ struct elf_dom_parms *parms, bool hvm)
{
if ( (ELF_PTRVAL_INVALID(parms->elf_note_start)) &&
(ELF_PTRVAL_INVALID(parms->guest_info)) )
@@ -382,7 +382,7 @@ static elf_errorstatus elf_xen_note_check(struct elf_binary
*elf,
}
/* PVH only requires one ELF note to be set */
-if ( parms->phys_entry != UNSET_ADDR32 )
+if ( parms->phys_entry != UNSET_ADDR32 && hvm
Re: [PATCH v2 1/2] xen/char: console: Use const whenever we point to literal strings
> On 18 May 2021, at 15:01, Julien Grall wrote:
>
> From: Julien Grall
>
> Literal strings are not meant to be modified. So we should use const
> char * rather than char * when we want to store a pointer to them.
>
> The array should also not be modified at all and is only used by
> xenlog_update_val(). So take the opportunity to add an extra const and
> move the definition in the function.
>
> Signed-off-by: Julien Grall
>
> ---
>Changes in v2:
>- The array content should never be modified
>- Move lvl2opt in xenlog_update_val()
> ---
> xen/drivers/char/console.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> index 23583751709c..7d0a603d0311 100644
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -168,10 +168,11 @@ static int parse_guest_loglvl(const char *s);
> static char xenlog_val[LOGLVL_VAL_SZ];
> static char xenlog_guest_val[LOGLVL_VAL_SZ];
>
> -static char *lvl2opt[] = { "none", "error", "warning", "info", "all" };
> -
> static void xenlog_update_val(int lower, int upper, char *val)
> {
> +static const char * const lvl2opt[] =
> +{ "none", "error", "warning", "info", "all" };
> +
> snprintf(val, LOGLVL_VAL_SZ, "%s/%s", lvl2opt[lower], lvl2opt[upper]);
> }
>
> @@ -262,7 +263,7 @@ static int parse_guest_loglvl(const char *s)
> return ret;
> }
>
> -static char *loglvl_str(int lvl)
> +static const char *loglvl_str(int lvl)
> {
> switch ( lvl )
> {
> --
> 2.17.1
>
Hi Julien,
Seems good to me and very sensible.
Reviewed-by: Luca Fancellu
Cheers,
Luca
Re: [PATCH 1/2] automation: use DOCKER_CMD for building containers too
On Tue, 2021-05-18 at 16:26 +0200, Roger Pau Monné wrote: > On Tue, May 18, 2021 at 02:04:07PM +0200, Dario Faggioli wrote: > > From: Dario Faggioli > > > > Use DOCKER_CMD from the environment (if defined) in the containers' > > makefile too, so that, e.g., when doing `export DOCKED_CMD=podman` > > podman is used for building the containers too. > > > > Signed-off-by: Dario Faggioli > > Acked-by: Roger Pau Monné > Thanks! If it has not been committed yet, can I resend with a From: that makes sense? And, either way, sorry for the noise... :-( Thanks and Regards -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ --- <> (Raistlin Majere) signature.asc Description: This is a digitally signed message part
Re: [PATCH 2/2] automation: fix dependencies on openSUSE Tumbleweed containers
On Tue, 2021-05-18 at 15:20 +0200, Roger Pau Monné wrote: > On Tue, May 18, 2021 at 02:04:13PM +0200, Dario Faggioli wrote: > > From: Dario Faggioli > > Mmm... this email address does not really exist, and it's a mistake that it ended up here. :-/ > > Fix the build inside our openSUSE Tumbleweed container by using > > the proper python development packages (and adding zstd headers). > > > > Signed-off-by: Dario Faggioli > > --- > > Cc: Doug Goldstein > > Cc: Roger Pau Monne > > Cc: Andrew Cooper > > --- > > .../build/suse/opensuse-tumbleweed.dockerfile | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/automation/build/suse/opensuse-tumbleweed.dockerfile > > b/automation/build/suse/opensuse-tumbleweed.dockerfile > > index 8ff7b9b5ce..5b99cb8a53 100644 > > --- a/automation/build/suse/opensuse-tumbleweed.dockerfile > > +++ b/automation/build/suse/opensuse-tumbleweed.dockerfile > > > > @@ -56,10 +57,8 @@ RUN zypper install -y --no-recommends \ > > pandoc \ > > patch \ > > pkg-config \ > > - python \ > > python-devel \ > > - python3 \ > > - python3-devel \ > > + python38-devel \ > > When I tested python3-devel was translated into python38-devel, > Oh, really? And when was it that you tested it, if I can ask? > which > I think is better as we don't need to modify the docker file for > every > new Python version? > That would definitely be better. Right now, I don't see any python3-devel package. If python3-devel can still be used (and it somehow translates to the proper -devel package), then sure we should use it. I'm not sure how that would happen, but maybe it's just me being unaware of some packaging magic. Let me put "python3-devel" there and test locally again, so we know if it actually works. Thanks and Regards -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ --- <> (Raistlin Majere) signature.asc Description: This is a digitally signed message part
Re: [PATCH 1/2] automation: use DOCKER_CMD for building containers too
On Tue, May 18, 2021 at 02:04:07PM +0200, Dario Faggioli wrote: > From: Dario Faggioli > > Use DOCKER_CMD from the environment (if defined) in the containers' > makefile too, so that, e.g., when doing `export DOCKED_CMD=podman` > podman is used for building the containers too. > > Signed-off-by: Dario Faggioli Acked-by: Roger Pau Monné Thanks!
Re: Discussion of Xenheap problems on AArch64
Hi Henry, On 17/05/2021 07:38, Henry Wang wrote: From: Julien Grall On a previous e-mail, you said you tweaked the FVP model to set those regions. Were you trying to mimick the memory layout of a real HW (either current or future)? Not really, I was just trying to cover as many cases as possible and these regions were just picked for testing your patchset in different scenarios. Thanks for the confirmation. It means we don't have to fix it right now. :). Cheers, -- Julien Grall
Re: [PATCH v3 2/5] xen/common: Guard iommu symbols with CONFIG_HAS_PASSTHROUGH
On 18/05/2021 07:27, Jan Beulich wrote:
On 18.05.2021 06:11, Connor Davis wrote:
On 5/17/21 9:42 AM, Julien Grall wrote:
Hi Jan,
On 17/05/2021 12:16, Jan Beulich wrote:
On 14.05.2021 20:53, Connor Davis wrote:
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -294,7 +294,9 @@ int guest_remove_page(struct domain *d, unsigned
long gmfn)
p2m_type_t p2mt;
#endif
mfn_t mfn;
+#ifdef CONFIG_HAS_PASSTHROUGH
bool *dont_flush_p, dont_flush;
+#endif
int rc;
#ifdef CONFIG_X86
@@ -385,13 +387,17 @@ int guest_remove_page(struct domain *d,
unsigned long gmfn)
* Since we're likely to free the page below, we need to suspend
* xenmem_add_to_physmap()'s suppressing of IOMMU TLB flushes.
*/
+#ifdef CONFIG_HAS_PASSTHROUGH
dont_flush_p = _cpu(iommu_dont_flush_iotlb);
dont_flush = *dont_flush_p;
*dont_flush_p = false;
+#endif
rc = guest_physmap_remove_page(d, _gfn(gmfn), mfn, 0);
+#ifdef CONFIG_HAS_PASSTHROUGH
*dont_flush_p = dont_flush;
+#endif
/*
* With the lack of an IOMMU on some platforms, domains with
DMA-capable
@@ -839,11 +845,13 @@ int xenmem_add_to_physmap(struct domain *d,
struct xen_add_to_physmap *xatp,
xatp->gpfn += start;
xatp->size -= start;
+#ifdef CONFIG_HAS_PASSTHROUGH
if ( is_iommu_enabled(d) )
{
this_cpu(iommu_dont_flush_iotlb) = 1;
extra.ppage = [0];
}
+#endif
while ( xatp->size > done )
{
@@ -868,6 +876,7 @@ int xenmem_add_to_physmap(struct domain *d,
struct xen_add_to_physmap *xatp,
}
}
+#ifdef CONFIG_HAS_PASSTHROUGH
if ( is_iommu_enabled(d) )
{
int ret;
@@ -894,6 +903,7 @@ int xenmem_add_to_physmap(struct domain *d,
struct xen_add_to_physmap *xatp,
if ( unlikely(ret) && rc >= 0 )
rc = ret;
}
+#endif
return rc;
}
I wonder whether all of these wouldn't better become CONFIG_X86:
ISTR Julien indicating that he doesn't see the override getting used
on Arm. (Julien, please correct me if I'm misremembering.)
Right, so far, I haven't been in favor to introduce it because:
1) The P2M code may free some memory. So you can't always ignore
the flush (I think this is wrong for the upper layer to know when this
can happen).
2) It is unclear what happen if the IOMMU TLBs and the PT contains
different mappings (I received conflicted advice).
So it is better to always flush and as early as possible.
So keep it as is or switch to CONFIG_X86?
Please switch, unless anyone else voices a strong opinion towards
keeping as is.
I would like to avoid adding more #ifdef CONFIG_X86 in the common code.
Can we instead provide a wrapper for them?
Cheers,
--
Julien Grall
Re: PING Re: [PATCH 00/14] Use const whether we point to literal strings (take 1)
Hi Wei, On 17/05/2021 19:41, Wei Liu wrote: On Mon, May 10, 2021 at 06:49:01PM +0100, Julien Grall wrote: Hi, Ian, Wei, Anthony, can I get some feedbacks on the tools side? I think this is moving to the right direction so Acked-by: Wei Liu Thanks! I had committed most of the tools code but one patch. I have kept your acked-by on the patch and will wait Anthony to do a full review. Cheers, -- Julien Grall
[PATCH v2 0/2] Use const whenever we point to literal strings (take 1)
From: Julien Grall Hi all, By default, both Clang and GCC will happily compile C code where non-const char * point to literal strings. This means the following code will be accepted: char *str = "test"; str[0] = 'a'; Literal strings will reside in rodata, so they are not modifiable. This will result to an permission fault at runtime if the permissions are enforced in the page-tables (this is the case in Xen). I am not aware of code trying to modify literal strings in Xen. However, there is a frequent use of non-const char * to point to literal strings. Given the size of the codebase, there is a risk to involuntarily introduce code that will modify literal strings. Therefore it would be better to enforce using const when pointing to such strings. Both GCC and Clang provide an option to warn for such case (see -Wwrite-strings) and therefore could be used by Xen. This series doesn't yet make use of -Wwrite-strings because the tree is not fully converted. Instead, it contains some easy and non-controversial use of const in the code. Julien Grall (2): xen/char: console: Use const whenever we point to literal strings tools/console: Use const whenever we point to literal strings tools/console/client/main.c | 4 ++-- tools/console/daemon/io.c | 15 --- xen/drivers/char/console.c | 7 --- 3 files changed, 14 insertions(+), 12 deletions(-) -- 2.17.1
[PATCH v2 1/2] xen/char: console: Use const whenever we point to literal strings
From: Julien Grall
Literal strings are not meant to be modified. So we should use const
char * rather than char * when we want to store a pointer to them.
The array should also not be modified at all and is only used by
xenlog_update_val(). So take the opportunity to add an extra const and
move the definition in the function.
Signed-off-by: Julien Grall
---
Changes in v2:
- The array content should never be modified
- Move lvl2opt in xenlog_update_val()
---
xen/drivers/char/console.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 23583751709c..7d0a603d0311 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -168,10 +168,11 @@ static int parse_guest_loglvl(const char *s);
static char xenlog_val[LOGLVL_VAL_SZ];
static char xenlog_guest_val[LOGLVL_VAL_SZ];
-static char *lvl2opt[] = { "none", "error", "warning", "info", "all" };
-
static void xenlog_update_val(int lower, int upper, char *val)
{
+static const char * const lvl2opt[] =
+{ "none", "error", "warning", "info", "all" };
+
snprintf(val, LOGLVL_VAL_SZ, "%s/%s", lvl2opt[lower], lvl2opt[upper]);
}
@@ -262,7 +263,7 @@ static int parse_guest_loglvl(const char *s)
return ret;
}
-static char *loglvl_str(int lvl)
+static const char *loglvl_str(int lvl)
{
switch ( lvl )
{
--
2.17.1
[PATCH v2 2/2] tools/console: Use const whenever we point to literal strings
From: Julien Grall
Literal strings are not meant to be modified. So we should use const
char * rather than char * when we want to store a pointer to them.
Take the opportunity to remove the cast (char *) in console_init(). It
is unnecessary and will remove the const.
Signed-off-by: Julien Grall
Acked-by: Wei Liu
---
Changes in v2:
- Remove the cast (char *) in console_init()
- Add Wei's acked-by
---
tools/console/client/main.c | 4 ++--
tools/console/daemon/io.c | 15 ---
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/tools/console/client/main.c b/tools/console/client/main.c
index 088be28dff02..80157be42144 100644
--- a/tools/console/client/main.c
+++ b/tools/console/client/main.c
@@ -325,7 +325,7 @@ int main(int argc, char **argv)
{
struct termios attr;
int domid;
- char *sopt = "hn:";
+ const char *sopt = "hn:";
int ch;
unsigned int num = 0;
int opt_ind=0;
@@ -345,7 +345,7 @@ int main(int argc, char **argv)
char *end;
console_type type = CONSOLE_INVAL;
bool interactive = 0;
- char *console_names = "serial, pv, vuart";
+ const char *console_names = "serial, pv, vuart";
while((ch = getopt_long(argc, argv, sopt, lopt, _ind)) != -1) {
switch(ch) {
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 4af27ffc5d02..200b575d76f8 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -87,14 +87,14 @@ struct buffer {
};
struct console {
- char *ttyname;
+ const char *ttyname;
int master_fd;
int master_pollfd_idx;
int slave_fd;
int log_fd;
struct buffer buffer;
char *xspath;
- char *log_suffix;
+ const char *log_suffix;
int ring_ref;
xenevtchn_handle *xce_handle;
int xce_pollfd_idx;
@@ -109,9 +109,9 @@ struct console {
};
struct console_type {
- char *xsname;
- char *ttyname;
- char *log_suffix;
+ const char *xsname;
+ const char *ttyname;
+ const char *log_suffix;
bool optional;
bool use_gnttab;
};
@@ -813,7 +813,8 @@ static int console_init(struct console *con, struct domain
*dom, void **data)
int err = -1;
struct timespec ts;
struct console_type **con_type = (struct console_type **)data;
- char *xsname, *xspath;
+ const char *xsname;
+ char *xspath;
if (clock_gettime(CLOCK_MONOTONIC, ) < 0) {
dolog(LOG_ERR, "Cannot get time of day %s:%s:L%d",
@@ -835,7 +836,7 @@ static int console_init(struct console *con, struct domain
*dom, void **data)
con->log_suffix = (*con_type)->log_suffix;
con->optional = (*con_type)->optional;
con->use_gnttab = (*con_type)->use_gnttab;
- xsname = (char *)(*con_type)->xsname;
+ xsname = (*con_type)->xsname;
xspath = xs_get_domain_path(xs, dom->domid);
s = realloc(xspath, strlen(xspath) +
strlen(xsname) + 1);
--
2.17.1
Re: [PATCH 09/14] tools/console: Use const whenever we point to literal strings
Hi Anthony,
On 11/05/2021 16:18, Anthony PERARD wrote:
On Mon, Apr 05, 2021 at 04:57:08PM +0100, Julien Grall wrote:
From: Julien Grall
literal strings are not meant to be modified. So we should use const
char * rather than char * when we want to store a pointer to them.
Signed-off-by: Julien Grall
---
diff --git a/tools/console/daemon/io.c b/tools/console/daemon/io.c
index 4af27ffc5d02..6a8a94e31b65 100644
--- a/tools/console/daemon/io.c
+++ b/tools/console/daemon/io.c
@@ -109,9 +109,9 @@ struct console {
};
struct console_type {
- char *xsname;
- char *ttyname;
- char *log_suffix;
+ const char *xsname;
I think that const of `xsname` is lost in console_init() in the same
file.
We have:
static int console_init(.. )
{
struct console_type **con_type = (struct console_type **)data;
char *xsname, *xspath;
xsname = (char *)(*con_type)->xsname;
}
So constify "xsname" in console_init() should be part of the patch, I
think.
Good point. I am not entirely sure why the cast has been added because
the field has always been a char *.
Anyway, I will remove it.
Cheers,
--
Julien Grall
Re: [PATCH 05/14] tools/libs: guest: Use const whenever we point to literal strings
Hi Anthony,
On 11/05/2021 15:58, Anthony PERARD wrote:
On Mon, Apr 05, 2021 at 04:57:04PM +0100, Julien Grall wrote:
From: Julien Grall
literal strings are not meant to be modified. So we should use const
*char rather than char * when we want to store a pointer to them.
Signed-off-by: Julien Grall
---
diff --git a/tools/libs/guest/xg_dom_x86.c b/tools/libs/guest/xg_dom_x86.c
index 2953aeb90b35..e379b07f9945 100644
--- a/tools/libs/guest/xg_dom_x86.c
+++ b/tools/libs/guest/xg_dom_x86.c
@@ -1148,11 +1148,12 @@ static int vcpu_hvm(struct xc_dom_image *dom)
/* */
-static int x86_compat(xc_interface *xch, uint32_t domid, char *guest_type)
+static int x86_compat(xc_interface *xch, uint32_t domid,
+ const char *guest_type)
{
static const struct {
-char *guest;
-uint32_tsize;
+const char *guest;
+uint32_t size;
It seems that one space have been removed by mistake just before "size".
Well spotted. I will fix on commit.
The rest looks good:
Reviewed-by: Anthony PERARD
Thank you!
Cheers,
--
Julien Grall
Re: [PATCH 2/2] automation: fix dependencies on openSUSE Tumbleweed containers
On Tue, May 18, 2021 at 02:04:13PM +0200, Dario Faggioli wrote: > From: Dario Faggioli > > Fix the build inside our openSUSE Tumbleweed container by using > the proper python development packages (and adding zstd headers). > > Signed-off-by: Dario Faggioli > --- > Cc: Doug Goldstein > Cc: Roger Pau Monne > Cc: Andrew Cooper > --- > .../build/suse/opensuse-tumbleweed.dockerfile |5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/automation/build/suse/opensuse-tumbleweed.dockerfile > b/automation/build/suse/opensuse-tumbleweed.dockerfile > index 8ff7b9b5ce..5b99cb8a53 100644 > --- a/automation/build/suse/opensuse-tumbleweed.dockerfile > +++ b/automation/build/suse/opensuse-tumbleweed.dockerfile > @@ -45,6 +45,7 @@ RUN zypper install -y --no-recommends \ > libtasn1-devel \ > libuuid-devel \ > libyajl-devel \ > +libzstd-devel \ > lzo-devel \ > make \ > nasm \ > @@ -56,10 +57,8 @@ RUN zypper install -y --no-recommends \ > pandoc \ > patch \ > pkg-config \ > -python \ > python-devel \ > -python3 \ > -python3-devel \ > +python38-devel \ When I tested python3-devel was translated into python38-devel, which I think is better as we don't need to modify the docker file for every new Python version? Thanks, Roger.
Re: [PATCH] tools/xenstore: claim resources when running as daemon
Hi Juergen,
On 18/05/2021 07:43, Juergen Gross wrote:
On 17.05.21 17:55, Julien Grall wrote:
So the admin should be able to configure them. At this point, I think
the two limit should be set my the initscript rather than xenstored
itself.
But the admin would need to know the Xen internals for selecting the
correct limits. In the end I'd be fine with moving this modification to
the script starting Xenstore (which would be launch-xenstore), but the
configuration item should be "max number of domains to support".
I would be fine with "max numer of domains to support". What I care the
most here is the limits are actually applied most of (if not all) the
time.
I did another test and found:
- the xl daemon for a guest will use 2 socket connections
- qemu for a HVM guest will use 3 socket connections
- qemu for a PV guest is using one socket connection
- 14 other files are used by xenstored
So we should set the limit to 5 * n_dom + 100 (some headroom will be
nice IMO).
This looks fine to me.
This would also avoid the problem where Xenstored is not allowed to
modify its limit (see more below).
Signed-off-by: Juergen Gross
---
tools/xenstore/xenstored_core.c | 2 ++
tools/xenstore/xenstored_core.h | 3 ++
tools/xenstore/xenstored_minios.c | 4 +++
tools/xenstore/xenstored_posix.c | 46
+++
4 files changed, 55 insertions(+)
diff --git a/tools/xenstore/xenstored_core.c
b/tools/xenstore/xenstored_core.c
index b66d119a98..964e693450 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2243,6 +2243,8 @@ int main(int argc, char *argv[])
xprintf = trace;
#endif
+ claim_resources();
+
signal(SIGHUP, trigger_reopen_log);
if (tracefile)
tracefile =
talloc_strdup(NULL, tracefile);
diff --git a/tools/xenstore/xenstored_core.h
b/tools/xenstore/xenstored_core.h
index 1467270476..ac26973648 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -255,6 +255,9 @@ void daemonize(void);
/* Close stdin/stdout/stderr to complete daemonize */
void finish_daemonize(void);
+/* Set OOM-killer score and raise ulimit. */
+void claim_resources(void);
+
/* Open a pipe for signal handling */
void init_pipe(int reopen_log_pipe[2]);
diff --git a/tools/xenstore/xenstored_minios.c
b/tools/xenstore/xenstored_minios.c
index c94493e52a..df8ff580b0 100644
--- a/tools/xenstore/xenstored_minios.c
+++ b/tools/xenstore/xenstored_minios.c
@@ -32,6 +32,10 @@ void finish_daemonize(void)
{
}
+void claim_resources(void)
+{
+}
+
void init_pipe(int reopen_log_pipe[2])
{
reopen_log_pipe[0] = -1;
diff --git a/tools/xenstore/xenstored_posix.c
b/tools/xenstore/xenstored_posix.c
index 48c37ffe3e..0074fbd8b2 100644
--- a/tools/xenstore/xenstored_posix.c
+++ b/tools/xenstore/xenstored_posix.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include "utils.h"
#include "xenstored_core.h"
@@ -87,6 +88,51 @@ void finish_daemonize(void)
close(devnull);
}
+static void avoid_oom_killer(void)
+{
+ char path[32];
+ char val[] = "-500";
+ int fd;
+
+ snprintf(path, sizeof(path), "/proc/%d/oom_score_adj",
(int)getpid());
This looks Linux specific. How about other OSes?
I don't know whether other OSes have an OOM killer, and if they do, how
to configure it. It is a best effort attempt, after all.
I have CCed Roger who should be able to help for FreeBSD.
It would be possible to set the OOM-score from the launch script, too.
It would be ideal if all the limits are set from the launch script. At
least, they can be changed by the admin and also possibly be enforced
(if Xenstored is not allowed to do it).
+
+ fd = open(path, O_WRONLY);
+ /* Do nothing if file doesn't exist. */
Your commit message leads to think that we *must* configure the OOM.
If not, then we should not continue. But here, this suggest this is
optional. In fact...
I can modify the commit message by adding a "Try to".
+ if (fd < 0)
+ return;
+ /* Ignore errors. */
+ write(fd, val, sizeof(val));
... xenstored may not be allowed to modify its own parameters. So
this would continue silently without the admin necessarily knowning
the limit wasn't applied.
I can add a line in the Xenstore log in this regard.
This feels wrong to me. If a limit cannot be applied then it should
fail early rather than possibly at the wrong moment a few days
(months?) after.
I think issuing a warning would be better here. We are running with
no OOM adjustments since years now.
Right, this is a sign that the OOM adjustment is not necessary in most
of the cases. But the fact you sent this patch suggests that you or
someone else saw Xenstored crashing.
The idea with failing hard and early is the admin will directly be aware
that didn't happen. It can then take action before it is too late (e.g.
Xenstored was killed
Re: [PATCH v3 2/2] tools/xenstore: simplify xenstored main loop
Hi Juergen,
On 18/05/2021 07:19, Juergen Gross wrote:
The main loop of xenstored is rather complicated due to different
handling of socket and ring-page interfaces. Unify that handling by
introducing interface type specific functions can_read() and
can_write().
Take the opportunity to remove the empty list check before calling
write_messages() because the function is already able to cope with an
empty list.
Signed-off-by: Juergen Gross
Reviewed-by: Julien Grall
I have also committed the series.
Cheers,
---
V2:
- split off function vector introduction (Julien Grall)
V3:
- expand commit message (Julien Grall)
---
tools/xenstore/xenstored_core.c | 77 +++
tools/xenstore/xenstored_core.h | 2 +
tools/xenstore/xenstored_domain.c | 2 +
3 files changed, 41 insertions(+), 40 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 856f518075..883a1a582a 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1659,9 +1659,34 @@ static int readfd(struct connection *conn, void *data,
unsigned int len)
return rc;
}
+static bool socket_can_process(struct connection *conn, int mask)
+{
+ if (conn->pollfd_idx == -1)
+ return false;
+
+ if (fds[conn->pollfd_idx].revents & ~(POLLIN | POLLOUT)) {
+ talloc_free(conn);
+ return false;
+ }
+
+ return (fds[conn->pollfd_idx].revents & mask) && !conn->is_ignored;
+}
+
+static bool socket_can_write(struct connection *conn)
+{
+ return socket_can_process(conn, POLLOUT);
+}
+
+static bool socket_can_read(struct connection *conn)
+{
+ return socket_can_process(conn, POLLIN);
+}
+
const struct interface_funcs socket_funcs = {
.write = writefd,
.read = readfd,
+ .can_write = socket_can_write,
+ .can_read = socket_can_read,
};
static void accept_connection(int sock)
@@ -2296,47 +2321,19 @@ int main(int argc, char *argv[])
if (>list != )
talloc_increase_ref_count(next);
- if (conn->domain) {
- if (domain_can_read(conn))
- handle_input(conn);
- if (talloc_free(conn) == 0)
- continue;
-
- talloc_increase_ref_count(conn);
- if (domain_can_write(conn) &&
- !list_empty(>out_list))
- handle_output(conn);
- if (talloc_free(conn) == 0)
- continue;
- } else {
- if (conn->pollfd_idx != -1) {
- if (fds[conn->pollfd_idx].revents
- & ~(POLLIN|POLLOUT))
- talloc_free(conn);
- else if ((fds[conn->pollfd_idx].revents
- & POLLIN) &&
-!conn->is_ignored)
- handle_input(conn);
- }
- if (talloc_free(conn) == 0)
- continue;
-
- talloc_increase_ref_count(conn);
-
- if (conn->pollfd_idx != -1) {
- if (fds[conn->pollfd_idx].revents
- & ~(POLLIN|POLLOUT))
- talloc_free(conn);
- else if ((fds[conn->pollfd_idx].revents
- & POLLOUT) &&
-!conn->is_ignored)
- handle_output(conn);
- }
- if (talloc_free(conn) == 0)
- continue;
+ if (conn->funcs->can_read(conn))
+ handle_input(conn);
+ if (talloc_free(conn) == 0)
+ continue;
-conn->pollfd_idx = -1;
- }
+ talloc_increase_ref_count(conn);
+
+ if (conn->funcs->can_write(conn))
+ handle_output(conn);
+ if (talloc_free(conn) == 0)
+ continue;
+
+ conn->pollfd_idx = -1;
}
if (delayed_requests) {
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 6c4845c196..bb36111ecc 100644
---
Re: [PATCH 07/10] xen/arm: intruduce alloc_domstatic_pages
Hi Penny,
On 18/05/2021 09:57, Penny Zheng wrote:
-Original Message-
From: Jan Beulich
Sent: Tuesday, May 18, 2021 3:35 PM
To: Penny Zheng
Cc: Bertrand Marquis ; Wei Chen
; nd ; xen-devel@lists.xenproject.org;
sstabell...@kernel.org; jul...@xen.org
Subject: Re: [PATCH 07/10] xen/arm: intruduce alloc_domstatic_pages
On 18.05.2021 07:21, Penny Zheng wrote:
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2447,6 +2447,9 @@ int assign_pages(
{
ASSERT(page_get_owner([i]) == NULL);
page_set_owner([i], d);
+/* use page_set_reserved_owner to set its reserved domain owner.
*/
+if ( (pg[i].count_info & PGC_reserved) )
+page_set_reserved_owner([i], d);
Now this is puzzling: What's the point of setting two owner fields to the same
value? I also don't recall you having introduced
page_set_reserved_owner() for x86, so how is this going to build there?
Thanks for pointing out that it will fail on x86.
As for the same value, sure, I shall change it to domid_t domid to record its
reserved owner.
Only domid is enough for differentiate.
And even when domain get rebooted, struct domain may be destroyed, but domid
will stays
The same.
Major user cases for domain on static allocation are referring to the whole
system are static,
No runtime creation.
One may want to have static memory yet doesn't care about the domid. So
I am not in favor to restrict about the domid unless there is no other way.
Cheers,
--
Julien Grall
[qemu-mainline test] 161986: regressions - FAIL
flight 161986 qemu-mainline real [real] http://logs.test-lab.xenproject.org/osstest/logs/161986/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-amd64-qemuu-freebsd11-amd64 16 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-freebsd10-i386 16 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-freebsd10-amd64 16 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-qemuu-freebsd12-amd64 16 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-debianhvm-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 15 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-ovmf-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-debianhvm-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-debianhvm-amd64-shadow 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-win7-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-ovmf-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-qemuu-nested-intel 20 debian-hvm-install/l1/l2 fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-win7-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-i386-xl-qemuu-ws16-amd64 15 guest-saverestore fail REGR. vs. 152631 test-amd64-amd64-xl-qemuu-ws16-amd64 15 guest-saverestore fail REGR. vs. 152631 Tests which did not succeed, but are not blocking: test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152631 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 152631 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 152631 test-amd64-i386-xl-pvshim14 guest-start fail never pass test-arm64-arm64-xl-seattle 15 migrate-support-checkfail never pass test-arm64-arm64-xl-seattle 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail never pass test-amd64-amd64-libvirt 15 migrate-support-checkfail never pass test-amd64-i386-libvirt 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-xl-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit1 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit1 16 saverestore-support-checkfail never pass test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail never pass test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail never pass test-arm64-arm64-xl 16 saverestore-support-checkfail never pass test-arm64-arm64-xl-credit2 15 migrate-support-checkfail never pass test-arm64-arm64-xl-credit2 16 saverestore-support-checkfail never pass test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check fail never pass test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl 15 migrate-support-checkfail never pass test-armhf-armhf-xl 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-rtds 15 migrate-support-checkfail never pass test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-cubietruck 15 migrate-support-checkfail never pass test-armhf-armhf-xl-cubietruck 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit2 15 migrate-support-checkfail never pass test-armhf-armhf-xl-credit2 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail never pass test-armhf-armhf-xl-multivcpu 16 saverestore-support-checkfail never pass test-armhf-armhf-libvirt 15 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 14 migrate-support-checkfail never pass test-armhf-armhf-xl-vhd 15 saverestore-support-checkfail never pass test-armhf-armhf-xl-arndale 15 migrate-support-checkfail never pass test-armhf-armhf-xl-arndale 16 saverestore-support-checkfail never pass test-armhf-armhf-xl-credit1 15 migrate-support-checkfail never pass
Re: [PATCH 09/10] xen/arm: parse `xen,static-mem` info during domain construction
Hi Penny,
On 18/05/2021 06:21, Penny Zheng wrote:
This commit parses `xen,static-mem` device tree property, to acquire
static memory info reserved for this domain, when constructing domain
during boot-up.
Related info shall be stored in new static_mem value under per domain
struct arch_domain.
So far, this seems to only be used during boot. So can't this be kept in
the kinfo structure?
Right now, the implementation of allocate_static_memory is missing, and
will be introduced later. It just BUG() out at the moment.
Signed-off-by: Penny Zheng
---
xen/arch/arm/domain_build.c | 58
xen/include/asm-arm/domain.h | 3 ++
2 files changed, 56 insertions(+), 5 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 282416e74d..30b55588b7 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2424,17 +2424,61 @@ static int __init construct_domU(struct domain *d,
{
struct kernel_info kinfo = {};
int rc;
-u64 mem;
+u64 mem, static_mem_size = 0;
+const struct dt_property *prop;
+u32 static_mem_len;
+bool static_mem = false;
+
+/*
+ * Guest RAM could be of static memory from static allocation,
+ * which will be specified through "xen,static-mem" property.
+ */
+prop = dt_find_property(node, "xen,static-mem", _mem_len);
+if ( prop )
+{
+const __be32 *cell;
+u32 addr_cells = 2, size_cells = 2, reg_cells;
+u64 start, size;
+int i, banks;
+static_mem = true;
+
+dt_property_read_u32(node, "#address-cells", _cells);
+dt_property_read_u32(node, "#size-cells", _cells);
+BUG_ON(size_cells > 2 || addr_cells > 2);
+reg_cells = addr_cells + size_cells;
+
+cell = (const __be32 *)prop->value;
+banks = static_mem_len / (reg_cells * sizeof (u32));
+BUG_ON(banks > NR_MEM_BANKS);
+
+for ( i = 0; i < banks; i++ )
+{
+device_tree_get_reg(, addr_cells, size_cells, , );
+d->arch.static_mem.bank[i].start = start;
+d->arch.static_mem.bank[i].size = size;
+static_mem_size += size;
+
+printk(XENLOG_INFO
+"Static Memory Bank[%d] for Domain %pd:"
+"0x%"PRIx64"-0x%"PRIx64"\n",
+i, d,
+d->arch.static_mem.bank[i].start,
+d->arch.static_mem.bank[i].start +
+d->arch.static_mem.bank[i].size);
+}
+d->arch.static_mem.nr_banks = banks;
+}
Could we allocate the memory as we parse?
rc = dt_property_read_u64(node, "memory", );
-if ( !rc )
+if ( !static_mem && !rc )
{
printk("Error building DomU: cannot read \"memory\" property\n");
return -EINVAL;
}
-kinfo.unassigned_mem = (paddr_t)mem * SZ_1K;
+kinfo.unassigned_mem = static_mem ? static_mem_size : (paddr_t)mem * SZ_1K;
-printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n", d->max_vcpus, mem);
+printk("*** LOADING DOMU cpus=%u memory=%"PRIx64"KB ***\n",
+d->max_vcpus, (kinfo.unassigned_mem) >> 10);
kinfo.vpl011 = dt_property_read_bool(node, "vpl011");
@@ -2452,7 +2496,11 @@ static int __init construct_domU(struct domain *d,
/* type must be set before allocate memory */
d->arch.type = kinfo.type;
#endif
-allocate_memory(d, );
+if ( static_mem )
+/* allocate_static_memory(d, ); */
+BUG();
+else
+allocate_memory(d, );
rc = prepare_dtb_domU(d, );
if ( rc < 0 )
diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
index c9277b5c6d..81b8eb453c 100644
--- a/xen/include/asm-arm/domain.h
+++ b/xen/include/asm-arm/domain.h
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
#include
struct hvm_domain
@@ -89,6 +90,8 @@ struct arch_domain
#ifdef CONFIG_TEE
void *tee;
#endif
+
+struct meminfo static_mem;
} __cacheline_aligned;
struct arch_vcpu
Cheers,
--
Julien Grall
Re: [PATCH 10/10] xen/arm: introduce allocate_static_memory
Hi Penny,
On 18/05/2021 06:21, Penny Zheng wrote:
This commit introduces allocate_static_memory to allocate static memory as
guest RAM for domain on Static Allocation.
It uses alloc_domstatic_pages to allocate pre-defined static memory banks
for this domain, and uses guest_physmap_add_page to set up P2M table,
guest starting at fixed GUEST_RAM0_BASE, GUEST_RAM1_BASE.
Signed-off-by: Penny Zheng
---
xen/arch/arm/domain_build.c | 157 +++-
1 file changed, 155 insertions(+), 2 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 30b55588b7..9f662313ad 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -437,6 +437,50 @@ static bool __init allocate_bank_memory(struct domain *d,
return true;
}
+/*
+ * #ram_index and #ram_index refer to the index and starting address of guest
+ * memory kank stored in kinfo->mem.
+ * Static memory at #smfn of #tot_size shall be mapped #sgfn, and
+ * #sgfn will be next guest address to map when returning.
+ */
+static bool __init allocate_static_bank_memory(struct domain *d,
+ struct kernel_info *kinfo,
+ int ram_index,
Please use unsigned.
+ paddr_t ram_addr,
+ gfn_t* sgfn,
I am confused, what is the difference between ram_addr and sgfn?
+ mfn_t smfn,
+ paddr_t tot_size)
+{
+int res;
+struct membank *bank;
+paddr_t _size = tot_size;
+
+bank = >mem.bank[ram_index];
+bank->start = ram_addr;
+bank->size = bank->size + tot_size;
+
+while ( tot_size > 0 )
+{
+unsigned int order = get_allocation_size(tot_size);
+
+res = guest_physmap_add_page(d, *sgfn, smfn, order);
+if ( res )
+{
+dprintk(XENLOG_ERR, "Failed map pages to DOMU: %d", res);
+return false;
+}
+
+*sgfn = gfn_add(*sgfn, 1UL << order);
+smfn = mfn_add(smfn, 1UL << order);
+tot_size -= (1ULL << (PAGE_SHIFT + order));
+}
+
+kinfo->mem.nr_banks = ram_index + 1;
+kinfo->unassigned_mem -= _size;
+
+return true;
+}
+
static void __init allocate_memory(struct domain *d, struct kernel_info
*kinfo)
{
unsigned int i;
@@ -480,6 +524,116 @@ fail:
(unsigned long)kinfo->unassigned_mem >> 10);
}
+/* Allocate memory from static memory as RAM for one specific domain d. */
+static void __init allocate_static_memory(struct domain *d,
+struct kernel_info *kinfo)
+{
+int nr_banks, _banks = 0;
AFAICT, _banks is the index in the array. I think it would be clearer if
it is caller 'bank' or 'idx'.
+size_t ram0_size = GUEST_RAM0_SIZE, ram1_size = GUEST_RAM1_SIZE;
+paddr_t bank_start, bank_size;
+gfn_t sgfn;
+mfn_t smfn;
+
+kinfo->mem.nr_banks = 0;
+sgfn = gaddr_to_gfn(GUEST_RAM0_BASE);
+nr_banks = d->arch.static_mem.nr_banks;
+ASSERT(nr_banks >= 0);
+
+if ( kinfo->unassigned_mem <= 0 )
+goto fail;
+
+while ( _banks < nr_banks )
+{
+bank_start = d->arch.static_mem.bank[_banks].start;
+smfn = maddr_to_mfn(bank_start);
+bank_size = d->arch.static_mem.bank[_banks].size;
The variable name are slightly confusing because it doesn't tell whether
this is physical are guest RAM. You might want to consider to prefix
them with p (resp. g) for physical (resp. guest) RAM.
+
+if ( !alloc_domstatic_pages(d, bank_size >> PAGE_SHIFT, bank_start, 0)
)
+{
+printk(XENLOG_ERR
+"%pd: cannot allocate static memory"
+"(0x%"PRIx64" - 0x%"PRIx64")",
bank_start and bank_size are both paddr_t. So this should be PRIpaddr.
+d, bank_start, bank_start + bank_size);
+goto fail;
+}
+
+/*
+ * By default, it shall be mapped to the fixed guest RAM address
+ * `GUEST_RAM0_BASE`, `GUEST_RAM1_BASE`.
+ * Starting from RAM0(GUEST_RAM0_BASE).
+ */
Ok. So you are first trying to exhaust the guest bank 0 and then moved
to bank 1. This wasn't entirely clear from the design document.
I am fine with that, but in this case, the developper should not need to
know that (in fact this is not part of the ABI).
Regarding this code, I am a bit concerned about the scalability if we
introduce a second bank.
Can we have an array of the possible guest banks and increment the index
when exhausting the current bank?
Cheers,
--
Julien Grall
[PATCH 2/2] automation: fix dependencies on openSUSE Tumbleweed containers
From: Dario Faggioli Fix the build inside our openSUSE Tumbleweed container by using the proper python development packages (and adding zstd headers). Signed-off-by: Dario Faggioli --- Cc: Doug Goldstein Cc: Roger Pau Monne Cc: Andrew Cooper --- .../build/suse/opensuse-tumbleweed.dockerfile |5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/automation/build/suse/opensuse-tumbleweed.dockerfile b/automation/build/suse/opensuse-tumbleweed.dockerfile index 8ff7b9b5ce..5b99cb8a53 100644 --- a/automation/build/suse/opensuse-tumbleweed.dockerfile +++ b/automation/build/suse/opensuse-tumbleweed.dockerfile @@ -45,6 +45,7 @@ RUN zypper install -y --no-recommends \ libtasn1-devel \ libuuid-devel \ libyajl-devel \ +libzstd-devel \ lzo-devel \ make \ nasm \ @@ -56,10 +57,8 @@ RUN zypper install -y --no-recommends \ pandoc \ patch \ pkg-config \ -python \ python-devel \ -python3 \ -python3-devel \ +python38-devel \ systemd-devel \ tar \ transfig \
[PATCH 1/2] automation: use DOCKER_CMD for building containers too
From: Dario Faggioli Use DOCKER_CMD from the environment (if defined) in the containers' makefile too, so that, e.g., when doing `export DOCKED_CMD=podman` podman is used for building the containers too. Signed-off-by: Dario Faggioli --- Cc: Doug Goldstein Cc: Roger Pau Monne Cc: Andrew Cooper --- automation/build/Makefile |5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/automation/build/Makefile b/automation/build/Makefile index 7c7612b1d9..a4b2b85178 100644 --- a/automation/build/Makefile +++ b/automation/build/Makefile @@ -2,6 +2,7 @@ # the base of where these containers will appear REGISTRY := registry.gitlab.com/xen-project/xen CONTAINERS = $(subst .dockerfile,,$(wildcard */*.dockerfile)) +DOCKER_CMD ?= docker help: @echo "Builds containers for building Xen based on different distros" @@ -10,9 +11,9 @@ help: @echo "To push container builds, set the env var PUSH" %: %.dockerfile ## Builds containers - docker build -t $(REGISTRY)/$(@D):$(@F) -f $< $(
[PATCH 0/2] automation: fix building in the openSUSE Tumbleweed
Building Xen in openSUSE Tumbleweed in the GitLab CI was broken due to python and libzstd development dependencies, so let's update the docker file to fix that. If this change is accepted, I'm happy to push a new, updated, image to our registry (ISTR that I used to have the right to do that). While there, extend the generalization of the container runtime to use (we have that in containerize already, through the DOCKER_CMD variable) to the local building of the containers as well. Regards --- Dario Faggioli (2): automation: use DOCKER_CMD for building containers too automation: fix dependencies on openSUSE Tumbleweed containers automation/build/suse/opensuse-tumbleweed.dockerfile | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- Dario Faggioli, Ph.D http://about.me/dario.faggioli Virtualization Software Engineer SUSE Labs, SUSE https://www.suse.com/ --- <> (Raistlin Majere)
Re: [PATCH] Xen: Design doc for 1:1 direct-map and static allocation
Hi Penny,
On 18/05/2021 06:07, Penny Zheng wrote:
Create one design doc for 1:1 direct-map and static allocation.
It is the first draft and aims to describe why and how we allocate
1:1 direct-map(guest physical == physical) domains, and why and how
we let domains on static allocation.
Signed-off-by: Penny Zheng
---
docs/designs/static_alloc_and_direct_map.md | 239
1 file changed, 239 insertions(+)
create mode 100644 docs/designs/static_alloc_and_direct_map.md
diff --git a/docs/designs/static_alloc_and_direct_map.md
b/docs/designs/static_alloc_and_direct_map.md
new file mode 100644
index 00..fdda162188
--- /dev/null
+++ b/docs/designs/static_alloc_and_direct_map.md
@@ -0,0 +1,239 @@
+# Preface
+
+The document is an early draft for 1:1 direct-map memory map
+(`guest physical == physical`) of domUs and Static Allocation.
+Since the implementation of these two features shares a lot, we would like
+to introduce both in one design.
+
+Right now, these two features are limited to ARM architecture.
+
+This design aims to describe why and how the guest would be created as 1:1
+direct-map domain, and why and what the static allocation is.
+
+This document is partly based on Stefano Stabellini's patch serie v1:
+[direct-map DomUs](
+https://lists.xenproject.org/archives/html/xen-devel/2020-04/msg00707.html).
While for the reviewer this is a useful information to have, I am not
sure a future reader needs to know all the history. So I would move this
to the commit message.
+
+This is a first draft and some questions are still unanswered. When this is
+the case, it will be included under chapter `DISCUSSION`.
+
+# Introduction on Static Allocation
+
+Static allocation refers to system or sub-system(domains) for which memory
+areas are pre-defined by configuration using physical address ranges.
+
+## Background
+
+Cases where needs static allocation:
+
+ * Static allocation needed whenever a system has a pre-defined non-changing
+behaviour. This is usually the case in safety world where system must behave
+the same upon reboot, so memory resource for both XEN and domains should be
+static and pre-defined.
+
+ * Static allocation needed whenever a guest wants to allocate memory
+from refined memory ranges. For example, a system has one high-speed RAM
+region, and would like to assign it to one specific domain.
+
+ * Static allocation needed whenever a system needs a guest restricted to some
+known memory area due to hardware limitations reason. For example, some device
+can only do DMA to a specific part of the memory.
+
+Limitations:
+ * There is no consideration for PV devices at the moment.
+
+## Design on Static Allocation
+
+Static allocation refers to system or sub-system(domains) for which memory
+areas are pre-defined by configuration using physical address ranges.
+
+These pre-defined memory, -- Static Momery, as parts of RAM reserved in the
s/Momery/Memory/
+beginning, shall never go to heap allocator or boot allocator for any use.
I think you mean "buddy" rather than "heap". Looking at your code, you
are treating static memory region as domheap pages.
+
+### Static Allocation for Domains
+
+### New Deivce Tree Node: `xen,static_mem`
S/Deivce/
+
+Here introduces new `xen,static_mem` node to define static memory nodes for
+one specific domain.
+
+For domains on static allocation, users need to pre-define guest RAM regions in
+configuration, through `xen,static_mem` node under approriate `domUx` node.
+
+Here is one example:
+
+
+domU1 {
+compatible = "xen,domain";
+#address-cells = <0x2>;
+#size-cells = <0x2>;
+cpus = <2>;
+xen,static-mem = <0x0 0xa000 0x0 0x2000>;
+...
+};
+
+RAM at 0xa000 of 512 MB are static memory reserved for domU1 as its RAM.
+
+### New Page Flag: `PGC_reserved`
+
+In order to differentiate and manage pages reserved as static memory with
+those which are allocated from heap allocator for normal domains, we shall
+introduce a new page flag `PGC_reserved` to tell.
+
+Grant pages `PGC_reserved` when initializing static memory.
+
+### New linked page list: `reserved_page_list` in `struct domain`
+
+Right now, for normal domains, on assigning pages to domain, pages allocated
+from heap allocator as guest RAM shall be inserted to one linked page
+list `page_list` for later managing and storing.
+
+So in order to tell, pages allocated from static memory, shall be inserted
+to a different linked page list `reserved_page_list`.
You already have the flag ``PGC_reserved`` to indicate whether the
memory is reserved or not. So why do you also need to link list it?
+
+Later, when domain get destroyed and memory relinquished, only pages in
+`page_list` go back to heap, and pages in `reserved_page_list` shall not.
While going through the series, I could not find any code implementing
this. However, this is not enough to prevent a
Re: [PATCH v4 03/10] libx86: introduce helper to fetch msr entry
On 18.05.2021 12:50, Roger Pau Monné wrote: > On Mon, May 17, 2021 at 05:43:33PM +0200, Jan Beulich wrote: >> On 07.05.2021 13:04, Roger Pau Monne wrote: >>> @@ -91,6 +91,21 @@ int x86_msr_copy_from_buffer(struct msr_policy *policy, >>> const msr_entry_buffer_t msrs, uint32_t >>> nr_entries, >>> uint32_t *err_msr); >>> >>> +/** >>> + * Get a MSR entry from a policy object. >>> + * >>> + * @param policy The msr_policy object. >>> + * @param idx The index. >>> + * @returns a pointer to the requested leaf or NULL in case of error. >>> + * >>> + * Do not call this function directly and instead use x86_msr_get_entry >>> that >>> + * will deal with both const and non-const policies returning a pointer >>> with >>> + * constness matching that of the input. >>> + */ >>> +const uint64_t *_x86_msr_get_entry(const struct msr_policy *policy, >>> + uint32_t idx); >>> +#define x86_msr_get_entry(p, i) \ >>> +((__typeof__(&(p)->platform_info.raw))_x86_msr_get_entry(p, i)) >>> #endif /* !XEN_LIB_X86_MSR_H */ >> >> Just two nits: I think it would be nice to retain a blank line ahead of >> the #endif. And here as well as in the CPUID counterpart you introduce, >> strictly speaking, name space violations (via the leading underscore). > > I guess another option would be to name the function > x86_msr_get_entry_const, and keep the x86_msr_get_entry macro as-is. > > Does that seem better? This would be fine with me, as would be a trailing underscore or a double underscore after e.g. the first name component. Jan
Re: [PATCH] libelf: improve PVH elfnote parsing
On Fri, May 14, 2021 at 11:11:14AM -0400, Jason Andryuk wrote: > On Fri, May 14, 2021 at 9:50 AM Roger Pau Monne wrote: > > > > Pass an hvm boolean parameter to the elf note parsing and checking > > routines, so that better checking can be done in case libelf is > > dealing with an hvm container. > > > > elf_xen_note_check shouldn't return early unless PHYS32_ENTRY is set > > and the container is of type HVM, or else the loader and version > > checks would be avoided for kernels intended to be booted as PV but > > that also have PHYS32_ENTRY set. > > > > Adjust elf_xen_addr_calc_check so that the virtual addresses are > > actually physical ones (by setting virt_base and elf_paddr_offset to > > zero) when the container is of type HVM, as that container is always > > started with paging disabled. > > Should elf_xen_addr_calc_check be changed so that PV operates on > virtual addresses and HVM operates on physical addresses? Right... I was aiming with getting away with something simpler and just assume phys == virt on HVM in order to avoid more complicated changes and the need to introduce new fields on the structure. > I worked on some patches for this a while back, but lost track when > other work pulled me away. I'll send out what I had, but I think I > had not tested many of the cases. Also, I had other questions about > the approach. Fundamentally, what notes and limits need to be checked > for PVH vs. PV? Those are only sanity checks to assert that the image is kind of fine, libelf also has checks when loading stuff to make sure a malicious elf payload cannot fool the loader. I'm unlikely to be able to do much work on this aside from this current patch. Thanks, Roger.
Re: [PATCH 08/10] xen/arm: introduce reserved_page_list
On 18.05.2021 10:38, Penny Zheng wrote: > Hi Jan > >> -Original Message- >> From: Jan Beulich >> Sent: Tuesday, May 18, 2021 3:39 PM >> To: Penny Zheng >> Cc: Bertrand Marquis ; Wei Chen >> ; nd ; xen-devel@lists.xenproject.org; >> sstabell...@kernel.org; jul...@xen.org >> Subject: Re: [PATCH 08/10] xen/arm: introduce reserved_page_list >> >> On 18.05.2021 07:21, Penny Zheng wrote: >>> Since page_list under struct domain refers to linked pages as gueast >>> RAM allocated from heap, it should not include reserved pages of static >> memory. >>> >>> The number of PGC_reserved pages assigned to a domain is tracked in a >>> new 'reserved_pages' counter. Also introduce a new reserved_page_list >>> to link pages of static memory. Let page_to_list return >>> reserved_page_list, when flag is PGC_reserved. >>> >>> Later, when domain get destroyed or restarted, those new values will >>> help relinquish memory to proper place, not been given back to heap. >>> >>> Signed-off-by: Penny Zheng >>> --- >>> xen/common/domain.c | 1 + >>> xen/common/page_alloc.c | 7 +-- >>> xen/include/xen/sched.h | 5 + >>> 3 files changed, 11 insertions(+), 2 deletions(-) >> >> This contradicts the title's prefix: There's no Arm-specific change here at >> all. >> But imo the title is correct, and the changes should be Arm-specific. There's >> no point having struct domain fields on e.g. x86 which aren't used there at >> all. >> > > Yep, you're right. > I'll add ifdefs in the following changes. As necessary, please. Moving stuff to Arm-specific files would be preferable. Jan
Re: [PATCH 07/10] xen/arm: intruduce alloc_domstatic_pages
On 18.05.2021 10:57, Penny Zheng wrote:
>> From: Jan Beulich
>> Sent: Tuesday, May 18, 2021 3:35 PM
>>
>> On 18.05.2021 07:21, Penny Zheng wrote:
>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -2447,6 +2447,9 @@ int assign_pages(
>>> {
>>> ASSERT(page_get_owner([i]) == NULL);
>>> page_set_owner([i], d);
>>> +/* use page_set_reserved_owner to set its reserved domain owner.
>> */
>>> +if ( (pg[i].count_info & PGC_reserved) )
>>> +page_set_reserved_owner([i], d);
>>
>> Now this is puzzling: What's the point of setting two owner fields to the
>> same
>> value? I also don't recall you having introduced
>> page_set_reserved_owner() for x86, so how is this going to build there?
>>
>
> Thanks for pointing out that it will fail on x86.
> As for the same value, sure, I shall change it to domid_t domid to record its
> reserved owner.
> Only domid is enough for differentiate.
> And even when domain get rebooted, struct domain may be destroyed, but domid
> will stays
> The same.
Will it? Are you intending to put in place restrictions that make it
impossible for the ID to get re-used by another domain?
> Major user cases for domain on static allocation are referring to the whole
> system are static,
> No runtime creation.
Right, but that's not currently enforced afaics. If you would
enforce it, it may simplify a number of things.
>>> @@ -2509,6 +2512,56 @@ struct page_info *alloc_domheap_pages(
>>> return pg;
>>> }
>>>
>>> +/*
>>> + * Allocate nr_pfns contiguous pages, starting at #start, of static
>>> +memory,
>>> + * then assign them to one specific domain #d.
>>> + * It is the equivalent of alloc_domheap_pages for static memory.
>>> + */
>>> +struct page_info *alloc_domstatic_pages(
>>> +struct domain *d, unsigned long nr_pfns, paddr_t start,
>>> +unsigned int memflags)
>>> +{
>>> +struct page_info *pg = NULL;
>>> +unsigned long dma_size;
>>> +
>>> +ASSERT(!in_irq());
>>> +
>>> +if ( memflags & MEMF_no_owner )
>>> +memflags |= MEMF_no_refcount;
>>> +
>>> +if ( !dma_bitsize )
>>> +memflags &= ~MEMF_no_dma;
>>> +else
>>> +{
>>> +dma_size = 1ul << bits_to_zone(dma_bitsize);
>>> +/* Starting address shall meet the DMA limitation. */
>>> +if ( dma_size && start < dma_size )
>>> +return NULL;
>>
>> It is the entire range (i.e. in particular the last byte) which needs to
>> meet such
>> a restriction. I'm not convinced though that DMA width restrictions and
>> static
>> allocation are sensible to coexist.
>>
>
> FWIT, if starting address meets the limitation, the last byte, greater than
> starting
> address shall meet it too.
I'm afraid I don't know what you're meaning to tell me here.
Jan
Re: [PATCH] libelf: improve PVH elfnote parsing
On Mon, May 17, 2021 at 01:09:11PM +0200, Jan Beulich wrote:
> On 14.05.2021 15:50, Roger Pau Monne wrote:
> > @@ -426,7 +426,7 @@ static elf_errorstatus elf_xen_addr_calc_check(struct
> > elf_binary *elf,
> > }
> >
> > /* Initial guess for virt_base is 0 if it is not explicitly defined. */
> > -if ( parms->virt_base == UNSET_ADDR )
> > +if ( parms->virt_base == UNSET_ADDR || hvm )
> > {
> > parms->virt_base = 0;
> > elf_msg(elf, "ELF: VIRT_BASE unset, using %#" PRIx64 "\n",
> > @@ -442,7 +442,7 @@ static elf_errorstatus elf_xen_addr_calc_check(struct
> > elf_binary *elf,
> > * If we are using the modern ELF notes interface then the default
> > * is 0.
> > */
> > -if ( parms->elf_paddr_offset == UNSET_ADDR )
> > +if ( parms->elf_paddr_offset == UNSET_ADDR || hvm )
> > {
> > if ( parms->elf_note_start )
> > parms->elf_paddr_offset = 0;
>
> Both of these would want their respective comments also updated, I
> think: There's no defaulting or guessing really in PVH mode, is
> there?
>
> > @@ -456,8 +456,13 @@ static elf_errorstatus elf_xen_addr_calc_check(struct
> > elf_binary *elf,
> > parms->virt_kstart = elf->pstart + virt_offset;
> > parms->virt_kend = elf->pend + virt_offset;
> >
> > -if ( parms->virt_entry == UNSET_ADDR )
> > -parms->virt_entry = elf_uval(elf, elf->ehdr, e_entry);
> > +if ( parms->virt_entry == UNSET_ADDR || hvm )
> > +{
> > +if ( parms->phys_entry != UNSET_ADDR32 )
>
> Don't you need "&& hvm" here to prevent ...
>
> > +parms->virt_entry = parms->phys_entry;
>
> ... this taking effect for a kernel capable of running in both
> PV and PVH modes, instead of ...
>
> > +else
> > +parms->virt_entry = elf_uval(elf, elf->ehdr, e_entry);
>
> ... this (when actually in PV mode)?
Oh, I somehow assumed that PV guests _must_ provide the entry point in
XEN_ELFNOTE_ENTRY, but I don't think that's the case. Will update and
send a new version.
Thanks, Roger.
Re: [PATCH 08/10] xen/arm: introduce reserved_page_list
Hi Penny,
On 18/05/2021 06:21, Penny Zheng wrote:
Since page_list under struct domain refers to linked pages as gueast RAM
s/gueast/guest/
allocated from heap, it should not include reserved pages of static memory.
You already have PGC_reserved to indicate they are "static memory". So
why do you need yet another list?
The number of PGC_reserved pages assigned to a domain is tracked in
a new 'reserved_pages' counter. Also introduce a new reserved_page_list
to link pages of static memory. Let page_to_list return reserved_page_list,
when flag is PGC_reserved.
Later, when domain get destroyed or restarted, those new values will help
relinquish memory to proper place, not been given back to heap.
Signed-off-by: Penny Zheng
---
xen/common/domain.c | 1 +
xen/common/page_alloc.c | 7 +--
xen/include/xen/sched.h | 5 +
3 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6b71c6d6a9..c38afd2969 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -578,6 +578,7 @@ struct domain *domain_create(domid_t domid,
INIT_PAGE_LIST_HEAD(>page_list);
INIT_PAGE_LIST_HEAD(>extra_page_list);
INIT_PAGE_LIST_HEAD(>xenpage_list);
+INIT_PAGE_LIST_HEAD(>reserved_page_list);
spin_lock_init(>node_affinity_lock);
d->node_affinity = NODE_MASK_ALL;
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index f1f1296a61..e3f07ec6c5 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2410,7 +2410,7 @@ int assign_pages(
for ( i = 0; i < nr_pfns; i++ )
{
-ASSERT(!(pg[i].count_info & ~PGC_extra));
+ASSERT(!(pg[i].count_info & ~(PGC_extra | PGC_reserved)));
I think this change belongs to the previous patch.
if ( pg[i].count_info & PGC_extra )
extra_pages++;
}
@@ -2439,6 +2439,9 @@ int assign_pages(
}
}
+if ( pg[0].count_info & PGC_reserved )
+d->reserved_pages += nr_pfns;
reserved_pages doesn't seem to be ever read or decremented. So why do
you need it?
+
if ( !(memflags & MEMF_no_refcount) &&
unlikely(domain_adjust_tot_pages(d, nr_pfns) == nr_pfns) )
get_knownalive_domain(d);
@@ -2452,7 +2455,7 @@ int assign_pages(
page_set_reserved_owner([i], d);
smp_wmb(); /* Domain pointer must be visible before updating refcnt.
*/
pg[i].count_info =
-(pg[i].count_info & PGC_extra) | PGC_allocated | 1;
+(pg[i].count_info & (PGC_extra | PGC_reserved)) | PGC_allocated |
1;
Same here.
page_list_add_tail([i], page_to_list(d, [i]));
}
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 3982167144..b6333ed8bb 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -368,6 +368,7 @@ struct domain
struct page_list_head page_list; /* linked list */
struct page_list_head extra_page_list; /* linked list (size extra_pages)
*/
struct page_list_head xenpage_list; /* linked list (size xenheap_pages) */
+struct page_list_head reserved_page_list; /* linked list (size reserved
pages) */
/*
* This field should only be directly accessed by
domain_adjust_tot_pages()
@@ -379,6 +380,7 @@ struct domain
unsigned int outstanding_pages; /* pages claimed but not possessed */
unsigned int max_pages; /* maximum value for
domain_tot_pages() */
unsigned int extra_pages; /* pages not included in
domain_tot_pages() */
+unsigned int reserved_pages;/* pages of static memory */
atomic_t shr_pages; /* shared pages */
atomic_t paged_pages; /* paged-out pages */
@@ -588,6 +590,9 @@ static inline struct page_list_head *page_to_list(
if ( pg->count_info & PGC_extra )
return >extra_page_list;
+if ( pg->count_info & PGC_reserved )
+return >reserved_page_list;
+
return >page_list;
}
Cheers,
--
Julien Grall
Re: [PATCH v4 03/10] libx86: introduce helper to fetch msr entry
On Mon, May 17, 2021 at 05:43:33PM +0200, Jan Beulich wrote: > On 07.05.2021 13:04, Roger Pau Monne wrote: > > @@ -91,6 +91,21 @@ int x86_msr_copy_from_buffer(struct msr_policy *policy, > > const msr_entry_buffer_t msrs, uint32_t > > nr_entries, > > uint32_t *err_msr); > > > > +/** > > + * Get a MSR entry from a policy object. > > + * > > + * @param policy The msr_policy object. > > + * @param idx The index. > > + * @returns a pointer to the requested leaf or NULL in case of error. > > + * > > + * Do not call this function directly and instead use x86_msr_get_entry > > that > > + * will deal with both const and non-const policies returning a pointer > > with > > + * constness matching that of the input. > > + */ > > +const uint64_t *_x86_msr_get_entry(const struct msr_policy *policy, > > + uint32_t idx); > > +#define x86_msr_get_entry(p, i) \ > > +((__typeof__(&(p)->platform_info.raw))_x86_msr_get_entry(p, i)) > > #endif /* !XEN_LIB_X86_MSR_H */ > > Just two nits: I think it would be nice to retain a blank line ahead of > the #endif. And here as well as in the CPUID counterpart you introduce, > strictly speaking, name space violations (via the leading underscore). I guess another option would be to name the function x86_msr_get_entry_const, and keep the x86_msr_get_entry macro as-is. Does that seem better? Thanks, Roger.
Re: [PATCH 04/10] xen/arm: static memory initialization
On 18.05.2021 11:51, Penny Zheng wrote: >> From: Jan Beulich >> Sent: Tuesday, May 18, 2021 3:16 PM >> >> On 18.05.2021 07:21, Penny Zheng wrote: >>> This patch introduces static memory initialization, during system RAM boot >> up. >>> >>> New func init_staticmem_pages is the equivalent of init_heap_pages, >>> responsible for static memory initialization. >>> >>> Helper func free_staticmem_pages is the equivalent of free_heap_pages, >>> to free nr_pfns pages of static memory. >>> For each page, it includes the following steps: >>> 1. change page state from in-use(also initialization state) to free >>> state and grant PGC_reserved. >>> 2. set its owner NULL and make sure this page is not a guest frame any >>> more >> >> But isn't the goal (as per the previous patch) to associate such pages with a >> _specific_ domain? >> > > Free_staticmem_pages are alike free_heap_pages, are not used only for > initialization. > Freeing used pages to unused are also included. > Here, setting its owner NULL is to set owner in used field NULL. I'm afraid I still don't understand. > Still, I need to add more explanation here. Yes please. >>> @@ -1512,6 +1515,49 @@ static void free_heap_pages( >>> spin_unlock(_lock); >>> } >>> >>> +/* Equivalent of free_heap_pages to free nr_pfns pages of static >>> +memory. */ static void free_staticmem_pages(struct page_info *pg, >> unsigned long nr_pfns, >>> + bool need_scrub) >> >> Right now this function gets called only from an __init one. Unless it is >> intended to gain further callers, it should be marked __init itself then. >> Otherwise it should be made sure that other architectures don't include this >> (dead there) code. >> > > Sure, I'll add __init. Thx. Didn't I see a 2nd call to the function later in the series? That one doesn't permit the function to be __init, iirc. Jan
Re: [PATCH] xen-netback: Check for hotplug-status existence before watching
On Tue, May 18, 2021 at 09:48:25AM +, Durrant, Paul wrote: > > -Original Message- > > From: Marek Marczykowski-Górecki > > > > On Tue, May 18, 2021 at 09:34:45AM +, Durrant, Paul wrote: > > > > -Original Message- > > > > From: Marek Marczykowski-Górecki > > > > > > > > On Tue, May 18, 2021 at 07:57:16AM +0100, Paul Durrant wrote: > > > > > Why is that missing? We're going behind the back of the toolstack to > > > > > do the > > > > > unbind and bind so why should we expect it to re-execute a hotplug > > > > > script? > > > > > > > > Ok, then simply execute the whole hotplug script (instead of its subset) > > > > after re-loading the backend module and everything will be fine. > > > > > > > > For example like this: > > > > XENBUS_PATH=backend/vif/$DOMID/$VIF \ > > > > XENBUS_TYPE=vif \ > > > > XENBUS_BASE_PATH=backend \ > > > > script=/etc/xen/scripts/vif-bridge \ > > > > vif=vif.$DOMID.$VIF \ > > > > /etc/xen/scripts/vif-bridge online > > > > > > > > > > ... as long as there's no xenstore fall-out that the guest can observe. > > > > Backend will set state to XenbusStateInitWait on load anyway... > > > > Oh, that sounds like a bug then... It ought to go straight to connected if > the frontend is already there. To me this sounds very suspicious. But if that's really what should backend do, then it would "solve" also hotplug-status node issue. See the end of netback_probe() function. But I think if you start processing traffic before hotplug script configures the interface (so - without switching to XenbusStateInitWait and waiting for hotplug-status node), you'll post some packets into not enabled interface, which I think will drop them (not queue). TCP will be fine with that, but many other protocols not. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab signature.asc Description: PGP signature
Re: [PATCH 07/10] xen/arm: intruduce alloc_domstatic_pages
Hi Penny,
Title: s/intruduce/introduce/
On 18/05/2021 06:21, Penny Zheng wrote:
alloc_domstatic_pages is the equivalent of alloc_domheap_pages for
static mmeory, and it is to allocate nr_pfns pages of static memory
and assign them to one specific domain.
It uses alloc_staticmen_pages to get nr_pages pages of static memory,
then on success, it will use assign_pages to assign those pages to
one specific domain, including using page_set_reserved_owner to set its
reserved domain owner.
Signed-off-by: Penny Zheng
---
xen/common/page_alloc.c | 53 +
xen/include/xen/mm.h| 4
2 files changed, 57 insertions(+)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 0eb9f22a00..f1f1296a61 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2447,6 +2447,9 @@ int assign_pages(
{
ASSERT(page_get_owner([i]) == NULL);
page_set_owner([i], d);
+/* use page_set_reserved_owner to set its reserved domain owner. */
+if ( (pg[i].count_info & PGC_reserved) )
+page_set_reserved_owner([i], d);
I have skimmed through the rest of the series and couldn't find anyone
calling page_get_reserved_owner(). The value is also going to be the
exact same as page_set_owner().
So why do we need it?
smp_wmb(); /* Domain pointer must be visible before updating refcnt.
*/
pg[i].count_info =
(pg[i].count_info & PGC_extra) | PGC_allocated | 1;
This will clobber PGC_reserved.
@@ -2509,6 +2512,56 @@ struct page_info *alloc_domheap_pages(
return pg;
}
+/*
+ * Allocate nr_pfns contiguous pages, starting at #start, of static memory,
s/nr_pfns/nr_mfns/
+ * then assign them to one specific domain #d.
+ * It is the equivalent of alloc_domheap_pages for static memory.
+ */
+struct page_info *alloc_domstatic_pages(
+struct domain *d, unsigned long nr_pfns, paddr_t start,
s/nr_pfns/nf_mfns/. Also, I would the third parameter to be an mfn_t.
+unsigned int memflags)
+{
+struct page_info *pg = NULL;
+unsigned long dma_size;
+
+ASSERT(!in_irq());
+
+if ( memflags & MEMF_no_owner )
+memflags |= MEMF_no_refcount;
+
+if ( !dma_bitsize )
+memflags &= ~MEMF_no_dma;
+else
+{
+dma_size = 1ul << bits_to_zone(dma_bitsize);
+/* Starting address shall meet the DMA limitation. */
+if ( dma_size && start < dma_size )
+return NULL;
+}
+
+pg = alloc_staticmem_pages(nr_pfns, start, memflags);
+if ( !pg )
+return NULL;
+
+if ( d && !(memflags & MEMF_no_owner) )
+{
+if ( memflags & MEMF_no_refcount )
+{
+unsigned long i;
+
+for ( i = 0; i < nr_pfns; i++ )
+pg[i].count_info = PGC_extra;
+}
+if ( assign_pages(d, pg, nr_pfns, memflags) )
+{
+free_staticmem_pages(pg, nr_pfns, memflags & MEMF_no_scrub);
+return NULL;
+}
+}
+
+return pg;
+}
+
void free_domheap_pages(struct page_info *pg, unsigned int order)
{
struct domain *d = page_get_owner(pg);
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index dcf9daaa46..e45987f0ed 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -111,6 +111,10 @@ unsigned long __must_check domain_adjust_tot_pages(struct
domain *d,
int domain_set_outstanding_pages(struct domain *d, unsigned long pages);
void get_outstanding_claims(uint64_t *free_pages, uint64_t
*outstanding_pages);
+/* Static Memory */
+struct page_info *alloc_domstatic_pages(struct domain *d,
+unsigned long nr_pfns, paddr_t start, unsigned int memflags);
+
/* Domain suballocator. These functions are *not* interrupt-safe.*/
void init_domheap_pages(paddr_t ps, paddr_t pe);
struct page_info *alloc_domheap_pages(
Cheers,
--
Julien Grall
Re: [PATCH 06/10] xen: replace order with nr_pfns in assign_pages for better compatibility
Hi Penny, On 18/05/2021 06:21, Penny Zheng wrote: Function parameter order in assign_pages is always used as 1ul << order, referring to 2@order pages. Now, for better compatibility with new static memory, order shall be replaced with nr_pfns pointing to page count with no constraint, like 250MB. We have similar requirements for LiveUpdate because are preserving the memory with a number of pages (rather than a power-of-two). With the current interface would be need to split the range in a power of 2 which is a bit of pain. However, I think I would prefer if we introduce a new interface (maybe assign_pages_nr()) rather than change the meaning of the field. This is for two reasons: 1) We limit the risk to make mistake when backporting a patch touch assign_pages(). 2) Adding (1UL << order) for pretty much all the caller is not nice. Cheers, -- Julien Grall
Re: [PATCH 05/10] xen/arm: introduce alloc_staticmem_pages
Hi Penny,
On 18/05/2021 06:21, Penny Zheng wrote:
alloc_staticmem_pages is designated to allocate nr_pfns contiguous
pages of static memory. And it is the equivalent of alloc_heap_pages
for static memory.
This commit only covers allocating at specified starting address.
For each page, it shall check if the page is reserved
(PGC_reserved) and free. It shall also do a set of necessary
initialization, which are mostly the same ones in alloc_heap_pages,
like, following the same cache-coherency policy and turning page
status into PGC_state_used, etc.
Signed-off-by: Penny Zheng
---
xen/common/page_alloc.c | 64 +
1 file changed, 64 insertions(+)
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 58b53c6ac2..adf2889e76 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1068,6 +1068,70 @@ static struct page_info *alloc_heap_pages(
return pg;
}
+/*
+ * Allocate nr_pfns contiguous pages, starting at #start, of static memory.
+ * It is the equivalent of alloc_heap_pages for static memory
+ */
+static struct page_info *alloc_staticmem_pages(unsigned long nr_pfns,
This wants to be nr_mfns.
+paddr_t start,
I would prefer if this helper takes an mfn_t in parameter.
+unsigned int memflags)
+{
+bool need_tlbflush = false;
+uint32_t tlbflush_timestamp = 0;
+unsigned int i;
+struct page_info *pg;
+mfn_t s_mfn;
+
+/* For now, it only supports allocating at specified address. */
+s_mfn = maddr_to_mfn(start);
+pg = mfn_to_page(s_mfn);
We should avoid to make the assumption the start address will be valid.
So you want to call mfn_valid() first.
At the same time, there is no guarantee that if the first page is valid,
then the next nr_pfns will be. So the check should be performed for all
of them.
+if ( !pg )
+return NULL;
+
+for ( i = 0; i < nr_pfns; i++)
+{
+/*
+ * Reference count must continuously be zero for free pages
+ * of static memory(PGC_reserved).
+ */
+ASSERT(pg[i].count_info & PGC_reserved);
+if ( (pg[i].count_info & ~PGC_reserved) != PGC_state_free )
+{
+printk(XENLOG_ERR
+"Reference count must continuously be zero for free pages"
+"pg[%u] MFN %"PRI_mfn" c=%#lx t=%#x\n",
+i, mfn_x(page_to_mfn(pg + i)),
+pg[i].count_info, pg[i].tlbflush_timestamp);
+BUG();
So we would crash Xen if the caller pass a wrong range. Is it what we want?
Also, who is going to prevent concurrent access?
+}
+
+if ( !(memflags & MEMF_no_tlbflush) )
+accumulate_tlbflush(_tlbflush, [i],
+_timestamp);
+
+/*
+ * Reserve flag PGC_reserved and change page state
+ * to PGC_state_inuse.
+ */
+pg[i].count_info = (pg[i].count_info & PGC_reserved) | PGC_state_inuse;
+/* Initialise fields which have other uses for free pages. */
+pg[i].u.inuse.type_info = 0;
+page_set_owner([i], NULL);
+
+/*
+ * Ensure cache and RAM are consistent for platforms where the
+ * guest can control its own visibility of/through the cache.
+ */
+flush_page_to_ram(mfn_x(page_to_mfn([i])),
+!(memflags & MEMF_no_icache_flush));
+}
+
+if ( need_tlbflush )
+filtered_flush_tlb_mask(tlbflush_timestamp);
+
+return pg;
+}
+
/* Remove any offlined page in the buddy pointed to by head. */
static int reserve_offlined_page(struct page_info *head)
{
Cheers,
--
Julien Grall
Re: [PATCH 05/10] xen/arm: introduce alloc_staticmem_pages
Hi Jan, On 18/05/2021 08:24, Jan Beulich wrote: On 18.05.2021 07:21, Penny Zheng wrote: + * to PGC_state_inuse. + */ +pg[i].count_info = (pg[i].count_info & PGC_reserved) | PGC_state_inuse; +/* Initialise fields which have other uses for free pages. */ +pg[i].u.inuse.type_info = 0; +page_set_owner([i], NULL); + +/* + * Ensure cache and RAM are consistent for platforms where the + * guest can control its own visibility of/through the cache. + */ +flush_page_to_ram(mfn_x(page_to_mfn([i])), +!(memflags & MEMF_no_icache_flush)); +} + +if ( need_tlbflush ) +filtered_flush_tlb_mask(tlbflush_timestamp); With reserved pages dedicated to a specific domain, in how far is it possible that stale mappings from a prior use can still be around, making such TLB flushing necessary? I would rather not make the assumption. I can see future where we just want to allocate memory from a static pool that may be shared with multiple domains. Cheers, -- Julien Grall
Re: [PATCH 04/10] xen/arm: static memory initialization
On 18/05/2021 11:00, Julien Grall wrote:
Hi Penny,
On 18/05/2021 06:21, Penny Zheng wrote:
This patch introduces static memory initialization, during system RAM
boot up.
New func init_staticmem_pages is the equivalent of init_heap_pages,
responsible
for static memory initialization.
Helper func free_staticmem_pages is the equivalent of free_heap_pages,
to free
nr_pfns pages of static memory.
For each page, it includes the following steps:
1. change page state from in-use(also initialization state) to free
state and
grant PGC_reserved.
2. set its owner NULL and make sure this page is not a guest frame any
more
3. follow the same cache coherency policy in free_heap_pages
4. scrub the page in need
Signed-off-by: Penny Zheng
---
xen/arch/arm/setup.c | 2 ++
xen/common/page_alloc.c | 70 +
xen/include/xen/mm.h | 3 ++
3 files changed, 75 insertions(+)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 444dbbd676..f80162c478 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -818,6 +818,8 @@ static void __init setup_mm(void)
setup_frametable_mappings(ram_start, ram_end);
max_page = PFN_DOWN(ram_end);
+
+ init_staticmem_pages();
}
#endif
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index ace6333c18..58b53c6ac2 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -150,6 +150,9 @@
#define p2m_pod_offline_or_broken_hit(pg) 0
#define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
#endif
+#ifdef CONFIG_ARM_64
+#include
+#endif
/*
* Comma-separated list of hexadecimal page numbers containing bad
bytes.
@@ -1512,6 +1515,49 @@ static void free_heap_pages(
spin_unlock(_lock);
}
+/* Equivalent of free_heap_pages to free nr_pfns pages of static
memory. */
+static void free_staticmem_pages(struct page_info *pg, unsigned long
nr_pfns,
This function is dealing with MFNs, so the second parameter should be
called nr_mfns.
+ bool need_scrub)
+{
+ mfn_t mfn = page_to_mfn(pg);
+ int i;
+
+ for ( i = 0; i < nr_pfns; i++ )
+ {
+ switch ( pg[i].count_info & PGC_state )
+ {
+ case PGC_state_inuse:
+ BUG_ON(pg[i].count_info & PGC_broken);
+ /* Make it free and reserved. */
+ pg[i].count_info = PGC_state_free | PGC_reserved;
+ break;
+
+ default:
+ printk(XENLOG_ERR
+ "Page state shall be only in PGC_state_inuse. "
+ "pg[%u] MFN %"PRI_mfn" count_info=%#lx
tlbflush_timestamp=%#x.\n",
+ i, mfn_x(mfn) + i,
+ pg[i].count_info,
+ pg[i].tlbflush_timestamp);
+ BUG();
+ }
+
+ /*
+ * Follow the same cache coherence scheme in free_heap_pages.
+ * If a page has no owner it will need no safety TLB flush.
+ */
+ pg[i].u.free.need_tlbflush = (page_get_owner([i]) != NULL);
+ if ( pg[i].u.free.need_tlbflush )
+ page_set_tlbflush_timestamp([i]);
+
+ /* This page is not a guest frame any more. */
+ page_set_owner([i], NULL);
+ set_gpfn_from_mfn(mfn_x(mfn) + i, INVALID_M2P_ENTRY);
The code looks quite similar to free_heap_pages(). Could we possibly
create an helper which can be called from both?
+
+ if ( need_scrub )
+ scrub_one_page([i]);
So the scrubbing will be synchronous. Is it what we want?
You also seem to miss the flush the call to flush_page_to_ram().
H... Sorry I looked at the wrong function. This is not necessary for
the free part.
+ }
+}
/*
* Following rules applied for page offline:
@@ -1828,6 +1874,30 @@ static void init_heap_pages(
}
}
+/* Equivalent of init_heap_pages to do static memory initialization */
+void __init init_staticmem_pages(void)
+{
+ int bank;
+
+ /*
+ * TODO: Considering NUMA-support scenario.
+ */
+ for ( bank = 0 ; bank < bootinfo.static_mem.nr_banks; bank++ )
bootinfo is arm specific, so this code should live in arch/arm rather
than common/.
+ {
+ paddr_t bank_start = bootinfo.static_mem.bank[bank].start;
+ paddr_t bank_size = bootinfo.static_mem.bank[bank].size;
+ paddr_t bank_end = bank_start + bank_size;
+
+ bank_start = round_pgup(bank_start);
+ bank_end = round_pgdown(bank_end);
+ if ( bank_end <= bank_start )
+ return;
+
+ free_staticmem_pages(maddr_to_page(bank_start),
+ (bank_end - bank_start) >> PAGE_SHIFT,
false);
+ }
+}
+
static unsigned long avail_heap_pages(
unsigned int zone_lo, unsigned int zone_hi, unsigned int node)
{
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 667f9dac83..8b1a2207b2 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -85,6 +85,9 @@ bool scrub_free_pages(void);
} while ( false
Re: [PATCH 04/10] xen/arm: static memory initialization
Hi Penny,
On 18/05/2021 06:21, Penny Zheng wrote:
This patch introduces static memory initialization, during system RAM boot up.
New func init_staticmem_pages is the equivalent of init_heap_pages, responsible
for static memory initialization.
Helper func free_staticmem_pages is the equivalent of free_heap_pages, to free
nr_pfns pages of static memory.
For each page, it includes the following steps:
1. change page state from in-use(also initialization state) to free state and
grant PGC_reserved.
2. set its owner NULL and make sure this page is not a guest frame any more
3. follow the same cache coherency policy in free_heap_pages
4. scrub the page in need
Signed-off-by: Penny Zheng
---
xen/arch/arm/setup.c| 2 ++
xen/common/page_alloc.c | 70 +
xen/include/xen/mm.h| 3 ++
3 files changed, 75 insertions(+)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 444dbbd676..f80162c478 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -818,6 +818,8 @@ static void __init setup_mm(void)
setup_frametable_mappings(ram_start, ram_end);
max_page = PFN_DOWN(ram_end);
+
+init_staticmem_pages();
}
#endif
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index ace6333c18..58b53c6ac2 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -150,6 +150,9 @@
#define p2m_pod_offline_or_broken_hit(pg) 0
#define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
#endif
+#ifdef CONFIG_ARM_64
+#include
+#endif
/*
* Comma-separated list of hexadecimal page numbers containing bad bytes.
@@ -1512,6 +1515,49 @@ static void free_heap_pages(
spin_unlock(_lock);
}
+/* Equivalent of free_heap_pages to free nr_pfns pages of static memory. */
+static void free_staticmem_pages(struct page_info *pg, unsigned long nr_pfns,
This function is dealing with MFNs, so the second parameter should be
called nr_mfns.
+ bool need_scrub)
+{
+mfn_t mfn = page_to_mfn(pg);
+int i;
+
+for ( i = 0; i < nr_pfns; i++ )
+{
+switch ( pg[i].count_info & PGC_state )
+{
+case PGC_state_inuse:
+BUG_ON(pg[i].count_info & PGC_broken);
+/* Make it free and reserved. */
+pg[i].count_info = PGC_state_free | PGC_reserved;
+break;
+
+default:
+printk(XENLOG_ERR
+ "Page state shall be only in PGC_state_inuse. "
+ "pg[%u] MFN %"PRI_mfn" count_info=%#lx
tlbflush_timestamp=%#x.\n",
+ i, mfn_x(mfn) + i,
+ pg[i].count_info,
+ pg[i].tlbflush_timestamp);
+BUG();
+}
+
+/*
+ * Follow the same cache coherence scheme in free_heap_pages.
+ * If a page has no owner it will need no safety TLB flush.
+ */
+pg[i].u.free.need_tlbflush = (page_get_owner([i]) != NULL);
+if ( pg[i].u.free.need_tlbflush )
+page_set_tlbflush_timestamp([i]);
+
+/* This page is not a guest frame any more. */
+page_set_owner([i], NULL);
+set_gpfn_from_mfn(mfn_x(mfn) + i, INVALID_M2P_ENTRY);
The code looks quite similar to free_heap_pages(). Could we possibly
create an helper which can be called from both?
+
+if ( need_scrub )
+scrub_one_page([i]);
So the scrubbing will be synchronous. Is it what we want?
You also seem to miss the flush the call to flush_page_to_ram().
+}
+}
/*
* Following rules applied for page offline:
@@ -1828,6 +1874,30 @@ static void init_heap_pages(
}
}
+/* Equivalent of init_heap_pages to do static memory initialization */
+void __init init_staticmem_pages(void)
+{
+int bank;
+
+/*
+ * TODO: Considering NUMA-support scenario.
+ */
+for ( bank = 0 ; bank < bootinfo.static_mem.nr_banks; bank++ )
bootinfo is arm specific, so this code should live in arch/arm rather
than common/.
+{
+paddr_t bank_start = bootinfo.static_mem.bank[bank].start;
+paddr_t bank_size = bootinfo.static_mem.bank[bank].size;
+paddr_t bank_end = bank_start + bank_size;
+
+bank_start = round_pgup(bank_start);
+bank_end = round_pgdown(bank_end);
+if ( bank_end <= bank_start )
+return;
+
+free_staticmem_pages(maddr_to_page(bank_start),
+(bank_end - bank_start) >> PAGE_SHIFT, false);
+}
+}
+
static unsigned long avail_heap_pages(
unsigned int zone_lo, unsigned int zone_hi, unsigned int node)
{
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 667f9dac83..8b1a2207b2 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -85,6 +85,9 @@ bool scrub_free_pages(void);
} while ( false )
#define FREE_XENHEAP_PAGE(p) FREE_XENHEAP_PAGES(p, 0)
+/* Static Memory */
+void init_staticmem_pages(void);
+
/* Map
RE: [PATCH 04/10] xen/arm: static memory initialization
Hi Jan
> -Original Message-
> From: Jan Beulich
> Sent: Tuesday, May 18, 2021 3:16 PM
> To: Penny Zheng
> Cc: Bertrand Marquis ; Wei Chen
> ; nd ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org; jul...@xen.org
> Subject: Re: [PATCH 04/10] xen/arm: static memory initialization
>
> On 18.05.2021 07:21, Penny Zheng wrote:
> > This patch introduces static memory initialization, during system RAM boot
> up.
> >
> > New func init_staticmem_pages is the equivalent of init_heap_pages,
> > responsible for static memory initialization.
> >
> > Helper func free_staticmem_pages is the equivalent of free_heap_pages,
> > to free nr_pfns pages of static memory.
> > For each page, it includes the following steps:
> > 1. change page state from in-use(also initialization state) to free
> > state and grant PGC_reserved.
> > 2. set its owner NULL and make sure this page is not a guest frame any
> > more
>
> But isn't the goal (as per the previous patch) to associate such pages with a
> _specific_ domain?
>
Free_staticmem_pages are alike free_heap_pages, are not used only for
initialization.
Freeing used pages to unused are also included.
Here, setting its owner NULL is to set owner in used field NULL.
Still, I need to add more explanation here.
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -150,6 +150,9 @@
> > #define p2m_pod_offline_or_broken_hit(pg) 0 #define
> > p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL) #endif
> > +#ifdef CONFIG_ARM_64
> > +#include
> > +#endif
>
> Whatever it is that's needed from this header suggests the code won't build
> for other architectures. I think init_staticmem_pages() in its current shape
> shouldn't live in this (common) file.
>
Yes, I should include them all both under one specific config, maybe like
CONFIG_STATIC_MEM, and this config is arm-specific.
> > @@ -1512,6 +1515,49 @@ static void free_heap_pages(
> > spin_unlock(_lock);
> > }
> >
> > +/* Equivalent of free_heap_pages to free nr_pfns pages of static
> > +memory. */ static void free_staticmem_pages(struct page_info *pg,
> unsigned long nr_pfns,
> > + bool need_scrub)
>
> Right now this function gets called only from an __init one. Unless it is
> intended to gain further callers, it should be marked __init itself then.
> Otherwise it should be made sure that other architectures don't include this
> (dead there) code.
>
Sure, I'll add __init. Thx.
> > +{
> > +mfn_t mfn = page_to_mfn(pg);
> > +int i;
>
> This type doesn't fit nr_pfns'es.
>
Sure, nr_mfns is better in also many other places.
> Jan
Re: [PATCH 03/10] xen/arm: introduce PGC_reserved
On 18/05/2021 06:21, Penny Zheng wrote:
In order to differentiate pages of static memory, from those allocated from
heap, this patch introduces a new page flag PGC_reserved to tell.
New struct reserved in struct page_info is to describe reserved page info,
that is, which specific domain this page is reserved to. >
Helper page_get_reserved_owner and page_set_reserved_owner are
designated to get/set reserved page's owner.
Struct domain is enlarged to more than PAGE_SIZE, due to newly-imported
struct reserved in struct page_info.
struct domain may embed a pointer to a struct page_info but never
directly embed the structure. So can you clarify what you mean?
Signed-off-by: Penny Zheng
---
xen/include/asm-arm/mm.h | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 0b7de3102e..d8922fd5db 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -88,7 +88,15 @@ struct page_info
*/
u32 tlbflush_timestamp;
};
-u64 pad;
+
+/* Page is reserved. */
+struct {
+/*
+ * Reserved Owner of this page,
+ * if this page is reserved to a specific domain.
+ */
+struct domain *domain;
+} reserved;
The space in page_info is quite tight, so I would like to avoid
introducing new fields unless we can't get away from it.
In this case, it is not clear why we need to differentiate the "Owner"
vs the "Reserved Owner". It might be clearer if this change is folded in
the first user of the field.
As an aside, for 32-bit Arm, you need to add a 4-byte padding.
};
#define PG_shift(idx) (BITS_PER_LONG - (idx))
@@ -108,6 +116,9 @@ struct page_info
/* Page is Xen heap? */
#define _PGC_xen_heap PG_shift(2)
#define PGC_xen_heap PG_mask(1, 2)
+ /* Page is reserved, referring static memory */
I would drop the second part of the sentence because the flag could be
used for other purpose. One example is reserved memory when Live Updating.
+#define _PGC_reserved PG_shift(3)
+#define PGC_reserved PG_mask(1, 3)
/* ... */
/* Page is broken? */
#define _PGC_broken PG_shift(7)
@@ -161,6 +172,9 @@ extern unsigned long xenheap_base_pdx;
#define page_get_owner(_p)(_p)->v.inuse.domain
#define page_set_owner(_p,_d) ((_p)->v.inuse.domain = (_d))
+#define page_get_reserved_owner(_p)(_p)->reserved.domain
+#define page_set_reserved_owner(_p,_d) ((_p)->reserved.domain = (_d))
+
#define maddr_get_owner(ma) (page_get_owner(maddr_to_page((ma
#define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
Cheers,
--
Julien Grall
RE: [PATCH 05/10] xen/arm: introduce alloc_staticmem_pages
Hi Jan
> -Original Message-
> From: Jan Beulich
> Sent: Tuesday, May 18, 2021 3:24 PM
> To: Penny Zheng
> Cc: Bertrand Marquis ; Wei Chen
> ; nd ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org; jul...@xen.org
> Subject: Re: [PATCH 05/10] xen/arm: introduce alloc_staticmem_pages
>
> On 18.05.2021 07:21, Penny Zheng wrote:
> > alloc_staticmem_pages is designated to allocate nr_pfns contiguous
> > pages of static memory. And it is the equivalent of alloc_heap_pages
> > for static memory.
> > This commit only covers allocating at specified starting address.
> >
> > For each page, it shall check if the page is reserved
> > (PGC_reserved) and free. It shall also do a set of necessary
> > initialization, which are mostly the same ones in alloc_heap_pages,
> > like, following the same cache-coherency policy and turning page
> > status into PGC_state_used, etc.
> >
> > Signed-off-by: Penny Zheng
> > ---
> > xen/common/page_alloc.c | 64
> > +
> > 1 file changed, 64 insertions(+)
> >
> > diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index
> > 58b53c6ac2..adf2889e76 100644
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -1068,6 +1068,70 @@ static struct page_info *alloc_heap_pages(
> > return pg;
> > }
> >
> > +/*
> > + * Allocate nr_pfns contiguous pages, starting at #start, of static memory.
> > + * It is the equivalent of alloc_heap_pages for static memory */
> > +static struct page_info *alloc_staticmem_pages(unsigned long nr_pfns,
> > +paddr_t start,
> > +unsigned int
> > +memflags)
>
> This is surely breaking the build (at this point in the series - recall that
> a series
> should build fine at every patch boundary), for introducing an unused static
> function, which most compilers will warn about.
>
Sure, I'll combine it with other commits
> Also again - please avoid introducing code that's always dead for certain
> architectures. Quite likely you want a Kconfig option to put a suitable #ifdef
> around such functions.
>
Sure, sorry for all the missing #ifdefs.
> And a nit: Please correct the apparently off-by-one indentation.
>
Sure, I'll check through the code more carefully.
> > +{
> > +bool need_tlbflush = false;
> > +uint32_t tlbflush_timestamp = 0;
> > +unsigned int i;
>
> This variable's type should (again) match nr_pfns'es (albeit I think that
> parameter really wants to be nr_mfns).
>
Correct if I understand you wrongly, you mean that parameters in
alloc_staticmem_pages
are better be named after unsigned long nr_mfns, right?
> > +struct page_info *pg;
> > +mfn_t s_mfn;
> > +
> > +/* For now, it only supports allocating at specified address. */
> > +s_mfn = maddr_to_mfn(start);
> > +pg = mfn_to_page(s_mfn);
> > +if ( !pg )
> > +return NULL;
>
> Under what conditions would mfn_to_page() return NULL?
Right, my mistake.
>
> > +for ( i = 0; i < nr_pfns; i++)
> > +{
> > +/*
> > + * Reference count must continuously be zero for free pages
> > + * of static memory(PGC_reserved).
> > + */
> > +ASSERT(pg[i].count_info & PGC_reserved);
> > +if ( (pg[i].count_info & ~PGC_reserved) != PGC_state_free )
> > +{
> > +printk(XENLOG_ERR
> > +"Reference count must continuously be zero for free
> > pages"
> > +"pg[%u] MFN %"PRI_mfn" c=%#lx t=%#x\n",
> > +i, mfn_x(page_to_mfn(pg + i)),
> > +pg[i].count_info, pg[i].tlbflush_timestamp);
>
> Nit: Indentation again.
>
Thx
> > +BUG();
> > +}
> > +
> > +if ( !(memflags & MEMF_no_tlbflush) )
> > +accumulate_tlbflush(_tlbflush, [i],
> > +_timestamp);
> > +
> > +/*
> > + * Reserve flag PGC_reserved and change page state
>
> DYM "Preserve ..."?
>
Sure, thx
> > + * to PGC_state_inuse.
> > + */
> > +pg[i].count_info = (pg[i].count_info & PGC_reserved) |
> PGC_state_inuse;
> > +/* Initialise fields which have other uses for free pages. */
> > +pg[i].u.inuse.type_info = 0;
> > +page_set_owner([i], NULL);
> > +
> > +/*
> > + * Ensure cache and RAM are consistent for platforms where the
> > + * guest can control its own visibility of/through the cache.
> > + */
> > +flush_page_to_ram(mfn_x(page_to_mfn([i])),
> > +!(memflags & MEMF_no_icache_flush));
> > +}
> > +
> > +if ( need_tlbflush )
> > +filtered_flush_tlb_mask(tlbflush_timestamp);
>
> With reserved pages dedicated to a specific domain, in how far is it possible
> that stale mappings from a prior use can still be around, making such TLB
> flushing necessary?
>
Yes, you're right.
> Jan
Re: [PATCH] xen-netback: Check for hotplug-status existence before watching
On Tue, May 18, 2021 at 07:57:16AM +0100, Paul Durrant wrote: > On 17/05/2021 22:43, Marek Marczykowski-Górecki wrote: > > On Tue, May 11, 2021 at 12:46:38PM +, Durrant, Paul wrote: > > > I really can't remember any detail. Perhaps try reverting both patches > > > then and check that the unbind/rmmod/modprobe/bind sequence still works > > > (and the backend actually makes it into connected state). > > > > Ok, I've tried this. I've reverted both commits, then used your test > > script from the 9476654bd5e8ad42abe8ee9f9e90069ff8e60c17: > > This has been tested by running iperf as a server in the test VM and > > then running a client against it in a continuous loop, whilst also > > running: > > while true; > >do echo vif-$DOMID-$VIF >unbind; > >echo down; > >rmmod xen-netback; > >echo unloaded; > >modprobe xen-netback; > >cd $(pwd); > >brctl addif xenbr0 vif$DOMID.$VIF; > >ip link set vif$DOMID.$VIF up; > >echo up; > >sleep 5; > >done > > in dom0 from /sys/bus/xen-backend/drivers/vif to continuously unbind, > > unload, re-load, re-bind and re-plumb the backend. > > In fact, the need to call `brctl` and `ip link` manually is exactly > > because the hotplug script isn't executed. When I execute it manually, > > the backend properly gets back to working. So, removing 'hotplug-status' > > was in the correct place (netback_remove). The missing part is the toolstack > > calling the hotplug script on xen-netback re-bind. > > > > Why is that missing? We're going behind the back of the toolstack to do the > unbind and bind so why should we expect it to re-execute a hotplug script? Ok, then simply execute the whole hotplug script (instead of its subset) after re-loading the backend module and everything will be fine. For example like this: XENBUS_PATH=backend/vif/$DOMID/$VIF \ XENBUS_TYPE=vif \ XENBUS_BASE_PATH=backend \ script=/etc/xen/scripts/vif-bridge \ vif=vif.$DOMID.$VIF \ /etc/xen/scripts/vif-bridge online (...) > > In short: if device gets XenbusStateInitWait for the first time (ddev == > > NULL case), it goes to add_device() which executes the hotplug script > > and stores the device. > > Then, if device goes to XenbusStateClosed + online==0 state, then it > > executes hotplug script again (with "offline" parameter) and forgets the > > device. If you unbind the driver, the device stays in > > XenbusStateConnected state (in xenstore), and after you bind it again, > > it goes to XenbusStateInitWait. It don't think it goes through > > XenbusStateClosed, and online stays at 1 too, so libxl doesn't execute > > the hotplug script again. > > This is pretty key. The frontend should not notice an unbind/bind i.e. there > should be no evidence of it happening by examining states in xenstore (from > the guest side). If you update the backend module, I think the frontend needs at least to re-evaluate feature-* nodes. In case of applying just a bug fix, they should not change (in theory), but technically that would be the correct thing to do. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab signature.asc Description: PGP signature
RE: [PATCH 06/10] xen: replace order with nr_pfns in assign_pages for better compatibility
Hi Jan > -Original Message- > From: Jan Beulich > Sent: Tuesday, May 18, 2021 3:28 PM > To: Penny Zheng > Cc: Bertrand Marquis ; Wei Chen > ; nd ; xen-devel@lists.xenproject.org; > sstabell...@kernel.org; jul...@xen.org > Subject: Re: [PATCH 06/10] xen: replace order with nr_pfns in assign_pages > for better compatibility > > On 18.05.2021 07:21, Penny Zheng wrote: > > Function parameter order in assign_pages is always used as 1ul << > > order, referring to 2@order pages. > > > > Now, for better compatibility with new static memory, order shall be > > replaced with nr_pfns pointing to page count with no constraint, like > > 250MB. > > While I'm not entirely opposed, I'm also not convinced. The new user could > as well break up the range into suitable power-of-2 chunks. In no case do I > view the wording "compatibility" here as appropriate. There's no > incompatibility at present. > Yes, maybe the incompatibility is not the good choice here. Sure, the new user definitely could choose the workaround to break up the range, while it may cost extra time. And while considering MPU system, memory range size is often not in the power-of-2. > Jan Thanks Penny
Re: [PATCH 02/10] xen/arm: handle static memory in dt_unreserved_regions
Hi Penny,
On 18/05/2021 06:21, Penny Zheng wrote:
static memory regions overlap with memory nodes. The
overlapping memory is reserved-memory and should be
handled accordingly:
dt_unreserved_regions should skip these regions the
same way they are already skipping mem-reserved regions.
Signed-off-by: Penny Zheng
---
xen/arch/arm/setup.c | 39 +--
1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 00aad1c194..444dbbd676 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -201,7 +201,7 @@ static void __init dt_unreserved_regions(paddr_t s, paddr_t
e,
void (*cb)(paddr_t, paddr_t),
int first)
{
-int i, nr = fdt_num_mem_rsv(device_tree_flattened);
+int i, nr_reserved, nr_static, nr = fdt_num_mem_rsv(device_tree_flattened);
for ( i = first; i < nr ; i++ )
{
@@ -222,18 +222,45 @@ static void __init dt_unreserved_regions(paddr_t s,
paddr_t e,
}
/*
- * i is the current bootmodule we are evaluating across all possible
- * kinds.
+ * i is the current reserved RAM banks we are evaluating across all
+ * possible kinds.
*
* When retrieving the corresponding reserved-memory addresses
* below, we need to index the bootinfo.reserved_mem bank starting
* from 0, and only counting the reserved-memory modules. Hence,
* we need to use i - nr.
*/
-for ( ; i - nr < bootinfo.reserved_mem.nr_banks; i++ )
+i = i - nr;
+nr_reserved = bootinfo.reserved_mem.nr_banks;
+for ( ; i < nr_reserved; i++ )
{
-paddr_t r_s = bootinfo.reserved_mem.bank[i - nr].start;
-paddr_t r_e = r_s + bootinfo.reserved_mem.bank[i - nr].size;
+paddr_t r_s = bootinfo.reserved_mem.bank[i].start;
+paddr_t r_e = r_s + bootinfo.reserved_mem.bank[i].size;
+
+if ( s < r_e && r_s < e )
+{
+dt_unreserved_regions(r_e, e, cb, i + 1);
+dt_unreserved_regions(s, r_s, cb, i + 1);
+return;
+}
+}
+
+/*
+ * i is the current reserved RAM banks we are evaluating across all
+ * possible kinds.
+ *
+ * When retrieving the corresponding static-memory bank address
+ * below, we need to index the bootinfo.static_mem starting
+ * from 0, and only counting the static-memory bank. Hence,
+ * we need to use i - nr_reserved.
+ */
+
+i = i - nr_reserved;
+nr_static = bootinfo.static_mem.nr_banks;
+for ( ; i < nr_static; i++ )
+{
+paddr_t r_s = bootinfo.static_mem.bank[i].start; > +paddr_t
r_e = r_s + bootinfo.static_mem.bank[i].size;
This is the 3rd loop we are adding in dt_unreserved_regions(). Each loop
are doing pretty much the same thing except with a different array. I'd
like to avoid the new loop if possible.
As mentionned in patch#1, the static memory is another kind of reserved
memory. So could we describe the static memory using the reserved-memory?
if ( s < r_e && r_s < e )
{
Cheers,
--
Julien Grall
Re: [PATCH 01/10] xen/arm: introduce domain on Static Allocation
Hi Penny,
On 18/05/2021 06:21, Penny Zheng wrote:
Static Allocation refers to system or sub-system(domains) for which memory
areas are pre-defined by configuration using physical address ranges.
Those pre-defined memory, -- Static Momery, as parts of RAM reserved in the
s/Momery/Memory/
beginning, shall never go to heap allocator or boot allocator for any use.
Domains on Static Allocation is supported through device tree property
`xen,static-mem` specifying reserved RAM banks as this domain's guest RAM.
By default, they shall be mapped to the fixed guest RAM address
`GUEST_RAM0_BASE`, `GUEST_RAM1_BASE`.
This patch introduces this new `xen,static-mem` property to define static
memory nodes in device tree file.
This patch also documents and parses this new attribute at boot time and
stores related info in static_mem for later initialization.
Signed-off-by: Penny Zheng
---
docs/misc/arm/device-tree/booting.txt | 33 +
xen/arch/arm/bootfdt.c| 52 +++
xen/include/asm-arm/setup.h | 2 ++
3 files changed, 87 insertions(+)
diff --git a/docs/misc/arm/device-tree/booting.txt
b/docs/misc/arm/device-tree/booting.txt
index 5243bc7fd3..d209149d71 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -268,3 +268,36 @@ The DTB fragment is loaded at 0xc00 in the example
above. It should
follow the convention explained in docs/misc/arm/passthrough.txt. The
DTB fragment will be added to the guest device tree, so that the guest
kernel will be able to discover the device.
+
+
+Static Allocation
+=
+
+Static Allocation refers to system or sub-system(domains) for which memory
+areas are pre-defined by configuration using physical address ranges.
+Those pre-defined memory, -- Static Momery, as parts of RAM reserved in the
s/Momery/Memory/
+beginning, shall never go to heap allocator or boot allocator for any use.
+
+Domains on Static Allocation is supported through device tree property
+`xen,static-mem` specifying reserved RAM banks as this domain's guest RAM.
I would suggest to use "physical RAM" when you refer to the host memory.
+By default, they shall be mapped to the fixed guest RAM address
+`GUEST_RAM0_BASE`, `GUEST_RAM1_BASE`.
There are a few bits that needs to clarified or part of the description:
1) "By default" suggests there is an alternative possibility.
However, I don't see any.
2) Will the first region of xen,static-mem be mapped to
GUEST_RAM0_BASE and the second to GUEST_RAM1_BASE? What if a third
region is specificed?
3) We don't guarantee the base address and the size of the banks.
Wouldn't it be better to let the admin select the region he/she wants?
4) How do you determine the number of cells for the address and the size?
+Static Allocation is only supported on AArch64 for now.
The code doesn't seem to be AArch64 specific. So why can't this be used
for 32-bit Arm?
+
+The dtb property should look like as follows:
+
+chosen {
+domU1 {
+compatible = "xen,domain";
+#address-cells = <0x2>;
+#size-cells = <0x2>;
+cpus = <2>;
+xen,static-mem = <0x0 0x3000 0x0 0x2000>;
+
+...
+};
+};
+
+DOMU1 on Static Allocation has reserved RAM bank at 0x3000 of 512MB size
Do you mean "DomU1 will have a static memory of 512MB reserved from the
physical address..."?
+as guest RAM.
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index dcff512648..e9f14e6a44 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -327,6 +327,55 @@ static void __init process_chosen_node(const void *fdt,
int node,
add_boot_module(BOOTMOD_RAMDISK, start, end-start, false);
}
+static int __init process_static_memory(const void *fdt, int node,
+const char *name,
+u32 address_cells, u32 size_cells,
+void *data)
+{
+int i;
+int banks;
+const __be32 *cell;
+paddr_t start, size;
+u32 reg_cells = address_cells + size_cells;
+struct meminfo *mem = data;
+const struct fdt_property *prop;
+
+if ( address_cells < 1 || size_cells < 1 )
+{
+printk("fdt: invalid #address-cells or #size-cells for static memory");
+return -EINVAL;
+}
+
+/*
+ * Check if static memory property belongs to a specific domain, that is,
+ * its node `domUx` has compatible string "xen,domain".
+ */
+if ( fdt_node_check_compatible(fdt, node, "xen,domain") != 0 )
+printk("xen,static-mem property can only locate under /domUx node.\n");
+
+prop = fdt_get_property(fdt, node, name, NULL);
+if ( !prop )
+return -ENOENT;
+
+cell = (const __be32 *)prop->data;
+banks = fdt32_to_cpu(prop->len) / (reg_cells * sizeof
RE: [PATCH 07/10] xen/arm: intruduce alloc_domstatic_pages
Hi Jan
> -Original Message-
> From: Jan Beulich
> Sent: Tuesday, May 18, 2021 3:35 PM
> To: Penny Zheng
> Cc: Bertrand Marquis ; Wei Chen
> ; nd ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org; jul...@xen.org
> Subject: Re: [PATCH 07/10] xen/arm: intruduce alloc_domstatic_pages
>
> On 18.05.2021 07:21, Penny Zheng wrote:
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -2447,6 +2447,9 @@ int assign_pages(
> > {
> > ASSERT(page_get_owner([i]) == NULL);
> > page_set_owner([i], d);
> > +/* use page_set_reserved_owner to set its reserved domain owner.
> */
> > +if ( (pg[i].count_info & PGC_reserved) )
> > +page_set_reserved_owner([i], d);
>
> Now this is puzzling: What's the point of setting two owner fields to the same
> value? I also don't recall you having introduced
> page_set_reserved_owner() for x86, so how is this going to build there?
>
Thanks for pointing out that it will fail on x86.
As for the same value, sure, I shall change it to domid_t domid to record its
reserved owner.
Only domid is enough for differentiate.
And even when domain get rebooted, struct domain may be destroyed, but domid
will stays
The same.
Major user cases for domain on static allocation are referring to the whole
system are static,
No runtime creation.
> > @@ -2509,6 +2512,56 @@ struct page_info *alloc_domheap_pages(
> > return pg;
> > }
> >
> > +/*
> > + * Allocate nr_pfns contiguous pages, starting at #start, of static
> > +memory,
> > + * then assign them to one specific domain #d.
> > + * It is the equivalent of alloc_domheap_pages for static memory.
> > + */
> > +struct page_info *alloc_domstatic_pages(
> > +struct domain *d, unsigned long nr_pfns, paddr_t start,
> > +unsigned int memflags)
> > +{
> > +struct page_info *pg = NULL;
> > +unsigned long dma_size;
> > +
> > +ASSERT(!in_irq());
> > +
> > +if ( memflags & MEMF_no_owner )
> > +memflags |= MEMF_no_refcount;
> > +
> > +if ( !dma_bitsize )
> > +memflags &= ~MEMF_no_dma;
> > +else
> > +{
> > +dma_size = 1ul << bits_to_zone(dma_bitsize);
> > +/* Starting address shall meet the DMA limitation. */
> > +if ( dma_size && start < dma_size )
> > +return NULL;
>
> It is the entire range (i.e. in particular the last byte) which needs to meet
> such
> a restriction. I'm not convinced though that DMA width restrictions and static
> allocation are sensible to coexist.
>
FWIT, if starting address meets the limitation, the last byte, greater than
starting
address shall meet it too.
> > +}
> > +
> > +pg = alloc_staticmem_pages(nr_pfns, start, memflags);
> > +if ( !pg )
> > +return NULL;
> > +
> > +if ( d && !(memflags & MEMF_no_owner) )
> > +{
> > +if ( memflags & MEMF_no_refcount )
> > +{
> > +unsigned long i;
> > +
> > +for ( i = 0; i < nr_pfns; i++ )
> > +pg[i].count_info = PGC_extra;
> > +}
>
> Is this as well as the MEMF_no_owner case actually meaningful for statically
> allocated pages?
>
Thanks for pointing out. Truly, we do not need to take it considered.
> Jan
RE: [PATCH 08/10] xen/arm: introduce reserved_page_list
Hi Jan
> -Original Message-
> From: Jan Beulich
> Sent: Tuesday, May 18, 2021 3:39 PM
> To: Penny Zheng
> Cc: Bertrand Marquis ; Wei Chen
> ; nd ; xen-devel@lists.xenproject.org;
> sstabell...@kernel.org; jul...@xen.org
> Subject: Re: [PATCH 08/10] xen/arm: introduce reserved_page_list
>
> On 18.05.2021 07:21, Penny Zheng wrote:
> > Since page_list under struct domain refers to linked pages as gueast
> > RAM allocated from heap, it should not include reserved pages of static
> memory.
> >
> > The number of PGC_reserved pages assigned to a domain is tracked in a
> > new 'reserved_pages' counter. Also introduce a new reserved_page_list
> > to link pages of static memory. Let page_to_list return
> > reserved_page_list, when flag is PGC_reserved.
> >
> > Later, when domain get destroyed or restarted, those new values will
> > help relinquish memory to proper place, not been given back to heap.
> >
> > Signed-off-by: Penny Zheng
> > ---
> > xen/common/domain.c | 1 +
> > xen/common/page_alloc.c | 7 +--
> > xen/include/xen/sched.h | 5 +
> > 3 files changed, 11 insertions(+), 2 deletions(-)
>
> This contradicts the title's prefix: There's no Arm-specific change here at
> all.
> But imo the title is correct, and the changes should be Arm-specific. There's
> no point having struct domain fields on e.g. x86 which aren't used there at
> all.
>
Yep, you're right.
I'll add ifdefs in the following changes.
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -2410,7 +2410,7 @@ int assign_pages(
> >
> > for ( i = 0; i < nr_pfns; i++ )
> > {
> > -ASSERT(!(pg[i].count_info & ~PGC_extra));
> > +ASSERT(!(pg[i].count_info & ~(PGC_extra |
> > + PGC_reserved)));
> > if ( pg[i].count_info & PGC_extra )
> > extra_pages++;
> > }
> > @@ -2439,6 +2439,9 @@ int assign_pages(
> > }
> > }
> >
> > +if ( pg[0].count_info & PGC_reserved )
> > +d->reserved_pages += nr_pfns;
>
> I guess this again will fail to build on x86.
>
> > @@ -588,6 +590,9 @@ static inline struct page_list_head *page_to_list(
> > if ( pg->count_info & PGC_extra )
> > return >extra_page_list;
> >
> > +if ( pg->count_info & PGC_reserved )
> > +return >reserved_page_list;
>
> Same here.
>
> Jan
Thanks
Penny
[libvirt test] 161988: regressions - FAIL
flight 161988 libvirt real [real] http://logs.test-lab.xenproject.org/osstest/logs/161988/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: build-armhf-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-amd64-libvirt 6 libvirt-buildfail REGR. vs. 151777 build-i386-libvirt6 libvirt-buildfail REGR. vs. 151777 build-arm64-libvirt 6 libvirt-buildfail REGR. vs. 151777 Tests which did not succeed, but are not blocking: test-amd64-amd64-libvirt 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-pair 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-vhd 1 build-check(1) blocked n/a test-amd64-amd64-libvirt-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt 1 build-check(1) blocked n/a test-amd64-i386-libvirt-pair 1 build-check(1) blocked n/a test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 1 build-check(1) blocked n/a test-amd64-i386-libvirt-xsm 1 build-check(1) blocked n/a test-arm64-arm64-libvirt 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-qcow2 1 build-check(1) blocked n/a test-arm64-arm64-libvirt-xsm 1 build-check(1) blocked n/a test-armhf-armhf-libvirt 1 build-check(1) blocked n/a test-armhf-armhf-libvirt-raw 1 build-check(1) blocked n/a version targeted for testing: libvirt 530715bd0b619b87c969571c0c5a10ebda514218 baseline version: libvirt 2c846fa6bcc11929c9fb857a22430fb9945654ad Last test of basis 151777 2020-07-10 04:19:19 Z 312 days Failing since151818 2020-07-11 04:18:52 Z 311 days 304 attempts Testing same since 161988 2021-05-18 04:19:59 Z0 days1 attempts People who touched revisions under test: Adolfo Jayme Barrientos Aleksandr Alekseev Aleksei Zakharov Andika Triwidada Andrea Bolognani Balázs Meskó Barrett Schonefeld Bastian Germann Bastien Orivel BiaoXiang Ye Bihong Yu Binfeng Wu Boris Fiuczynski Brian Turek Bruno Haible Chris Mayo Christian Ehrhardt Christian Schoenebeck Cole Robinson Collin Walling Cornelia Huck Cédric Bosdonnat Côme Borsoi Daniel Henrique Barboza Daniel Letai Daniel P. Berrange Daniel P. Berrangé Dmytro Linkin Eiichi Tsukata Eric Farman Erik Skultety Fabian Affolter Fabian Freyer Fangge Jin Farhan Ali Fedora Weblate Translation gongwei Guoyi Tu Göran Uddeborg Halil Pasic Han Han Hao Wang Hela Basa Helmut Grohne Ian Wienand Jakob Meng Jamie Strandboge Jamie Strandboge Jan Kuparinen Jean-Baptiste Holcroft Jianan Gao Jim Fehlig Jin Yan Jiri Denemark John Ferlan Jonathan Watt Jonathon Jongsma Julio Faracco Ján Tomko Kashyap Chamarthy Kevin Locke Kristina Hanicova Laine Stump Laszlo Ersek Liao Pingfang Lin Ma Lin Ma Lin Ma Luke Yue Luyao Zhong Marc Hartmayer Marc-André Lureau Marek Marczykowski-Górecki Markus Schade Martin Kletzander Masayoshi Mizuma Matt Coleman Matt Coleman Mauro Matteo Cascella Meina Li Michal Privoznik Michał Smyk Milo Casagrande Moshe Levi Muha Aliss Neal Gompa Nick Shyrokovskiy Nickys Music Group Nico Pache Nikolay Shirokovskiy Olaf Hering Olesya Gerasimenko Orion Poplawski Pany Patrick Magauran Paulo de Rezende Pinatti Pavel Hrdina Peng Liang Peter Krempa Pino Toscano Pino Toscano Piotr Drąg Prathamesh Chavan Ricky Tigg Roman Bogorodskiy Roman Bolshakov Ryan Gahagan Ryan Schmidt Sam Hartman Scott Shambarger Sebastian Mitterle SeongHyun Jo Shalini Chellathurai Saroja Shaojun Yang Shi Lei simmon Simon Gaiser Stefan Bader Stefan Berger Stefan Berger Szymon Scholz Thomas Huth Tim Wiederhake Tomáš Golembiovský Tomáš Janoušek Tuguoyi Ville Skyttä Wang Xin WangJian Weblate Yalei Li <274268...@qq.com> Yalei Li Yang Hang Yanqiu Zhang Yaroslav Kargin Yi Li Yi Wang Yuri Chornoivan Zheng Chuan zhenwei pi Zhenyu Zheng jobs: build-amd64-xsm pass build-arm64-xsm pass build-i386-xsm pass build-amd64 pass build-arm64 pass build-armhf pass build-i386
Re: [PATCH 08/10] xen/arm: introduce reserved_page_list
On 18.05.2021 07:21, Penny Zheng wrote:
> Since page_list under struct domain refers to linked pages as gueast RAM
> allocated from heap, it should not include reserved pages of static memory.
>
> The number of PGC_reserved pages assigned to a domain is tracked in
> a new 'reserved_pages' counter. Also introduce a new reserved_page_list
> to link pages of static memory. Let page_to_list return reserved_page_list,
> when flag is PGC_reserved.
>
> Later, when domain get destroyed or restarted, those new values will help
> relinquish memory to proper place, not been given back to heap.
>
> Signed-off-by: Penny Zheng
> ---
> xen/common/domain.c | 1 +
> xen/common/page_alloc.c | 7 +--
> xen/include/xen/sched.h | 5 +
> 3 files changed, 11 insertions(+), 2 deletions(-)
This contradicts the title's prefix: There's no Arm-specific change
here at all. But imo the title is correct, and the changes should
be Arm-specific. There's no point having struct domain fields on
e.g. x86 which aren't used there at all.
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -2410,7 +2410,7 @@ int assign_pages(
>
> for ( i = 0; i < nr_pfns; i++ )
> {
> -ASSERT(!(pg[i].count_info & ~PGC_extra));
> +ASSERT(!(pg[i].count_info & ~(PGC_extra | PGC_reserved)));
> if ( pg[i].count_info & PGC_extra )
> extra_pages++;
> }
> @@ -2439,6 +2439,9 @@ int assign_pages(
> }
> }
>
> +if ( pg[0].count_info & PGC_reserved )
> +d->reserved_pages += nr_pfns;
I guess this again will fail to build on x86.
> @@ -588,6 +590,9 @@ static inline struct page_list_head *page_to_list(
> if ( pg->count_info & PGC_extra )
> return >extra_page_list;
>
> +if ( pg->count_info & PGC_reserved )
> +return >reserved_page_list;
Same here.
Jan
Re: [PATCH 07/10] xen/arm: intruduce alloc_domstatic_pages
On 18.05.2021 07:21, Penny Zheng wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -2447,6 +2447,9 @@ int assign_pages(
> {
> ASSERT(page_get_owner([i]) == NULL);
> page_set_owner([i], d);
> +/* use page_set_reserved_owner to set its reserved domain owner. */
> +if ( (pg[i].count_info & PGC_reserved) )
> +page_set_reserved_owner([i], d);
Now this is puzzling: What's the point of setting two owner fields
to the same value? I also don't recall you having introduced
page_set_reserved_owner() for x86, so how is this going to build
there?
> @@ -2509,6 +2512,56 @@ struct page_info *alloc_domheap_pages(
> return pg;
> }
>
> +/*
> + * Allocate nr_pfns contiguous pages, starting at #start, of static memory,
> + * then assign them to one specific domain #d.
> + * It is the equivalent of alloc_domheap_pages for static memory.
> + */
> +struct page_info *alloc_domstatic_pages(
> +struct domain *d, unsigned long nr_pfns, paddr_t start,
> +unsigned int memflags)
> +{
> +struct page_info *pg = NULL;
> +unsigned long dma_size;
> +
> +ASSERT(!in_irq());
> +
> +if ( memflags & MEMF_no_owner )
> +memflags |= MEMF_no_refcount;
> +
> +if ( !dma_bitsize )
> +memflags &= ~MEMF_no_dma;
> +else
> +{
> +dma_size = 1ul << bits_to_zone(dma_bitsize);
> +/* Starting address shall meet the DMA limitation. */
> +if ( dma_size && start < dma_size )
> +return NULL;
It is the entire range (i.e. in particular the last byte) which needs
to meet such a restriction. I'm not convinced though that DMA width
restrictions and static allocation are sensible to coexist.
> +}
> +
> +pg = alloc_staticmem_pages(nr_pfns, start, memflags);
> +if ( !pg )
> +return NULL;
> +
> +if ( d && !(memflags & MEMF_no_owner) )
> +{
> +if ( memflags & MEMF_no_refcount )
> +{
> +unsigned long i;
> +
> +for ( i = 0; i < nr_pfns; i++ )
> +pg[i].count_info = PGC_extra;
> +}
Is this as well as the MEMF_no_owner case actually meaningful for
statically allocated pages?
Jan
[linux-linus test] 161984: regressions - FAIL
flight 161984 linux-linus real [real] http://logs.test-lab.xenproject.org/osstest/logs/161984/ Regressions :-( Tests which did not succeed and are blocking, including tests which could not be run: test-amd64-i386-xl-xsm7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-dmrestrict-amd64-dmrestrict 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-intel 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-examine 6 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-ws16-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-debianhvm-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt 7 xen-install fail REGR. vs. 152332 test-amd64-i386-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemut-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-libvirt-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-coresched-i386-xl 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl7 xen-install fail REGR. vs. 152332 test-amd64-i386-qemuu-rhel6hvm-amd 7 xen-installfail REGR. vs. 152332 test-amd64-i386-freebsd10-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-pvshim 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-raw7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-debianhvm-i386-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-shadow 7 xen-install fail REGR. vs. 152332 test-amd64-i386-freebsd10-i386 7 xen-installfail REGR. vs. 152332 test-amd64-i386-xl-qemuu-ovmf-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-xl-qemut-stubdom-debianhvm-amd64-xsm 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 10 xen-install/src_host fail REGR. vs. 152332 test-amd64-i386-xl-qemut-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-i386-libvirt-pair 11 xen-install/dst_host fail REGR. vs. 152332 test-amd64-i386-xl-qemuu-win7-amd64 7 xen-install fail REGR. vs. 152332 test-amd64-amd64-libvirt-vhd 13 guest-start fail REGR. vs. 152332 test-arm64-arm64-xl 14 guest-start fail REGR. vs. 152332 test-arm64-arm64-xl-xsm 14 guest-start fail REGR. vs. 152332 test-arm64-arm64-xl-thunderx 13 debian-fixup fail REGR. vs. 152332 test-arm64-arm64-xl-credit1 14 guest-start fail REGR. vs. 152332 test-amd64-amd64-amd64-pvgrub 20 guest-stop fail REGR. vs. 152332 test-amd64-amd64-i386-pvgrub 20 guest-stop fail REGR. vs. 152332 test-amd64-amd64-xl-qcow213 guest-start fail REGR. vs. 152332 test-armhf-armhf-xl-vhd 13 guest-start fail REGR. vs. 152332 Tests which are failing intermittently (not blocking): test-amd64-amd64-qemuu-freebsd12-amd64 13 guest-start fail in 161977 pass in 161984 test-arm64-arm64-xl 13 debian-fixup fail in 161977 pass in 161984 test-arm64-arm64-xl-xsm 13 debian-fixup fail in 161977 pass in 161984 test-arm64-arm64-xl-credit1 13 debian-fixup fail in 161977 pass in 161984 test-arm64-arm64-libvirt-xsm 13 debian-fixup fail pass in 161972 test-arm64-arm64-xl-credit2 13 debian-fixup fail pass in 161977 Tests which did not succeed, but are not blocking: test-arm64-arm64-libvirt-xsm 15 migrate-support-check fail in 161972 never pass test-arm64-arm64-libvirt-xsm 16 saverestore-support-check fail in 161972 never pass test-arm64-arm64-xl-credit2 15 migrate-support-check fail in 161977 never pass test-arm64-arm64-xl-credit2 16 saverestore-support-check fail in 161977 never pass test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 152332 test-armhf-armhf-libvirt 16 saverestore-support-checkfail like 152332 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 152332 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 152332 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 152332 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail like 152332 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like
Re: [PATCH 06/10] xen: replace order with nr_pfns in assign_pages for better compatibility
On 18.05.2021 07:21, Penny Zheng wrote: > Function parameter order in assign_pages is always used as 1ul << order, > referring to 2@order pages. > > Now, for better compatibility with new static memory, order shall > be replaced with nr_pfns pointing to page count with no constraint, > like 250MB. While I'm not entirely opposed, I'm also not convinced. The new user could as well break up the range into suitable power-of-2 chunks. In no case do I view the wording "compatibility" here as appropriate. There's no incompatibility at present. Jan
Re: [PATCH 05/10] xen/arm: introduce alloc_staticmem_pages
On 18.05.2021 07:21, Penny Zheng wrote:
> alloc_staticmem_pages is designated to allocate nr_pfns contiguous
> pages of static memory. And it is the equivalent of alloc_heap_pages
> for static memory.
> This commit only covers allocating at specified starting address.
>
> For each page, it shall check if the page is reserved
> (PGC_reserved) and free. It shall also do a set of necessary
> initialization, which are mostly the same ones in alloc_heap_pages,
> like, following the same cache-coherency policy and turning page
> status into PGC_state_used, etc.
>
> Signed-off-by: Penny Zheng
> ---
> xen/common/page_alloc.c | 64 +
> 1 file changed, 64 insertions(+)
>
> diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
> index 58b53c6ac2..adf2889e76 100644
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -1068,6 +1068,70 @@ static struct page_info *alloc_heap_pages(
> return pg;
> }
>
> +/*
> + * Allocate nr_pfns contiguous pages, starting at #start, of static memory.
> + * It is the equivalent of alloc_heap_pages for static memory
> + */
> +static struct page_info *alloc_staticmem_pages(unsigned long nr_pfns,
> +paddr_t start,
> +unsigned int memflags)
This is surely breaking the build (at this point in the series -
recall that a series should build fine at every patch boundary),
for introducing an unused static function, which most compilers
will warn about.
Also again - please avoid introducing code that's always dead for
certain architectures. Quite likely you want a Kconfig option to
put a suitable #ifdef around such functions.
And a nit: Please correct the apparently off-by-one indentation.
> +{
> +bool need_tlbflush = false;
> +uint32_t tlbflush_timestamp = 0;
> +unsigned int i;
This variable's type should (again) match nr_pfns'es (albeit I
think that parameter really wants to be nr_mfns).
> +struct page_info *pg;
> +mfn_t s_mfn;
> +
> +/* For now, it only supports allocating at specified address. */
> +s_mfn = maddr_to_mfn(start);
> +pg = mfn_to_page(s_mfn);
> +if ( !pg )
> +return NULL;
Under what conditions would mfn_to_page() return NULL?
> +for ( i = 0; i < nr_pfns; i++)
> +{
> +/*
> + * Reference count must continuously be zero for free pages
> + * of static memory(PGC_reserved).
> + */
> +ASSERT(pg[i].count_info & PGC_reserved);
> +if ( (pg[i].count_info & ~PGC_reserved) != PGC_state_free )
> +{
> +printk(XENLOG_ERR
> +"Reference count must continuously be zero for free
> pages"
> +"pg[%u] MFN %"PRI_mfn" c=%#lx t=%#x\n",
> +i, mfn_x(page_to_mfn(pg + i)),
> +pg[i].count_info, pg[i].tlbflush_timestamp);
Nit: Indentation again.
> +BUG();
> +}
> +
> +if ( !(memflags & MEMF_no_tlbflush) )
> +accumulate_tlbflush(_tlbflush, [i],
> +_timestamp);
> +
> +/*
> + * Reserve flag PGC_reserved and change page state
DYM "Preserve ..."?
> + * to PGC_state_inuse.
> + */
> +pg[i].count_info = (pg[i].count_info & PGC_reserved) |
> PGC_state_inuse;
> +/* Initialise fields which have other uses for free pages. */
> +pg[i].u.inuse.type_info = 0;
> +page_set_owner([i], NULL);
> +
> +/*
> + * Ensure cache and RAM are consistent for platforms where the
> + * guest can control its own visibility of/through the cache.
> + */
> +flush_page_to_ram(mfn_x(page_to_mfn([i])),
> +!(memflags & MEMF_no_icache_flush));
> +}
> +
> +if ( need_tlbflush )
> +filtered_flush_tlb_mask(tlbflush_timestamp);
With reserved pages dedicated to a specific domain, in how far is it
possible that stale mappings from a prior use can still be around,
making such TLB flushing necessary?
Jan
Re: [PATCH 04/10] xen/arm: static memory initialization
On 18.05.2021 07:21, Penny Zheng wrote:
> This patch introduces static memory initialization, during system RAM boot up.
>
> New func init_staticmem_pages is the equivalent of init_heap_pages,
> responsible
> for static memory initialization.
>
> Helper func free_staticmem_pages is the equivalent of free_heap_pages, to free
> nr_pfns pages of static memory.
> For each page, it includes the following steps:
> 1. change page state from in-use(also initialization state) to free state and
> grant PGC_reserved.
> 2. set its owner NULL and make sure this page is not a guest frame any more
But isn't the goal (as per the previous patch) to associate such pages
with a _specific_ domain?
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -150,6 +150,9 @@
> #define p2m_pod_offline_or_broken_hit(pg) 0
> #define p2m_pod_offline_or_broken_replace(pg) BUG_ON(pg != NULL)
> #endif
> +#ifdef CONFIG_ARM_64
> +#include
> +#endif
Whatever it is that's needed from this header suggests the code won't
build for other architectures. I think init_staticmem_pages() in its
current shape shouldn't live in this (common) file.
> @@ -1512,6 +1515,49 @@ static void free_heap_pages(
> spin_unlock(_lock);
> }
>
> +/* Equivalent of free_heap_pages to free nr_pfns pages of static memory. */
> +static void free_staticmem_pages(struct page_info *pg, unsigned long nr_pfns,
> + bool need_scrub)
Right now this function gets called only from an __init one. Unless
it is intended to gain further callers, it should be marked __init
itself then. Otherwise it should be made sure that other
architectures don't include this (dead there) code.
> +{
> +mfn_t mfn = page_to_mfn(pg);
> +int i;
This type doesn't fit nr_pfns'es.
Jan
