Re: [PATCH v2] tools: convert bitfields to unsigned type

[Juergen Gross]

On 08.05.23 18:46, Olaf Hering wrote:

clang complains about the signed type:

implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 
to -1 [-Wsingle-bit-bitfield-constant-conversion]

The potential ABI change in libxenvchan is covered by the Xen version based 
SONAME.

Signed-off-by: Olaf Hering 


Reviewed-by: Juergen Gross 


Juergen



OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

[linux-linus test] 180582: regressions - FAIL

[osstest service owner]

flight 180582 linux-linus real [real]
flight 180585 linux-linus real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/180582/
http://logs.test-lab.xenproject.org/osstest/logs/180585/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit1   8 xen-boot fail REGR. vs. 180278

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-examine  8 reboot   fail  like 180278
 test-armhf-armhf-libvirt  8 xen-boot fail  like 180278
 test-armhf-armhf-libvirt-raw  8 xen-boot fail  like 180278
 test-armhf-armhf-xl-rtds  8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278
 test-armhf-armhf-xl   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278
 test-armhf-armhf-xl-multivcpu  8 xen-boot fail like 180278
 test-armhf-armhf-libvirt-qcow2  8 xen-bootfail like 180278
 test-armhf-armhf-xl-credit2   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-vhd   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-arndale   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-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  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail 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-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 linuxba0ad6ed89fd5dada3b7b65ef2b08e95d449d4ab
baseline version:
 linux6c538e1adbfc696ac4747fb10d63e704344f763d

Last test of basis   180278  2023-04-16 19:41:46 Z   22 days
Failing since180281  2023-04-17 06:24:36 Z   22 days   40 attempts
Testing same since   180582  2023-05-08 21:11:46 Z0 days1 attempts


2359 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-xl  pass
 test-amd64-cores

Re: xenstored: EACCESS error accessing control/feature-balloon 1

[Yann Dirson]

On 5/4/23 20:04, zithro wrote:
> On 04 May 2023 17:59, Yann Dirson wrote:
>>
>> On 5/4/23 15:58, zithro wrote:
>>> Hi,
>>>
>>> [ snipped for brevity, report summary:
>>> XAPI daemon in domU tries to write to a non-existent xenstore node in
>>> a non-XAPI dom0 ]
>>>
>>> On 12 Apr 2023 18:41, Yann Dirson wrote:
 Is there anything besides XAPI using this node, or the other data
 published by xe-daemon?
>>>
>>> On my vanilla Xen (ie. non-XAPI), I have no node about "balloon"-ing
>>> in xenstore (either dom0 or domU nodes, but I'm not using ballooning
>>> in both).
>>>
 Maybe the original issue is just that there is no reason to have
 xe-guest-utilities installed in this setup?
>>>
>>> That's what I thought as I'm not using XAPI, so maybe the problem
>>> should only be addressed to the truenas team ? I posted on their forum
>>> but got no answer.
>>> I killed the 'xe-daemon' in both setups without loss of functionality.
>>>
>>> My wild guess is that 'xe-daemon', 'xe-update-guest-attrs' and all
>>> 'xenstore* commands' are leftovers from when Xen was working as a dom0
>>> under FreeBSD (why would a *domU* have them ?).
>>
>> That would not be correct: xenstore* are useful in guests, should you
>> want to read/write to the XenStore manually or from scripts;
>
> Didn't know that, can you give some use cases (or URLs) for which it 
> is useful, with or without XAPI ?
> I've read xenstore* man pages and could not infer a use case.
> Although I may already see some : updating ballooned memory values, or 
> as Julien Grall pointed out, updating "feature-s3/4" values ?

You can get other examples in 
https://xenbits.xen.org/docs/unstable/misc/xenstore-paths.html#domain-controlled-paths

>
> PS: small mistake in "man/xenstore-write.1.html" (from at least 4.14, 
> and onward), the synopsis reads "xenstore-read" ieof "xenstore-read".
Patch sent, thanks.
> Also, the -s option disappeared from unstable, although it may be 
> expected. I don't know it's purpose either.

See 
https://github.com/xen-project/xen/commit/c65687ed16d2289ec91036ec2862a4b4bd34ea4f

Best regards,

-- 
Yann Dirson | Vates Platform Developer
XCP-ng & Xen Orchestra - Vates solutions
w: vates.tech | xcp-ng.org | xen-orchestra.com



Yann Dirson | Vates Platform Developer

XCP-ng & Xen Orchestra - Vates solutions
w: vates.fr | xcp-ng.org | xen-orchestra.com

[PATCH] docs/man: fix xenstore-write synopsis

[Yann Dirson]

Reported-by: zithro 
Signed-off-by: Yann Dirson 
---
 docs/man/xenstore-write.1.pod | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/man/xenstore-write.1.pod b/docs/man/xenstore-write.1.pod
index a0b1bca333..74f80f7b1b 100644
--- a/docs/man/xenstore-write.1.pod
+++ b/docs/man/xenstore-write.1.pod
@@ -4,7 +4,7 @@ xenstore-write - write Xenstore values
 
 =head1 SYNOPSIS
 
-B [I]... I I...
+B [I]... I I...
 
 =head1 DESCRIPTION
 
-- 
2.30.2



Yann Dirson | Vates Platform Developer

XCP-ng & Xen Orchestra - Vates solutions
w: vates.fr | xcp-ng.org | xen-orchestra.com

Re: [patch v3 33/36] x86/apic: Save the APIC virtual base address

[Sergey Shtylyov]

Hello!

On 5/8/23 10:44 PM, Thomas Gleixner wrote:

> From: Thomas Gleixner 
> 
> For parallel CPU brinugp it's required to read the APIC ID in the low level
> startup code. The virtual APIC base address is a constant because its a
> fix-mapped address. Exposing that constant which is composed via macros to
> assembly code is non-trivial dues to header inclusion hell.

   s/dues/due/?

> Aside of that it's constant only because of the vsyscall ABI
> requirement. Once vsyscall is out of the picture the fixmap can be placed
> at runtime.
> 
> Avoid header hell, stay flexible and store the address in a variable which
> can be exposed to the low level startup code.
> 
> Signed-off-by: Thomas Gleixner 
> Tested-by: Michael Kelley 

[...]

MBR, Sergey

Re: [PATCH] docs/man: fix xenstore-write synopsis

[Andrew Cooper]

On 09/05/2023 10:01 am, Yann Dirson wrote:
> Reported-by: zithro 
> Signed-off-by: Yann Dirson 

Oops.

Reviewed-by: Andrew Cooper 

Re: [PATCH 3/3] x86: Use CpuidUserDis if an AMD HVM guest toggles CPUID faulting

[Andrew Cooper]

On 08/05/2023 2:18 pm, Jan Beulich wrote:
> On 05.05.2023 19:57, Alejandro Vallejo wrote:
>> This is in order to aid guests of AMD hardware that we have exposed
>> CPUID faulting to. If they try to modify the Intel MSR that enables
>> the feature, trigger levelling so AMD's version of it (CpuidUserDis)
>> is used instead.
>>
>> Signed-off-by: Alejandro Vallejo 
>> ---
>>  xen/arch/x86/msr.c | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
> Don't you also need to update cpu-policy.c:calculate_host_policy()
> for the guest to actually know it can use the functionality? Which
> in turn would appear to require some form of adjustment to
> lib/x86/policy.c:x86_cpu_policies_are_compatible().

I asked Alejandro to do it like this.

Advertising this to guests requires plumbing another MSR into the
infrastructure which isn't quite set up properly let, and is in flux
from my work.

For now, this just lets Xen enforce the policy over PV guests, which is
an improvement in and of itself.

~Andrew

Re: [patch v3 08/36] x86/smpboot: Split up native_cpu_up() into separate phases and document them

[Peter Zijlstra]

On Mon, May 08, 2023 at 09:43:39PM +0200, Thomas Gleixner wrote:

> @@ -233,14 +237,31 @@ static void notrace start_secondary(void
>   load_cr3(swapper_pg_dir);
>   __flush_tlb_all();
>  #endif
> + /*
> +  * Sync point with wait_cpu_initialized(). Before proceeding through
> +  * cpu_init(), the AP will call wait_for_master_cpu() which sets its
> +  * own bit in cpu_initialized_mask and then waits for the BSP to set
> +  * its bit in cpu_callout_mask to release it.
> +  */
>   cpu_init_secondary();
>   rcu_cpu_starting(raw_smp_processor_id());
>   x86_cpuinit.early_percpu_clock_init();
> +
> + /*
> +  * Sync point with wait_cpu_callin(). The AP doesn't wait here
> +  * but just sets the bit to let the controlling CPU (BSP) know that
> +  * it's got this far.
> +  */
>   smp_callin();
>  
> - /* otherwise gcc will move up smp_processor_id before the cpu_init */
> + /* Otherwise gcc will move up smp_processor_id() before cpu_init() */
>   barrier();

Not to the detriment of this patch, but this barrier() and it's comment
seem weird vs smp_callin(). That function ends with an atomic bitop (it
has to, at the very least it must not be weaker than store-release) but
also has an explicit wmb() to order setup vs CPU_STARTING.

(arguably that should be a full fence *AND* get a comment)

There is no way the smp_processor_id() referred to in this comment can
land before cpu_init() even without the barrier().

> - /* Check TSC synchronization with the control CPU: */
> +
> + /*
> +  * Check TSC synchronization with the control CPU, which will do
> +  * its part of this from wait_cpu_online(), making it an implicit
> +  * synchronization point.
> +  */
>   check_tsc_sync_target();
>  
>   /*

Re: [patch v3 08/36] x86/smpboot: Split up native_cpu_up() into separate phases and document them

[Peter Zijlstra]

Again, not really this patch, but since I had to look at this code 

On Mon, May 08, 2023 at 09:43:39PM +0200, Thomas Gleixner wrote:
> @@ -1048,60 +1066,89 @@ static int do_boot_cpu(int apicid, int c

/*
 * AP might wait on cpu_callout_mask in cpu_init() with
 * cpu_initialized_mask set if previous attempt to online
 * it timed-out. Clear cpu_initialized_mask so that after
 * INIT/SIPI it could start with a clean state.
 */
cpumask_clear_cpu(cpu, cpu_initialized_mask);
smp_mb();

^^^ that barrier is weird too, cpumask_clear_cpu() is an atomic op and
implies much the same (this is x86 after all). If you want to be super
explicit about it write:

smp_mb__after_atomic();

(which is a no-op) but then it still very much requires a comment as to
what exactly it orders against what.


/*
 * Wake up a CPU in difference cases:
 * - Use a method from the APIC driver if one defined, with wakeup
 *   straight to 64-bit mode preferred over wakeup to RM.
 * Otherwise,
>* - Use an INIT boot APIC message
>*/
>   if (apic->wakeup_secondary_cpu_64)
> + return apic->wakeup_secondary_cpu_64(apicid, start_ip);
>   else if (apic->wakeup_secondary_cpu)
> + return apic->wakeup_secondary_cpu(apicid, start_ip);
>  
> + return wakeup_secondary_cpu_via_init(apicid, start_ip);
> +}

[PATCH] docs: fix xenstore-paths doc structure

[Yann Dirson]

We currently have "Per Domain Paths" as an empty section, whereas it
looks like "General Paths" was not indended to include all the
following sections.

Signed-off-by: Yann Dirson 
---
 docs/misc/xenstore-paths.pandoc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/misc/xenstore-paths.pandoc b/docs/misc/xenstore-paths.pandoc
index bffb8ea544..f07ef90f63 100644
--- a/docs/misc/xenstore-paths.pandoc
+++ b/docs/misc/xenstore-paths.pandoc
@@ -129,7 +129,7 @@ create writable subdirectories as necessary.
 
 ## Per Domain Paths
 
-## General Paths
+### General Paths
 
  ~/vm = PATH []
 
-- 
2.30.2



Yann Dirson | Vates Platform Developer

XCP-ng & Xen Orchestra - Vates solutions
w: vates.fr | xcp-ng.org | xen-orchestra.com

Re: [patch v3 08/36] x86/smpboot: Split up native_cpu_up() into separate phases and document them

[Peter Zijlstra]

And since I'm commenting on existing things anyway, let me continue...

On Mon, May 08, 2023 at 09:43:39PM +0200, Thomas Gleixner wrote:

> +static int wait_cpu_cpumask(unsigned int cpu, const struct cpumask *mask)
> +{
> + unsigned long timeout;
>  
> + /*
> +  * Wait up to 10s for the CPU to report in.
> +  */
> + timeout = jiffies + 10*HZ;
> + while (time_before(jiffies, timeout)) {
> + if (cpumask_test_cpu(cpu, mask))
> + return 0;
> +
> + schedule();
>   }
> + return -1;
> +}

> +/*
> + * Bringup step three: Wait for the target AP to reach smp_callin().
> + * The AP is not waiting for us here so we don't need to parallelise
> + * this step. Not entirely clear why we care about this, since we just
> + * proceed directly to TSC synchronization which is the next sync
> + * point with the AP anyway.
> + */
> +static void wait_cpu_callin(unsigned int cpu)
> +{
> + while (!cpumask_test_cpu(cpu, cpu_callin_mask))
> + schedule();
> +}
> +
> +/*
> + * Bringup step four: Synchronize the TSC and wait for the target AP
> + * to reach set_cpu_online() in start_secondary().
> + */
> +static void wait_cpu_online(unsigned int cpu)
>  {
>   unsigned long flags;
> +
> + /*
> +  * Check TSC synchronization with the AP (keep irqs disabled
> +  * while doing so):
> +  */
> + local_irq_save(flags);
> + check_tsc_sync_source(cpu);
> + local_irq_restore(flags);
> +
> + /*
> +  * Wait for the AP to mark itself online, so the core caller
> +  * can drop sparse_irq_lock.
> +  */
> + while (!cpu_online(cpu))
> + schedule();
> +}

These schedule() loops make me itch... this is basically Ye Olde yield()
loop with all it's known 'benefits'.

Now, I don't think it's horribly broken, we're explicitly waiting on
another CPU and can't have priority inversions, but yuck!

It could all be somewhat cleaned up with wait_var_event{_timeout}() and
wake_up_var(), but I'm really not sure that's worth it. But at least it
requires a comment to justify.

[PATCH] iommu/vtd: fix address translation for superpages

[Roger Pau Monne]

When translating an address that falls inside of a superpage in the
IOMMU page tables the fetching of the PTE physical address field
wasn't using dma_pte_addr(), which caused the returned data to be
corrupt as it would contain bits not related to the address field.

Fix this by re-using the value of pte_maddr which is obtained using
dma_pte_addr().  Such change requires adjusting the previous error
path to zero pte_maddr.

Fixes: c71e55501a61 ('VT-d: have callers specify the target level for page 
table walks')
Signed-off-by: Roger Pau Monné 
---
 xen/drivers/passthrough/vtd/iommu.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 130a159cde07..819e996e6269 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -359,16 +359,18 @@ static uint64_t addr_to_dma_page_maddr(struct domain 
*domain, daddr_t addr,
 
 if ( !alloc )
 {
-pte_maddr = 0;
 if ( !dma_pte_present(*pte) )
+{
+pte_maddr = 0;
 break;
+}
 
 /*
  * When the leaf entry was requested, pass back the full PTE,
  * with the address adjusted to account for the residual of
  * the walk.
  */
-pte_maddr = pte->val +
+pte_maddr +=
 (addr & ((1UL << level_to_offset_bits(level)) - 1) &
  PAGE_MASK);
 if ( !target )
-- 
2.40.0

Re: [patch v3 13/36] x86/smpboot: Remove cpu_callin_mask

[Peter Zijlstra]

On Mon, May 08, 2023 at 09:43:47PM +0200, Thomas Gleixner wrote:

> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c

> @@ -167,21 +166,16 @@ static inline void smpboot_restore_warm_
>   */
>  static void smp_callin(void)
>  {
> - int cpuid;
> + int cpuid = smp_processor_id();
>  
>   /*
>* If waken up by an INIT in an 82489DX configuration
> -  * cpu_callout_mask guarantees we don't get here before
> -  * an INIT_deassert IPI reaches our local APIC, so it is
> -  * now safe to touch our local APIC.
> -  */
> - cpuid = smp_processor_id();
> -
> - /*
> -  * the boot CPU has finished the init stage and is spinning
> -  * on callin_map until we finish. We are free to set up this
> -  * CPU, first the APIC. (this is probably redundant on most
> -  * boards)
> +  * cpu_callout_mask guarantees we don't get here before an
> +  * INIT_deassert IPI reaches our local APIC, so it is now safe to
> +  * touch our local APIC.
> +  *
> +  * Set up this CPU, first the APIC, which is probably redundant on
> +  * most boards.
>*/
>   apic_ap_setup();
>  
> @@ -192,7 +186,7 @@ static void smp_callin(void)
>* The topology information must be up to date before
>* calibrate_delay() and notify_cpu_starting().
>*/
> - set_cpu_sibling_map(raw_smp_processor_id());
> + set_cpu_sibling_map(cpuid);
>  
>   ap_init_aperfmperf();
>  
> @@ -205,11 +199,6 @@ static void smp_callin(void)
>* state (CPUHP_ONLINE in the case of serial bringup).
>*/
>   notify_cpu_starting(cpuid);
> -
> - /*
> -  * Allow the master to continue.
> -  */
> - cpumask_set_cpu(cpuid, cpu_callin_mask);
>  }
>  
>  static void ap_calibrate_delay(void)
> @@ -268,11 +257,6 @@ static void notrace start_secondary(void
>   rcu_cpu_starting(raw_smp_processor_id());
>   x86_cpuinit.early_percpu_clock_init();
>  
> - /*
> -  * Sync point with wait_cpu_callin(). The AP doesn't wait here
> -  * but just sets the bit to let the controlling CPU (BSP) know that
> -  * it's got this far.
> -  */
>   smp_callin();
>  
>   /* Otherwise gcc will move up smp_processor_id() before cpu_init() */

Good riddance to that mask; however is smp_callin() still an appropriate
name for that function?

Would smp_starting() -- seeing how this kicks of CPU_STARTING not be a
better name?

Re: [XEN][PATCH v6 05/19] xen/arm: Add CONFIG_OVERLAY_DTB

[Michal Orzel]

On 04/05/2023 06:11, Henry Wang wrote:
> 
> 
> Hi Vikram,
> 
>> -Original Message-
>> Subject: [XEN][PATCH v6 05/19] xen/arm: Add CONFIG_OVERLAY_DTB
>>
>> Introduce a config option where the user can enable support for
>> adding/removing
>> device tree nodes using a device tree binary overlay.
> 
> May I please also suggest adding a CHANGELOG entry in the "### Added"
> section? I personally think this series deserves a CHANGELOG entry but I
> am open to others' opinion though. Thanks!
Yes, this definitely deserves an entry.
+
please mention the SUPPORT (and CHANGELOG) changes in a commit msg (for now
it only covers the Kconfig option).

~Michal

Re: [patch v3 14/36] [patch V2 14/38] cpu/hotplug: Rework sparse_irq locking in bringup_cpu()

[Peter Zijlstra]

On Mon, May 08, 2023 at 09:43:49PM +0200, Thomas Gleixner wrote:
> From: Thomas Gleixner 
> 
> There is no harm to hold sparse_irq lock until the upcoming CPU completes
> in cpuhp_online_idle(). This allows to remove cpu_online() synchronization
> from architecture code.

Uhh.. damn. Can you at the very least ammend the comment near
irq_lock_sparse() to mention these extra duties?

[PATCH] x86/iommu: fix wrong iterator type in arch_iommu_hwdom_init()

[Roger Pau Monne]

The 'i' iterator index stores a pdx, not a pfn, and hence the initial
assignation of start (which stores a pfn) needs a conversion from pfn
to pdx.

Fixes: 6b4f6a31ace1 ('x86/PVH: de-duplicate mappings for first Mb of Dom0 
memory')
Signed-off-by: Roger Pau Monné 
---
 xen/drivers/passthrough/x86/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/drivers/passthrough/x86/iommu.c 
b/xen/drivers/passthrough/x86/iommu.c
index cb0788960a08..6bc79e7ec843 100644
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -406,7 +406,7 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
  */
 start = paging_mode_translate(d) ? PFN_DOWN(MB(1)) : 0;
 
-for ( i = start, count = 0; i < top; )
+for ( i = pfn_to_pdx(start), count = 0; i < top; )
 {
 unsigned long pfn = pdx_to_pfn(i);
 unsigned int perms = hwdom_iommu_map(d, pfn, max_pfn);
-- 
2.40.0

Re: [patch v3 18/36] [patch V2 18/38] cpu/hotplug: Add CPU state tracking and synchronization

[Peter Zijlstra]

On Mon, May 08, 2023 at 09:43:55PM +0200, Thomas Gleixner wrote:

> +static inline void cpuhp_ap_update_sync_state(enum cpuhp_sync_state state)
> +{
> + atomic_t *st = this_cpu_ptr(&cpuhp_state.ap_sync_state);
> + int sync = atomic_read(st);
> +
> + while (!atomic_try_cmpxchg(st, &sync, state));
> +}

Why isn't:

atomic_set(st, state);

any good?

Re: [XEN][PATCH v6 08/19] xen/device-tree: Add device_tree_find_node_by_path() to find nodes in device tree

[Michal Orzel]

On 04/05/2023 06:23, Henry Wang wrote:
> 
> 
> Hi Vikram,
> 
>> -Original Message-
>> Subject: [XEN][PATCH v6 08/19] xen/device-tree: Add
>> device_tree_find_node_by_path() to find nodes in device tree
>>
>> Add device_tree_find_node_by_path() to find a matching node with path for
>> a
>> dt_device_node.
>>
>> Reason behind this function:
>> Each time overlay nodes are added using .dtbo, a new fdt(memcpy of
>> device_tree_flattened) is created and updated with overlay nodes. This
>> updated fdt is further unflattened to a dt_host_new. Next, we need to 
>> find
>> the overlay nodes in dt_host_new, find the overlay node's parent in 
>> dt_host
>> and add the nodes as child under their parent in the dt_host. Thus we 
>> need
>> this function to search for node in different unflattened device trees.
>>
>> Also, make dt_find_node_by_path() static inline.
>>
>> Signed-off-by: Vikram Garhwal 
>> ---
>>  xen/common/device_tree.c  |  5 +++--
>>  xen/include/xen/device_tree.h | 17 +++--
>>  2 files changed, 18 insertions(+), 4 deletions(-)
>>
> 
> [...]
> 
>>  /**
>> - * dt_find_node_by_path - Find a node matching a full DT path
>> + * device_tree_find_node_by_path - Generic function to find a node
>> matching the
>> + * full DT path for any given unflatten device tree
>> + * @dt_node: The device tree to search
> 
> I noticed that you missed Michal's comment here about renaming the
> "dt_node" here to "dt" to match below function prototype...
This is one thing. The other is that in v5 you said this is to be a generic 
function
where you can search from a middle of a device tree. This means that the 
parameter should be
named "node" or "from" and the description needs to say "The node to start 
searching from" +
seeing the lack of ->allnext you can mention that this is inclusive (i.e. the 
passed node will also be searched).

~Michal

Re: [patch v3 18/36] [patch V2 18/38] cpu/hotplug: Add CPU state tracking and synchronization

[Peter Zijlstra]

On Tue, May 09, 2023 at 01:07:23PM +0200, Peter Zijlstra wrote:
> On Mon, May 08, 2023 at 09:43:55PM +0200, Thomas Gleixner wrote:
> 
> > +static inline void cpuhp_ap_update_sync_state(enum cpuhp_sync_state state)
> > +{
> > +   atomic_t *st = this_cpu_ptr(&cpuhp_state.ap_sync_state);
> > +   int sync = atomic_read(st);
> > +
> > +   while (!atomic_try_cmpxchg(st, &sync, state));
> > +}
> 
> Why isn't:
> 
>   atomic_set(st, state);
> 
> any good?

Hmm, should at the very least be atomic_set_release(), but if you want
the full barrier then:

(void)atomic_xchg(st, state);

is the much saner way to write the above.

Re: [patch v3 08/36] x86/smpboot: Split up native_cpu_up() into separate phases and document them

[Thomas Gleixner]

On Tue, May 09 2023 at 12:04, Peter Zijlstra wrote:
> On Mon, May 08, 2023 at 09:43:39PM +0200, Thomas Gleixner wrote:
>> +/*
>> + * Sync point with wait_cpu_callin(). The AP doesn't wait here
>> + * but just sets the bit to let the controlling CPU (BSP) know that
>> + * it's got this far.
>> + */
>>  smp_callin();
>>  
>> -/* otherwise gcc will move up smp_processor_id before the cpu_init */
>> +/* Otherwise gcc will move up smp_processor_id() before cpu_init() */
>>  barrier();
>
> Not to the detriment of this patch, but this barrier() and it's comment
> seem weird vs smp_callin(). That function ends with an atomic bitop (it
> has to, at the very least it must not be weaker than store-release) but
> also has an explicit wmb() to order setup vs CPU_STARTING.
>
> (arguably that should be a full fence *AND* get a comment)
>
> There is no way the smp_processor_id() referred to in this comment can
> land before cpu_init() even without the barrier().

Right. Let me clean that up.

Thanks,

tglx

Re: [patch v3 08/36] x86/smpboot: Split up native_cpu_up() into separate phases and document them

[Thomas Gleixner]

On Tue, May 09 2023 at 12:19, Peter Zijlstra wrote:
> On Mon, May 08, 2023 at 09:43:39PM +0200, Thomas Gleixner wrote:
>> @@ -1048,60 +1066,89 @@ static int do_boot_cpu(int apicid, int c
>
>   /*
>* AP might wait on cpu_callout_mask in cpu_init() with
>* cpu_initialized_mask set if previous attempt to online
>* it timed-out. Clear cpu_initialized_mask so that after
>* INIT/SIPI it could start with a clean state.
>*/
>   cpumask_clear_cpu(cpu, cpu_initialized_mask);
>   smp_mb();
>
> ^^^ that barrier is weird too, cpumask_clear_cpu() is an atomic op and
> implies much the same (this is x86 after all). If you want to be super
> explicit about it write:
>
>   smp_mb__after_atomic();
>
> (which is a no-op) but then it still very much requires a comment as to
> what exactly it orders against what.

As this is gone a few patches later, I just be lazy and leave it alone.

Re: [patch v3 08/36] x86/smpboot: Split up native_cpu_up() into separate phases and document them

[Thomas Gleixner]

On Tue, May 09 2023 at 12:31, Peter Zijlstra wrote:
> On Mon, May 08, 2023 at 09:43:39PM +0200, Thomas Gleixner wrote:
>> +/*
>> + * Wait for the AP to mark itself online, so the core caller
>> + * can drop sparse_irq_lock.
>> + */
>> +while (!cpu_online(cpu))
>> +schedule();
>> +}
>
> These schedule() loops make me itch... this is basically Ye Olde yield()
> loop with all it's known 'benefits'.
>
> Now, I don't think it's horribly broken, we're explicitly waiting on
> another CPU and can't have priority inversions, but yuck!
>
> It could all be somewhat cleaned up with wait_var_event{_timeout}() and
> wake_up_var(), but I'm really not sure that's worth it. But at least it
> requires a comment to justify.

All of them are gone with the later patches and the control CPU will
just go straight to wait for the completion in the core code.

Re: [patch v3 13/36] x86/smpboot: Remove cpu_callin_mask

[Thomas Gleixner]

On Tue, May 09 2023 at 12:49, Peter Zijlstra wrote:
> On Mon, May 08, 2023 at 09:43:47PM +0200, Thomas Gleixner wrote:
>> -/*
>> - * Sync point with wait_cpu_callin(). The AP doesn't wait here
>> - * but just sets the bit to let the controlling CPU (BSP) know that
>> - * it's got this far.
>> - */
>>  smp_callin();
>>  
>>  /* Otherwise gcc will move up smp_processor_id() before cpu_init() */
>
> Good riddance to that mask; however is smp_callin() still an appropriate
> name for that function?
>
> Would smp_starting() -- seeing how this kicks of CPU_STARTING not be a
> better name?

Something like that, yes.

Re: [patch v3 14/36] [patch V2 14/38] cpu/hotplug: Rework sparse_irq locking in bringup_cpu()

[Thomas Gleixner]

On Tue, May 09 2023 at 13:02, Peter Zijlstra wrote:
> On Mon, May 08, 2023 at 09:43:49PM +0200, Thomas Gleixner wrote:
>> From: Thomas Gleixner 
>> 
>> There is no harm to hold sparse_irq lock until the upcoming CPU completes
>> in cpuhp_online_idle(). This allows to remove cpu_online() synchronization
>> from architecture code.
>
> Uhh.. damn. Can you at the very least ammend the comment near
> irq_lock_sparse() to mention these extra duties?

Will do.

Re: [patch v3 18/36] [patch V2 18/38] cpu/hotplug: Add CPU state tracking and synchronization

[Thomas Gleixner]

On Tue, May 09 2023 at 13:07, Peter Zijlstra wrote:
> On Mon, May 08, 2023 at 09:43:55PM +0200, Thomas Gleixner wrote:
>
>> +static inline void cpuhp_ap_update_sync_state(enum cpuhp_sync_state state)
>> +{
>> +atomic_t *st = this_cpu_ptr(&cpuhp_state.ap_sync_state);
>> +int sync = atomic_read(st);
>> +
>> +while (!atomic_try_cmpxchg(st, &sync, state));
>> +}
>
> Why isn't:
>
>   atomic_set(st, state);
>
> any good?

Good question. It's only the other side (control CPU) which needs to be
careful.

Thanks,

tglx

[xen-unstable-smoke test] 180588: tolerable all pass - PUSHED

[osstest service owner]

flight 180588 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180588/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-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-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  8b1ac353b4db7c5bb2f82cb6afee9cc641e756a4
baseline version:
 xen  a16fb78515d54be95f81c0d1c0a3a7b954a54d0a

Last test of basis   180577  2023-05-08 13:03:54 Z0 days
Testing same since   180588  2023-05-09 10:01:52 Z0 days1 attempts


People who touched revisions under test:
  Andrew Cooper 
  Jan Beulich 
  Stefano Stabellini 
  Yann Dirson 

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
   a16fb78515..8b1ac353b4  8b1ac353b4db7c5bb2f82cb6afee9cc641e756a4 -> smoke

Re: [PATCH v2] Fix install.sh for systemd

[Andrew Cooper]

On 08/05/2023 6:14 pm, Olaf Hering wrote:
> On a fedora system, if you run `sudo sh install.sh` you break your
> system.  The installation clobbers /var/run, a symlink to /run.  A
> subsequent boot fails when /var/run and /run are different since
> accesses through /var/run can't find items that now only exist in /run
> and vice-versa.
>
> Skip populating /var/run/xen during make install.
> The directory is already created by some scripts. Adjust all remaining
> scripts to create XEN_RUN_DIR at runtime.
>
> XEN_RUN_STORED is covered by XEN_RUN_DIR because xenstored is usually
> started afterwards.
>
> Reported-by: Jason Andryuk 
> Signed-off-by: Olaf Hering 

TBH, I think this goes to show how many people do `make install` by hand
on a system, rather than using a packaged version.

One query...

> diff --git a/tools/hotplug/Linux/init.d/xendriverdomain.in 
> b/tools/hotplug/Linux/init.d/xendriverdomain.in
> index c63060f62a..1055d0b942 100644
> --- a/tools/hotplug/Linux/init.d/xendriverdomain.in
> +++ b/tools/hotplug/Linux/init.d/xendriverdomain.in
> @@ -49,6 +49,7 @@ fi
>  
>  do_start () {
>   echo Starting xl devd...
> + mkdir -m700 -p @XEN_RUN_DIR@

Why is this 700, and the others just using regular perms?

Also, doesn't it want quoting like the other examples too?

~Andrew

Re: [PATCH v6 2/4] xen/riscv: introduce setup_initial_pages

[Oleksii]

On Mon, 2023-05-08 at 10:58 +0200, Jan Beulich wrote:
> On 03.05.2023 18:31, Oleksii Kurochko wrote:
> > --- a/xen/arch/riscv/include/asm/config.h
> > +++ b/xen/arch/riscv/include/asm/config.h
> > @@ -70,12 +70,23 @@
> >    name:
> >  #endif
> >  
> > -#define XEN_VIRT_START  _AT(UL, 0x8020)
> > +#ifdef CONFIG_RISCV_64
> > +#define XEN_VIRT_START 0xC000 /* (_AC(-1, UL) + 1 -
> > GB(1)) */
> > +#else
> > +#error "RV32 isn't supported"
> > +#endif
> >  
> >  #define SMP_CACHE_BYTES (1 << 6)
> >  
> >  #define STACK_SIZE PAGE_SIZE
> >  
> > +#ifdef CONFIG_RISCV_64
> > +#define CONFIG_PAGING_LEVELS 3
> > +#define RV_STAGE1_MODE SATP_MODE_SV39
> > +#else
> 
> #define CONFIG_PAGING_LEVELS 2
> 
> (or else I would think ...
> 
> > +#define RV_STAGE1_MODE SATP_MODE_SV32
> 
> ... this and hence the entire "#else" should also be omitted)
Agree. it will be better to set CONFIG_PAGING_LEVELS for RV32 too.

> 
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/current.h
> > @@ -0,0 +1,10 @@
> > +#ifndef __ASM_CURRENT_H
> > +#define __ASM_CURRENT_H
> > +
> > +#define switch_stack_and_jump(stack, fn)    \
> > +    asm volatile (  \
> > +    "mv sp, %0 \n"  \
> > +    "j " #fn :: "r" (stack) :   \
> > +    )
> 
> Nit: Style. Our normal way of writing this would be
> 
> #define switch_stack_and_jump(stack, fn)    \
>     asm volatile (  \
>     "mv sp, %0\n"   \
>     "j " #fn :: "r" (stack) )
> 
> i.e. unnecessary colon omitted, no trailin blank on any generated
> assembly line, and closing paren not placed on its own line (only
> closing figure braces would normally live on their own lines).
Thanks for clarification

> 
> However, as to the 3rd colon: Can you really get away here without a
> memory clobber (i.e. the construct being a full compiler barrier)?
I am not 100% sure so to be safe I would add memory clobber. Thanks.

> 
> Further I think you want to make the use of "fn" visible to the
> compiler, by using an "X" constraint just like Arm does.
> 
> Finally I think you want to add unreachable(), like both Arm and x86
> have it. Plus the "noreturn" on relevant functions.
It makes sense. I'll take into account.
> 
> > --- /dev/null
> > +++ b/xen/arch/riscv/include/asm/page.h
> > @@ -0,0 +1,62 @@
> > +#ifndef _ASM_RISCV_PAGE_H
> > +#define _ASM_RISCV_PAGE_H
> > +
> > +#include 
> > +#include 
> > =)+
> > +#define VPN_MASK    ((unsigned
> > long)(PAGETABLE_ENTRIES - 1))
> > +
> > +#define XEN_PT_LEVEL_ORDER(lvl) ((lvl) * PAGETABLE_ORDER)
> > +#define XEN_PT_LEVEL_SHIFT(lvl) (XEN_PT_LEVEL_ORDER(lvl) +
> > PAGE_SHIFT)
> > +#define XEN_PT_LEVEL_SIZE(lvl)  (_AT(paddr_t, 1) <<
> > XEN_PT_LEVEL_SHIFT(lvl))
> > +#define XEN_PT_LEVEL_MAP_MASK(lvl)  (~(XEN_PT_LEVEL_SIZE(lvl) -
> > 1))
> > +#define XEN_PT_LEVEL_MASK(lvl)  (VPN_MASK <<
> > XEN_PT_LEVEL_SHIFT(lvl))
> > +
> > +#define PTE_VALID   BIT(0, UL)
> > +#define PTE_READABLE    BIT(1, UL)
> > +#define PTE_WRITABLE    BIT(2, UL)
> > +#define PTE_EXECUTABLE  BIT(3, UL)
> > +#define PTE_USER    BIT(4, UL)
> > +#define PTE_GLOBAL  BIT(5, UL)
> > +#define PTE_ACCESSED    BIT(6, UL)
> > +#define PTE_DIRTY   BIT(7, UL)
> > +#define PTE_RSW (BIT(8, UL) | BIT(9, UL))
> > +
> > +#define PTE_LEAF_DEFAULT    (PTE_VALID | PTE_READABLE |
> > PTE_WRITABLE)
> > +#define PTE_TABLE   (PTE_VALID)
> > +
> > +/* Calculate the offsets into the pagetables for a given VA */
> > +#define pt_linear_offset(lvl, va)   ((va) >>
> > XEN_PT_LEVEL_SHIFT(lvl))
> > +
> > +#define pt_index(lvl, va) pt_linear_offset(lvl, (va) &
> > XEN_PT_LEVEL_MASK(lvl))
> 
> Maybe better
> 
> #define pt_index(lvl, va) (pt_linear_offset(lvl, va) & VPN_MASK)
> 
> as the involved constant will be easier to use for the compiler?
But VPN_MASK should be shifted by level shift value.

Or did you mean that it will be better to pre-calculate MASK for each
level instead of define MASK as (VPN_MASK <<
XEN_PT_LEVEL_SHIFT(lvl)).

Probably I have to re-check that but taking into account that all
values are defined during compile time so they will be pre-calculated
so it shouldn't be big difference for the compiler.
> 
> > +/* Page Table entry */
> > +typedef struct {
> > +#ifdef CONFIG_RISCV_64
> > +    uint64_t pte;
> > +#else
> > +    uint32_t pte;
> > +#endif
> > +} pte_t;
> > +
> > +#define pte_to_addr(x) (((paddr_t)(x) >> PTE_PPN_SHIFT) <<
> > PAGE_SHIFT)
> > +
> > +#define addr_to_pte(x) (((x) >> PAGE_SHIFT) << PTE_PPN_SHIFT)
> 
> Are these two macros useful on their own? I ask because I still
> consider
> them somewhat misnamed, as they don't produce / consume a PTE (but
> the
> raw value). Yet generally you want to avoid any code operating on raw
> values, using pte_t instead. IOW I would hope to be able to conv

Re: [PATCH 1/6] x86/cpu-policy: Drop build time cross-checks of featureset sizes

[Andrew Cooper]

On 08/05/2023 7:47 am, Jan Beulich wrote:
> On 04.05.2023 21:39, Andrew Cooper wrote:
>> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
>> code which looks like:
>>
>>   uint32_t foo[1] = { 1, 2, 3 };
>>
>> However, GCC 12 at least does now warn for this:
>>
>>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>> 884 | uint32_t foo[1] = { 1, 2, 3 };
>> |^
>>   foo.c:1:24: note: (near initialization for 'foo')
> I'm pretty sure all gcc versions we support diagnose such cases. In turn
> the arrays in question don't have explicit dimensions at their
> definition sites, and hence they derive their dimensions from their
> initializers. So the build-time-checks are about the arrays in fact
> obtaining the right dimensions, i.e. the initializers being suitable.
>
> With the core part of the reasoning not being applicable, I'm afraid I
> can't even say "okay with an adjusted description".

Now I'm extra confused.

I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
when I was expecting one, and there was a bug in the original featureset
work caused by this going wrong.

But godbolt seems to agree that even GCC 4.1 notices.

Maybe it was some other error (C file not seeing the header properly?)
which disappeared across the upstream review?


Either way, these aren't appropriate, and need deleting before patch 5,
because the check is no longer valid when a featureset can be longer
than the autogen length.

~Andrew

Re: [patch v3 34/36] x86/smpboot: Implement a bit spinlock to protect the realmode stack

[Peter Zijlstra]

On Mon, May 08, 2023 at 09:44:22PM +0200, Thomas Gleixner wrote:

> @@ -252,6 +252,17 @@ SYM_INNER_LABEL(secondary_startup_64_no_
>   movqTASK_threadsp(%rax), %rsp
>  
>   /*
> +  * Now that this CPU is running on its own stack, drop the realmode
> +  * protection. For the boot CPU the pointer is NULL!
> +  */
> + movqtrampoline_lock(%rip), %rax
movl$0, (%rax)

> +.Lsetup_gdt:
> + /*
>* We must switch to a new descriptor in kernel space for the GDT
>* because soon the kernel won't have access anymore to the userspace
>* addresses where we're currently running on. We have to do that here

> --- a/arch/x86/realmode/rm/trampoline_64.S
> +++ b/arch/x86/realmode/rm/trampoline_64.S
> @@ -37,6 +37,24 @@
>   .text
>   .code16
>  
> +.macro LOAD_REALMODE_ESP
> + /*
> +  * Make sure only one CPU fiddles with the realmode stack
> +  */
> +.Llock_rm\@:
> + btl $0, tr_lock
> + jnc 2f
> + pause
> + jmp .Llock_rm\@
> +2:
> + lock
> + btsl$0, tr_lock
> + jc  .Llock_rm\@

Do we really care about performance here; or should we pick the simpler
form? Also, 'lock' is a prefix, not an instruction.

.Llock_rm\@:
lock btsl   $0, tr_lock;
jnc 2f
pause
jmp .Llock_rm\@
2:

> +
> + # Setup stack
> + movl$rm_stack_end, %esp
> +.endm
> +
>   .balign PAGE_SIZE

[xen-unstable test] 180584: tolerable FAIL

[osstest service owner]

flight 180584 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180584/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-i386-libvirt-raw   7 xen-install  fail in 180580 pass in 180584
 test-amd64-i386-pair 11 xen-install/dst_host   fail pass in 180580
 test-amd64-coresched-i386-xl 20 guest-localmigrate/x10 fail pass in 180580
 test-amd64-amd64-libvirt-vhd 19 guest-start/debian.repeat  fail pass in 180580
 test-amd64-i386-xl-vhd   21 guest-start/debian.repeat  fail pass in 180580

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180580
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180580
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180580
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180580
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180580
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180580
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180580
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180580
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180580
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180580
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180580
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180580
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  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  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-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  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-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-amd64-i386-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-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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-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-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 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-libvirt 15 migrate-support-checkfail   never pass

version targe

Re: [PATCH v2] Fix install.sh for systemd

[Olaf Hering]

Tue, 9 May 2023 13:47:11 +0100 Andrew Cooper :

> > +++ b/tools/hotplug/Linux/init.d/xendriverdomain.in
> > @@ -49,6 +49,7 @@ fi
> >  
> >  do_start () {
> > echo Starting xl devd...
> > +   mkdir -m700 -p @XEN_RUN_DIR@  
> 
> Why is this 700, and the others just using regular perms?

I think the permissions are just copy&paste from elsewhere.
I have to check why -m700 is used anyway, and why it is not used consistently.

> Also, doesn't it want quoting like the other examples too?

There is a mix of quoting and non-quoting. Not sure if having spaces in any of 
the path names does actually work today.

I will double check this detail as well.

Olaf


pgpha8EJvdIwd.pgp
Description: Digitale Signatur von OpenPGP

Re: [patch v3 34/36] x86/smpboot: Implement a bit spinlock to protect the realmode stack

[Thomas Gleixner]

On Tue, May 09 2023 at 15:13, Peter Zijlstra wrote:
> On Mon, May 08, 2023 at 09:44:22PM +0200, Thomas Gleixner wrote:
>
> Do we really care about performance here; or should we pick the simpler
> form? Also, 'lock' is a prefix, not an instruction.

Right. KISS is the way to go.

Re: [patch v3 35/36] x86/smpboot: Support parallel startup of secondary CPUs

[Peter Zijlstra]

On Mon, May 08, 2023 at 09:44:23PM +0200, Thomas Gleixner wrote:
> + /*  APIC ID not found in the table. Drop the trampoline lock and bail. 
> */
> + movqtrampoline_lock(%rip), %rax

Again:

movl$0, (%rax)

is sufficient for unlock.

> + lock
> + btrl$0, (%rax)

Re: [PATCH 5/6] x86/cpu-policy: Disentangle X86_NR_FEAT and FEATURESET_NR_ENTRIES

[Andrew Cooper]

On 08/05/2023 8:45 am, Jan Beulich wrote:
> On 04.05.2023 21:39, Andrew Cooper wrote:
>> When adding new words to a featureset, there is a reasonable amount of
>> boilerplate and it is preforable to split the addition into multiple patches.
>>
>> GCC 12 spotted a real (transient) error which occurs when splitting additions
>> like this.  Right now, FEATURESET_NR_ENTRIES is dynamically generated from 
>> the
>> highest numeric XEN_CPUFEATURE() value, and can be less than what the
>> FEATURESET_* constants suggest the length of a featureset bitmap ought to be.
>>
>> This causes the policy <-> featureset converters to genuinely access
>> out-of-bounds on the featureset array.
>>
>> Rework X86_NR_FEAT to be related to FEATURESET_* alone, allowing it
>> specifically to grow larger than FEATURESET_NR_ENTRIES.
>>
>> Reported-by: Jan Beulich 
>> Signed-off-by: Andrew Cooper 
> While, like you, I could live with the previous patch even if I don't
> particularly like it, I'm not convinced of the route you take here.

It's the route you tentatively agreed to in
https://lore.kernel.org/xen-devel/a282c338-98ab-6c3f-314b-267a5a82b...@suse.com/

> Can't
> we instead improve build-time checking, so the issue spotted late in the
> build by gcc12 can be spotted earlier and/or be connected better to the
> underlying reason?

I don't understand what you mean by this.  For the transient period of
time, Xen's idea of a featureset *is* longer the autogen idea, hence the
work in this patch to decouple the two.

>
> One idea I have would deal with another aspect I don't like about our
> present XEN_CPUFEATURE() as well: The *32 that's there in every use of
> the macro. How about
>
> XEN_CPUFEATURE(FSRCS,10, 12) /*A  Fast Short REP CMPSB/SCASB */
>
> as the common use and e.g.
>
> XEN_CPUFEATURE(16)
>
> or (if that ends up easier in gen-cpuid-py and/or the public header)
> something like
>
> XEN_CPUFEATURE(, 16, )
>
> as the placeholder required for (at least trailing) unpopulated slots? Of
> course the macro used may also be one of a different name, which may even
> be necessary to keep the public header reasonably simple; maybe as much
> as avoiding use of compiler extensions there. (This would then mean to
> leave alone XEN_CPUFEATURE(), and my secondary adjustment would perhaps
> become an independent change to make.)

Honestly, I don't want to hide the *32 part of the expression.  This
logic is already magic enough.

If we were to do something like this, I don't see what's wrong with just
having the value as a regular define at the end anyway.

One way or another with this approach, something needs updating in the
tail of cpufeatureset.h, and gen-cpuid.py can easily parse for a
specific named constant, and it will be less magic than overloading
XEN_CPUFEATURE().

>> To preempt what I expect will be the first review question, no FEATURESET_*
>> can't become an enumeration, because the constants undergo token 
>> concatination
>> in the preprocess as part of making DECL_BITFIELD() work.
> Just as a remark: I had trouble understanding this. Which was a result of
> you referring to token concatenation being the problem (which is fine when
> the results are enumerators), when really the issue is with the result of
> the concatenation wanting to be expanded to a literal number.
>
> Then again - do CPUID_BITFIELD_ really need to be named that way?
> Couldn't they equally well be CPUID_BITFIELD_1d, CPUID_BITFIELD_e1c, and
> alike, thus removing the need for intermediate macro expansion?

gen-cpuid.py doesn't know the short names; only Xen does, which is why
the expansion needs to know the name->word mapping.

I suppose this can be fixed, but it will require more magic comments and
more parsing to achieve.

~Andrew

Re: [PATCH 5/6] x86/cpu-policy: Disentangle X86_NR_FEAT and FEATURESET_NR_ENTRIES

[Jan Beulich]

On 09.05.2023 16:03, Andrew Cooper wrote:
> On 08/05/2023 8:45 am, Jan Beulich wrote:
>> On 04.05.2023 21:39, Andrew Cooper wrote:
>>> When adding new words to a featureset, there is a reasonable amount of
>>> boilerplate and it is preforable to split the addition into multiple 
>>> patches.
>>>
>>> GCC 12 spotted a real (transient) error which occurs when splitting 
>>> additions
>>> like this.  Right now, FEATURESET_NR_ENTRIES is dynamically generated from 
>>> the
>>> highest numeric XEN_CPUFEATURE() value, and can be less than what the
>>> FEATURESET_* constants suggest the length of a featureset bitmap ought to 
>>> be.
>>>
>>> This causes the policy <-> featureset converters to genuinely access
>>> out-of-bounds on the featureset array.
>>>
>>> Rework X86_NR_FEAT to be related to FEATURESET_* alone, allowing it
>>> specifically to grow larger than FEATURESET_NR_ENTRIES.
>>>
>>> Reported-by: Jan Beulich 
>>> Signed-off-by: Andrew Cooper 
>> While, like you, I could live with the previous patch even if I don't
>> particularly like it, I'm not convinced of the route you take here.
> 
> It's the route you tentatively agreed to in
> https://lore.kernel.org/xen-devel/a282c338-98ab-6c3f-314b-267a5a82b...@suse.com/

Right. Yet I deliberately said "may be the best" there, as something
better might turn up. And getting the two numbers to always agree, as
suggested, might end up being better.

>> Can't
>> we instead improve build-time checking, so the issue spotted late in the
>> build by gcc12 can be spotted earlier and/or be connected better to the
>> underlying reason?
> 
> I don't understand what you mean by this.  For the transient period of
> time, Xen's idea of a featureset *is* longer the autogen idea, hence the
> work in this patch to decouple the two.

Well, this part of my reply was just aiming at diagnosing the issue
as early as possible and as clearly as possible, such that one can
easily and quickly adjust whatever is missing in a change being worked
on. The main goal of course needs to be that this can't easily go
entirely unnoticed (as had happened, which has prompted this attempt
of yours of addressing the issue). I.e. diagnosing late is still far
better than failing to (without the compiler spotting it) altogether.

>> One idea I have would deal with another aspect I don't like about our
>> present XEN_CPUFEATURE() as well: The *32 that's there in every use of
>> the macro. How about
>>
>> XEN_CPUFEATURE(FSRCS,10, 12) /*A  Fast Short REP CMPSB/SCASB */
>>
>> as the common use and e.g.
>>
>> XEN_CPUFEATURE(16)
>>
>> or (if that ends up easier in gen-cpuid-py and/or the public header)
>> something like
>>
>> XEN_CPUFEATURE(, 16, )
>>
>> as the placeholder required for (at least trailing) unpopulated slots? Of
>> course the macro used may also be one of a different name, which may even
>> be necessary to keep the public header reasonably simple; maybe as much
>> as avoiding use of compiler extensions there. (This would then mean to
>> leave alone XEN_CPUFEATURE(), and my secondary adjustment would perhaps
>> become an independent change to make.)
> 
> Honestly, I don't want to hide the *32 part of the expression.  This
> logic is already magic enough.

Well, I certainly wouldn't insist, but to me it looks pretty odd to have
it on all the lines.

> If we were to do something like this, I don't see what's wrong with just
> having the value as a regular define at the end anyway.
> 
> One way or another with this approach, something needs updating in the
> tail of cpufeatureset.h, and gen-cpuid.py can easily parse for a
> specific named constant, and it will be less magic than overloading
> XEN_CPUFEATURE().

If less overloading is deemed better - fine with me. Looking at the
script I wasn't sure hunting for an entirely different construct would
end up being more tidy.

What isn't really clear to me from your reply: Are you okay with trying
such an alternative approach? Or are you opposed to it? Or something in
the middle, like you being okay, but only if it's not you to actually
try it out?

>>> To preempt what I expect will be the first review question, no FEATURESET_*
>>> can't become an enumeration, because the constants undergo token 
>>> concatination
>>> in the preprocess as part of making DECL_BITFIELD() work.
>> Just as a remark: I had trouble understanding this. Which was a result of
>> you referring to token concatenation being the problem (which is fine when
>> the results are enumerators), when really the issue is with the result of
>> the concatenation wanting to be expanded to a literal number.
>>
>> Then again - do CPUID_BITFIELD_ really need to be named that way?
>> Couldn't they equally well be CPUID_BITFIELD_1d, CPUID_BITFIELD_e1c, and
>> alike, thus removing the need for intermediate macro expansion?
> 
> gen-cpuid.py doesn't know the short names; only Xen does, which is why
> the expansion needs to know the name->word mapping.
> 
> I suppose this can be fixed, but it will

Re: [PATCH 1/6] x86/cpu-policy: Drop build time cross-checks of featureset sizes

[Jan Beulich]

On 09.05.2023 15:04, Andrew Cooper wrote:
> On 08/05/2023 7:47 am, Jan Beulich wrote:
>> On 04.05.2023 21:39, Andrew Cooper wrote:
>>> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic for
>>> code which looks like:
>>>
>>>   uint32_t foo[1] = { 1, 2, 3 };
>>>
>>> However, GCC 12 at least does now warn for this:
>>>
>>>   foo.c:1:24: error: excess elements in array initializer [-Werror]
>>> 884 | uint32_t foo[1] = { 1, 2, 3 };
>>> |^
>>>   foo.c:1:24: note: (near initialization for 'foo')
>> I'm pretty sure all gcc versions we support diagnose such cases. In turn
>> the arrays in question don't have explicit dimensions at their
>> definition sites, and hence they derive their dimensions from their
>> initializers. So the build-time-checks are about the arrays in fact
>> obtaining the right dimensions, i.e. the initializers being suitable.
>>
>> With the core part of the reasoning not being applicable, I'm afraid I
>> can't even say "okay with an adjusted description".
> 
> Now I'm extra confused.
> 
> I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
> when I was expecting one, and there was a bug in the original featureset
> work caused by this going wrong.
> 
> But godbolt seems to agree that even GCC 4.1 notices.
> 
> Maybe it was some other error (C file not seeing the header properly?)
> which disappeared across the upstream review?

Or maybe, by mistake, too few initializer fields? But what exactly it
was probably doesn't matter. If this patch is to stay (see below), some
different description will be needed anyway (or the change be folded
into the one actually invalidating those BUILD_BUG_ON()s).

> Either way, these aren't appropriate, and need deleting before patch 5,
> because the check is no longer valid when a featureset can be longer
> than the autogen length.

Well, they need deleting if we stick to the approach chosen there right
now. If we switched to my proposed alternative, they better would stay.

Jan

Re: [XEN][PATCH v6 15/19] xen/arm: Implement device tree node removal functionalities

[Michal Orzel]

On 03/05/2023 01:36, Vikram Garhwal wrote:
> Introduce sysctl XEN_SYSCTL_dt_overlay to remove device-tree nodes added using
> device tree overlay.
> 
> xl dt-overlay remove file.dtbo:
> Removes all the nodes in a given dtbo.
> First, removes IRQ permissions and MMIO accesses. Next, it finds the nodes
> in dt_host and delete the device node entries from dt_host.
> 
> The nodes get removed only if it is not used by any of dom0 or domio.
> 
> Also, added overlay_track struct to keep the track of added node through 
> device
> tree overlay. overlay_track has dt_host_new which is unflattened form of 
> updated
> fdt and name of overlay nodes. When a node is removed, we also free the memory
> used by overlay_track for the particular overlay node.
> 
> Nested overlay removal is supported in sequential manner only i.e. if
> overlay_child nests under overlay_parent, it is assumed that user first 
> removes
> overlay_child and then removes overlay_parent.
> 
> Signed-off-by: Vikram Garhwal 
> ---
>  xen/arch/arm/sysctl.c|  16 +-
>  xen/common/Makefile  |   1 +
>  xen/common/dt-overlay.c  | 419 +++
>  xen/include/public/sysctl.h  |  23 ++
>  xen/include/xen/dt-overlay.h |  58 +
>  5 files changed, 516 insertions(+), 1 deletion(-)
>  create mode 100644 xen/common/dt-overlay.c
>  create mode 100644 xen/include/xen/dt-overlay.h
> 
> diff --git a/xen/arch/arm/sysctl.c b/xen/arch/arm/sysctl.c
> index b0a78a8b10..456358166c 100644
> --- a/xen/arch/arm/sysctl.c
> +++ b/xen/arch/arm/sysctl.c
> @@ -9,6 +9,7 @@
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -21,7 +22,20 @@ void arch_do_physinfo(struct xen_sysctl_physinfo *pi)
>  long arch_do_sysctl(struct xen_sysctl *sysctl,
>  XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
>  {
> -return -ENOSYS;
> +long ret = 0;
> +
> +switch ( sysctl->cmd )
> +{
> +case XEN_SYSCTL_dt_overlay:
> +ret = dt_sysctl(&sysctl->u.dt_overlay);
> +break;
> +
> +default:
> +ret = -ENOSYS;
> +break;
> +}
> +
> +return ret;
>  }
>  
>  /*
> diff --git a/xen/common/Makefile b/xen/common/Makefile
> index 46049eac35..e7e96b1087 100644
> --- a/xen/common/Makefile
> +++ b/xen/common/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_DEBUG_TRACE) += debugtrace.o
>  obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
>  obj-$(CONFIG_IOREQ_SERVER) += dm.o
>  obj-y += domain.o
> +obj-$(CONFIG_OVERLAY_DTB) += dt-overlay.o
>  obj-y += event_2l.o
>  obj-y += event_channel.o
>  obj-y += event_fifo.o
> diff --git a/xen/common/dt-overlay.c b/xen/common/dt-overlay.c
> new file mode 100644
> index 00..b89cceab84
> --- /dev/null
> +++ b/xen/common/dt-overlay.c
> @@ -0,0 +1,419 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
GPL-2.0-only according to the latest series from Andrew (GPL-2.0 is a 
depracated tag)

[...]

> +
> +/* Remove mmio access. */
> +for ( i = 0; i < naddr; i++ )
> +{
> +uint64_t addr, size;
> +
> +rc = dt_device_get_address(device_node, i, &addr, &size);
Given that Ayan's 32-bit series might be merged first, this will have to be 
changed (to use paddr_t for addr,size, etc.)

> +if ( rc )
> +{
> +printk(XENLOG_ERR "Unable to retrieve address %u for %s\n",
> +   i, dt_node_full_name(device_node));
> +return rc;
> +}
> +
> +rc = iomem_deny_access(d, paddr_to_pfn(addr),
> +   paddr_to_pfn(PAGE_ALIGN(addr + size - 1)));
> +if ( rc )
> +{
> +printk(XENLOG_ERR "Unable to remove dom%d access to"
> +   " 0x%"PRIx64" - 0x%"PRIx64"\n",
> +   d->domain_id,
> +   addr & PAGE_MASK, PAGE_ALIGN(addr + size) - 1);
> +return rc;
> +}
> +
> +}
> +
> +return rc;
> +}
> +
> +/* Removes all descendants of the given node. */
> +static int remove_all_descendant_nodes(struct dt_device_node *device_node)
> +{
> +int rc = 0;
> +struct dt_device_node *child_node;
> +
> +for ( child_node = device_node->child; child_node != NULL;
> + child_node = child_node->sibling )
> +{
> +if ( child_node->child )
> +remove_all_descendant_nodes(child_node);
> +
> +rc = handle_remove_irq_iommu(child_node);
> +if ( rc )
> +return rc;
> +}
> +
> +return rc;
> +}
> +
> +/* Remove nodes from dt_host. */
> +static int remove_nodes(const struct overlay_track *tracker)
> +{
> +int rc = 0;
> +struct dt_device_node *overlay_node;
> +unsigned int j;
> +
> +for ( j = 0; j < tracker->num_nodes; j++ )
> +{
> +overlay_node = (struct dt_device_node *)tracker->nodes_address[j];
> +if ( overlay_node == NULL )
> +{
> +printk(XENLOG_ERR "Device %s is not present in the tree. 
> Removing nodes failed\n",
> +

Re: [PATCH v6 2/4] xen/riscv: introduce setup_initial_pages

[Jan Beulich]

On 09.05.2023 14:59, Oleksii wrote:
> On Mon, 2023-05-08 at 10:58 +0200, Jan Beulich wrote:
>> On 03.05.2023 18:31, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/riscv/include/asm/page.h
>>> @@ -0,0 +1,62 @@
>>> +#ifndef _ASM_RISCV_PAGE_H
>>> +#define _ASM_RISCV_PAGE_H
>>> +
>>> +#include 
>>> +#include 
>>> =)+
>>> +#define VPN_MASK    ((unsigned
>>> long)(PAGETABLE_ENTRIES - 1))
>>> +
>>> +#define XEN_PT_LEVEL_ORDER(lvl) ((lvl) * PAGETABLE_ORDER)
>>> +#define XEN_PT_LEVEL_SHIFT(lvl) (XEN_PT_LEVEL_ORDER(lvl) +
>>> PAGE_SHIFT)
>>> +#define XEN_PT_LEVEL_SIZE(lvl)  (_AT(paddr_t, 1) <<
>>> XEN_PT_LEVEL_SHIFT(lvl))
>>> +#define XEN_PT_LEVEL_MAP_MASK(lvl)  (~(XEN_PT_LEVEL_SIZE(lvl) -
>>> 1))
>>> +#define XEN_PT_LEVEL_MASK(lvl)  (VPN_MASK <<
>>> XEN_PT_LEVEL_SHIFT(lvl))
>>> +
>>> +#define PTE_VALID   BIT(0, UL)
>>> +#define PTE_READABLE    BIT(1, UL)
>>> +#define PTE_WRITABLE    BIT(2, UL)
>>> +#define PTE_EXECUTABLE  BIT(3, UL)
>>> +#define PTE_USER    BIT(4, UL)
>>> +#define PTE_GLOBAL  BIT(5, UL)
>>> +#define PTE_ACCESSED    BIT(6, UL)
>>> +#define PTE_DIRTY   BIT(7, UL)
>>> +#define PTE_RSW (BIT(8, UL) | BIT(9, UL))
>>> +
>>> +#define PTE_LEAF_DEFAULT    (PTE_VALID | PTE_READABLE |
>>> PTE_WRITABLE)
>>> +#define PTE_TABLE   (PTE_VALID)
>>> +
>>> +/* Calculate the offsets into the pagetables for a given VA */
>>> +#define pt_linear_offset(lvl, va)   ((va) >>
>>> XEN_PT_LEVEL_SHIFT(lvl))
>>> +
>>> +#define pt_index(lvl, va) pt_linear_offset(lvl, (va) &
>>> XEN_PT_LEVEL_MASK(lvl))
>>
>> Maybe better
>>
>> #define pt_index(lvl, va) (pt_linear_offset(lvl, va) & VPN_MASK)
>>
>> as the involved constant will be easier to use for the compiler?
> But VPN_MASK should be shifted by level shift value.

Why? pt_linear_offset() already does the necessary shifting.

>>> +    csr_write(CSR_SATP, 0);
>>> +
>>> +    /* Clean MMU root page table */
>>> +    stage1_pgtbl_root[index] = paddr_to_pte(0x0, 0x0);
>>> +
>>> +    asm volatile ( "sfence.vma" );
>>
>> Doesn't this want to move between the SATP write and the clearing of
>> the
>> root table slot? Also here and elsewhere - don't these asm()s need
>> memory
>> clobbers? And anyway - could I talk you into introducing an inline
>> wrapper
>> (e.g. named sfence_vma()) so all uses end up consistent?
> I think clearing of root page table should be done before "sfence.vma"
> because we have to first clear slot of MMU's root page table and then
> make updating root page table visible for all. ( by usage of sfence
> instruction )

I disagree. The SATP write has removed the connection of the CPU
to the page tables. That's the action you want to fence, not the
altering of some (then) no longer referenced data structure.

>>> +void __init setup_initial_pagetables(void)
>>> +{
>>> +    struct mmu_desc mmu_desc = { 0, 0, NULL, NULL };
>>> +
>>> +    /*
>>> + * Access to _stard, _end is always PC-relative
>>
>> Nit: Typo-ed symbol name. Also ...
>>
>>> + * thereby when access them we will get load adresses
>>> + * of start and end of Xen
>>> + * To get linker addresses LOAD_TO_LINK() is required
>>> + * to use
>>> + */
>>
>> see the earlier line wrapping remark again. Finally in multi-sentence
>> comments full stops are required.
> Full stops mean '.' at the end of sentences?

Yes. Please see ./CODING_STYLE.

Jan

Re: [PATCH 3/3] x86: Use CpuidUserDis if an AMD HVM guest toggles CPUID faulting

[Jan Beulich]

On 09.05.2023 12:05, Andrew Cooper wrote:
> On 08/05/2023 2:18 pm, Jan Beulich wrote:
>> On 05.05.2023 19:57, Alejandro Vallejo wrote:
>>> This is in order to aid guests of AMD hardware that we have exposed
>>> CPUID faulting to. If they try to modify the Intel MSR that enables
>>> the feature, trigger levelling so AMD's version of it (CpuidUserDis)
>>> is used instead.
>>>
>>> Signed-off-by: Alejandro Vallejo 
>>> ---
>>>  xen/arch/x86/msr.c | 9 -
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>> Don't you also need to update cpu-policy.c:calculate_host_policy()
>> for the guest to actually know it can use the functionality? Which
>> in turn would appear to require some form of adjustment to
>> lib/x86/policy.c:x86_cpu_policies_are_compatible().
> 
> I asked Alejandro to do it like this.
> 
> Advertising this to guests requires plumbing another MSR into the
> infrastructure which isn't quite set up properly let, and is in flux
> from my work.
> 
> For now, this just lets Xen enforce the policy over PV guests, which is
> an improvement in and of itself.

But as per the title this patch is about HVM guests (aiui the PV aspect
is taken care of already without the patch here). In any event - if the
omissions are intentional (for the time being), then I think that wants
mentioning in the description.

Jan

Re: [XEN][PATCH v6 15/19] xen/arm: Implement device tree node removal functionalities

[Andrew Cooper]

On 09/05/2023 3:30 pm, Michal Orzel wrote:
> On 03/05/2023 01:36, Vikram Garhwal wrote:
>> diff --git a/xen/include/xen/dt-overlay.h b/xen/include/xen/dt-overlay.h
>> new file mode 100644
>> index 00..5b369f8eb7
>> --- /dev/null
>> +++ b/xen/include/xen/dt-overlay.h
>> @@ -0,0 +1,58 @@
>> + /* SPDX-License-Identifier: GPL-2.0 */
> GPL-2.0-only according to the latest series from Andrew (GPL-2.0 is a 
> depracated tag)

Well, or "-or-later" at your choosing/as applicable, but one of the
explicitly suffixed forms please.

~Andrew

Re: [PATCH 3/3] x86: Use CpuidUserDis if an AMD HVM guest toggles CPUID faulting

[Alejandro Vallejo]

On Tue, May 09, 2023 at 04:41:49PM +0200, Jan Beulich wrote:
> > I asked Alejandro to do it like this.
> > 
> > Advertising this to guests requires plumbing another MSR into the
> > infrastructure which isn't quite set up properly let, and is in flux
> > from my work.
> > 
> > For now, this just lets Xen enforce the policy over PV guests, which is
> > an improvement in and of itself.
> 
> But as per the title this patch is about HVM guests (aiui the PV aspect
> is taken care of already without the patch here). In any event - if the
> omissions are intentional (for the time being), then I think that wants
> mentioning in the description.
> 
> Jan

HVM guests are always exposed the Intel interface (emulated if not natively
available). The HVM max policy forces it on, and I don't see anything in
the default policy overriding it. My attempt here was to let AMD guests use
the emulated Intel MSR and trigger levellling that would itself rely on
AMD's CpuidUserDis without guest intervention. That said, several cans of
worms exist in mantaining this internal routing. I'll get rid of that last
patch and leave HVM guests alone for the time being. They are functionally 
correct (albeit their CPUID take 2 faults, whereas 1 would suffice).

Alejandro

Re: [PATCH] libxl: arm: Allow grant mappings for backends running on Dom0

[Anthony PERARD]

On Fri, May 05, 2023 at 03:08:35PM +0530, Viresh Kumar wrote:
> Hi Anthony,
> 
> On 02-05-23, 15:44, Anthony PERARD wrote:
> > > diff --git a/tools/libs/light/libxl_virtio.c 
> > > b/tools/libs/light/libxl_virtio.c
> > > index faada49e184e..e1f15344ef97 100644
> > > --- a/tools/libs/light/libxl_virtio.c
> > > +++ b/tools/libs/light/libxl_virtio.c
> > > @@ -48,11 +48,13 @@ static int libxl__set_xenstore_virtio(libxl__gc *gc, 
> > > uint32_t domid,
> > >  flexarray_append_pair(back, "base", GCSPRINTF("%#"PRIx64, 
> > > virtio->base));
> > >  flexarray_append_pair(back, "type", GCSPRINTF("%s", virtio->type));
> > >  flexarray_append_pair(back, "transport", GCSPRINTF("%s", transport));
> > > +flexarray_append_pair(back, "forced_grant", GCSPRINTF("%u", 
> > > virtio->forced_grant));
> > >  
> > >  flexarray_append_pair(front, "irq", GCSPRINTF("%u", virtio->irq));
> > >  flexarray_append_pair(front, "base", GCSPRINTF("%#"PRIx64, 
> > > virtio->base));
> > >  flexarray_append_pair(front, "type", GCSPRINTF("%s", virtio->type));
> > >  flexarray_append_pair(front, "transport", GCSPRINTF("%s", 
> > > transport));
> > > +flexarray_append_pair(front, "forced_grant", GCSPRINTF("%u", 
> > > virtio->forced_grant));
> > 
> > This "forced_grant" feels weird to me in the protocol, I feel like this
> > use of grant or not could be handled by the backend. For example in
> > "blkif" protocol, there's plenty of "feature-*" which allows both
> > front-end and back-end to advertise which feature they can or want to
> > use.
> > But maybe the fact that the device tree needs to be modified to be able
> > to accommodate grant mapping means that libxl needs to ask the backend to
> > use grant or not, and the frontend needs to now if it needs to use
> > grant.
> 
> I am not sure if I fully understand what you are suggesting here.

I guess the way virtio devices are implemented in libxl suggest to me
that the are just Xen PV devices. So I guess some documentation in the
tree would be useful, maybe some comments in libxl_virtio.c.

> The eventual fronend drivers (like drivers/i2c/busses/i2c-virtio.c)
> aren't Xen aware and the respective virtio protocol doesn't talk about
> how memory is mapped for the guest. The guest kernel allows both
> memory mapping models and the decision is made based on the presence
> or absence of the iommu node in the DT.

So, virtio's frontend don't know about xenstore? In this case, there's
no need to have all those nodes in xenstore under the frontend path.

I guess the nodes for the backends are at least somewhat useful for
libxl to reload the configuration of the virtio device. But even that
isn't probably useful if we can't hot-plug or hot-unplug virtio devices.

Are the xenstore node for the backend actually been used by a virtio
backend?

Cheers,

-- 
Anthony PERARD

Re: [PATCH] xen/evtchn: Introduce new IOCTL to bind static evtchn

[Rahul Singh]

Hi Stefano,

Thanks for the review.

On 6 May 2023, at 1:52 am, Stefano Stabellini  wrote:

On Fri, 28 Apr 2023, Rahul Singh wrote:
Xen 4.17 supports the creation of static evtchns. To allow user space
application to bind static evtchns introduce new ioctl
"IOCTL_EVTCHN_BIND_STATIC". Existing IOCTL doing more than binding
that’s why we need to introduce the new IOCTL to only bind the static
event channels.

Also, static evtchns to be available for use during the lifetime of the
guest. When the application exits, __unbind_from_irq() end up
being called from release() fop because of that static evtchns are
getting closed. To avoid closing the static event channel, add the new
bool variable "is_static" in "struct irq_info" to mark the event channel
static when creating the event channel to avoid closing the static
evtchn.

Signed-off-by: Rahul Singh 

I think the patch is OK but evtchn_bind_to_user on the error path calls
EVTCHNOP_close. Could that be a problem for static evtchns? I wonder if
we need to skip that EVTCHNOP_close call too.


err:
/* bind failed, should close the port now */
close.port = port;
if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
BUG();
del_evtchn(u, evtchn);

Yes, we need to avoid to close the static event channel in case of error path 
also.
I will fix this in next version.

Regards,
Rahul

Re: [PATCH 1/6] x86/cpu-policy: Drop build time cross-checks of featureset sizes

[Andrew Cooper]

On 09/05/2023 3:28 pm, Jan Beulich wrote:
> On 09.05.2023 15:04, Andrew Cooper wrote:
>> On 08/05/2023 7:47 am, Jan Beulich wrote:
>>> On 04.05.2023 21:39, Andrew Cooper wrote:
 These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic 
 for
 code which looks like:

   uint32_t foo[1] = { 1, 2, 3 };

 However, GCC 12 at least does now warn for this:

   foo.c:1:24: error: excess elements in array initializer [-Werror]
 884 | uint32_t foo[1] = { 1, 2, 3 };
 |^
   foo.c:1:24: note: (near initialization for 'foo')
>>> I'm pretty sure all gcc versions we support diagnose such cases. In turn
>>> the arrays in question don't have explicit dimensions at their
>>> definition sites, and hence they derive their dimensions from their
>>> initializers. So the build-time-checks are about the arrays in fact
>>> obtaining the right dimensions, i.e. the initializers being suitable.
>>>
>>> With the core part of the reasoning not being applicable, I'm afraid I
>>> can't even say "okay with an adjusted description".
>> Now I'm extra confused.
>>
>> I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
>> when I was expecting one, and there was a bug in the original featureset
>> work caused by this going wrong.
>>
>> But godbolt seems to agree that even GCC 4.1 notices.
>>
>> Maybe it was some other error (C file not seeing the header properly?)
>> which disappeared across the upstream review?
> Or maybe, by mistake, too few initializer fields? But what exactly it
> was probably doesn't matter. If this patch is to stay (see below), some
> different description will be needed anyway (or the change be folded
> into the one actually invalidating those BUILD_BUG_ON()s).
>
>> Either way, these aren't appropriate, and need deleting before patch 5,
>> because the check is no longer valid when a featureset can be longer
>> than the autogen length.
> Well, they need deleting if we stick to the approach chosen there right
> now. If we switched to my proposed alternative, they better would stay.

Given that all versions of GCC do warn, I don't see any justification
for them to stay.

i.e. this should be committed, even if the commit message says "no idea
what they were taken originally, but they're superfluous in the logic as
it exists today".

~Andrew

Re: [PATCH] iommu/vtd: fix address translation for superpages

[Jan Beulich]

On 09.05.2023 12:41, Roger Pau Monne wrote:
> When translating an address that falls inside of a superpage in the
> IOMMU page tables the fetching of the PTE physical address field
> wasn't using dma_pte_addr(), which caused the returned data to be
> corrupt as it would contain bits not related to the address field.

I'm afraid I don't understand:

> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -359,16 +359,18 @@ static uint64_t addr_to_dma_page_maddr(struct domain 
> *domain, daddr_t addr,
>  
>  if ( !alloc )
>  {
> -pte_maddr = 0;
>  if ( !dma_pte_present(*pte) )
> +{
> +pte_maddr = 0;
>  break;
> +}
>  
>  /*
>   * When the leaf entry was requested, pass back the full PTE,
>   * with the address adjusted to account for the residual of
>   * the walk.
>   */
> -pte_maddr = pte->val +
> +pte_maddr +=
>  (addr & ((1UL << level_to_offset_bits(level)) - 1) &
>   PAGE_MASK);

With this change you're now violating what the comment says (plus what
the comment ahead of the function says). And it says what it says for
a reason - see intel_iommu_lookup_page(), which I think your change is
breaking.

Note also the following code:

if ( !target )
break;
}

pte_maddr = level - 1;

IOW the local variable is overwritten right away unless target == 0.

Jan

[PATCH v4 0/3] Rationalize usage of xc_domain_getinfo{,list}()

[Alejandro Vallejo]

The first 4 patches of v2 already made it to staging. v4 includes a fix
so libxl preserves its previous error handling behaviour and a style change

Original cover letter:

xc_domain_getinfo() returns the list of domains with domid >= first_domid.
It does so by repeatedly invoking XEN_DOMCTL_getdomaininfo, which leads to
unintuitive behaviour (asking for domid=1 might succeed returning domid=2).
Furthermore, N hypercalls are required whereas the equivalent functionality
can be achieved using with XEN_SYSCTL_getdomaininfo.

Ideally, we want a DOMCTL interface that operates over a single precisely
specified domain and a SYSCTL interface that can be used for bulk queries.

All callers of xc_domain_getinfo() that are better off using SYSCTL are
migrated to use that instead. That includes callers performing domain
discovery and those requesting info for more than 1 domain per hypercall.

A new xc_domain_getinfo_single() is introduced with stricter semantics than
xc_domain_getinfo() (failing if domid isn't found) to migrate the rest to.

With no callers left the xc_dominfo_t structure and the xc_domain_getinfo()
call itself can be cleanly removed, and the DOMCTL interface simplified to
only use its fastpath.

With the DOMCTL ammended, the new xc_domain_getinfo_single() drops its
stricter check, becoming a simple wrapper to invoke the hypercall itself.

Alejandro Vallejo (3):
  tools: Modify single-domid callers of xc_domain_getinfolist()
  tools: Use new xc function for some xc_domain_getinfo() calls
  domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found

 tools/console/client/main.c |  7 +--
 tools/debugger/kdd/kdd-xen.c|  5 +-
 tools/include/xenctrl.h | 43 -
 tools/libs/ctrl/xc_domain.c | 82 ++---
 tools/libs/ctrl/xc_pagetab.c|  7 +--
 tools/libs/ctrl/xc_private.c|  9 +--
 tools/libs/ctrl/xc_private.h|  7 ++-
 tools/libs/guest/xg_core.c  | 23 +++
 tools/libs/guest/xg_core.h  |  6 +-
 tools/libs/guest/xg_core_arm.c  | 10 +--
 tools/libs/guest/xg_core_x86.c  | 18 +++---
 tools/libs/guest/xg_cpuid_x86.c | 40 ++--
 tools/libs/guest/xg_dom_boot.c  | 16 ++---
 tools/libs/guest/xg_domain.c|  8 +--
 tools/libs/guest/xg_offline_page.c  | 12 ++--
 tools/libs/guest/xg_private.h   |  1 +
 tools/libs/guest/xg_resume.c| 20 +++---
 tools/libs/guest/xg_sr_common.h |  2 +-
 tools/libs/guest/xg_sr_restore.c| 17 ++---
 tools/libs/guest/xg_sr_restore_x86_pv.c |  2 +-
 tools/libs/guest/xg_sr_save.c   | 27 
 tools/libs/guest/xg_sr_save_x86_pv.c|  6 +-
 tools/libs/light/libxl_dom.c| 17 ++---
 tools/libs/light/libxl_dom_suspend.c|  7 +--
 tools/libs/light/libxl_domain.c | 18 +++---
 tools/libs/light/libxl_mem.c|  4 +-
 tools/libs/light/libxl_sched.c  | 30 -
 tools/libs/light/libxl_x86_acpi.c   |  6 +-
 tools/misc/xen-hvmcrash.c   |  6 +-
 tools/misc/xen-lowmemd.c|  6 +-
 tools/misc/xen-mfndump.c| 22 +++
 tools/misc/xen-vmtrace.c|  6 +-
 tools/ocaml/libs/xc/xenctrl_stubs.c |  6 +-
 tools/vchan/vchan-socket-proxy.c|  6 +-
 tools/xenpaging/xenpaging.c | 10 +--
 tools/xenstore/xenstored_domain.c   | 15 +++--
 tools/xentrace/xenctx.c |  8 +--
 xen/common/domctl.c | 32 +-
 38 files changed, 190 insertions(+), 377 deletions(-)

-- 
2.34.1

[PATCH v4 1/3] tools: Modify single-domid callers of xc_domain_getinfolist()

[Alejandro Vallejo]

xc_domain_getinfolist() internally relies on a sysctl that performs
a linear search for the domids. Many callers of xc_domain_getinfolist()
who require information about a precise domid are much better off calling
xc_domain_getinfo_single() instead, that will use the getdomaininfo domctl
instead and ensure the returned domid matches the requested one. The domtctl
will find the domid faster too, because that uses hashed lists.

Signed-off-by: Alejandro Vallejo 
Reviewed-by: Andrew Cooper 
Acked-by: Christian Lindig 
---
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Anthony PERARD 
Cc: Juergen Gross 
Cc: Christian Lindig 

v4:
  libxl:
* Preserve distinction between "domain-not-found" and other errors
* Renamed new rc variable to r, as the coding style
---
 tools/libs/light/libxl_dom.c | 17 ++---
 tools/libs/light/libxl_dom_suspend.c |  7 +--
 tools/libs/light/libxl_domain.c  | 18 --
 tools/libs/light/libxl_mem.c |  4 ++--
 tools/libs/light/libxl_sched.c   | 14 +-
 tools/ocaml/libs/xc/xenctrl_stubs.c  |  6 ++
 tools/xenpaging/xenpaging.c  | 10 +-
 7 files changed, 29 insertions(+), 47 deletions(-)

diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
index 25fb716084..94fef37401 100644
--- a/tools/libs/light/libxl_dom.c
+++ b/tools/libs/light/libxl_dom.c
@@ -32,9 +32,9 @@ libxl_domain_type libxl__domain_type(libxl__gc *gc, uint32_t 
domid)
 xc_domaininfo_t info;
 int ret;
 
-ret = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
-if (ret != 1 || info.domain != domid) {
-LOG(ERROR, "unable to get domain type for domid=%"PRIu32, domid);
+ret = xc_domain_getinfo_single(ctx->xch, domid, &info);
+if (ret < 0) {
+LOGED(ERROR, domid, "unable to get dominfo");
 return LIBXL_DOMAIN_TYPE_INVALID;
 }
 if (info.flags & XEN_DOMINF_hvm_guest) {
@@ -70,15 +70,10 @@ int libxl__domain_cpupool(libxl__gc *gc, uint32_t domid)
 xc_domaininfo_t info;
 int ret;
 
-ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
-if (ret != 1)
+ret = xc_domain_getinfo_single(CTX->xch, domid, &info);
+if (ret < 0)
 {
-LOGE(ERROR, "getinfolist failed %d", ret);
-return ERROR_FAIL;
-}
-if (info.domain != domid)
-{
-LOGE(ERROR, "got info for dom%d, wanted dom%d\n", info.domain, domid);
+LOGED(ERROR, domid, "get domaininfo failed");
 return ERROR_FAIL;
 }
 return info.cpupool;
diff --git a/tools/libs/light/libxl_dom_suspend.c 
b/tools/libs/light/libxl_dom_suspend.c
index 4fa22bb739..6091a5f3f6 100644
--- a/tools/libs/light/libxl_dom_suspend.c
+++ b/tools/libs/light/libxl_dom_suspend.c
@@ -332,13 +332,8 @@ static void suspend_common_wait_guest_check(libxl__egc 
*egc,
 /* Convenience aliases */
 const uint32_t domid = dsps->domid;
 
-ret = xc_domain_getinfolist(CTX->xch, domid, 1, &info);
+ret = xc_domain_getinfo_single(CTX->xch, domid, &info);
 if (ret < 0) {
-LOGED(ERROR, domid, "unable to check for status of guest");
-goto err;
-}
-
-if (!(ret == 1 && info.domain == domid)) {
 LOGED(ERROR, domid, "guest we were suspending has been destroyed");
 goto err;
 }
diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
index 7f0986c185..5ee1544d9c 100644
--- a/tools/libs/light/libxl_domain.c
+++ b/tools/libs/light/libxl_domain.c
@@ -349,15 +349,11 @@ int libxl_domain_info(libxl_ctx *ctx, libxl_dominfo 
*info_r,
 int ret;
 GC_INIT(ctx);
 
-ret = xc_domain_getinfolist(ctx->xch, domid, 1, &xcinfo);
-if (ret<0) {
-LOGED(ERROR, domid, "Getting domain info list");
-GC_FREE;
-return ERROR_FAIL;
-}
-if (ret==0 || xcinfo.domain != domid) {
+ret = xc_domain_getinfo_single(ctx->xch, domid, &xcinfo);
+if (ret < 0) {
+LOGED(ERROR, domid, "Getting domain info");
 GC_FREE;
-return ERROR_DOMAIN_NOTFOUND;
+return errno == ESRCH ? ERROR_DOMAIN_NOTFOUND : ERROR_FAIL;
 }
 
 if (info_r)
@@ -1657,14 +1653,16 @@ int libxl__resolve_domid(libxl__gc *gc, const char 
*name, uint32_t *domid)
 libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
int *nr_vcpus_out, int *nr_cpus_out)
 {
+int r;
 GC_INIT(ctx);
 libxl_vcpuinfo *ptr, *ret;
 xc_domaininfo_t domaininfo;
 xc_vcpuinfo_t vcpuinfo;
 unsigned int nr_vcpus;
 
-if (xc_domain_getinfolist(ctx->xch, domid, 1, &domaininfo) != 1) {
-LOGED(ERROR, domid, "Getting infolist");
+r = xc_domain_getinfo_single(ctx->xch, domid, &domaininfo);
+if (r < 0) {
+LOGED(ERROR, domid, "Getting dominfo");
 GC_FREE;
 return NULL;
 }
diff --git a/tools/libs/light/libxl_mem.c b/tools/libs/light/libxl_mem.c
index 92ec09f4cf..44e554adba 100644
--- a/tools/libs/light/libxl_mem.c
+++ b/tools/libs/light/libxl_me

[PATCH v4 3/3] domctl: Modify XEN_DOMCTL_getdomaininfo to fail if domid is not found

[Alejandro Vallejo]

It previously mimicked the getdomaininfo sysctl semantics by returning
the first domid higher than the requested domid that does exist. This
unintuitive behaviour causes quite a few mistakes and makes the call
needlessly slow in its error path.

This patch removes the fallback search, returning -ESRCH if the requested
domain doesn't exist. Domain discovery can still be done through the sysctl
interface as that performs a linear search on the list of domains.

With this modification the xc_domain_getinfo() function is deprecated and
removed to make sure it's not mistakenly used expecting the old behaviour.
The new xc wrapper is xc_domain_getinfo_single().

All previous callers of xc_domain_getinfo() have been updated to use
xc_domain_getinfo_single() or xc_domain_getinfolist() instead. This also
means xc_dominfo_t is no longer used by anything and can be purged.

Resolves: xen-project/xen#105
Signed-off-by: Alejandro Vallejo 
Reviewed-by: Andrew Cooper 
Acked-by: Anthony PERARD 

---
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Jan Beulich 
Cc: Julien Grall 
Cc: Stefano Stabellini 
Cc: Wei Liu 
Cc: Anthony PERARD 
Cc: Juergen Gross 
---
 tools/include/xenctrl.h | 43 --
 tools/libs/ctrl/xc_domain.c | 73 -
 xen/common/domctl.c | 32 +---
 3 files changed, 2 insertions(+), 146 deletions(-)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 086314d28a..dba33d5d0f 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -444,28 +444,6 @@ typedef struct xc_core_header {
  * DOMAIN MANAGEMENT FUNCTIONS
  */
 
-typedef struct xc_dominfo {
-uint32_t  domid;
-uint32_t  ssidref;
-unsigned int  dying:1, crashed:1, shutdown:1,
-  paused:1, blocked:1, running:1,
-  hvm:1, debugged:1, xenstore:1, hap:1;
-unsigned int  shutdown_reason; /* only meaningful if shutdown==1 */
-unsigned long nr_pages; /* current number, not maximum */
-unsigned long nr_outstanding_pages;
-unsigned long nr_shared_pages;
-unsigned long nr_paged_pages;
-unsigned long shared_info_frame;
-uint64_t  cpu_time;
-unsigned long max_memkb;
-unsigned int  nr_online_vcpus;
-unsigned int  max_vcpu_id;
-xen_domain_handle_t handle;
-unsigned int  cpupool;
-uint8_t   gpaddr_bits;
-struct xen_arch_domainconfig arch_config;
-} xc_dominfo_t;
-
 typedef xen_domctl_getdomaininfo_t xc_domaininfo_t;
 
 static inline unsigned int dominfo_shutdown_reason(const xc_domaininfo_t *info)
@@ -721,27 +699,6 @@ int xc_domain_getinfo_single(xc_interface *xch,
  uint32_t domid,
  xc_domaininfo_t *info);
 
-/**
- * This function will return information about one or more domains. It is
- * designed to iterate over the list of domains. If a single domain is
- * requested, this function will return the next domain in the list - if
- * one exists. It is, therefore, important in this case to make sure the
- * domain requested was the one returned.
- *
- * @parm xch a handle to an open hypervisor interface
- * @parm first_domid the first domain to enumerate information from.  Domains
- *   are currently enumerate in order of creation.
- * @parm max_doms the number of elements in info
- * @parm info an array of max_doms size that will contain the information for
- *the enumerated domains.
- * @return the number of domains enumerated or -1 on error
- */
-int xc_domain_getinfo(xc_interface *xch,
-  uint32_t first_domid,
-  unsigned int max_doms,
-  xc_dominfo_t *info);
-
-
 /**
  * This function will set the execution context for the specified vcpu.
  *
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index 66179e6f12..724fa6f753 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -357,85 +357,12 @@ int xc_domain_getinfo_single(xc_interface *xch,
 if ( do_domctl(xch, &domctl) < 0 )
 return -1;
 
-if ( domctl.u.getdomaininfo.domain != domid )
-{
-errno = ESRCH;
-return -1;
-}
-
 if ( info )
 *info = domctl.u.getdomaininfo;
 
 return 0;
 }
 
-int xc_domain_getinfo(xc_interface *xch,
-  uint32_t first_domid,
-  unsigned int max_doms,
-  xc_dominfo_t *info)
-{
-unsigned int nr_doms;
-uint32_t next_domid = first_domid;
-DECLARE_DOMCTL;
-int rc = 0;
-
-memset(info, 0, max_doms*sizeof(xc_dominfo_t));
-
-for ( nr_doms = 0; nr_doms < max_doms; nr_doms++ )
-{
-domctl.cmd = XEN_DOMCTL_getdomaininfo;
-domctl.domain = next_domid;
-if ( (rc = do_domctl(xch, &domctl)) < 0 )
-break;
-info->domid  = domctl.domain;
-
-info->dying= !!(domctl.u.getdomaininfo.flags&XEN_DOMINF_dying);
-info->shu

[PATCH v4 2/3] tools: Use new xc function for some xc_domain_getinfo() calls

[Alejandro Vallejo]

Move calls that require a information about a single precisely identified
domain to the new xc_domain_getinfo_single().

Signed-off-by: Alejandro Vallejo 
Reviewed-by: Andrew Cooper 

---
Cc: Andrew Cooper 
Cc: Wei Liu 
Cc: Anthony PERARD 
Cc: Tim Deegan 
Cc: George Dunlap 
Cc: Juergen Gross 

v4: Changed a line in libxl_x86_acpi.c to print errno/domid instead of rc
---
 tools/console/client/main.c |  7 ++---
 tools/debugger/kdd/kdd-xen.c|  5 ++--
 tools/libs/ctrl/xc_domain.c |  9 +++---
 tools/libs/ctrl/xc_pagetab.c|  7 ++---
 tools/libs/ctrl/xc_private.c|  9 +++---
 tools/libs/ctrl/xc_private.h|  7 +++--
 tools/libs/guest/xg_core.c  | 23 ++
 tools/libs/guest/xg_core.h  |  6 ++--
 tools/libs/guest/xg_core_arm.c  | 10 +++
 tools/libs/guest/xg_core_x86.c  | 18 +--
 tools/libs/guest/xg_cpuid_x86.c | 40 +
 tools/libs/guest/xg_dom_boot.c  | 16 +++---
 tools/libs/guest/xg_domain.c|  8 ++---
 tools/libs/guest/xg_offline_page.c  | 12 
 tools/libs/guest/xg_private.h   |  1 +
 tools/libs/guest/xg_resume.c| 20 ++---
 tools/libs/guest/xg_sr_common.h |  2 +-
 tools/libs/guest/xg_sr_restore.c| 17 ---
 tools/libs/guest/xg_sr_restore_x86_pv.c |  2 +-
 tools/libs/guest/xg_sr_save.c   | 27 +++--
 tools/libs/guest/xg_sr_save_x86_pv.c|  6 ++--
 tools/libs/light/libxl_sched.c  | 16 +-
 tools/libs/light/libxl_x86_acpi.c   |  6 ++--
 tools/misc/xen-hvmcrash.c   |  6 ++--
 tools/misc/xen-lowmemd.c|  6 ++--
 tools/misc/xen-mfndump.c| 22 ++
 tools/misc/xen-vmtrace.c|  6 ++--
 tools/vchan/vchan-socket-proxy.c|  6 ++--
 tools/xenstore/xenstored_domain.c   | 15 +-
 tools/xentrace/xenctx.c |  8 ++---
 30 files changed, 159 insertions(+), 184 deletions(-)

diff --git a/tools/console/client/main.c b/tools/console/client/main.c
index 1a6fa162f7..6775006488 100644
--- a/tools/console/client/main.c
+++ b/tools/console/client/main.c
@@ -408,17 +408,16 @@ int main(int argc, char **argv)
if (dom_path == NULL)
err(errno, "xs_get_domain_path()");
if (type == CONSOLE_INVAL) {
-   xc_dominfo_t xcinfo;
+   xc_domaininfo_t xcinfo;
xc_interface *xc_handle = xc_interface_open(0,0,0);
if (xc_handle == NULL)
err(errno, "Could not open xc interface");
-   if ( (xc_domain_getinfo(xc_handle, domid, 1, &xcinfo) != 1) ||
-(xcinfo.domid != domid) ) {
+   if (xc_domain_getinfo_single(xc_handle, domid, &xcinfo) < 0) {
xc_interface_close(xc_handle);
err(errno, "Failed to get domain information");
}
/* default to pv console for pv guests and serial for hvm 
guests */
-   if (xcinfo.hvm)
+   if (xcinfo.flags & XEN_DOMINF_hvm_guest)
type = CONSOLE_SERIAL;
else
type = CONSOLE_PV;
diff --git a/tools/debugger/kdd/kdd-xen.c b/tools/debugger/kdd/kdd-xen.c
index e78c9311c4..e63e267023 100644
--- a/tools/debugger/kdd/kdd-xen.c
+++ b/tools/debugger/kdd/kdd-xen.c
@@ -570,7 +570,7 @@ kdd_guest *kdd_guest_init(char *arg, FILE *log, int 
verbosity)
 kdd_guest *g = NULL;
 xc_interface *xch = NULL;
 uint32_t domid;
-xc_dominfo_t info;
+xc_domaininfo_t info;
 
 g = calloc(1, sizeof (kdd_guest));
 if (!g) 
@@ -590,7 +590,8 @@ kdd_guest *kdd_guest_init(char *arg, FILE *log, int 
verbosity)
 g->domid = domid;
 
 /* Check that the domain exists and is HVM */
-if (xc_domain_getinfo(xch, domid, 1, &info) != 1 || !info.hvm)
+if (xc_domain_getinfo_single(xch, domid, &info) < 0 ||
+!(info.flags & XEN_DOMINF_hvm_guest))
 goto err;
 
 snprintf(g->id, (sizeof g->id) - 1, 
diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index d5f0923088..66179e6f12 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -1960,15 +1960,14 @@ int xc_domain_memory_mapping(
 uint32_t add_mapping)
 {
 DECLARE_DOMCTL;
-xc_dominfo_t info;
+xc_domaininfo_t info;
 int ret = 0, rc;
 unsigned long done = 0, nr, max_batch_sz;
 
-if ( xc_domain_getinfo(xch, domid, 1, &info) != 1 ||
- info.domid != domid )
+if ( xc_domain_getinfo_single(xch, domid, &info) < 0 )
 {
-PERROR("Could not get info for domain");
-return -EINVAL;
+PERROR("Could not get info for dom%u", domid);
+return -1;
 }
 if ( !xc_core_arch_auto_translated_physmap(&info) )
 return 0;
diff --git a/tools/libs/ctrl/xc_pagetab.c b/tools/libs/ctrl/x

Re: xenstored: EACCESS error accessing control/feature-balloon 1

[zithro]

On 09 May 2023 10:50, Yann Dirson wrote:

On 5/4/23 20:04, zithro wrote:

On 04 May 2023 17:59, Yann Dirson wrote:


On 5/4/23 15:58, zithro wrote:

Hi,

[ snipped for brevity, report summary:
XAPI daemon in domU tries to write to a non-existent xenstore node in
a non-XAPI dom0 ]

On 12 Apr 2023 18:41, Yann Dirson wrote:

Is there anything besides XAPI using this node, or the other data
published by xe-daemon?


On my vanilla Xen (ie. non-XAPI), I have no node about "balloon"-ing
in xenstore (either dom0 or domU nodes, but I'm not using ballooning
in both).


Maybe the original issue is just that there is no reason to have
xe-guest-utilities installed in this setup?


That's what I thought as I'm not using XAPI, so maybe the problem
should only be addressed to the truenas team ? I posted on their forum
but got no answer.
I killed the 'xe-daemon' in both setups without loss of functionality.

My wild guess is that 'xe-daemon', 'xe-update-guest-attrs' and all
'xenstore* commands' are leftovers from when Xen was working as a dom0
under FreeBSD (why would a *domU* have them ?).


That would not be correct: xenstore* are useful in guests, should you
want to read/write to the XenStore manually or from scripts;


Didn't know that, can you give some use cases (or URLs) for which it
is useful, with or without XAPI ?


You can get other examples in
https://xenbits.xen.org/docs/unstable/misc/xenstore-paths.html#domain-controlled-paths


Thanks, I should pay more attention to headings ;)
From the comparison between the docs and the xe-daemon, I saw some 
inconsistencies.

I decided to post a bug report in TrueNAS bug tracker :
https://ixsystems.atlassian.net/browse/NAS-121872
(It's a bit messy though, /me not proud).
I didn't check if FreeBSD and TrueNAS do some changes from upstream, but 
it may be of interest for Citrix devs too ?

Apologizes in advance for mistakes due to misunderstanding/noobiness.


PS: small mistake in "man/xenstore-write.1.html" (from at least 4.14,
and onward), the synopsis reads "xenstore-read" ieof "xenstore-read".

Patch sent, thanks.


No prob and happy to help back !
(@Yann: please tell that to your colleague C. Schultz, so he's more 
inclined to lemme write articles for the Xen docs ^_^).



Also, the -s option disappeared from unstable, although it may be
expected. I don't know it's purpose either.


See
https://github.com/xen-project/xen/commit/c65687ed16d2289ec91036ec2862a4b4bd34ea4f


Ah yes, understood.
To complete the commit, I found a few other places where "-s" has not 
been removed from "xenstore_client.c", in func "usage()" :


Example for line 97, diff style:
- errx(1, "Usage: %s %s[-h] [-p] [-s] [-R] key [...]", progname, mstr);
+ errx(1, "Usage: %s %s[-h] [-p] [-R] key [...]", progname, mstr);

Same deletions needed for lines: 100, 103, 109, 112, 115.


KR,
zithro/Cyril

Re: [PATCH 1/6] x86/cpu-policy: Drop build time cross-checks of featureset sizes

[Jan Beulich]

On 09.05.2023 17:59, Andrew Cooper wrote:
> On 09/05/2023 3:28 pm, Jan Beulich wrote:
>> On 09.05.2023 15:04, Andrew Cooper wrote:
>>> On 08/05/2023 7:47 am, Jan Beulich wrote:
 On 04.05.2023 21:39, Andrew Cooper wrote:
> These BUILD_BUG_ON()s exist to cover the curious absence of a diagnostic 
> for
> code which looks like:
>
>   uint32_t foo[1] = { 1, 2, 3 };
>
> However, GCC 12 at least does now warn for this:
>
>   foo.c:1:24: error: excess elements in array initializer [-Werror]
> 884 | uint32_t foo[1] = { 1, 2, 3 };
> |^
>   foo.c:1:24: note: (near initialization for 'foo')
 I'm pretty sure all gcc versions we support diagnose such cases. In turn
 the arrays in question don't have explicit dimensions at their
 definition sites, and hence they derive their dimensions from their
 initializers. So the build-time-checks are about the arrays in fact
 obtaining the right dimensions, i.e. the initializers being suitable.

 With the core part of the reasoning not being applicable, I'm afraid I
 can't even say "okay with an adjusted description".
>>> Now I'm extra confused.
>>>
>>> I put those BUILD_BUG_ON()'s in because I was not getting a diagnostic
>>> when I was expecting one, and there was a bug in the original featureset
>>> work caused by this going wrong.
>>>
>>> But godbolt seems to agree that even GCC 4.1 notices.
>>>
>>> Maybe it was some other error (C file not seeing the header properly?)
>>> which disappeared across the upstream review?
>> Or maybe, by mistake, too few initializer fields? But what exactly it
>> was probably doesn't matter. If this patch is to stay (see below), some
>> different description will be needed anyway (or the change be folded
>> into the one actually invalidating those BUILD_BUG_ON()s).
>>
>>> Either way, these aren't appropriate, and need deleting before patch 5,
>>> because the check is no longer valid when a featureset can be longer
>>> than the autogen length.
>> Well, they need deleting if we stick to the approach chosen there right
>> now. If we switched to my proposed alternative, they better would stay.
> 
> Given that all versions of GCC do warn, I don't see any justification
> for them to stay.

All versions warn when the variable declarations / definitions have a
dimension specified, and then there are excess initializers. Yet none
of the five affected arrays have a dimension specified in their
definitions.

Even if dimensions were added, we'd then have only covered half of
what the BUILD_BUG_ON()s cover right now: There could then be fewer
than intended initializer fields, and things may still be screwed. I
think it was for this very reason why BUILD_BUG_ON() was chosen.

Jan

> i.e. this should be committed, even if the commit message says "no idea
> what they were taken originally, but they're superfluous in the logic as
> it exists today".
> 
> ~Andrew

Re: [PATCH] x86/iommu: fix wrong iterator type in arch_iommu_hwdom_init()

[Jan Beulich]

On 09.05.2023 13:03, Roger Pau Monne wrote:
> The 'i' iterator index stores a pdx, not a pfn, and hence the initial
> assignation of start (which stores a pfn) needs a conversion from pfn
> to pdx.

Strictly speaking: Yes. But pdx compression skips the bottom MAX_ORDER
bits, so ...

> --- a/xen/drivers/passthrough/x86/iommu.c
> +++ b/xen/drivers/passthrough/x86/iommu.c
> @@ -406,7 +406,7 @@ void __hwdom_init arch_iommu_hwdom_init(struct domain *d)
>   */
>  start = paging_mode_translate(d) ? PFN_DOWN(MB(1)) : 0;

... with this, ...

> -for ( i = start, count = 0; i < top; )
> +for ( i = pfn_to_pdx(start), count = 0; i < top; )

... this is an expensive identity transformation. Could I talk you into
adding

ASSERT(start == pfn_to_pdx(start));

instead (or the corresponding BUG_ON() if you'd prefer that, albeit then
the expensive identity transformation will still be there even in release
builds; not that it matters all that much right here, but still)?

In any event, with no real bug fixed (unless I'm overlooking something),
I would suggest to drop the Fixes: tag.

Jan

[linux-linus test] 180587: regressions - FAIL

[osstest service owner]

flight 180587 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180587/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit1   8 xen-boot fail REGR. vs. 180278
 build-arm64-pvops 6 kernel-build fail REGR. vs. 180278

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-libvirt-qcow2 19 guest-start/debian.repeat fail pass in 180582
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail pass 
in 180582

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-vhd   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked  n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked  n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked  n/a
 test-arm64-arm64-xl-credit2 15 migrate-support-check fail in 180582 never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-check fail in 180582 never pass
 test-arm64-arm64-xl-xsm 15 migrate-support-check fail in 180582 never pass
 test-arm64-arm64-xl-credit2 16 saverestore-support-check fail in 180582 never 
pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-check fail in 180582 never 
pass
 test-arm64-arm64-xl-xsm 16 saverestore-support-check fail in 180582 never pass
 test-arm64-arm64-xl 15 migrate-support-check fail in 180582 never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-check fail in 180582 never pass
 test-arm64-arm64-xl 16 saverestore-support-check fail in 180582 never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-check fail in 180582 never 
pass
 test-arm64-arm64-xl-credit1 15 migrate-support-check fail in 180582 never pass
 test-arm64-arm64-xl-credit1 16 saverestore-support-check fail in 180582 never 
pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-check fail in 180582 never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-check fail in 180582 never 
pass
 test-arm64-arm64-xl-vhd 14 migrate-support-check fail in 180582 never pass
 test-arm64-arm64-xl-vhd 15 saverestore-support-check fail in 180582 never pass
 test-armhf-armhf-examine  8 reboot   fail  like 180278
 test-armhf-armhf-libvirt  8 xen-boot fail  like 180278
 test-armhf-armhf-libvirt-raw  8 xen-boot fail  like 180278
 test-armhf-armhf-xl-rtds  8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278
 test-armhf-armhf-xl   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278
 test-armhf-armhf-xl-multivcpu  8 xen-boot fail like 180278
 test-armhf-armhf-libvirt-qcow2  8 xen-bootfail like 180278
 test-armhf-armhf-xl-credit2   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-vhd   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-arndale   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278
 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-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass

version targeted for testing:
 linuxba0ad6ed89fd5dada3b7b65ef2b08e95d449d4ab
baseline version:
 linux6c538e1adbfc696ac4747fb10d63e704344f763d

Last test of basis   180278  2023-04-16 19:41:46 Z   22 days
Failing since180281  2023-04-17 06:24:36 Z   22 days   41 attempts
Testing same since   180582  2023-05-08 21:11:46 Z0 days2 attempts


2359 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  

[PATCH v2 0/3] Add CpuidUserDis support

[Alejandro Vallejo]

v2:
 * Style changes
 * Remove v1/patch3: HVM not to be addressed by this series
 * Adds one patch between v1/patch1 and v1/patch2 with the vendor-specific
   refactor of probe_cpuid_faulting()

Nowadays AMD supports trapping the CPUID instruction from ring>0 to ring0,
(CpuidUserDis) akin to Intel's "CPUID faulting". There is a difference in
that the toggle bit is in a different MSR and the support bit is in CPUID
itself rather than yet another MSR. This patch enables AMD hosts to use it
when supported in order to provide correct CPUID contents to PV guests.

Patch 1 merely adds definitions to various places in CPUID and MSR

Patch 2 moves vendor-specific code on probe_cpuid_faulting() to amd.c/intel.c

Patch 3 adds support for CpuidUserDis, hooking it in the probing path and
the context switching path.

Alejandro Vallejo (3):
  x86: Add AMD's CpuidUserDis bit definitions
  x86: Refactor conditional guard in probe_cpuid_faulting()
  x86: Add support for CpuidUserDis

 tools/libs/light/libxl_cpuid.c  |  1 +
 tools/misc/xen-cpuid.c  |  2 +
 xen/arch/x86/cpu/amd.c  | 28 ++-
 xen/arch/x86/cpu/common.c   | 51 +++--
 xen/arch/x86/cpu/intel.c| 12 -
 xen/arch/x86/include/asm/amd.h  |  1 +
 xen/arch/x86/include/asm/msr-index.h|  1 +
 xen/include/public/arch-x86/cpufeatureset.h |  1 +
 8 files changed, 71 insertions(+), 26 deletions(-)

-- 
2.34.1

[PATCH v2 2/3] x86: Refactor conditional guard in probe_cpuid_faulting()

[Alejandro Vallejo]

Move vendor-specific checks to the vendor-specific callers.

No functional change.

Signed-off-by: Alejandro Vallejo 
---
v2:
  * Patch factored out from patch2 of v1
---
 xen/arch/x86/cpu/amd.c| 10 +-
 xen/arch/x86/cpu/common.c | 11 ---
 xen/arch/x86/cpu/intel.c  |  9 -
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index caafe44740..899bae7a10 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -271,7 +271,15 @@ static void __init noinline amd_init_levelling(void)
 {
const struct cpuidmask *m = NULL;
 
-   if (probe_cpuid_faulting())
+   /*
+* If there's support for CpuidUserDis or CPUID faulting then
+* we can skip levelling because CPUID accesses are trapped anyway.
+*
+* CPUID faulting is an Intel feature analogous to CpuidUserDis, so
+* that can only be present when Xen is itself virtualized (because
+* it can be emulated)
+*/
+   if (cpu_has_hypervisor && probe_cpuid_faulting())
return;
 
probe_masking_msrs();
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index edc4db1335..4bfaac4590 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -131,17 +131,6 @@ bool __init probe_cpuid_faulting(void)
uint64_t val;
int rc;
 
-   /*
-* Don't bother looking for CPUID faulting if we aren't virtualised on
-* AMD or Hygon hardware - it won't be present.  Likewise for Fam0F
-* Intel hardware.
-*/
-   if (((boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON)) ||
-((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
- boot_cpu_data.x86 == 0xf)) &&
-   !cpu_has_hypervisor)
-   return false;
-
if ((rc = rdmsr_safe(MSR_INTEL_PLATFORM_INFO, val)) == 0)
raw_cpu_policy.platform_info.cpuid_faulting =
val & MSR_PLATFORM_INFO_CPUID_FAULTING;
diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c
index 71fc1a1e18..a04414ba1d 100644
--- a/xen/arch/x86/cpu/intel.c
+++ b/xen/arch/x86/cpu/intel.c
@@ -226,7 +226,14 @@ static void cf_check intel_ctxt_switch_masking(const 
struct vcpu *next)
  */
 static void __init noinline intel_init_levelling(void)
 {
-   if (probe_cpuid_faulting())
+   /*
+* Intel Fam0f is old enough that probing for CPUID faulting support
+* introduces spurious #GP(0) when the appropriate MSRs are read,
+* so skip it altogether. In the case where Xen is virtualized these
+* MSRs may be emulated though, so we allow it in that case.
+*/
+   if ((boot_cpu_data.x86 != 0xf || cpu_has_hypervisor) &&
+   probe_cpuid_faulting())
return;
 
probe_masking_msrs();
-- 
2.34.1

[PATCH v2 1/3] x86: Add AMD's CpuidUserDis bit definitions

[Alejandro Vallejo]

AMD reports support for CpuidUserDis in CPUID and provides the toggle in HWCR.
This patch adds the positions of both of those bits to both xen and tools.

No functional change.

Signed-off-by: Alejandro Vallejo 
---
 tools/libs/light/libxl_cpuid.c  | 1 +
 tools/misc/xen-cpuid.c  | 2 ++
 xen/arch/x86/include/asm/msr-index.h| 1 +
 xen/include/public/arch-x86/cpufeatureset.h | 1 +
 4 files changed, 5 insertions(+)

diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
index 5f0bf93810..4d2fab5414 100644
--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -317,6 +317,7 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list 
*cpuid, const char* str)
 
 {"lfence+",  0x8021, NA, CPUID_REG_EAX,  2,  1},
 {"nscb", 0x8021, NA, CPUID_REG_EAX,  6,  1},
+{"cpuid-user-dis", 0x8021, NA, CPUID_REG_EAX, 17,  1},
 
 {"maxhvleaf",0x4000, NA, CPUID_REG_EAX,  0,  8},
 
diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c
index d7efc59d31..8ec143ebc8 100644
--- a/tools/misc/xen-cpuid.c
+++ b/tools/misc/xen-cpuid.c
@@ -199,6 +199,8 @@ static const char *const str_e21a[32] =
 {
 [ 2] = "lfence+",
 [ 6] = "nscb",
+
+/* 16 */[17] = "cpuid-user-dis",
 };
 
 static const char *const str_7b1[32] =
diff --git a/xen/arch/x86/include/asm/msr-index.h 
b/xen/arch/x86/include/asm/msr-index.h
index fa771ed0b5..082fb2e0d9 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -337,6 +337,7 @@
 
 #define MSR_K8_HWCR0xc0010015
 #define K8_HWCR_TSC_FREQ_SEL   (1ULL << 24)
+#define K8_HWCR_CPUID_USER_DIS (1ULL << 35)
 
 #define MSR_K7_FID_VID_CTL 0xc0010041
 #define MSR_K7_FID_VID_STATUS  0xc0010042
diff --git a/xen/include/public/arch-x86/cpufeatureset.h 
b/xen/include/public/arch-x86/cpufeatureset.h
index 12e3dc80c6..623dcb1bce 100644
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -287,6 +287,7 @@ XEN_CPUFEATURE(AVX_IFMA, 10*32+23) /*A  AVX-IFMA 
Instructions */
 /* AMD-defined CPU features, CPUID level 0x8021.eax, word 11 */
 XEN_CPUFEATURE(LFENCE_DISPATCH,11*32+ 2) /*A  LFENCE always serializing */
 XEN_CPUFEATURE(NSCB,   11*32+ 6) /*A  Null Selector Clears Base 
(and limit too) */
+XEN_CPUFEATURE(CPUID_USER_DIS, 11*32+17) /*   CPUID disable for 
non-privileged software */
 
 /* Intel-defined CPU features, CPUID level 0x0007:1.ebx, word 12 */
 XEN_CPUFEATURE(INTEL_PPIN, 12*32+ 0) /*   Protected Processor 
Inventory Number */
-- 
2.34.1

[PATCH v2 3/3] x86: Add support for CpuidUserDis

[Alejandro Vallejo]

Because CpuIdUserDis is reported in CPUID itself, the extended leaf
containing that bit must be retrieved before calling c_early_init()

Signed-off-by: Alejandro Vallejo 
---
v2:
 * Style fixes
 * MSR index inlined in rdmsr/wrmsr
 * Swapped Intel's conditional guard so typically true condition goes first
 * Factored the vendor-specific non functional changes into another patch
---
 xen/arch/x86/cpu/amd.c | 20 -
 xen/arch/x86/cpu/common.c  | 40 +++---
 xen/arch/x86/cpu/intel.c   |  5 -
 xen/arch/x86/include/asm/amd.h |  1 +
 4 files changed, 51 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index 899bae7a10..cc9c89fd19 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -279,8 +279,12 @@ static void __init noinline amd_init_levelling(void)
 * that can only be present when Xen is itself virtualized (because
 * it can be emulated)
 */
-   if (cpu_has_hypervisor && probe_cpuid_faulting())
+   if ((cpu_has_hypervisor && probe_cpuid_faulting()) ||
+   boot_cpu_has(X86_FEATURE_CPUID_USER_DIS)) {
+   expected_levelling_cap |= LCAP_faulting;
+   levelling_caps |=  LCAP_faulting;
return;
+   }
 
probe_masking_msrs();
 
@@ -371,6 +375,20 @@ static void __init noinline amd_init_levelling(void)
ctxt_switch_masking = amd_ctxt_switch_masking;
 }
 
+void amd_set_cpuid_user_dis(bool enable)
+{
+   const uint64_t bit = K8_HWCR_CPUID_USER_DIS;
+   uint64_t val;
+
+   rdmsrl(MSR_K8_HWCR, val);
+
+   if (!!(val & bit) == enable)
+   return;
+
+   val ^= bit;
+   wrmsrl(MSR_K8_HWCR, val);
+}
+
 /*
  * Check for the presence of an AMD erratum. Arguments are defined in amd.h 
  * for each known erratum. Return 1 if erratum is found.
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 4bfaac4590..9bbb385db4 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -4,6 +4,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -144,8 +145,6 @@ bool __init probe_cpuid_faulting(void)
return false;
}
 
-   expected_levelling_cap |= LCAP_faulting;
-   levelling_caps |=  LCAP_faulting;
setup_force_cpu_cap(X86_FEATURE_CPUID_FAULTING);
 
return true;
@@ -168,8 +167,10 @@ static void set_cpuid_faulting(bool enable)
 void ctxt_switch_levelling(const struct vcpu *next)
 {
const struct domain *nextd = next ? next->domain : NULL;
+   bool enable_cpuid_faulting;
 
-   if (cpu_has_cpuid_faulting) {
+   if (cpu_has_cpuid_faulting ||
+   boot_cpu_has(X86_FEATURE_CPUID_USER_DIS)) {
/*
 * No need to alter the faulting setting if we are switching
 * to idle; it won't affect any code running in idle context.
@@ -190,12 +191,18 @@ void ctxt_switch_levelling(const struct vcpu *next)
 * an interim escape hatch in the form of
 * `dom0=no-cpuid-faulting` to restore the older behaviour.
 */
-   set_cpuid_faulting(nextd && (opt_dom0_cpuid_faulting ||
-!is_control_domain(nextd) ||
-!is_pv_domain(nextd)) &&
-  (is_pv_domain(nextd) ||
-   next->arch.msrs->
-   misc_features_enables.cpuid_faulting));
+   enable_cpuid_faulting = nextd && (opt_dom0_cpuid_faulting ||
+ !is_control_domain(nextd) ||
+ !is_pv_domain(nextd)) &&
+   (is_pv_domain(nextd) ||
+next->arch.msrs->
+misc_features_enables.cpuid_faulting);
+
+   if (cpu_has_cpuid_faulting)
+   set_cpuid_faulting(enable_cpuid_faulting);
+   else
+   amd_set_cpuid_user_dis(enable_cpuid_faulting);
+
return;
}
 
@@ -404,6 +411,17 @@ static void generic_identify(struct cpuinfo_x86 *c)
c->apicid = phys_pkg_id((ebx >> 24) & 0xFF, 0);
c->phys_proc_id = c->apicid;
 
+   eax = cpuid_eax(0x8000);
+   if ((eax >> 16) == 0x8000)
+   c->extended_cpuid_level = eax;
+
+   /*
+* These AMD-defined flags are out of place, but we need
+* them early for the CPUID faulting probe code
+*/
+   if (c->extended_cpuid_level >= 0x8021)
+   c->x86_capability[FEATURESET_e21a] = cpuid_eax(0x8021);
+
if (this_cpu->c_early_init)
this_cpu->c_early_init(c);
 
@@ -420,10 +438,6 @@ static void generic_identify(struct cpuinfo_x86 *c)
 

Re: [PATCH v1] tools: drop bogus and obsolete ptyfuncs.m4

[Anthony PERARD]

On Tue, May 02, 2023 at 08:48:00PM +, Olaf Hering wrote:
> According to openpty(3) it is required to include  to get the
> prototypes for openpty() and login_tty(). But this is not what the
> function AX_CHECK_PTYFUNCS actually does. It makes no attempt to include
> the required header.
> 
> The two source files which call openpty() and login_tty() already contain
> the conditionals to include the required header.
> 
> Remove the bogus m4 file to fix build with clang, which complains about
> calls to undeclared functions.
> 
> Signed-off-by: Olaf Hering 

That change isn't enough. And I'm not convinced that it needs to be
removed.

First, AX_CHECK_PTYFUNCS is still called in "tools/configure.ac".

Then, AX_CHECK_PTYFUNCS define INCLUDE_LIBUTIL_H and PTYFUNCS_LIBS.
Those two are still used in the tree.

Also, that that macro isn't just about the header, but also about the
needed library.

Thanks,

-- 
Anthony PERARD

Re: [PATCH v5 00/21] block: remove aio_disable_external() API

[Stefan Hajnoczi]

On Thu, May 04, 2023 at 11:44:42PM +0200, Kevin Wolf wrote:
> Am 04.05.2023 um 21:53 hat Stefan Hajnoczi geschrieben:
> > v5:
> > - Use atomic accesses for in_flight counter in vhost-user-server.c [Kevin]
> > - Stash SCSIDevice id/lun values for VIRTIO_SCSI_T_TRANSPORT_RESET event
> >   before unrealizing the SCSIDevice [Kevin]
> > - Keep vhost-user-blk export .detach() callback so ctx is set to NULL 
> > [Kevin]
> > - Narrow BdrvChildClass and BlockDriver drained_{begin/end/poll} callbacks 
> > from
> >   IO_OR_GS_CODE() to GLOBAL_STATE_CODE() [Kevin]
> > - Include Kevin's "block: Fix use after free in blockdev_mark_auto_del()" to
> >   fix a latent bug that was exposed by this series
> 
> I only just finished reviewing v4 when you had already sent v5, but it
> hadn't arrived yet. I had a few more comments on what are now patches
> 17, 18, 19 and 21 in v5. I think they all still apply.

I'm not sure which comments from v4 still apply. In my email client all
your replies were already read when I sent v5.

Maybe you can share the Message-Id of something I still need to address?

Thanks,
Stefan


signature.asc
Description: PGP signature

Re: [patch v3 08/36] x86/smpboot: Split up native_cpu_up() into separate phases and document them

[Thomas Gleixner]

On Tue, May 09 2023 at 14:07, Thomas Gleixner wrote:
> On Tue, May 09 2023 at 12:04, Peter Zijlstra wrote:
>> On Mon, May 08, 2023 at 09:43:39PM +0200, Thomas Gleixner wrote:
>>> +   /*
>>> +* Sync point with wait_cpu_callin(). The AP doesn't wait here
>>> +* but just sets the bit to let the controlling CPU (BSP) know that
>>> +* it's got this far.
>>> +*/
>>> smp_callin();
>>>  
>>> -   /* otherwise gcc will move up smp_processor_id before the cpu_init */
>>> +   /* Otherwise gcc will move up smp_processor_id() before cpu_init() */
>>> barrier();
>>
>> Not to the detriment of this patch, but this barrier() and it's comment
>> seem weird vs smp_callin(). That function ends with an atomic bitop (it
>> has to, at the very least it must not be weaker than store-release) but
>> also has an explicit wmb() to order setup vs CPU_STARTING.
>>
>> (arguably that should be a full fence *AND* get a comment)
>>
>> There is no way the smp_processor_id() referred to in this comment can
>> land before cpu_init() even without the barrier().
>
> Right. Let me clean that up.

So I went and tried to figure out where this comes from. It's from
d8f19f2cac70 ("[PATCH] x86-64 merge") in the history tree. One of those
well documented combo patches which change world and some more. The
context back then was:

/*
 * Dont put anything before smp_callin(), SMP
 * booting is too fragile that we want to limit the
 * things done here to the most necessary things.
 */
cpu_init();
smp_callin();

Dprintk("cpu %d: waiting for commence\n", smp_processor_id()); 

That still does not explain what the barrier is doing. I tried to
harvest mailing list archives, but did not find anything. The back then
list disc...@x86-64.org was never publicly archived... Boris gave me an
tarball, but this 'barrier()' add on was nowhere discussed in public.

As the barrier has no obvious value, I'm adding a patch upfront which
removes it.

Thanks,

tglx

Re: [patch v3 08/36] x86/smpboot: Split up native_cpu_up() into separate phases and document them

[Thomas Gleixner]

On Tue, May 09 2023 at 12:19, Peter Zijlstra wrote:
> Again, not really this patch, but since I had to look at this code 
>
> On Mon, May 08, 2023 at 09:43:39PM +0200, Thomas Gleixner wrote:
>> @@ -1048,60 +1066,89 @@ static int do_boot_cpu(int apicid, int c
>
>   /*
>* AP might wait on cpu_callout_mask in cpu_init() with
>* cpu_initialized_mask set if previous attempt to online
>* it timed-out. Clear cpu_initialized_mask so that after
>* INIT/SIPI it could start with a clean state.
>*/
>   cpumask_clear_cpu(cpu, cpu_initialized_mask);
>   smp_mb();
>
> ^^^ that barrier is weird too, cpumask_clear_cpu() is an atomic op and
> implies much the same (this is x86 after all). If you want to be super
> explicit about it write:
>
>   smp_mb__after_atomic();
>
> (which is a no-op) but then it still very much requires a comment as to
> what exactly it orders against what.

Won't bother either as that mask is gone a few patches later.

Re: [PATCH v8 0/7] Add pci_dev_for_each_resource() helper and update users

[Bjorn Helgaas]

On Tue, Apr 04, 2023 at 11:11:01AM -0500, Bjorn Helgaas wrote:
> On Thu, Mar 30, 2023 at 07:24:27PM +0300, Andy Shevchenko wrote:
> > Provide two new helper macros to iterate over PCI device resources and
> > convert users.

> Applied 2-7 to pci/resource for v6.4, thanks, I really like this!

This is 09cc90063240 ("PCI: Introduce pci_dev_for_each_resource()")
upstream now.

Coverity complains about each use, sample below from
drivers/pci/vgaarb.c.  I didn't investigate at all, so it might be a
false positive; just FYI.

  1. Condition screen_info.capabilities & (2U /* 1 << 1 */), taking 
true branch.
  556if (screen_info.capabilities & VIDEO_CAPABILITY_64BIT_BASE)
  557base |= (u64)screen_info.ext_lfb_base << 32;
  558
  559limit = base + size;
  560
  561/* Does firmware framebuffer belong to us? */
  2. Condition __b < PCI_NUM_RESOURCES, taking true branch.
  3. Condition (r = &pdev->resource[__b]) , (__b < PCI_NUM_RESOURCES), 
taking true branch.
  6. Condition __b < PCI_NUM_RESOURCES, taking true branch.
  7. cond_at_most: Checking __b < PCI_NUM_RESOURCES implies that __b 
may be up to 16 on the true branch.
  8. Condition (r = &pdev->resource[__b]) , (__b < PCI_NUM_RESOURCES), 
taking true branch.
  11. incr: Incrementing __b. The value of __b may now be up to 17.
  12. alias: Assigning: r = &pdev->resource[__b]. r may now point to as 
high as element 17 of pdev->resource (which consists of 17 64-byte elements).
  13. Condition __b < PCI_NUM_RESOURCES, taking true branch.
  14. Condition (r = &pdev->resource[__b]) , (__b < PCI_NUM_RESOURCES), 
taking true branch.
  562pci_dev_for_each_resource(pdev, r) {
  4. Condition resource_type(r) != 512, taking true branch.
  9. Condition resource_type(r) != 512, taking true branch.

  CID 1529911 (#1 of 1): Out-of-bounds read (OVERRUN)
  15. overrun-local: Overrunning array of 1088 bytes at byte offset 1088 by 
dereferencing pointer r. [show details]
  563if (resource_type(r) != IORESOURCE_MEM)
  5. Continuing loop.
  10. Continuing loop.
  564continue;

Re: [PATCH v5 04/14] tools/xenstore: add framework to commit accounting data on success only

[Julien Grall]

Hi Juergen,

On 08/05/2023 12:47, Juergen Gross wrote:

Instead of modifying accounting data and undo those modifications in
case of an error during further processing, add a framework for
collecting the needed changes and commit them only when the whole
operation has succeeded.

This scheme can reuse large parts of the per transaction accounting.
The changed_domain handling can be reused, but the array size of the
accounting data should be possible to be different for both use cases.

Signed-off-by: Juergen Gross 


Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall

Re: [PATCH v5 00/21] block: remove aio_disable_external() API

[Kevin Wolf]

Am 09.05.2023 um 19:51 hat Stefan Hajnoczi geschrieben:
> On Thu, May 04, 2023 at 11:44:42PM +0200, Kevin Wolf wrote:
> > Am 04.05.2023 um 21:53 hat Stefan Hajnoczi geschrieben:
> > > v5:
> > > - Use atomic accesses for in_flight counter in vhost-user-server.c [Kevin]
> > > - Stash SCSIDevice id/lun values for VIRTIO_SCSI_T_TRANSPORT_RESET event
> > >   before unrealizing the SCSIDevice [Kevin]
> > > - Keep vhost-user-blk export .detach() callback so ctx is set to NULL 
> > > [Kevin]
> > > - Narrow BdrvChildClass and BlockDriver drained_{begin/end/poll} 
> > > callbacks from
> > >   IO_OR_GS_CODE() to GLOBAL_STATE_CODE() [Kevin]
> > > - Include Kevin's "block: Fix use after free in blockdev_mark_auto_del()" 
> > > to
> > >   fix a latent bug that was exposed by this series
> > 
> > I only just finished reviewing v4 when you had already sent v5, but it
> > hadn't arrived yet. I had a few more comments on what are now patches
> > 17, 18, 19 and 21 in v5. I think they all still apply.
> 
> I'm not sure which comments from v4 still apply. In my email client all
> your replies were already read when I sent v5.

Yes, but I added some more replies after you had sent v5 (and before I
fetched mail again to actually see v5).

> Maybe you can share the Message-Id of something I still need to address?

I thought the patch numbers identified them and were easier, but sure:

Message-ID: 
Message-ID: 
Message-ID: 
Message-ID: 

Kevin


signature.asc
Description: PGP signature

Re: [PATCH v5 05/14] tools/xenstore: use accounting buffering for node accounting

[Julien Grall]

Hi Juergen,

On 08/05/2023 12:47, Juergen Gross wrote:

Add the node accounting to the accounting information buffering in
order to avoid having to undo it in case of failure.

This requires to call domain_nbentry_dec() before any changes to the
data base, as it can return an error now.

Signed-off-by: Juergen Gross 
---
V5:
- add error handling after domain_nbentry_dec() calls (Julien Grall)
---
  tools/xenstore/xenstored_core.c   | 29 +++--
  tools/xenstore/xenstored_domain.h |  4 ++--
  2 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 8392bdec9b..22da434e2a 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1454,7 +1454,6 @@ static void destroy_node_rm(struct connection *conn, 
struct node *node)
  static int destroy_node(struct connection *conn, struct node *node)
  {
destroy_node_rm(conn, node);
-   domain_nbentry_dec(conn, get_node_owner(node));
  
  	/*

 * It is not possible to easily revert the changes in a transaction.
@@ -1645,6 +1644,9 @@ static int delnode_sub(const void *ctx, struct connection 
*conn,
if (ret > 0)
return WALK_TREE_SUCCESS_STOP;
  
+	if (domain_nbentry_dec(conn, get_node_owner(node)))

+   return WALK_TREE_ERROR_STOP;


I think there is a potential issue with the buffering here. In case of 
failure, the node could have been removed, but the quota would not be 
properly accounted.


Also, I think a comment would be warrant to explain why we are returning 
WALK_TREE_ERROR_STOP here when...



+
/* In case of error stop the walk. */
if (!ret && do_tdb_delete(conn, &key, &node->acc))
return WALK_TREE_SUCCESS_STOP;


... this is not the case when do_tdb_delete() fails for some reasons.


@@ -1657,8 +1659,6 @@ static int delnode_sub(const void *ctx, struct connection 
*conn,
watch_exact = strcmp(root, node->name);
fire_watches(conn, ctx, node->name, node, watch_exact, NULL);
  
-	domain_nbentry_dec(conn, get_node_owner(node));

-
return WALK_TREE_RM_CHILDENTRY;
  }
  
@@ -1797,29 +1797,14 @@ static int do_set_perms(const void *ctx, struct connection *conn,

return EPERM;
  
  	old_perms = node->perms;

-   domain_nbentry_dec(conn, get_node_owner(node));
+   if (domain_nbentry_dec(conn, get_node_owner(node)))
+   return ENOMEM;
node->perms = perms;
-   if (domain_nbentry_inc(conn, get_node_owner(node))) {
-   node->perms = old_perms;
-   /*
-* This should never fail because we had a reference on the
-* domain before and Xenstored is single-threaded.
-*/
-   domain_nbentry_inc(conn, get_node_owner(node));
+   if (domain_nbentry_inc(conn, get_node_owner(node)))
return ENOMEM;
-   }
-
-   if (write_node(conn, node, false)) {
-   int saved_errno = errno;
  
-		domain_nbentry_dec(conn, get_node_owner(node));

-   node->perms = old_perms;
-   /* No failure possible as above. */
-   domain_nbentry_inc(conn, get_node_owner(node));
-
-   errno = saved_errno;
+   if (write_node(conn, node, false))
return errno;
-   }
  
  	fire_watches(conn, ctx, name, node, false, &old_perms);

send_ack(conn, XS_SET_PERMS);
diff --git a/tools/xenstore/xenstored_domain.h 
b/tools/xenstore/xenstored_domain.h
index e40657216b..466549709f 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -25,9 +25,9 @@
   * a per transaction array.
   */
  enum accitem {
+   ACC_NODES,
ACC_REQ_N,  /* Number of elements per request. */
-   ACC_NODES = ACC_REQ_N,
-   ACC_TR_N,   /* Number of elements per transaction. */
+   ACC_TR_N = ACC_REQ_N,   /* Number of elements per transaction. */
ACC_CHD_N = ACC_TR_N,   /* max(ACC_REQ_N, ACC_TR_N), for changed dom. */
ACC_N = ACC_TR_N,   /* Number of elements per domain. */
  };



Cheers,

--
Julien Grall

Re: [PATCH v5 07/14] tools/xenstore: use accounting data array for per-domain values

[Julien Grall]

On 08/05/2023 12:47, Juergen Gross wrote:

Add the accounting of per-domain usage of Xenstore memory, watches, and
outstanding requests to the array based mechanism.

Signed-off-by: Juergen Gross 


Acked-by: Julien Grall 


---
V5:
- drop domid parameter from domain_outstanding_inc() (Julien Grall)
---
  tools/xenstore/xenstored_core.c   |   4 +-
  tools/xenstore/xenstored_domain.c | 109 +++---
  tools/xenstore/xenstored_domain.h |   8 ++-
  3 files changed, 48 insertions(+), 73 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 4d1debeba1..e7f86f9487 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -255,7 +255,7 @@ static void free_buffered_data(struct buffered_data *out,
req->pend.ref.event_cnt--;
if (!req->pend.ref.event_cnt && !req->on_out_list) {
if (req->on_ref_list) {
-   domain_outstanding_domid_dec(
+   domain_outstanding_dec(conn,
req->pend.ref.domid);
list_del(&req->list);
}
@@ -271,7 +271,7 @@ static void free_buffered_data(struct buffered_data *out,
out->on_ref_list = true;
return;
} else
-   domain_outstanding_dec(conn);
+   domain_outstanding_dec(conn, conn->id);
  
  	talloc_free(out);

  }
diff --git a/tools/xenstore/xenstored_domain.c 
b/tools/xenstore/xenstored_domain.c
index 7770c4f395..a35ed97fd7 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -72,19 +72,12 @@ struct domain
/* Accounting data for this domain. */
unsigned int acc[ACC_N];
  
-	/* Amount of memory allocated for this domain. */

-   int memory;
+   /* Memory quota data for this domain. */
bool soft_quota_reported;
bool hard_quota_reported;
time_t mem_last_msg;
  #define MEM_WARN_MINTIME_SEC 10
  
-	/* number of watch for this domain */

-   int nbwatch;
-
-   /* Number of outstanding requests. */
-   int nboutstanding;
-
/* write rate limit */
wrl_creditt wrl_credit; /* [ -wrl_config_writecost, +_dburst ] */
struct wrl_timestampt wrl_timestamp;
@@ -200,14 +193,15 @@ static bool domain_can_write(struct connection *conn)
  
  static bool domain_can_read(struct connection *conn)

  {
-   struct xenstore_domain_interface *intf = conn->domain->interface;
+   struct domain *domain = conn->domain;
+   struct xenstore_domain_interface *intf = domain->interface;
  
  	if (domain_is_unprivileged(conn)) {

-   if (conn->domain->wrl_credit < 0)
+   if (domain->wrl_credit < 0)
return false;
-   if (conn->domain->nboutstanding >= quota_req_outstanding)
+   if (domain->acc[ACC_OUTST] >= quota_req_outstanding)
return false;
-   if (conn->domain->memory >= quota_memory_per_domain_hard &&
+   if (domain->acc[ACC_MEM] >= quota_memory_per_domain_hard &&
quota_memory_per_domain_hard)
return false;
}
@@ -438,10 +432,10 @@ int domain_get_quota(const void *ctx, struct connection 
*conn,
if (!resp) return ENOMEM
  
  	ent(nodes, d->acc[ACC_NODES]);

-   ent(watches, d->nbwatch);
+   ent(watches, d->acc[ACC_WATCH]);
ent(transactions, ta);
-   ent(outstanding, d->nboutstanding);
-   ent(memory, d->memory);
+   ent(outstanding, d->acc[ACC_OUTST]);
+   ent(memory, d->acc[ACC_MEM]);
  
  #undef ent
  
@@ -1187,14 +1181,16 @@ unsigned int domain_nbentry(struct connection *conn)

   ? domain_acc_add(conn, conn->id, ACC_NODES, 0, true) : 0;
  }
  
-static bool domain_chk_quota(struct domain *domain, int mem)

+static bool domain_chk_quota(struct connection *conn, unsigned int mem)
  {
time_t now;
+   struct domain *domain;
  
-	if (!domain || !domid_is_unprivileged(domain->domid) ||

-   (domain->conn && domain->conn->is_ignored))
+   if (!conn || !domid_is_unprivileged(conn->id) ||
+   conn->is_ignored)
return false;
  
+	domain = conn->domain;

now = time(NULL);
  
  	if (mem >= quota_memory_per_domain_hard &&

@@ -1239,80 +1235,57 @@ static bool domain_chk_quota(struct domain *domain, int 
mem)
  int domain_memory_add(struct connection *conn, unsigned int domid, int mem,
  bool no_quota_check)
  {
-   struct domain *domain;
+   int ret;
  
-	domain = find_domain_struct(domid);

-   if (domain) {
-   /*
-* domain_chk_quota() will print warning and also store whether
-* the soft/hard quota has been hit. So check no_quota_check
-

Re: [PATCH v5 10/14] tools/xenstore: switch transaction accounting to generic accounting

[Julien Grall]

Hi Juergen,

On 08/05/2023 12:47, Juergen Gross wrote:

As transaction accounting is active for unprivileged domains only, it
can easily be added to the generic per-domain accounting.

Signed-off-by: Juergen Gross 
---
V5:
- use list_empty(&conn->transaction_list) for detection of "no
   transaction active" (Julien Grall)
---
  tools/xenstore/xenstored_core.c|  3 +--
  tools/xenstore/xenstored_core.h|  1 -
  tools/xenstore/xenstored_domain.c  | 21 ++---
  tools/xenstore/xenstored_domain.h  |  4 
  tools/xenstore/xenstored_transaction.c | 14 ++
  5 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 3caf9e45dc..c98d30561f 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2087,7 +2087,7 @@ static void consider_message(struct connection *conn)
 * stalled. This will ignore new requests until Live-Update happened
 * or it was aborted.
 */
-   if (lu_is_pending() && conn->transaction_started == 0 &&
+   if (lu_is_pending() && list_empty(&conn->transaction_list) &&
conn->in->hdr.msg.type == XS_TRANSACTION_START) {
trace("Delaying transaction start for connection %p req_id 
%u\n",
  conn, conn->in->hdr.msg.req_id);
@@ -2194,7 +2194,6 @@ struct connection *new_connection(const struct 
interface_funcs *funcs)
new->funcs = funcs;
new->is_ignored = false;
new->is_stalled = false;
-   new->transaction_started = 0;
INIT_LIST_HEAD(&new->out_list);
INIT_LIST_HEAD(&new->acc_list);
INIT_LIST_HEAD(&new->ref_list);
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 5a11dc1231..3564d85d7d 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -151,7 +151,6 @@ struct connection
/* List of in-progress transactions. */
struct list_head transaction_list;
uint32_t next_transaction_id;
-   unsigned int transaction_started;
time_t ta_start_time;
  
  	/* List of delayed requests. */

diff --git a/tools/xenstore/xenstored_domain.c 
b/tools/xenstore/xenstored_domain.c
index 03825ca24b..25c6d20036 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -417,12 +417,10 @@ int domain_get_quota(const void *ctx, struct connection 
*conn,
  {
struct domain *d = find_domain_struct(domid);
char *resp;
-   int ta;
  
  	if (!d)

return ENOENT;
  
-	ta = d->conn ? d->conn->transaction_started : 0;

resp = talloc_asprintf(ctx, "Domain %u:\n", domid);
if (!resp)
return ENOMEM;
@@ -433,7 +431,7 @@ int domain_get_quota(const void *ctx, struct connection 
*conn,
  
  	ent(nodes, d->acc[ACC_NODES]);

ent(watches, d->acc[ACC_WATCH]);
-   ent(transactions, ta);
+   ent(transactions, d->acc[ACC_TRANS]);
ent(outstanding, d->acc[ACC_OUTST]);
ent(memory, d->acc[ACC_MEM]);
  
@@ -1298,6 +1296,23 @@ void domain_outstanding_dec(struct connection *conn, unsigned int domid)

domain_acc_add(conn, domid, ACC_OUTST, -1, true);
  }
  
+void domain_transaction_inc(struct connection *conn)

+{
+   domain_acc_add(conn, conn->id, ACC_TRANS, 1, true);
+}
+
+void domain_transaction_dec(struct connection *conn)
+{
+   domain_acc_add(conn, conn->id, ACC_TRANS, -1, true);
+}
+
+unsigned int domain_transaction_get(struct connection *conn)
+{
+   return (domain_is_unprivileged(conn))
+   ? domain_acc_add(conn, conn->id, ACC_TRANS, 0, true)
+   : 0;
+}
+
  static wrl_creditt wrl_config_writecost  = WRL_FACTOR;
  static wrl_creditt wrl_config_rate   = WRL_RATE   * WRL_FACTOR;
  static wrl_creditt wrl_config_dburst = WRL_DBURST * WRL_FACTOR;
diff --git a/tools/xenstore/xenstored_domain.h 
b/tools/xenstore/xenstored_domain.h
index 086133407b..01b6f1861b 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -32,6 +32,7 @@ enum accitem {
ACC_WATCH = ACC_TR_N,
ACC_OUTST,
ACC_MEM,
+   ACC_TRANS,
ACC_N,  /* Number of elements per domain. */
  };
  
@@ -113,6 +114,9 @@ void domain_watch_dec(struct connection *conn);

  int domain_watch(struct connection *conn);
  void domain_outstanding_inc(struct connection *conn);
  void domain_outstanding_dec(struct connection *conn, unsigned int domid);
+void domain_transaction_inc(struct connection *conn);
+void domain_transaction_dec(struct connection *conn);
+unsigned int domain_transaction_get(struct connection *conn);
  int domain_get_quota(const void *ctx, struct connection *conn,
 unsigned int domid);
  
diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c

index 11c8bcec84..b9e9d76a1f 100644
--- a/to

[qemu-mainline test] 180586: tolerable FAIL - PUSHED

[osstest service owner]

flight 180586 qemu-mainline real [real]
flight 180591 qemu-mainline real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/180586/
http://logs.test-lab.xenproject.org/osstest/logs/180591/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl   8 xen-bootfail pass in 180591-retest

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl 15 migrate-support-check fail in 180591 never pass
 test-armhf-armhf-xl 16 saverestore-support-check fail in 180591 never pass
 test-amd64-i386-qemuu-rhel6hvm-amd  7 xen-install fail like 180559
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180559
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180559
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180559
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180559
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180559
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180559
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180559
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180559
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  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-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  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-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  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-armhf-armhf-xl-credit1  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit1  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-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 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-libvirt-raw 14 migrate-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-libvirt 15 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-qcow2 14 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

version targeted for testing:
 qemuu271477b59e723250f17a7e20f139262057921b6a
baseline version:
 qemuu792f77f376adef944f9a03e601f6ad90c2f891b2

Last test of basis   180559  2023-05-06 14:16:57 Z3 days
Testing same since   180586  2023-05-09 06:38:31 Z0 days1 attempts


People who touched revisions under test:
  Juan Quintela 
  Lukas Straub 
  Peter Xu 
  Richard

Re: [PATCH v5 05/21] virtio-scsi: stop using aio_disable_external() during unplug

[Kevin Wolf]

Am 04.05.2023 um 21:53 hat Stefan Hajnoczi geschrieben:
> This patch is part of an effort to remove the aio_disable_external()
> API because it does not fit in a multi-queue block layer world where
> many AioContexts may be submitting requests to the same disk.
> 
> The SCSI emulation code is already in good shape to stop using
> aio_disable_external(). It was only used by commit 9c5aad84da1c
> ("virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi
> disk") to ensure that virtio_scsi_hotunplug() works while the guest
> driver is submitting I/O.
> 
> Ensure virtio_scsi_hotunplug() is safe as follows:
> 
> 1. qdev_simple_device_unplug_cb() -> qdev_unrealize() ->
>device_set_realized() calls qatomic_set(&dev->realized, false) so
>that future scsi_device_get() calls return NULL because they exclude
>SCSIDevices with realized=false.
> 
>That means virtio-scsi will reject new I/O requests to this
>SCSIDevice with VIRTIO_SCSI_S_BAD_TARGET even while
>virtio_scsi_hotunplug() is still executing. We are protected against
>new requests!
> 
> 2. scsi_device_unrealize() already contains a call to

I think you mean scsi_qdev_unrealize(). Can be fixed while applying.

>scsi_device_purge_requests() so that in-flight requests are cancelled
>synchronously. This ensures that no in-flight requests remain once
>qdev_simple_device_unplug_cb() returns.
> 
> Thanks to these two conditions we don't need aio_disable_external()
> anymore.
> 
> Cc: Zhengui Li 
> Reviewed-by: Paolo Bonzini 
> Reviewed-by: Daniil Tatianin 
> Signed-off-by: Stefan Hajnoczi 

Kevin

Re: [PATCH v5 00/21] block: remove aio_disable_external() API

[Kevin Wolf]

Am 04.05.2023 um 21:53 hat Stefan Hajnoczi geschrieben:
> v5:
> - Use atomic accesses for in_flight counter in vhost-user-server.c [Kevin]
> - Stash SCSIDevice id/lun values for VIRTIO_SCSI_T_TRANSPORT_RESET event
>   before unrealizing the SCSIDevice [Kevin]
> - Keep vhost-user-blk export .detach() callback so ctx is set to NULL [Kevin]
> - Narrow BdrvChildClass and BlockDriver drained_{begin/end/poll} callbacks 
> from
>   IO_OR_GS_CODE() to GLOBAL_STATE_CODE() [Kevin]
> - Include Kevin's "block: Fix use after free in blockdev_mark_auto_del()" to
>   fix a latent bug that was exposed by this series
> 
> v4:
> - Remove external_disable_cnt variable [Philippe]
> - Add Patch 1 to fix assertion failure in .drained_end() -> 
> blk_get_aio_context()
> v3:
> - Resend full patch series. v2 was sent in the middle of a git rebase and was
>   missing patches. [Eric]
> - Apply Reviewed-by tags.
> v2:
> - Do not rely on BlockBackend request queuing, implement .drained_begin/end()
>   instead in xen-block, virtio-blk, and virtio-scsi [Paolo]
> - Add qdev_is_realized() API [Philippe]
> - Add patch to avoid AioContext lock around blk_exp_ref/unref() [Paolo]
> - Add patch to call .drained_begin/end() from main loop thread to simplify
>   callback implementations
> 
> The aio_disable_external() API temporarily suspends file descriptor monitoring
> in the event loop. The block layer uses this to prevent new I/O requests being
> submitted from the guest and elsewhere between bdrv_drained_begin() and
> bdrv_drained_end().
> 
> While the block layer still needs to prevent new I/O requests in drained
> sections, the aio_disable_external() API can be replaced with
> .drained_begin/end/poll() callbacks that have been added to BdrvChildClass and
> BlockDevOps.
> 
> This newer .bdrained_begin/end/poll() approach is attractive because it works
> without specifying a specific AioContext. The block layer is moving towards
> multi-queue and that means multiple AioContexts may be processing I/O
> simultaneously.
> 
> The aio_disable_external() was always somewhat hacky. It suspends all file
> descriptors that were registered with is_external=true, even if they have
> nothing to do with the BlockDriverState graph nodes that are being drained.
> It's better to solve a block layer problem in the block layer than to have an
> odd event loop API solution.
> 
> The approach in this patch series is to implement BlockDevOps
> .drained_begin/end() callbacks that temporarily stop file descriptor handlers.
> This ensures that new I/O requests are not submitted in drained sections.

Patches 2-16: Reviewed-by: Kevin Wolf 

Re: [PATCH v5 12/14] tools/xenstore: use generic accounting for remaining quotas

[Julien Grall]

Hi Juergen,

On 08/05/2023 12:47, Juergen Gross wrote:

The maxrequests, node size, number of node permissions, and path length
quota are a little bit special, as they are either active in
transactions only (maxrequests), or they are just per item instead of
count values. Nevertheless being able to know the maximum number of
those quota related values per domain would be beneficial, so add them
to the generic accounting.

The per domain value will never show current numbers other than zero,
but the maximum number seen can be gathered the same way as the number
of nodes during a transaction.

To be able to use the const qualifier for a new function switch
domain_is_unprivileged() to take a const pointer, too.

For printing the quota/max values, adapt the print format string to
the longest quota name (now 17 characters long).

Signed-off-by: Juergen Gross 
---
V5:
- add comment (Julien Grall)
- add missing quota printing (Julien Grall)
---
  tools/xenstore/xenstored_core.c| 15 +
  tools/xenstore/xenstored_core.h|  2 +-
  tools/xenstore/xenstored_domain.c  | 45 +-
  tools/xenstore/xenstored_domain.h  |  6 
  tools/xenstore/xenstored_transaction.c |  4 +--
  tools/xenstore/xenstored_watch.c   |  2 +-
  6 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index c98d30561f..fce73b883e 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -799,8 +799,9 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, 
struct node *node,
+ node->perms.num * sizeof(node->perms.p[0])
+ node->datalen + node->childlen;
  
-	if (!no_quota_check && domain_is_unprivileged(conn) &&

-   data.dsize >= quota_max_entry_size) {
+   /* Call domain_max_chk() in any case in order to record max values. */
+   if (domain_max_chk(conn, ACC_NODESZ, data.dsize, quota_max_entry_size)
+   && !no_quota_check) {
errno = ENOSPC;
return errno;
}
@@ -1170,7 +1171,7 @@ static bool valid_chars(const char *node)
   "0123456789-/_@") == strlen(node));
  }
  
-bool is_valid_nodename(const char *node)

+bool is_valid_nodename(const struct connection *conn, const char *node)
  {
int local_off = 0;
unsigned int domid;
@@ -1190,7 +1191,8 @@ bool is_valid_nodename(const char *node)
if (sscanf(node, "/local/domain/%5u/%n", &domid, &local_off) != 1)
local_off = 0;
  
-	if (strlen(node) > local_off + quota_max_path_len)

+   if (domain_max_chk(conn, ACC_PATHLEN, strlen(node) - local_off,
+  quota_max_path_len))
return false;
  
  	return valid_chars(node);

@@ -1252,7 +1254,7 @@ static struct node *get_node_canonicalized(struct 
connection *conn,
*canonical_name = canonicalize(conn, ctx, name);
if (!*canonical_name)
return NULL;
-   if (!is_valid_nodename(*canonical_name)) {
+   if (!is_valid_nodename(conn, *canonical_name)) {
errno = EINVAL;
return NULL;
}
@@ -1778,8 +1780,7 @@ static int do_set_perms(const void *ctx, struct 
connection *conn,
return EINVAL;
  
  	perms.num--;

-   if (domain_is_unprivileged(conn) &&
-   perms.num > quota_nb_perms_per_node)
+   if (domain_max_chk(conn, ACC_NPERM, perms.num, quota_nb_perms_per_node))
return ENOSPC;
  
  	permstr = in->buffer + strlen(in->buffer) + 1;

diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 3564d85d7d..9339820156 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -258,7 +258,7 @@ void check_store(void);
  void corrupt(struct connection *conn, const char *fmt, ...);
  
  /* Is this a valid node name? */

-bool is_valid_nodename(const char *node);
+bool is_valid_nodename(const struct connection *conn, const char *node);
  
  /* Get name of parent node. */

  char *get_parent(const void *ctx, const char *node);
diff --git a/tools/xenstore/xenstored_domain.c 
b/tools/xenstore/xenstored_domain.c
index 6f3a27765a..b3a719569e 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -431,7 +431,7 @@ int domain_get_quota(const void *ctx, struct connection 
*conn,
return ENOMEM;
  
  #define ent(t, e) \

-   resp = talloc_asprintf_append(resp, "%-16s: %8u (max: %8u\n", #t, \
+   resp = talloc_asprintf_append(resp, "%-17s: %8u (max: %8u\n", #t, \
  d->acc[e].val, d->acc[e].max); \
if (!resp) return ENOMEM
  
@@ -440,6 +440,10 @@ int domain_get_quota(const void *ctx, struct connection *conn,

ent(transactions, ACC_TRANS);
ent(outstanding, ACC_OUTST);
ent(memory, ACC_MEM);
+   ent(transaction-nodes, ACC_TRANSNODES);
+

Re: [PATCH v5 13/14] tools/xenstore: switch get_optval_int() to get_optval_uint()

[Julien Grall]

Hi Juergen,

On 08/05/2023 12:47, Juergen Gross wrote:

Let get_optval_int() return an unsigned value and rename it
accordingly.

Signed-off-by: Juergen Gross 


Reviewed-by: Julien Grall 

Cheers,

--
Julien Grall

Re: [PATCH v5 14/14] tools/xenstore: switch quota management to be table based

[Julien Grall]

Hi Juergen,

On 08/05/2023 12:47, Juergen Gross wrote:

@@ -2714,15 +2710,19 @@ static unsigned int get_optval_uint(const char *arg)
  
  static bool what_matches(const char *arg, const char *what)

  {
-   unsigned int what_len = strlen(what);
+   unsigned int what_len;
+
+   if (!what)
+   return false;
  
+	what_len = strlen(what);


NIT: Keep the newline before the return.

Acked-by: Julien Grall 

--
Julien Grall

Re: [XEN v6 03/12] xen/arm: Introduce a wrapper for dt_device_get_address() to handle paddr_t

[Julien Grall]

Hi,

On 03/05/2023 15:06, Ayan Kumar Halder wrote:


On 03/05/2023 12:25, Julien Grall wrote:

Hi Ayan,

Hi Julien,


On 28/04/2023 18:55, Ayan Kumar Halder wrote:

dt_device_get_address() can accept uint64_t only for address and size.
However, the address/size denotes physical addresses. Thus, they should
be represented by 'paddr_t'.
Consequently, we introduce a wrapper for dt_device_get_address() ie
dt_device_get_paddr() which accepts address/size as paddr_t and inturn
invokes dt_device_get_address() after converting address/size to
uint64_t.

The reason for introducing this is that in future 'paddr_t' may not
always be 64-bit. Thus, we need an explicit wrapper to do the type
conversion and return an error in case of truncation.

With this, callers can now invoke dt_device_get_paddr().

Signed-off-by: Ayan Kumar Halder 
---
Changes from -

v1 - 1. New patch.

v2 - 1. Extracted part of "[XEN v2 05/11] xen/arm: Use paddr_t 
instead of u64 for address/size"

into this patch.

2. dt_device_get_address() callers now invoke dt_device_get_paddr() 
instead.


3. Logged error in case of truncation.

v3 - 1. Modified the truncation checks as "dt_addr != (paddr_t)dt_addr".
2. Some sanity fixes.

v4 - 1. Some sanity fixes.
2. Preserved the declaration of dt_device_get_address() in
xen/include/xen/device_tree.h. The reason being it is currently used by
ns16550.c. This driver requires some more changes as pointed by Jan in
https://lore.kernel.org/xen-devel/6196e90f-752e-e61a-45ce-37e46c22b...@suse.com/
which is to be addressed as a separate series.

v5 - 1. Removed initialization of variables.
2. In dt_device_get_paddr(), added the check
if ( !addr )
 return -EINVAL;

  xen/arch/arm/domain_build.c    | 10 +++---
  xen/arch/arm/gic-v2.c  | 10 +++---
  xen/arch/arm/gic-v3-its.c  |  4 +--
  xen/arch/arm/gic-v3.c  | 10 +++---
  xen/arch/arm/pci/pci-host-common.c |  6 ++--
  xen/arch/arm/platforms/brcm-raspberry-pi.c |  2 +-
  xen/arch/arm/platforms/brcm.c  |  6 ++--
  xen/arch/arm/platforms/exynos5.c   | 32 +-
  xen/arch/arm/platforms/sunxi.c |  2 +-
  xen/arch/arm/platforms/xgene-storm.c   |  2 +-
  xen/common/device_tree.c   | 39 ++
  xen/drivers/char/cadence-uart.c    |  4 +--
  xen/drivers/char/exynos4210-uart.c |  4 +--
  xen/drivers/char/imx-lpuart.c  |  4 +--
  xen/drivers/char/meson-uart.c  |  4 +--
  xen/drivers/char/mvebu-uart.c  |  4 +--
  xen/drivers/char/omap-uart.c   |  4 +--
  xen/drivers/char/pl011.c   |  6 ++--
  xen/drivers/char/scif-uart.c   |  4 +--


What about the call in xen/drivers/char/ns16550.c?


Refer to 
https://lore.kernel.org/xen-devel/6196e90f-752e-e61a-45ce-37e46c22b...@suse.com/ , Jan mentioned that this driver needs some prior cleanup.


So, I decided to take it out and do it after the current series has been 
committed.


See 
https://patchew.org/Xen/20230413173735.48387-1-ayan.kumar.hal...@amd.com/ , Jan agreed to this.


Is this ok with you ?


I am OK with that. Can you mention it in the commit message? This would 
help the reviewer or a future reader to understand why you left out 
ns16550 (this is the only call left).


With the remarks from Michal addressed:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

Re: xen cache colors in ARM

[Stefano Stabellini]

We test Xen Cache Coloring regularly on zcu102. Every Petalinux release
(twice a year) is tested with cache coloring enabled. The last Petalinux
release is 2023.1 and the kernel used is this:
https://github.com/Xilinx/linux-xlnx/tree/xlnx_rebase_v6.1_LTS


On Tue, 9 May 2023, Oleg Nikitenko wrote:
> Hello guys,
> 
> I have a couple of more questions.
> Have you ever run xen with the cache coloring at Zynq UltraScale+ MPSoC 
> zcu102 xczu15eg ?
> When did you run xen with the cache coloring last time ?
> What kernel version did you use for Dom0 when you ran xen with the cache 
> coloring last time ?
> 
> Regards,
> Oleg
> 
> пт, 5 мая 2023 г. в 11:48, Oleg Nikitenko :
>   Hi Michal,
> 
> Thanks.
> 
> Regards,
> Oleg
> 
> пт, 5 мая 2023 г. в 11:34, Michal Orzel :
>   Hi Oleg,
> 
>   Replying, so that you do not need to wait for Stefano.
> 
>   On 05/05/2023 10:28, Oleg Nikitenko wrote:
>   >       
>   >
>   >
>   > Hello Stefano,
>   >
>   > I would like to try a xen cache color property from this repo  
> https://xenbits.xen.org/git-http/xen.git
>   
>   > Could you tell whot branch I should use ?
>   Cache coloring feature is not part of the upstream tree and it is still 
> under review.
>   You can only find it integrated in the Xilinx Xen tree.
> 
>   ~Michal
> 
>   >
>   > Regards,
>   > Oleg
>   >
>   > пт, 28 апр. 2023 г. в 00:51, Stefano Stabellini 
> mailto:sstabell...@kernel.org>>:
>   >
>   >     I am familiar with the zcu102 but I don't know how you could 
> possibly
>   >     generate a SError.
>   >
>   >     I suggest to try to use ImageBuilder [1] to generate the boot
>   >     configuration as a test because that is known to work well for 
> zcu102.
>   >
>   >     [1] https://gitlab.com/xen-project/imagebuilder 
> 
>   >
>   >
>   >     On Thu, 27 Apr 2023, Oleg Nikitenko wrote:
>   >     > Hello Stefano,
>   >     >
>   >     > Thanks for clarification.
>   >     > We nighter use ImageBuilder nor uboot boot script.
>   >     > A model is zcu102 compatible.
>   >     >
>   >     > Regards,
>   >     > O.
>   >     >
>   >     > вт, 25 апр. 2023 г. в 21:21, Stefano Stabellini 
> mailto:sstabell...@kernel.org>>:
>   >     >       This is interesting. Are you using Xilinx hardware by any 
> chance? If so,
>   >     >       which board?
>   >     >
>   >     >       Are you using ImageBuilder to generate your boot.scr boot 
> script? If so,
>   >     >       could you please post your ImageBuilder config file? If 
> not, can you
>   >     >       post the source of your uboot boot script?
>   >     >
>   >     >       SErrors are supposed to be related to a hardware failure 
> of some kind.
>   >     >       You are not supposed to be able to trigger an SError 
> easily by
>   >     >       "mistake". I have not seen SErrors due to wrong cache 
> coloring
>   >     >       configurations on any Xilinx board before.
>   >     >
>   >     >       The differences between Xen with and without cache 
> coloring from a
>   >     >       hardware perspective are:
>   >     >
>   >     >       - With cache coloring, the SMMU is enabled and does 
> address translations
>   >     >         even for dom0. Without cache coloring the SMMU could be 
> disabled, and
>   >     >         if enabled, the SMMU doesn't do any address 
> translations for Dom0. If
>   >     >         there is a hardware failure related to SMMU address 
> translation it
>   >     >         could only trigger with cache coloring. This would be 
> my normal
>   >     >         suggestion for you to explore, but the failure happens 
> too early
>   >     >         before any DMA-capable device is programmed. So I don't 
> think this can
>   >     >         be the issue.
>   >     >
>   >     >       - With cache coloring, the memory allocation is very 
> different so you'll
>   >     >         end up using different DDR regions for Dom0. So if your 
> DDR is
>   >     >         defective, you might only see a failure with cache 
> coloring enabled
>   >     >         because you end up using different regions.
>   >     >
>   >     >
>   >     >       On Tue, 25 Apr 2023, Oleg Nikitenko wrote:
>   >     >       > Hi Stefano,
>   >     >       >
>   >     >       > Thank you.
>   >     >       > If I build xen without colors support there is not this 
> error.
>   >     >       > All the domains are booted well.
>   >     >       > Hense it can not be a hardware issue.
>   >     >       > This panic arrived during unpacking the rootfs.
>   >     >       > Here I attached the boot log xen/Dom0 without color.
>   >     >       > A high

Re: [patch v3 08/36] x86/smpboot: Split up native_cpu_up() into separate phases and document them

[Thomas Gleixner]

On Tue, May 09 2023 at 12:04, Peter Zijlstra wrote:
> On Mon, May 08, 2023 at 09:43:39PM +0200, Thomas Gleixner wrote:
> Not to the detriment of this patch, but this barrier() and it's comment
> seem weird vs smp_callin(). That function ends with an atomic bitop (it
> has to, at the very least it must not be weaker than store-release) but
> also has an explicit wmb() to order setup vs CPU_STARTING.
>
> (arguably that should be a full fence *AND* get a comment)

TBH: I'm grasping for something 'arguable': What's the point of this
wmb() or even a mb()?

Most of the [w]mb()'s in smpboot.c except those in mwait_play_dead()
have a very distinct voodoo programming smell.

Thanks,

tglx

Re: [PATCH v6 00/16] x86/mtrr: fix handling with PAT but without MTRR

[Borislav Petkov]

On Tue, May 02, 2023 at 02:09:15PM +0200, Juergen Gross wrote:
> This series tries to fix the rather special case of PAT being available
> without having MTRRs (either due to CONFIG_MTRR being not set, or
> because the feature has been disabled e.g. by a hypervisor).

More weird stuff. With the series:

[root@vh: ~> cat /proc/mtrr 
cat: /proc/mtrr: Input/output error

before:

[root@vh: ~> cat /proc/mtrr 
reg00: base=0x0 (0MB), size= 2048MB, count=1: write-back
reg01: base=0x08000 ( 2048MB), size= 1024MB, count=1: write-back
reg02: base=0x0c000 ( 3072MB), size=  256MB, count=1: write-back
reg03: base=0x0ff00 ( 4080MB), size=   16MB, count=1: write-protect

I think it wrongly determines that MTRRs are disabled by BIOS:

MTRRs disabled by BIOS
x86/PAT: Configuration [0-7]: WB  WC  UC- UC  WB  WP  UC- WT

which is obviously wrong.

But more debugging later.

-- 
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v5 05/21] virtio-scsi: stop using aio_disable_external() during unplug

[Stefan Hajnoczi]

On Tue, May 09, 2023 at 08:55:14PM +0200, Kevin Wolf wrote:
> Am 04.05.2023 um 21:53 hat Stefan Hajnoczi geschrieben:
> > This patch is part of an effort to remove the aio_disable_external()
> > API because it does not fit in a multi-queue block layer world where
> > many AioContexts may be submitting requests to the same disk.
> > 
> > The SCSI emulation code is already in good shape to stop using
> > aio_disable_external(). It was only used by commit 9c5aad84da1c
> > ("virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi
> > disk") to ensure that virtio_scsi_hotunplug() works while the guest
> > driver is submitting I/O.
> > 
> > Ensure virtio_scsi_hotunplug() is safe as follows:
> > 
> > 1. qdev_simple_device_unplug_cb() -> qdev_unrealize() ->
> >device_set_realized() calls qatomic_set(&dev->realized, false) so
> >that future scsi_device_get() calls return NULL because they exclude
> >SCSIDevices with realized=false.
> > 
> >That means virtio-scsi will reject new I/O requests to this
> >SCSIDevice with VIRTIO_SCSI_S_BAD_TARGET even while
> >virtio_scsi_hotunplug() is still executing. We are protected against
> >new requests!
> > 
> > 2. scsi_device_unrealize() already contains a call to
> 
> I think you mean scsi_qdev_unrealize(). Can be fixed while applying.

Yes, it should be scsi_qdev_unrealize(). I'll review your other comments
and fix this if I need to respin.

Stefan


signature.asc
Description: PGP signature

Re: [PATCH v2] piix: fix regression during unplug in Xen HVM domUs

[Olaf Hering]

Resuming this old thread about an unfixed bug, which was introduced in qemu-4.2:

qemu ends up in piix_ide_reset from pci_unplug_disks.
This was not the case prior 4.2, the removed call to
qemu_register_reset(piix3_reset, d) in
ee358e919e385fdc79d59d0d47b4a81e349cd5c9 did apparently nothing.

In my debugging (with v8.0.0) it turned out the three pci_set_word
causes the domU to hang. In fact, it is just the last one:

   pci_set_byte(pci_conf + 0x20, 0x01);  /* BMIBA: 20-23h */

It changes the value from 0xc121 to 0x1.

The question is: what does this do in practice?

Starting with recent qemu (like 7.2), the domU sometimes proceeds with
these messages:

[1.631161] uhci_hcd :00:01.2: host system error, PCI problems?
[1.634965] uhci_hcd :00:01.2: host controller process error, 
something bad happened!
[1.634965] uhci_hcd :00:01.2: host controller halted, very bad!
[1.634965] uhci_hcd :00:01.2: HC died; cleaning up
Loading basic drivers...[2.398048] Disabling IRQ #23

Is anyone familiar enough with PIIX3 and knows how these devices are
interacting?

00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
00:01.2 USB controller: Intel Corporation 82371SB PIIX3 USB [Natoma/Triton II] 
(rev 01)
00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
00:03.0 VGA compatible controller: Cirrus Logic GD 5446


Thanks,
Olaf

On Thu, Mar 25, Paolo Bonzini wrote:

> On 25/03/21 12:12, Olaf Hering wrote:
> > Am Mon, 22 Mar 2021 18:09:17 -0400
> > schrieb John Snow :
> > 
> > > My understanding is that XEN has some extra disks that it unplugs when
> > > it later figures out it doesn't need them. How exactly this works is
> > > something I've not looked into too closely.
> > 
> > It has no extra disks, why would it?
> > 
> > I assume each virtualization variant has some sort of unplug if it has to 
> > support guests that lack PV/virtio/enlightened/whatever drivers.
> 
> No, it's Xen only and really should be legacy.  Ideally one would just have
> devices supported at all levels from firmware to kernel.
> 
> > > So if these IDE devices have been "unplugged" already, we avoid
> > > resetting them here. What about this reset causes the bug you describe
> > > in the commit message?
> > > 
> > > Does this reset now happen earlier/later as compared to what it did
> > > prior to ee358e91 ?
> > 
> > Prior this commit, piix_ide_reset was only called when the entire
> > emulated machine was reset. Like: never. With this commit,
> > piix_ide_reset will be called from pci_piix3_xen_ide_unplug. For some
> > reason it confuses the emulated USB hardware. Why it does confused
> > it, no idea.
> 
> > I wonder what the purpose of the qdev_reset_all() call really is. It
> > is 10 years old. It might be stale.
> 
> piix_ide_reset is only calling ide_bus_reset, and from there ide_reset and
> bmdma_reset.  All of these functions do just two things: reset internal
> registers and ensure pending I/O is completed or canceled.  The latter is
> indeed unnecessary; drain/flush/detach is already done before the call to
> qdev_reset_all.
> 
> But the fact that it breaks USB is weird.  That's the part that needs to be
> debugged, because changing IDE to unbreak USB needs an explanation even if
> it's the right thing to do.
> 
> If you don't want to debug it, removing the qdev_reset_all call might do the
> job; you'll have to see what the Xen maintainers think of it.  But if you
> don't debug the USB issue now, it will come back later almost surely.
> 
> Paolo


signature.asc
Description: Digital signature

Re: [PATCH v6 00/16] x86/mtrr: fix handling with PAT but without MTRR

[Borislav Petkov]

On Tue, May 09, 2023 at 10:14:37PM +0200, Borislav Petkov wrote:
> On Tue, May 02, 2023 at 02:09:15PM +0200, Juergen Gross wrote:
> > This series tries to fix the rather special case of PAT being available
> > without having MTRRs (either due to CONFIG_MTRR being not set, or
> > because the feature has been disabled e.g. by a hypervisor).
> 
> More weird stuff. With the series:

Yah, that was me.

That ->enabled thing is *two* bits. FFS.

More staring at this tomorrow, on a clear head.

diff --git a/arch/x86/include/uapi/asm/mtrr.h b/arch/x86/include/uapi/asm/mtrr.h
index a28e6bbd8f21..f476a1355182 100644
--- a/arch/x86/include/uapi/asm/mtrr.h
+++ b/arch/x86/include/uapi/asm/mtrr.h
@@ -84,7 +84,7 @@ typedef __u8 mtrr_type;
 struct mtrr_state_type {
struct mtrr_var_range var_ranges[MTRR_MAX_VAR_RANGES];
mtrr_type fixed_ranges[MTRR_NUM_FIXED_RANGES];
-   bool enabled;
+   unsigned char enabled;
bool have_fixed;
mtrr_type def_type;
 };

-- 
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[ovmf test] 180593: all pass - PUSHED

[osstest service owner]

flight 180593 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180593/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf bee67e0c142af6599a85aa7640094816b8a24c4f
baseline version:
 ovmf 5215cd5baf6609e54050c69909273b7f5161c59e

Last test of basis   180581  2023-05-08 19:13:38 Z1 days
Testing same since   180593  2023-05-09 22:10:44 Z0 days1 attempts


People who touched revisions under test:
  Laszlo Ersek 
  Michael Brown 

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
   5215cd5baf..bee67e0c14  bee67e0c142af6599a85aa7640094816b8a24c4f -> 
xen-tested-master

[xen-unstable-smoke test] 180592: tolerable all pass - PUSHED

[osstest service owner]

flight 180592 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180592/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-amd64-amd64-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-armhf-armhf-xl  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass

version targeted for testing:
 xen  ed6b7c0266e512c1207c07911da14e684f47b909
baseline version:
 xen  8b1ac353b4db7c5bb2f82cb6afee9cc641e756a4

Last test of basis   180588  2023-05-09 10:01:52 Z0 days
Testing same since   180592  2023-05-09 21:03:16 Z0 days1 attempts


People who touched revisions under test:
  Michal Orzel 

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
   8b1ac353b4..ed6b7c0266  ed6b7c0266e512c1207c07911da14e684f47b909 -> smoke

[xen-unstable test] 180589: tolerable FAIL - PUSHED

[osstest service owner]

flight 180589 xen-unstable real [real]
flight 180594 xen-unstable real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/180589/
http://logs.test-lab.xenproject.org/osstest/logs/180594/

Failures :-/ but no regressions.

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail pass 
in 180594-retest

Tests which did not succeed, but are not blocking:
 test-amd64-i386-pair 11 xen-install/dst_host fail  like 180584
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 180584
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 180584
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180584
 test-armhf-armhf-libvirt-raw 15 saverestore-support-checkfail  like 180584
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 180584
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 180584
 test-armhf-armhf-libvirt-qcow2 15 saverestore-support-check   fail like 180584
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180584
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 180584
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180584
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 180584
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180584
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-i386-xl-pvshim14 guest-start  fail   never pass
 test-amd64-i386-libvirt  15 migrate-support-checkfail   never pass
 test-amd64-i386-libvirt-xsm  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  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-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  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-armhf-armhf-xl-multivcpu 15 migrate-support-checkfail  never pass
 test-armhf-armhf-xl-multivcpu 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-amd64-i386-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail 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-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   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-credit2  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-credit2  16 saverestore-support-checkfail   never pass
 test-amd64-i386-libvirt-raw  14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt-raw 14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-armhf-armhf-libvirt-qcow2 14 migrate-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-xl-vhd  14 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-vhd  15 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass

version targeted for testing:
 xen  8b1ac353b4db7c5bb2f82cb6afee9cc641e756a4
baseline version:
 xen  a

[ovmf test] 180595: all pass - PUSHED

[osstest service owner]

flight 180595 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180595/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf e97b9b4e5a4bd53fd5f18c44390b266a2a89881a
baseline version:
 ovmf bee67e0c142af6599a85aa7640094816b8a24c4f

Last test of basis   180593  2023-05-09 22:10:44 Z0 days
Testing same since   180595  2023-05-10 00:40:40 Z0 days1 attempts


People who touched revisions under test:
  Gua Guo 

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
   bee67e0c14..e97b9b4e5a  e97b9b4e5a4bd53fd5f18c44390b266a2a89881a -> 
xen-tested-master

[linux-linus test] 180590: regressions - FAIL

[osstest service owner]

flight 180590 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180590/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 test-armhf-armhf-xl-credit1   8 xen-boot fail REGR. vs. 180278
 build-arm64-pvops 6 kernel-build   fail in 180587 REGR. vs. 180278

Tests which are failing intermittently (not blocking):
 test-amd64-amd64-libvirt-qcow2 19 guest-start/debian.repeat fail in 180587 
pass in 180590
 test-amd64-amd64-xl-qemuu-debianhvm-i386-xsm 12 debian-hvm-install fail pass 
in 180582
 test-amd64-amd64-xl-vhd  21 guest-start/debian.repeat  fail pass in 180587
 test-amd64-amd64-xl-qemut-win7-amd64 18 guest-localmigrate/x10 fail pass in 
180587

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked in 180587 n/a
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked in 180587 n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked in 180587 n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked in 180587 n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked in 180587 n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked in 180587 n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked in 180587 n/a
 test-arm64-arm64-xl-vhd   1 build-check(1)   blocked in 180587 n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked in 180587 n/a
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stop  fail in 180587 like 180278
 test-armhf-armhf-libvirt  8 xen-boot fail  like 180278
 test-armhf-armhf-libvirt-raw  8 xen-boot fail  like 180278
 test-armhf-armhf-xl-rtds  8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 180278
 test-armhf-armhf-xl   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 180278
 test-armhf-armhf-examine  8 reboot   fail  like 180278
 test-armhf-armhf-xl-multivcpu  8 xen-boot fail like 180278
 test-armhf-armhf-libvirt-qcow2  8 xen-bootfail like 180278
 test-armhf-armhf-xl-credit2   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-vhd   8 xen-boot fail  like 180278
 test-armhf-armhf-xl-arndale   8 xen-boot fail  like 180278
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 180278
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-xsm  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-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  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail 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-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-raw 15 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  14 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-vhd  15 saverestore-support-checkfail   never pass

version targeted for testing:
 linuxba0ad6ed89fd5dada3b7b65ef2b08e95d449d4ab
baseline version:
 linux6c538e1adbfc696ac4747fb10d63e704344f763d

Last test of basis   180278  2023-04-16 19:41:46 Z   23 days
Failing since180281  2023-04-17 06:24:36 Z   22 days   42 attempts
Testing same since   180582  2023-05-08 21:11:46 Z1 days3 attempts


2359 people touched revisions under test,
not listing them all

jobs:
 build-amd64-xsm 

[XEN PATCH 0/2] Use streaming decompression for ZSTD kernels

[Rafaël Kooi]

I've attempted to get Xen to boot Arch Linux as a unified EFI binary.
Using https://xenbits.xen.org/docs/unstable/misc/efi.html as my source
of information, I've been able to build a unified binary. When trying to
boot the kernel Xen complains that the stream is corrupt
("ZSTD-compressed data is corrupt"). I've been able to reproduce the
issue locally in user-mode, and confirmed that the issue is also present
in the latest ZSTD version.

Using streaming decompression the kernel gets unpacked properly and the
output is the same as if doing `cat kernel.zst | unzstd > bzImage`.

A problem I ran into was that adding book keeping to decompress.c would
result in either a .data section being added or a .bss.* section. The
linker would complain about this. And since I am not familiar with this
code, and why it is this way, I opted to add a user-pointer to the
internal decompression API.

Rafaël Kooi (2):
  xen/decompress: Add a user pointer for book keeping in the callbacks
  x86/Dom0: Use streaming decompression for ZSTD compressed kernels

 xen/common/bunzip2.c | 23 --
 xen/common/decompress.c  | 37 ++--
 xen/common/unlz4.c   | 15 ---
 xen/common/unlzma.c  | 30 +
 xen/common/unlzo.c   | 13 +++--
 xen/common/unxz.c| 11 ++-
 xen/common/unzstd.c  | 13 +++--
 xen/include/xen/decompress.h | 10 +++---
 8 files changed, 97 insertions(+), 55 deletions(-)

--
2.40.0

[XEN PATCH 1/2] xen/decompress: Add a user pointer for book keeping in the callbacks

[Rafaël Kooi]

Before this change the callbacks either needed to be completely
stateless or use global state for book keeping. In the case where book
keeping is needed we can't use global state in decompress.c because the
linker disallows the existence of the .data and .bss segments. This
change allows for localized book keeping with the user pointer.

Signed-off-by: Rafaël Kooi 
---
 xen/common/bunzip2.c | 23 +--
 xen/common/unlz4.c   | 15 ---
 xen/common/unlzma.c  | 30 ++
 xen/common/unlzo.c   | 13 +++--
 xen/common/unxz.c| 11 ++-
 xen/common/unzstd.c  | 13 +++--
 xen/include/xen/decompress.h | 10 +++---
 7 files changed, 66 insertions(+), 49 deletions(-)

diff --git a/xen/common/bunzip2.c b/xen/common/bunzip2.c
index 4466426941..a854046524 100644
--- a/xen/common/bunzip2.c
+++ b/xen/common/bunzip2.c
@@ -1,4 +1,4 @@
-/* vi: set sw = 4 ts = 4: */
+/* vi: set sw=4 ts=4: */
 /* Small bzip2 deflate implementation, by Rob Landley (r...@landley.net).
 
Based on bzip2 decompression code by Julian R Seward (jsew...@acm.org),
@@ -86,7 +86,7 @@ struct bunzip_data {
/* State for interrupting output loop */
int writeCopies, writePos, writeRunCountdown, writeCount, writeCurrent;
/* I/O tracking data (file handles, buffers, positions, etc.) */
-   int (*fill)(void*, unsigned int);
+   int (*fill)(void*, unsigned int, void*);
int inbufCount, inbufPos /*, outbufPos*/;
unsigned char *inbuf /*,*outbuf*/;
unsigned int inbufBitCount, inbufBits;
@@ -99,6 +99,7 @@ struct bunzip_data {
unsigned char selectors[32768]; /* nSelectors = 15 bits */
struct group_data groups[MAX_GROUPS];   /* Huffman coding tables */
int io_error;   /* non-zero if we have IO error */
+   void *userptr;  /* user pointer to pass to fill */
 };
 
 
@@ -117,7 +118,7 @@ static unsigned int __init get_bits(struct bunzip_data *bd, 
char bits_wanted)
if (bd->inbufPos == bd->inbufCount) {
if (bd->io_error)
return 0;
-   bd->inbufCount = bd->fill(bd->inbuf, BZIP2_IOBUF_SIZE);
+   bd->inbufCount = bd->fill(bd->inbuf, BZIP2_IOBUF_SIZE, 
bd->userptr);
if (bd->inbufCount <= 0) {
bd->io_error = RETVAL_UNEXPECTED_INPUT_EOF;
return 0;
@@ -612,7 +613,7 @@ decode_next_byte:
goto decode_next_byte;
 }
 
-static int __init cf_check nofill(void *buf, unsigned int len)
+static int __init cf_check nofill(void *buf, unsigned int len, void *userptr)
 {
return -1;
 }
@@ -621,7 +622,7 @@ static int __init cf_check nofill(void *buf, unsigned int 
len)
a complete bunzip file (len bytes long).  If in_fd!=-1, inbuf and len are
ignored, and data is read from file handle into temporary buffer. */
 static int __init start_bunzip(struct bunzip_data **bdp, void *inbuf, int len,
-  int (*fill)(void*, unsigned int))
+  int (*fill)(void*, unsigned int, void*), void 
*userptr)
 {
struct bunzip_data *bd;
unsigned int i, j, c;
@@ -644,6 +645,7 @@ static int __init start_bunzip(struct bunzip_data **bdp, 
void *inbuf, int len,
bd->fill = fill;
else
bd->fill = nofill;
+   bd->userptr = userptr;
 
/* Init the CRC32 table (big endian) */
for (i = 0; i < 256; i++) {
@@ -671,10 +673,11 @@ static int __init start_bunzip(struct bunzip_data **bdp, 
void *inbuf, int len,
 /* Example usage: decompress src_fd to dst_fd.  (Stops at end of bzip2 data,
not end of file.) */
 int __init bunzip2(unsigned char *buf, unsigned int len,
-  int(*fill)(void*, unsigned int),
-  int(*flush)(void*, unsigned int),
+  int(*fill)(void*, unsigned int, void*),
+  int(*flush)(void*, unsigned int, void*),
   unsigned char *outbuf, unsigned int *pos,
-  void(*error)(const char *x))
+  void(*error)(const char *x),
+  void *userptr)
 {
struct bunzip_data *bd;
int i = -1;
@@ -696,7 +699,7 @@ int __init bunzip2(unsigned char *buf, unsigned int len,
i = RETVAL_OUT_OF_MEMORY;
goto exit_0;
}
-   i = start_bunzip(&bd, inbuf, len, fill);
+   i = start_bunzip(&bd, inbuf, len, fill, userptr);
if (!i) {
for (;;) {
i = read_bunzip(bd, outbuf, BZIP2_IOBUF_SIZE);
@@ -705,7 +708,7 @@ int __init bunzip2(unsigned char *buf, unsigned int len,
if (!flush)
outbuf += i;
else
-   if (i != flush(outbuf

[XEN PATCH 2/2] x86/Dom0: Use streaming decompression for ZSTD compressed kernels

[Rafaël Kooi]

On Arch Linux kernel decompression will fail when Xen has been unified
with the kernel and initramfs as a single binary. This change works for
both streaming and non-streaming ZSTD content.

Signed-off-by: Rafaël Kooi 
---
 xen/common/decompress.c | 37 +++--
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/xen/common/decompress.c b/xen/common/decompress.c
index 989336983f..cde754ffb1 100644
--- a/xen/common/decompress.c
+++ b/xen/common/decompress.c
@@ -3,11 +3,26 @@
 #include 
 #include 
 
+typedef struct _ZSTD_state
+{
+void *write_buf;
+unsigned int write_pos;
+} ZSTD_state;
+
 static void __init cf_check error(const char *msg)
 {
 printk("%s\n", msg);
 }
 
+static int __init cf_check ZSTD_flush(void *buf, unsigned int pos,
+  void *userptr)
+{
+ZSTD_state *state = (ZSTD_state*)userptr;
+memcpy(state->write_buf + state->write_pos, buf, pos);
+state->write_pos += pos;
+return pos;
+}
+
 int __init decompress(void *inbuf, unsigned int len, void *outbuf)
 {
 #if 0 /* Not needed here yet. */
@@ -17,22 +32,32 @@ int __init decompress(void *inbuf, unsigned int len, void 
*outbuf)
 #endif
 
 if ( len >= 3 && !memcmp(inbuf, "\x42\x5a\x68", 3) )
-return bunzip2(inbuf, len, NULL, NULL, outbuf, NULL, error);
+return bunzip2(inbuf, len, NULL, NULL, outbuf, NULL, error, NULL);
 
 if ( len >= 6 && !memcmp(inbuf, "\3757zXZ", 6) )
-return unxz(inbuf, len, NULL, NULL, outbuf, NULL, error);
+return unxz(inbuf, len, NULL, NULL, outbuf, NULL, error, NULL);
 
 if ( len >= 2 && !memcmp(inbuf, "\135\000", 2) )
-return unlzma(inbuf, len, NULL, NULL, outbuf, NULL, error);
+return unlzma(inbuf, len, NULL, NULL, outbuf, NULL, error, NULL);
 
 if ( len >= 5 && !memcmp(inbuf, "\x89LZO", 5) )
-return unlzo(inbuf, len, NULL, NULL, outbuf, NULL, error);
+return unlzo(inbuf, len, NULL, NULL, outbuf, NULL, error, NULL);
 
 if ( len >= 2 && !memcmp(inbuf, "\x02\x21", 2) )
-   return unlz4(inbuf, len, NULL, NULL, outbuf, NULL, error);
+return unlz4(inbuf, len, NULL, NULL, outbuf, NULL, error, NULL);
 
 if ( len >= 4 && !memcmp(inbuf, "\x28\xb5\x2f\xfd", 4) )
-   return unzstd(inbuf, len, NULL, NULL, outbuf, NULL, error);
+{
+// NOTE (Rafaël): On Arch Linux the kernel is compressed in a way
+// that requires streaming ZSTD decompression. Otherwise decompression
+// will fail when using a unified EFI binary. Somehow decompression
+// works when not using a unified EFI binary, I suspect this is the
+// kernel self decompressing. Or there is a code path that I am not
+// aware of that takes care of the use case properly.
+
+ZSTD_state state = (ZSTD_state){ outbuf, 0 };
+return unzstd(inbuf, len, NULL, ZSTD_flush, NULL, NULL, error, &state);
+}
 
 return 1;
 }
-- 
2.40.0

[ovmf test] 180597: all pass - PUSHED

[osstest service owner]

flight 180597 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/180597/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 9165a7e95ec6263c04c8babfdbe8bee133959300
baseline version:
 ovmf e97b9b4e5a4bd53fd5f18c44390b266a2a89881a

Last test of basis   180595  2023-05-10 00:40:40 Z0 days
Testing same since   180597  2023-05-10 03:12:17 Z0 days1 attempts


People who touched revisions under test:
  Rebecca Cran 

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
   e97b9b4e5a..9165a7e95e  9165a7e95ec6263c04c8babfdbe8bee133959300 -> 
xen-tested-master