Re: [PATCH 11/15] tools/helpers: Add get_overlay

2024-04-24 Thread Jan Beulich
On 24.04.2024 05:34, Henry Wang wrote:
> From: Vikram Garhwal 
> 
> This user level application copies the overlay dtbo shared by dom0 while doing
> overlay node assignment operation. It uses xenstore to communicate with dom0.
> More information on the protocol is writtien in docs/misc/overlay.txt file.
> 
> Signed-off-by: Vikram Garhwal 
> Signed-off-by: Stefano Stabellini 
> Signed-off-by: Henry Wang 
> ---
>  tools/helpers/Makefile  |   8 +
>  tools/helpers/get_overlay.c | 393 
>  2 files changed, 401 insertions(+)
>  create mode 100644 tools/helpers/get_overlay.c

As mentioned before on various occasions - new files preferably use dashes as
separators in preference to underscores. You not doing so is particularly
puzzling seeing ...

> --- a/tools/helpers/Makefile
> +++ b/tools/helpers/Makefile
> @@ -12,6 +12,7 @@ TARGETS += init-xenstore-domain
>  endif
>  ifeq ($(CONFIG_ARM),y)
>  TARGETS += init-dom0less
> +TARGETS += get_overlay

... patch context here (demonstrating a whopping 3 dashes used in similar
cases).

Jan



Re: [PATCH 14/15] add a domU script to fetch overlays and applying them to linux

2024-04-24 Thread Jan Beulich
On 24.04.2024 05:34, Henry Wang wrote:
> From: Vikram Garhwal 
> 
> Introduce a shell script that runs in the background and calls
> get_overlay to retrive overlays and add them (or remove them) to Linux
> device tree (running as a domU).
> 
> Signed-off-by: Vikram Garhwal 
> Signed-off-by: Stefano Stabellini 
> Signed-off-by: Henry Wang 
> ---
>  tools/helpers/Makefile   |  2 +-
>  tools/helpers/get_overlay.sh | 81 
>  2 files changed, 82 insertions(+), 1 deletion(-)
>  create mode 100755 tools/helpers/get_overlay.sh

Besides the same naming issue as in the earlier patch, the script also
looks very Linux-ish. Yet ...

> --- a/tools/helpers/Makefile
> +++ b/tools/helpers/Makefile
> @@ -58,7 +58,6 @@ init-dom0less: $(INIT_DOM0LESS_OBJS)
>  get_overlay: $(SHARE_OVERLAY_OBJS)
>   $(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) 
> $(LDLIBS_libxenctrl) $(LDLIBS_libxengnttab) $(APPEND_LDFLAGS)
>  
> -
>  .PHONY: install
>  install: all
>   $(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
> @@ -67,6 +66,7 @@ install: all
>  .PHONY: uninstall
>  uninstall:
>   for i in $(TARGETS); do rm -f $(DESTDIR)$(LIBEXEC_BIN)/$$i; done
> + $(RM) $(DESTDIR)$(LIBEXEC_BIN)/get_overlay.sh
>  
>  .PHONY: clean
>  clean:

... you touching only the uninstall target, it's not even clear to me
how (and under what conditions) the script is going to make it into
$(DESTDIR)$(LIBEXEC_BIN)/. Did you mean to add to $(TARGETS), perhaps,
alongside the earlier added get-overlay binary?

Jan



Re: [PATCH 00/15] Remaining patches for dynamic node programming using overlay dtbo

2024-04-24 Thread Jan Beulich
On 24.04.2024 05:34, Henry Wang wrote:
> Hi all,
> 
> This is the remaining series for the full functional "dynamic node
> programming using overlay dtbo" feature. The first part [1] has
> already been merged.
> 
> Quoting from the original series, the first part has already made
> Xen aware of new device tree node which means updating the dt_host
> with overlay node information, and in this series, the goal is to
> map IRQ and IOMMU during runtime, where we will do the actual IOMMU
> and IRQ mapping to a running domain and will call unmap_mmio_regions()
> to remove the mapping.
> 
> Also, documentation of the "dynamic node programming using overlay dtbo"
> feature is added.
> 
> Patch 1 is a fix of [1] which is noticed during my local test, details
> please see the commit message.
> 
> Gitlab CI for this series can be found in [2].
> 
> [1] 
> https://lore.kernel.org/xen-devel/20230906011631.30310-1-vikram.garh...@amd.com/
> [2] https://gitlab.com/xen-project/people/henryw/xen/-/pipelines/1265297506
> 
> Henry Wang (1):
>   xen/commom/dt-overlay: Fix missing lock when remove the device
> 
> Vikram Garhwal (14):
>   xen/arm/gic: Enable interrupt assignment to running VM
>   xen/arm: Always enable IOMMU
>   tools/libs/light: Always enable IOMMU
>   tools/libs/light: Increase nr_spi to 160
>   rangeset: Move struct range and struct rangeset to headerfile
>   xen/overlay: Enable device tree overlay assignment to running domains
>   tools: Add domain_id and expert mode for overlay operations
>   tools/libs/light: Modify dtbo to domU linux dtbo format
>   tools/xl: Share overlay with domU
>   tools/helpers: Add get_overlay
>   get_overlay: remove domU overlay
>   xl/overlay: add remove operation to xenstore
>   add a domU script to fetch overlays and applying them to linux
>   docs: add device tree overlay documentation

For all the replies I sent, Vikram's addresses - Xilinx or AMD - bounced.
What's the value of Cc-ing dead addresses?

Jan



Re: [PATCH 01/15] xen/commom/dt-overlay: Fix missing lock when remove the device

2024-04-24 Thread Henry Wang

Hi Jan,

On 4/24/2024 1:58 PM, Jan Beulich wrote:

On 24.04.2024 05:34, Henry Wang wrote:

--- a/xen/common/dt-overlay.c
+++ b/xen/common/dt-overlay.c
@@ -381,9 +381,14 @@ static int remove_node_resources(struct dt_device_node 
*device_node)
  {
  if ( dt_device_is_protected(device_node) )
  {
+write_lock(_host_lock);
  rc = iommu_remove_dt_device(device_node);

Any particular reason you add two call sites to the unlock function,
instead of putting it here?


Oh...you are correct. It is indeed better to put the unlock here. If 
this is the only comment for this patch, can I respin this only patch as 
a v1.1 or would one of the committers be ok to fix on commit? Sorry for 
the trouble and thanks for the suggestion.


Kind regards,
Henry


Jan


  if ( rc < 0 )
+{
+write_unlock(_host_lock);
  return rc;
+}
+write_unlock(_host_lock);
  }
  }
  





Re: [PATCH 1/4] x86/P2M: write_p2m_entry() is HVM-only anyway

2024-04-24 Thread Jan Beulich
On 23.04.2024 21:29, Andrew Cooper wrote:
> On 23/04/2024 3:31 pm, Jan Beulich wrote:
>> The latest as of e2b2ff677958 ("x86/P2M: split out init/teardown
>> functions") the function is obviously unreachable for PV guests.
> 
> This doesn't parse.  Do you mean "Since e2b2ff677958 ..." ?

Well. I'm sure you at least get the point of "the lastest as of", even
if that may not be proper English. I specifically didn't use "since"
because the fact mentioned may have been true before (more or less
obviously). I'd therefore appreciate a wording suggestion which gets
this across.

>>  Hence
>> the paging_mode_enabled(d) check is pointless.
>>
>> Further host mode of a vCPU is always set, by virtue of
>> paging_vcpu_init() being part of vCPU creation. Hence the
>> paging_get_hostmode() check is pointless.
>>
>> With that the v local variable is unnecessary too. Drop the "if()"
>> conditional and its corresponding "else".
>>
>> Signed-off-by: Jan Beulich 
>> ---
>> I have to confess that this if() has been puzzling me before.
> 
> Puzzling yes, but it can't blindly be dropped.

And I'm not doing so "blindly". Every part of what is being dropped is
being explained.

> This is the "did the toolstack initiate this update" check.  i.e. I
> think it's "bypass the normal side effects of making this update".

Why would we want to bypass side effects?

> I suspect it exists because of improper abstraction between the guest
> physmap and the shadow pagetables as-were - which were/are tighly
> coupled to vCPUs even for aspects where they shouldn't have been.
> 
> For better or worse, the toolstack can add_to_physmap() before it
> creates vCPUs, and it will take this path you're trying to delete. 
> There may be other cases too; I could see foreign mapping ending up
> ticking this too.
> 
> Whether we ought to permit a toolstack to do this is a different
> question, but seeing as we explicitly intend (eventually for AMX) have a
> set_policy call between domain_create() and vcpu_create(), I don't think
> we can reasably restrict other hypercalls too in this period.

None of which explains what's wrong with the provided justification.
The P2M isn't per-vCPU. Presence of vCPU-s therefore shouldn't matter
for alterations of the P2M.

Jan



Re: [PATCH 6/6] x86/spec-ctrl: Introduce and use DO_COND_BHB_SEQ

2024-04-24 Thread Jan Beulich
On 22.04.2024 20:14, Andrew Cooper wrote:
> --- a/xen/arch/x86/hvm/vmx/entry.S
> +++ b/xen/arch/x86/hvm/vmx/entry.S
> @@ -62,12 +62,12 @@ ENTRY(vmx_asm_vmexit_handler)
>   * Clear the BHB to mitigate BHI.  Used on eIBRS parts, and uses RETs
>   * itself so must be after we've perfomed all the RET-safety we can.
>   */
> -testb $SCF_entry_bhb, CPUINFO_scf(%rsp)
> -jz .L_skip_bhb
> -ALTERNATIVE_2 "",\
> -"call clear_bhb_loops", X86_SPEC_BHB_LOOPS,  \
> -"call clear_bhb_tsx", X86_SPEC_BHB_TSX
> -.L_skip_bhb:
> +.macro VMX_BHB_SEQ fn:req
> +DO_COND_BHB_SEQ \fn scf=CPUINFO_scf(%rsp)

While the assembler permits this, can we please avoid mixing positional and
named macro arguments? Instead, ...

> +.endm
> +ALTERNATIVE_2 "", \
> +"VMX_BHB_SEQ fn=clear_bhb_loops", X86_SPEC_BHB_LOOPS, \
> +"VMX_BHB_SEQ fn=clear_bhb_tsx",   X86_SPEC_BHB_TSX

... as done further down, named arguments don't need using here. With the
former addressed and preferably with the latter in consistent shape with
what's below (which way round I don't care as much, albeit with a slight
preference for "shorter"):
Reviewed-by: Jan Beulich 

Jan

> --- a/xen/arch/x86/include/asm/spec_ctrl_asm.h
> +++ b/xen/arch/x86/include/asm/spec_ctrl_asm.h
> @@ -92,6 +92,21 @@
>  .L\@_skip:
>  .endm
>  
> +.macro DO_COND_BHB_SEQ fn:req, scf=%bl
> +/*
> + * Requires SCF (defaults to %rbx), fn=clear_bhb_{loops,tsx}
> + * Clobbers %rax, %rcx
> + *
> + * Conditionally use a BHB clearing software sequence.
> + */
> +testb  $SCF_entry_bhb, \scf
> +jz .L\@_skip_bhb
> +
> +call   \fn
> +
> +.L\@_skip_bhb:
> +.endm
> +
>  .macro DO_OVERWRITE_RSB tmp=rax, xu
>  /*
>   * Requires nothing
> @@ -277,12 +292,9 @@
>   * Clear the BHB to mitigate BHI.  Used on eIBRS parts, and uses RETs
>   * itself so must be after we've perfomed all the RET-safety we can.
>   */
> -testb $SCF_entry_bhb, %bl
> -jz .L\@_skip_bhb
> -ALTERNATIVE_2 "",\
> -"call clear_bhb_loops", X86_SPEC_BHB_LOOPS,  \
> -"call clear_bhb_tsx", X86_SPEC_BHB_TSX
> -.L\@_skip_bhb:
> +ALTERNATIVE_2 "",  \
> +"DO_COND_BHB_SEQ clear_bhb_loops", X86_SPEC_BHB_LOOPS, \
> +"DO_COND_BHB_SEQ clear_bhb_tsx",   X86_SPEC_BHB_TSX
>  
>  ALTERNATIVE "lfence", "", X86_SPEC_NO_LFENCE_ENTRY_PV
>  .endm
> @@ -322,12 +334,9 @@
>  ALTERNATIVE "", __stringify(DO_SPEC_CTRL_ENTRY maybexen=1), \
>  X86_FEATURE_SC_MSR_PV
>  
> -testb $SCF_entry_bhb, %bl
> -jz .L\@_skip_bhb
> -ALTERNATIVE_2 "",\
> -"call clear_bhb_loops", X86_SPEC_BHB_LOOPS,  \
> -"call clear_bhb_tsx", X86_SPEC_BHB_TSX
> -.L\@_skip_bhb:
> +ALTERNATIVE_2 "",  \
> +"DO_COND_BHB_SEQ clear_bhb_loops", X86_SPEC_BHB_LOOPS, \
> +"DO_COND_BHB_SEQ clear_bhb_tsx",   X86_SPEC_BHB_TSX
>  
>  ALTERNATIVE "lfence", "", X86_SPEC_NO_LFENCE_ENTRY_INTR
>  .endm
> @@ -433,13 +442,9 @@
>   * Clear the BHB to mitigate BHI.  Used on eIBRS parts, and uses RETs
>   * itself so must be after we've perfomed all the RET-safety we can.
>   */
> -testb $SCF_entry_bhb, %bl
> -jz .L\@_skip_bhb
> -
> -ALTERNATIVE_2 "",\
> -"call clear_bhb_loops", X86_SPEC_BHB_LOOPS,  \
> -"call clear_bhb_tsx", X86_SPEC_BHB_TSX
> -.L\@_skip_bhb:
> +ALTERNATIVE_2 "",  \
> +"DO_COND_BHB_SEQ clear_bhb_loops", X86_SPEC_BHB_LOOPS, \
> +"DO_COND_BHB_SEQ clear_bhb_tsx",   X86_SPEC_BHB_TSX
>  
>  lfence
>  .endm




Re: [PATCH 07/15] xen/overlay: Enable device tree overlay assignment to running domains

2024-04-24 Thread Jan Beulich
On 24.04.2024 05:34, Henry Wang wrote:
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -1197,7 +1197,9 @@ struct xen_sysctl_dt_overlay {
>  #define XEN_SYSCTL_DT_OVERLAY_ADD   1
>  #define XEN_SYSCTL_DT_OVERLAY_REMOVE2
>  uint8_t overlay_op; /* IN: Add or remove. */
> -uint8_t pad[3]; /* IN: Must be zero. */
> +bool domain_mapping;/* IN: True of False. */
> +uint8_t pad[2]; /* IN: Must be zero. */
> +uint32_t domain_id;
>  };

If you merely re-purposed padding fields, all would be fine without
bumping the interface version. Yet you don't, albeit for an unclear
reason: Why uint32_t rather than domid_t? And on top of that - why a
separate boolean when you could use e.g. DOMID_INVALID to indicate
"no domain mapping"?

That said - anything taking a domain ID is certainly suspicious in a
sysctl. Judging from the description you really mean this to be a
domctl. Anything else will require extra justification.

Jan



Re: [PATCH 06/15] rangeset: Move struct range and struct rangeset to headerfile

2024-04-24 Thread Jan Beulich
On 24.04.2024 05:34, Henry Wang wrote:
> From: Vikram Garhwal 
> 
> Move struct range, rangeset and removed static from first_range and 
> next_range().

NAK, for going against what we do elsewhere (limiting exposure of internals).
At least as long as the justification isn't any better than ...

> IRQs and IOMEMs for nodes are stored as rangeset in the dynamic node addition
> part. While removing the nodes we need to access every IRQ and IOMEM ranges to
> unmap IRQ and IOMEM from the domain.

... this. You're aware of rangeset_report_ranges() and 
rangeset_consume_ranges(),
aren't you? If neither is suitable for your purpose, can you explain what you
need in addition?

Jan



Re: [XEN PATCH v2 5/5] xen/arm: ffa: support notification

2024-04-24 Thread Julien Grall

Hi Bertrand & Jens,

On 24/04/2024 07:53, Bertrand Marquis wrote:

Hi Jens,


On 23 Apr 2024, at 17:26, Jens Wiklander  wrote:

Hi Julien,

On Mon, Apr 22, 2024 at 1:40 PM Julien Grall  wrote:


Hi Jens,

This is not a full review of the code. I will let Bertrand doing it.

On 22/04/2024 08:37, Jens Wiklander wrote:

+void ffa_notif_init(void)
+{
+const struct arm_smccc_1_2_regs arg = {
+.a0 = FFA_FEATURES,
+.a1 = FFA_FEATURE_SCHEDULE_RECV_INTR,
+};
+struct arm_smccc_1_2_regs resp;
+unsigned int irq;
+int ret;
+
+arm_smccc_1_2_smc(, );
+if ( resp.a0 != FFA_SUCCESS_32 )
+return;
+
+irq = resp.a2;
+if ( irq >= NR_GIC_SGI )
+irq_set_type(irq, IRQ_TYPE_EDGE_RISING);
+ret = request_irq(irq, 0, notif_irq_handler, "FF-A notif", NULL);


If I am not mistaken, ffa_notif_init() is only called once on the boot
CPU. However, request_irq() needs to be called on every CPU so the
callback is registered every where and the interrupt enabled.

I know the name of the function is rather confusing. So can you confirm
this is what you expected?


Good catch, no this wasn't what I expected. I'll need to change this.



[...]


diff --git a/xen/arch/arm/tee/ffa_private.h b/xen/arch/arm/tee/ffa_private.h
index 98236cbf14a3..ef8ffd4526bd 100644
--- a/xen/arch/arm/tee/ffa_private.h
+++ b/xen/arch/arm/tee/ffa_private.h
@@ -25,6 +25,7 @@
  #define FFA_RET_DENIED  -6
  #define FFA_RET_RETRY   -7
  #define FFA_RET_ABORTED -8
+#define FFA_RET_NO_DATA -9

  /* FFA_VERSION helpers */
  #define FFA_VERSION_MAJOR_SHIFT 16U
@@ -97,6 +98,18 @@
   */
  #define FFA_MAX_SHM_COUNT   32

+/*
+ * TODO How to manage the available SGIs? SGI 8-15 seem to be entirely
+ * unused, but that may change.


Are the value below intended for the guests? If so, can they be moved in
public/arch-arm.h along with the others guest interrupts?


Yes, I'll move it.




+ *
+ * SGI is the preferred delivery mechanism. SGIs 8-15 are normally not used
+ * by a guest as they in a non-virtualized system typically are assigned to
+ * the secure world. Here we're free to use SGI 8-15 since they are virtual
+ * and have nothing to do with the secure world.


Do you have a pointer to the specification?


There's one at the top of arch/arm/tee/ffa.c,
https://developer.arm.com/documentation/den0077/e
Do you want the link close to the defines when I've moved them to
public/arch-arm.h?
Or is it perhaps better to give a link to "Arm Base System
Architecture v1.0C", https://developer.arm.com/documentation/den0094/
instead?


I would say we need the link to Arm Base System Architecture in arch-arm.


+1

Cheers,

--
Julien Grall



Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2024-04-24 Thread Philippe Mathieu-Daudé

Hi,

On 1/6/23 05:18, Akihiko Odaki wrote:

Recently MemReentrancyGuard was added to DeviceState to record that the
device is engaging in I/O. The network device backend needs to update it
when delivering a packet to a device.

In preparation for such a change, add MemReentrancyGuard * as a
parameter of qemu_new_nic().


An user on IRC asked if this patch is related/fixing CVE-2021-20255,
any clue?


Signed-off-by: Akihiko Odaki 
---
  include/net/net.h | 1 +
  hw/net/allwinner-sun8i-emac.c | 3 ++-
  hw/net/allwinner_emac.c   | 3 ++-
  hw/net/cadence_gem.c  | 3 ++-
  hw/net/dp8393x.c  | 3 ++-
  hw/net/e1000.c| 3 ++-
  hw/net/e1000e.c   | 2 +-
  hw/net/eepro100.c | 4 +++-
  hw/net/etraxfs_eth.c  | 3 ++-
  hw/net/fsl_etsec/etsec.c  | 3 ++-
  hw/net/ftgmac100.c| 3 ++-
  hw/net/i82596.c   | 2 +-
  hw/net/igb.c  | 2 +-
  hw/net/imx_fec.c  | 2 +-
  hw/net/lan9118.c  | 3 ++-
  hw/net/mcf_fec.c  | 3 ++-
  hw/net/mipsnet.c  | 3 ++-
  hw/net/msf2-emac.c| 3 ++-
  hw/net/mv88w8618_eth.c| 3 ++-
  hw/net/ne2000-isa.c   | 3 ++-
  hw/net/ne2000-pci.c   | 3 ++-
  hw/net/npcm7xx_emc.c  | 3 ++-
  hw/net/opencores_eth.c| 3 ++-
  hw/net/pcnet.c| 3 ++-
  hw/net/rocker/rocker_fp.c | 4 ++--
  hw/net/rtl8139.c  | 3 ++-
  hw/net/smc91c111.c| 3 ++-
  hw/net/spapr_llan.c   | 3 ++-
  hw/net/stellaris_enet.c   | 3 ++-
  hw/net/sungem.c   | 2 +-
  hw/net/sunhme.c   | 3 ++-
  hw/net/tulip.c| 3 ++-
  hw/net/virtio-net.c   | 6 --
  hw/net/vmxnet3.c  | 2 +-
  hw/net/xen_nic.c  | 4 ++--
  hw/net/xgmac.c| 3 ++-
  hw/net/xilinx_axienet.c   | 3 ++-
  hw/net/xilinx_ethlite.c   | 3 ++-
  hw/usb/dev-network.c  | 3 ++-
  net/net.c | 1 +
  40 files changed, 75 insertions(+), 41 deletions(-)





Re: [PATCH 4/4] x86/shadow: correct shadow_vcpu_init()'s comment

2024-04-24 Thread Roger Pau Monné
On Tue, Apr 23, 2024 at 04:33:09PM +0200, Jan Beulich wrote:
> As of the commit referenced below the update_paging_modes() hook is per-
> domain and hence also set (already) during domain construction.
> 
> Fixes: d0816a9085b5 ("x86/paging: move update_paging_modes() hook")
> Signed-off-by: Jan Beulich 
> 
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -99,11 +99,12 @@ int shadow_domain_init(struct domain *d)
>  return 0;
>  }
>  
> -/* Setup the shadow-specfic parts of a vcpu struct. Note: The most important
> - * job is to initialize the update_paging_modes() function pointer, which is
> - * used to initialized the rest of resources. Therefore, it really does not
> - * matter to have v->arch.paging.mode pointing to any mode, as long as it can
> - * be compiled.
> +/*
> + * Setup the shadow-specific parts of a vcpu struct. Note: The
> + * update_paging_modes() function pointer, which is used to initialize other
> + * resources, was already set during domain creation. Therefore it really 
> does
> + * not matter to have v->arch.paging.mode pointing to any (legitimate) mode,
> + * as long as it can be compiled.

Do you need to keep the last sentence?  If update_paging_modes is
already set at domain create, the 'Therefore it really does...'
doesn't seem to make much sense anymore, as it's no longer
shadow_vcpu_init() that sets it.

Possibly with that dropped:

Acked-by: Roger Pau Monné 

Thanks, Roger.



Re: [PATCH v3 12/12] xen/arm: List static shared memory regions as /memory nodes

2024-04-24 Thread Luca Fancellu


Hi Julien,

 
> 
> Rest LGTM:
> Reviewed-by: Michal Orzel 
 
 Thanks, I will send the next one shortly.
>>> I don't think there is a need to respin the whole series just for these 
>>> fixes.
>>> You should wait for the committers opinion.
>> AFAICT, there are multiple changes requested in various line. So I would 
>> rather prefer if this is respinned.
>> If this is the only patch that requires to change. You could send a new one 
>> in reply-to this patch. I think b4 is clever enough to pick up the new 
>> version in that case.
> 
> I was wrong. b4 didn't picked up the new version. Anyway, I have applied the 
> new patch and send to gitlab for testing. I will merge it once it passes.

Thanks a lot for that!

Cheers,
Luca




Re: [XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

2024-04-24 Thread Teddy Astie
Le 24/04/2024 à 14:11, Alessandro Zucchelli a écrit :
> This addresses violations of MISRA C:2012 Rule 7.2 which states as
> following: A “u” or “U” suffix shall be applied to all integer constants
> that are represented in an unsigned type.
>
> No functional change.
>
> Signed-off-by: Alessandro Zucchelli 
> ---
>   xen/arch/x86/include/asm/msr-index.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/include/asm/msr-index.h 
> b/xen/arch/x86/include/asm/msr-index.h
> index 92dd9fa496..9cdb5b2625 100644
> --- a/xen/arch/x86/include/asm/msr-index.h
> +++ b/xen/arch/x86/include/asm/msr-index.h
> @@ -236,7 +236,7 @@
>
>   #define MSR_VIRT_SPEC_CTRL  _AC(0xc001011f, U) /* Layout 
> matches MSR_SPEC_CTRL */
>
> -#define MSR_AMD_CSTATE_CFG  0xc0010296
> +#define MSR_AMD_CSTATE_CFG  0xc0010296U
>
>   /*
>* Legacy MSR constants in need of cleanup.  No new MSRs below this comment.

Hello, thanks for the patches

I wonder if the same approach should be also taken for all the other MSR
constants of this file that are similar ?

Teddy


Teddy Astie | Vates XCP-ng Intern

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech




Re: [XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

2024-04-24 Thread Jan Beulich
On 24.04.2024 14:11, Alessandro Zucchelli wrote:
> This addresses violations of MISRA C:2012 Rule 7.2 which states as
> following: A “u” or “U” suffix shall be applied to all integer constants
> that are represented in an unsigned type.
> 
> No functional change.

I'm inclined to suggest
Fixes: 652683e1aeaa ("x86/hvm: address violations of MISRA C:2012 Rule 7.2")
as that change clearly should have taken care of this already. The line
changed here is even visible in patch context there.

> Signed-off-by: Alessandro Zucchelli 

Acked-by: Jan Beulich 





Re: [PATCH 03/15] xen/arm: Always enable IOMMU

2024-04-24 Thread Julien Grall

Hi Henry,

On 24/04/2024 04:34, Henry Wang wrote:

From: Vikram Garhwal 

For overlay with iommu functionality to work with running VMs, we need to enable
IOMMU by default for the domains.

Signed-off-by: Vikram Garhwal 
Signed-off-by: Stefano Stabellini 
Signed-off-by: Henry Wang 
---
  xen/arch/arm/dom0less-build.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index fb63ec6fd1..2d1fd1e214 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -894,7 +894,8 @@ void __init create_domUs(void)
  panic("Missing property 'cpus' for domain %s\n",
dt_node_name(node));
  
-if ( dt_find_compatible_node(node, NULL, "multiboot,device-tree") &&

+if ( (IS_ENABLED(CONFIG_OVERLAY_DTB) ||


Similar to the first patch, building Xen with the DTB overlay doesn't 
mean the user will want to use it (think of distros that may want to 
provide a generic Xen).


Instead, we should introduce a new DT property "passthrough" that would 
indicate whether the IOMMU should be used.


To be futureproof, I would match the values used by xl.cfg (see 
docs/man/xl.cfg.5.pod.in).



+  dt_find_compatible_node(node, NULL, "multiboot,device-tree")) &&
   iommu_enabled )
  d_cfg.flags |= XEN_DOMCTL_CDF_iommu;
  


Cheers,

--
Julien Grall



Re: [XEN PATCH 1/2] pci: add suffix 'U' to PCI_CONF_ADDRESS macro.

2024-04-24 Thread Alessandro Zucchelli

On 2024-04-24 14:47, Jan Beulich wrote:

On 24.04.2024 14:11, Alessandro Zucchelli wrote:

This addresses violations of MISRA C:2012 Rule 7.2 which states as
following: A “u” or “U” suffix shall be applied to all integer 
constants

that are represented in an unsigned type.

No functional change.

Signed-off-by: Alessandro Zucchelli 


Acked-by: Jan Beulich 


--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -9,7 +9,7 @@
 #include 

 #define PCI_CONF_ADDRESS(sbdf, reg) \
-(0x8000 | ((sbdf).bdf << 8) | ((reg) & ~3))
+(0x8000U | ((sbdf).bdf << 8) | ((reg) & ~3))

 uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
 {


This is x86'es PCI, so the subject prefix may ant to be "x86/pci:".


Noted, thanks.

--
Alessandro Zucchelli, B.Sc.

Software Engineer, BUGSENG (https://bugseng.com)



[XEN PATCH 0/2] x86: address remaining violation of Rule 7.2

2024-04-24 Thread Alessandro Zucchelli
This patch series addresses the last violations of MISRA C:2012 Rule 7.2.
This rule will soon be tagged as clean for ECLAIR in a future patch. 

Alessandro Zucchelli (2):
  pci: add suffix 'U' to PCI_CONF_ADDRESS macro.
  x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

 xen/arch/x86/include/asm/msr-index.h | 2 +-
 xen/arch/x86/x86_64/pci.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.25.1




[XEN PATCH 1/2] pci: add suffix 'U' to PCI_CONF_ADDRESS macro.

2024-04-24 Thread Alessandro Zucchelli
This addresses violations of MISRA C:2012 Rule 7.2 which states as
following: A “u” or “U” suffix shall be applied to all integer constants
that are represented in an unsigned type.

No functional change.

Signed-off-by: Alessandro Zucchelli 
---
 xen/arch/x86/x86_64/pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/x86_64/pci.c b/xen/arch/x86/x86_64/pci.c
index aad1c3f7cf..8d33429103 100644
--- a/xen/arch/x86/x86_64/pci.c
+++ b/xen/arch/x86/x86_64/pci.c
@@ -9,7 +9,7 @@
 #include 
 
 #define PCI_CONF_ADDRESS(sbdf, reg) \
-(0x8000 | ((sbdf).bdf << 8) | ((reg) & ~3))
+(0x8000U | ((sbdf).bdf << 8) | ((reg) & ~3))
 
 uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
 {
-- 
2.25.1




[XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

2024-04-24 Thread Alessandro Zucchelli
This addresses violations of MISRA C:2012 Rule 7.2 which states as
following: A “u” or “U” suffix shall be applied to all integer constants
that are represented in an unsigned type.

No functional change.

Signed-off-by: Alessandro Zucchelli 
---
 xen/arch/x86/include/asm/msr-index.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/msr-index.h 
b/xen/arch/x86/include/asm/msr-index.h
index 92dd9fa496..9cdb5b2625 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -236,7 +236,7 @@
 
 #define MSR_VIRT_SPEC_CTRL  _AC(0xc001011f, U) /* Layout 
matches MSR_SPEC_CTRL */
 
-#define MSR_AMD_CSTATE_CFG  0xc0010296
+#define MSR_AMD_CSTATE_CFG  0xc0010296U
 
 /*
  * Legacy MSR constants in need of cleanup.  No new MSRs below this comment.
-- 
2.25.1




Re: [XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

2024-04-24 Thread Andrew Cooper
On 24/04/2024 1:51 pm, Jan Beulich wrote:
> On 24.04.2024 14:11, Alessandro Zucchelli wrote:
>> This addresses violations of MISRA C:2012 Rule 7.2 which states as
>> following: A “u” or “U” suffix shall be applied to all integer constants
>> that are represented in an unsigned type.
>>
>> No functional change.
> I'm inclined to suggest
> Fixes: 652683e1aeaa ("x86/hvm: address violations of MISRA C:2012 Rule 7.2")
> as that change clearly should have taken care of this already. The line
> changed here is even visible in patch context there.
>
>> Signed-off-by: Alessandro Zucchelli 
> Acked-by: Jan Beulich 
>
>

I expect it was a race condition.  MSR_AMD_CSTATE_CFG is a recent addition.

~Andrew



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

2024-04-24 Thread osstest service owner
flight 185780 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/185780/

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  77e25f0e30ddd11e043e6fce84bf108ce7de5b6f
baseline version:
 xen  cccb7878f386fb8691b7e28957a562a66d9b875f

Last test of basis   185767  2024-04-23 05:03:58 Z1 days
Testing same since   185780  2024-04-24 01:37:24 Z0 days1 attempts


People who touched revisions under test:
  Anthony PERARD 
  Jan Beulich 
  Leigh Brown 
  Roger Pau Monné 
  Ross Lagerwall 

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

Re: [PATCH] tools/xl: add suspend-to-ram and resume subcommands

2024-04-24 Thread Jason Andryuk

On 2024-02-29 02:00, zithro / Cyril Rébert wrote:

The xl command doesn't provide suspend/resume, so add them :
   xl suspend-to-ram 
   xl resume 

This patch follows a discussion on XenDevel: when you want the
virtualized equivalent of "sleep"-ing a host, it's better to
suspend/resume than to pause/unpause a domain.

Suggested-by: Andrew Cooper 
Suggested-by: Marek Marczykowski-Górecki 


You should include you S-o-B here to certify your patch under the 
Developer's Certificate of Origin.  You can read what that means in 
CONTRIBUTING.  tl;dr: You are stating you can make the open source 
contribution.


I tested this with a PVH and HVM guest.  suspend-to-ram and resume seem 
to function properly.  The VCPUs stop, but the domain & qemu remain. 
Resume works - the VCPUs start running again.


However, the domain destruction seems to hang on poweroff.  The VM 
transitions to powered off - state shutdown - but the domain and QEMU 
instance are not cleaned up.


If I power off without a suspend-to-ram, everything is cleaned up properly.

Have you seen this?  It's not your code, but I guess something with 
libxl or qemu.



---
- Tested on v4.17, x86
- the function "libxl_domain_resume" is called like libvirt does, so
   using a "co-operative resume", but I don't know what it means (:
- there may be a problem with the words resume <-> restore, like
   for "LIBXL_HAVE_NO_SUSPEND_RESUME"
- for the docs, I only slightly adapted a copy/paste from xl pause ...
---



diff --git a/tools/xl/xl_vmcontrol.c b/tools/xl/xl_vmcontrol.c
index 98f6bd2e76..ba45f89c5a 100644
--- a/tools/xl/xl_vmcontrol.c
+++ b/tools/xl/xl_vmcontrol.c
@@ -42,6 +42,16 @@ static void unpause_domain(uint32_t domid)
  libxl_domain_unpause(ctx, domid, NULL);
  }
  
+static void suspend_domain_toram(uint32_t domid)

+{
+libxl_domain_suspend_only(ctx, domid, NULL);
+}
+
+static void resume_domain(uint32_t domid)
+{
+libxl_domain_resume(ctx, domid, 1, NULL);
+}
+


I would just inline these functions below.


  static void destroy_domain(uint32_t domid, int force)
  {
  int rc;
@@ -82,6 +92,32 @@ int main_unpause(int argc, char **argv)
  return EXIT_SUCCESS;
  }
  
+int main_suspendtoram(int argc, char **argv)


Maybe main_suspend_to_ram to be closer to the command line suspend-to-ram.

Thanks,
Jason



Re: [XEN PATCH 04/10] drivers: char: address violation of MISRA C Rule 20.7

2024-04-24 Thread Nicola Vetrini

On 2024-04-24 09:23, Jan Beulich wrote:

On 23.04.2024 17:12, Nicola Vetrini wrote:

--- a/xen/drivers/char/omap-uart.c
+++ b/xen/drivers/char/omap-uart.c
@@ -48,8 +48,9 @@
 /* System configuration register */
 #define UART_OMAP_SYSC_DEF_CONF   0x0d   /* autoidle mode, wakeup is 
enabled */


-#define omap_read(uart, off)   readl((uart)->regs + 
(off<-#define omap_write(uart, off, val) writel((val), (uart)->regs + 
(off<+#define omap_read(uart, off)   readl((uart)->regs + ((off) << 
REG_SHIFT))

+#define omap_write(uart, off, val) writel((val), (uart)->regs + \


Would have been nice to drop the excess parentheses at the same time.

Jan


Right. I think I'll have a few more patches on this rule, so maybe I can 
adjust it.


--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)



Re: [PATCH 2/4] x86/P2M: un-indent write_p2m_entry()

2024-04-24 Thread Jan Beulich
On 24.04.2024 11:16, Roger Pau Monné wrote:
> On Tue, Apr 23, 2024 at 04:32:14PM +0200, Jan Beulich wrote:
>> Drop the inner scope that was left from earlier if/else removal. Take
>> the opportunity and make the paging_unlock() invocation common to
>> success and error paths, though.
> 
> TBH I'm not sure I prefer the fact to continue function execution
> after an error is found, I specially dislike that you have to add a
> !rc check to the nestedhvm conditional block, and because anything
> that we further add to the function would also need a !rc check.
> 
>>
>> Signed-off-by: Jan Beulich 
> 
> Acked-by: Roger Pau Monné 

Thanks.

> Albeit I do prefer the extra call to paging_unlock() and early return
> from the function in case of error.

Which puts me in the middle of your preference and the one George voiced
in the context of what is now cc950c49ae6a ("x86/PoD: tie together P2M
update and increment of entry count"). Doing the extra adjustment was
merely in the hope of meeting his desires ...

Jan



Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

2024-04-24 Thread Jan Beulich
On 24.04.2024 11:00, Federico Serafini wrote:
> On 24/04/24 10:30, Jan Beulich wrote:
>> On 24.04.2024 10:25, Federico Serafini wrote:
>>> Update ECLAIR configuration to take into account the deviations
>>> agreed during MISRA meetings for Rule 16.4.
>>>
>>> Signed-off-by: Federico Serafini 
>>> ---
>>>   automation/eclair_analysis/ECLAIR/deviations.ecl |  8 
>>>   docs/misra/deviations.rst| 13 +
>>>   2 files changed, 21 insertions(+)
>>>
>>
>> So what has changed here from v1? It looks all the same to me, with it still
>> remaining unclear what exactly ...
>>
>>> --- a/docs/misra/deviations.rst
>>> +++ b/docs/misra/deviations.rst
>>> @@ -334,6 +334,19 @@ Deviations related to MISRA C:2012 Rules:
>>>- /\* Fallthrough \*/
>>>- /\* Fallthrough. \*/
>>>   
>>> +   * - R16.4
>>> + - Switch statements having a controlling expression of enum type
>>> +   deliberately do not have a default case: gcc -Wall enables -Wswitch
>>> +   which warns (and breaks the build as we use -Werror) if one of the 
>>> enum
>>> +   labels is missing from the switch.
>>> + - Tagged as `deliberate` for ECLAIR.
>>> +
>>> +   * - R16.4
>>> + - A switch statement with a single switch clause and no default label 
>>> may
>>> +   be used in place of an equivalent if statement if it is considered 
>>> to
>>> +   improve readability.
>>> + - Tagged as `deliberate` for ECLAIR.
>>> +
>>>  * - R16.6
>>>- A switch statement with a single switch clause and no default 
>>> label may
>>>  be used in place of an equivalent if statement if it is considered 
>>> to
>>
>> ... a "switch clause" is.
> 
> I would define a switch clause as:
> "the non-empy list of statements which follows a non-empty list of
> case/default labels".
> If you agree, I will place it near the occurrences of the term
> "switch clause".

I'm afraid I don't (quite) agree, and I had hoped that I would have got my
point across that such a definition wants to be in terms used by the C spec.
"statement" is too broad here, as that in particular includes
"labeled-statement" as well. Ordinary labels are (aiui) okay to have in
there, so entirely excluding "labeled-statement" wouldn't be quite right
either.

Jan



Re: [PATCH 4/4] x86/shadow: correct shadow_vcpu_init()'s comment

2024-04-24 Thread Roger Pau Monné
On Wed, Apr 24, 2024 at 01:44:39PM +0200, Jan Beulich wrote:
> On 24.04.2024 12:06, Roger Pau Monné wrote:
> > On Tue, Apr 23, 2024 at 04:33:09PM +0200, Jan Beulich wrote:
> >> As of the commit referenced below the update_paging_modes() hook is per-
> >> domain and hence also set (already) during domain construction.
> >>
> >> Fixes: d0816a9085b5 ("x86/paging: move update_paging_modes() hook")
> >> Signed-off-by: Jan Beulich 
> >>
> >> --- a/xen/arch/x86/mm/shadow/common.c
> >> +++ b/xen/arch/x86/mm/shadow/common.c
> >> @@ -99,11 +99,12 @@ int shadow_domain_init(struct domain *d)
> >>  return 0;
> >>  }
> >>  
> >> -/* Setup the shadow-specfic parts of a vcpu struct. Note: The most 
> >> important
> >> - * job is to initialize the update_paging_modes() function pointer, which 
> >> is
> >> - * used to initialized the rest of resources. Therefore, it really does 
> >> not
> >> - * matter to have v->arch.paging.mode pointing to any mode, as long as it 
> >> can
> >> - * be compiled.
> >> +/*
> >> + * Setup the shadow-specific parts of a vcpu struct. Note: The
> >> + * update_paging_modes() function pointer, which is used to initialize 
> >> other
> >> + * resources, was already set during domain creation. Therefore it really 
> >> does
> >> + * not matter to have v->arch.paging.mode pointing to any (legitimate) 
> >> mode,
> >> + * as long as it can be compiled.
> > 
> > Do you need to keep the last sentence?  If update_paging_modes is
> > already set at domain create, the 'Therefore it really does...'
> > doesn't seem to make much sense anymore, as it's no longer
> > shadow_vcpu_init() that sets it.
> 
> I thought about dropping, but the "any mode does" seemed to me to be still
> relevant to mention. I thought about re-wording, too, without coming to any
> good alternative. Hence, despite agreeing with you that 'Therefore ...' does
> not quite fit (anymore), I left that as is.

To me the "was already set during domain creation" does already imply
it's set to "any (legitimate) mode", and hence the tailing sentence
feels out of place.

> > Possibly with that dropped:
> > 
> > Acked-by: Roger Pau Monné 

Feel free to keep the chunk and the Ack, I guess we could always
remove at a later point anyway.

Thanks, Roger.



Re: [PATCH 02/15] xen/arm/gic: Enable interrupt assignment to running VM

2024-04-24 Thread Julien Grall

Hi Henry,

On 24/04/2024 04:34, Henry Wang wrote:

From: Vikram Garhwal 

Enable interrupt assign/remove for running VMs in CONFIG_OVERLAY_DTB.

Currently, irq_route and mapping is only allowed at the domain creation. Adding
exception for CONFIG_OVERLAY_DTB.


AFAICT, this is mostly reverting b8577547236f ("xen/arm: Restrict when a 
physical IRQ can be routed/removed from/to a domain").




Signed-off-by: Vikram Garhwal 
Signed-off-by: Stefano Stabellini 
Signed-off-by: Henry Wang 
---
  xen/arch/arm/gic.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 44c40e86de..a775f886ed 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -140,8 +140,10 @@ int gic_route_irq_to_guest(struct domain *d, unsigned int 
virq,
   * back to the physical IRQ. To prevent get unsync, restrict the
   * routing to when the Domain is been created.
   */


The above comment explains why the check was added. But the commit 
message doesn't explain why this can be disregarded for your use-case.


Looking at the history, I don't think you can simply remove the checks.

Regardless that...


+#ifndef CONFIG_OVERLAY_DTB


... I am against such #ifdef. A distros may want to have OVERLAY_DTB 
enabled, yet the user will not use it.


Instead, you want to remove the check once the code can properly handle 
routing an IRQ the domain is created or ...



  if ( d->creation_finished )
  return -EBUSY;
+#endif
  
  ret = vgic_connect_hw_irq(d, NULL, virq, desc, true);

  if ( ret )
@@ -171,8 +173,10 @@ int gic_remove_irq_from_guest(struct domain *d, unsigned 
int virq,
   * Removing an interrupt while the domain is running may have
   * undesirable effect on the vGIC emulation.
   */
+#ifndef CONFIG_OVERLAY_DTB
  if ( !d->is_dying )
  return -EBUSY;
+#endif


... removed before they domain is destroyed.

  
  desc->handler->shutdown(desc);
  


Cheers,

--
Julien Grall



Re: [XEN PATCH 1/2] pci: add suffix 'U' to PCI_CONF_ADDRESS macro.

2024-04-24 Thread Jan Beulich
On 24.04.2024 14:11, Alessandro Zucchelli wrote:
> This addresses violations of MISRA C:2012 Rule 7.2 which states as
> following: A “u” or “U” suffix shall be applied to all integer constants
> that are represented in an unsigned type.
> 
> No functional change.
> 
> Signed-off-by: Alessandro Zucchelli 

Acked-by: Jan Beulich 

> --- a/xen/arch/x86/x86_64/pci.c
> +++ b/xen/arch/x86/x86_64/pci.c
> @@ -9,7 +9,7 @@
>  #include 
>  
>  #define PCI_CONF_ADDRESS(sbdf, reg) \
> -(0x8000 | ((sbdf).bdf << 8) | ((reg) & ~3))
> +(0x8000U | ((sbdf).bdf << 8) | ((reg) & ~3))
>  
>  uint8_t pci_conf_read8(pci_sbdf_t sbdf, unsigned int reg)
>  {

This is x86'es PCI, so the subject prefix may ant to be "x86/pci:".

Jan



Re: [PATCH 3/4] x86/paging: vCPU host mode is always set

2024-04-24 Thread Roger Pau Monné
On Wed, Apr 24, 2024 at 01:41:25PM +0200, Jan Beulich wrote:
> On 24.04.2024 11:34, Roger Pau Monné wrote:
> > On Tue, Apr 23, 2024 at 04:32:32PM +0200, Jan Beulich wrote:
> >> ... thanks to paging_vcpu_init() being part of vCPU creation. Further
> >> if paging is enabled on a domain, it's also guaranteed to be either HAP
> >> or shadow. Drop respective unnecessary (parts of) conditionals.
> > 
> > Is there some commit that changed when arch.paging.mode gets set, so
> > that this is actually safe to do now, but not when the code in
> > paging_dump_vcpu_info() was introduced?
> > 
> > I get the feeling we want to reference some change here in order to
> > explain why is now always guaranteed to be set.
> 
> I was indeed meaning to, but when I found the same even in 3.2, I stopped
> searching further.

Fair enough :).

Thanks, Roger.



Re: [XEN PATCH v5 1/3] x86/iommu: Disable IOMMU if cx16 isn't supported

2024-04-24 Thread Jan Beulich
On 18.04.2024 13:57, Teddy Astie wrote:
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -305,6 +305,12 @@ static int __init cf_check iov_detect(void)
>  if ( !iommu_enable && !iommu_intremap )
>  return 0;
>  
> +if ( unlikely(!cpu_has_cx16) )
> +{
> +printk("AMD-Vi: CPU doesn't support CMPXCHG16B, disabling\n");
> +return -ENODEV;
> +}

One additional nit: Here you neatly have just AMD-Vi: as a prefix for the
log message. Why ...

> @@ -2630,6 +2598,15 @@ static int __init cf_check vtd_setup(void)
>  int ret;
>  bool reg_inval_supported = true;
>  
> +if ( unlikely(!cpu_has_cx16) )
> +{
> +printk(XENLOG_ERR VTDPREFIX
> +   "IOMMU: CPU doesn't support CMPXCHG16B, disabling\n");
> +
> +ret = -ENODEV;
> +goto error;
> +}

... both VTDPREFIX and "IOMMU:" here?

Jan



Re: [PATCH 4/4] x86/shadow: correct shadow_vcpu_init()'s comment

2024-04-24 Thread Jan Beulich
On 24.04.2024 12:06, Roger Pau Monné wrote:
> On Tue, Apr 23, 2024 at 04:33:09PM +0200, Jan Beulich wrote:
>> As of the commit referenced below the update_paging_modes() hook is per-
>> domain and hence also set (already) during domain construction.
>>
>> Fixes: d0816a9085b5 ("x86/paging: move update_paging_modes() hook")
>> Signed-off-by: Jan Beulich 
>>
>> --- a/xen/arch/x86/mm/shadow/common.c
>> +++ b/xen/arch/x86/mm/shadow/common.c
>> @@ -99,11 +99,12 @@ int shadow_domain_init(struct domain *d)
>>  return 0;
>>  }
>>  
>> -/* Setup the shadow-specfic parts of a vcpu struct. Note: The most important
>> - * job is to initialize the update_paging_modes() function pointer, which is
>> - * used to initialized the rest of resources. Therefore, it really does not
>> - * matter to have v->arch.paging.mode pointing to any mode, as long as it 
>> can
>> - * be compiled.
>> +/*
>> + * Setup the shadow-specific parts of a vcpu struct. Note: The
>> + * update_paging_modes() function pointer, which is used to initialize other
>> + * resources, was already set during domain creation. Therefore it really 
>> does
>> + * not matter to have v->arch.paging.mode pointing to any (legitimate) mode,
>> + * as long as it can be compiled.
> 
> Do you need to keep the last sentence?  If update_paging_modes is
> already set at domain create, the 'Therefore it really does...'
> doesn't seem to make much sense anymore, as it's no longer
> shadow_vcpu_init() that sets it.

I thought about dropping, but the "any mode does" seemed to me to be still
relevant to mention. I thought about re-wording, too, without coming to any
good alternative. Hence, despite agreeing with you that 'Therefore ...' does
not quite fit (anymore), I left that as is.

> Possibly with that dropped:
> 
> Acked-by: Roger Pau Monné 

Thanks.

Jan



Re: [XEN PATCH] automation/eclair: add deviation of MISRA C:2012 Rule 14.4

2024-04-24 Thread Jan Beulich
On 24.04.2024 14:23, Federico Serafini wrote:
> Update ECLAIR configuration to take into account the deviations
> agreed during MISRA meetings.
> 
> Amend an existing entry of Rule 14.4 in deviations.rst:
> it is not a project-wide deviation.

Who / how is it not? ->is_dying is a globally visible struct field.

> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -294,7 +294,13 @@ Deviations related to MISRA C:2012 Rules:
>   - The XEN team relies on the fact that the enum is_dying has the
> constant with assigned value 0 act as false and the other ones as 
> true,
> therefore have the same behavior of a boolean.
> - - Project-wide deviation; tagged as `deliberate` for ECLAIR.
> + - Tagged as `deliberate` for ECLAIR.
> +
> +   * - R14.4
> + - A controlling expression of 'if' and iteration statements having
> +   integer, character or pointer type has a semantics that is well-known 
> to
> +   all Xen developers.
> + - Tagged as `deliberate` for ECLAIR.

I'm inclined to suggest that this more generic deviation be inserted ahead
of the more specific ->is_dying one.

Jan



Re: [XEN PATCH v5 1/3] x86/iommu: Disable IOMMU if cx16 isn't supported

2024-04-24 Thread Jan Beulich
On 18.04.2024 13:57, Teddy Astie wrote:
> All hardware with VT-d/AMD-Vi has CMPXCHG16B support. Check this at
> initialisation time, and remove the effectively-dead logic for the
> non-cx16 case.

As before: What about Xen itself running virtualized, and the underlying
hypervisor surfacing an IOMMU but not CX16? It may be okay to ignore the
IOMMU in such an event, but by not mentioning the case you give the
appearance of not having considered it at all.

> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -305,6 +305,12 @@ static int __init cf_check iov_detect(void)
>  if ( !iommu_enable && !iommu_intremap )
>  return 0;
>  
> +if ( unlikely(!cpu_has_cx16) )
> +{
> +printk("AMD-Vi: CPU doesn't support CMPXCHG16B, disabling\n");
> +return -ENODEV;
> +}
> +
>  if ( (init_done ? amd_iommu_init_late()
>  : amd_iommu_init(false)) != 0 )
>  {

I did previously point out (and that's even visible in patch context here)
that the per-vendor .setup() hooks aren't necessarily the first thing to
run. Please can you make sure you address (verbally or by code) prior to
submitting new versions?

Jan



Re: [PATCH v4 6/6] xen/ppc: mm-radix: Allocate all paging structures at runtime

2024-04-24 Thread Jan Beulich
On 12.04.2024 05:55, Shawn Anastasio wrote:
> In the initial mm-radix implementation, the in-memory partition and
> process tables required to configure the MMU, as well as the page tables
> themselves were all allocated statically since the boot allocator was
> not yet available.
> 
> Now that it is, allocate these structures at runtime and bump the size
> of the Process Table to its maximum supported value (on POWER9).
> 
> Signed-off-by: Shawn Anastasio 
> ---
> Changes in v4:
>   - use mfn_add in initial_page_alloc()
>   - zero pages returned by initial_page_alloc()
> 
>  xen/arch/ppc/mm-radix.c | 231 +---
>  1 file changed, 123 insertions(+), 108 deletions(-)
> 
> diff --git a/xen/arch/ppc/mm-radix.c b/xen/arch/ppc/mm-radix.c
> index ab5a10695c..8f4bfa87c0 100644
> --- a/xen/arch/ppc/mm-radix.c
> +++ b/xen/arch/ppc/mm-radix.c
> @@ -21,69 +21,105 @@ void enable_mmu(void);
>  #define radix_dprintk(...)
>  #endif
> 
> -#define INITIAL_LVL1_PD_COUNT  1
> -#define INITIAL_LVL2_LVL3_PD_COUNT 2
> -#define INITIAL_LVL4_PT_COUNT  256
> -
> -static size_t __initdata initial_lvl1_pd_pool_used;
> -static struct lvl1_pd initial_lvl1_pd_pool[INITIAL_LVL1_PD_COUNT];
> -
> -static size_t __initdata initial_lvl2_lvl3_pd_pool_used;
> -static struct lvl2_pd initial_lvl2_lvl3_pd_pool[INITIAL_LVL2_LVL3_PD_COUNT];
> -
> -static size_t __initdata initial_lvl4_pt_pool_used;
> -static struct lvl4_pt initial_lvl4_pt_pool[INITIAL_LVL4_PT_COUNT];
> -
> -/* Only reserve minimum Partition and Process tables  */
>  #define PATB_SIZE_LOG2 16 /* Only supported partition table size on POWER9 */
>  #define PATB_SIZE  (1UL << PATB_SIZE_LOG2)
> -#define PRTB_SIZE_LOG2 12
> +#define PRTB_SIZE_LOG2 24 /* Maximum process table size on POWER9 */
>  #define PRTB_SIZE  (1UL << PRTB_SIZE_LOG2)
> 
> -static struct patb_entry
> -__aligned(PATB_SIZE) initial_patb[PATB_SIZE / sizeof(struct patb_entry)];
> +static struct patb_entry *initial_patb;
> +static struct prtb_entry *initial_prtb;
> 
> -static struct prtb_entry
> -__aligned(PRTB_SIZE) initial_prtb[PRTB_SIZE / sizeof(struct prtb_entry)];
> +static mfn_t __initdata min_alloc_mfn = {-1};
> +static mfn_t __initdata max_alloc_mfn = {0};
> 
> -static __init struct lvl1_pd *lvl1_pd_pool_alloc(void)
> +/*
> + * A thin wrapper for alloc_boot_pages that keeps track of the maximum and
> + * minimum mfns that have been allocated. This information is used by
> + * setup_initial_mapping to include the allocated pages in the initial
> + * page mapping.

Plus everything in between. Together with the further comment below I'm
afraid I still think that the constraints (and the justification for them
being tolerable at least for the time being) aren't made sufficiently
clear (perhaps in another code comment next to the respective two
variables' definitions).

> + * Additionally, allocated pages are zeroed before return.
> + */
> +static mfn_t __init initial_page_alloc(unsigned long nr_pfns,
> +   unsigned long pfn_align)
>  {
> -if ( initial_lvl1_pd_pool_used >= INITIAL_LVL1_PD_COUNT )
> -{
> -early_printk("Ran out of space for LVL1 PD!\n");
> -die();
> -}
> +mfn_t mfn_first, mfn_last;
> 
> -return _lvl1_pd_pool[initial_lvl1_pd_pool_used++];
> -}
> +mfn_first = alloc_boot_pages(nr_pfns, pfn_align);
> +mfn_last = mfn_add(mfn_first, nr_pfns - 1);
> 
> -static __init struct lvl2_pd *lvl2_pd_pool_alloc(void)
> -{
> -if ( initial_lvl2_lvl3_pd_pool_used >= INITIAL_LVL2_LVL3_PD_COUNT )
> -{
> -early_printk("Ran out of space for LVL2/3 PD!\n");
> -die();
> -}
> +min_alloc_mfn = _mfn(min(mfn_x(min_alloc_mfn), mfn_x(mfn_first)));
> +max_alloc_mfn = _mfn(max(mfn_x(max_alloc_mfn), mfn_x(mfn_last)));
> 
> -return _lvl2_lvl3_pd_pool[initial_lvl2_lvl3_pd_pool_used++];
> +memset(__va(mfn_to_maddr(mfn_first)), 0, nr_pfns << PAGE_SHIFT);
> +
> +return mfn_first;
>  }
> 
> -static __init struct lvl3_pd *lvl3_pd_pool_alloc(void)
> +static __init void *initial_pd_pt_alloc(void)
>  {
> -BUILD_BUG_ON(sizeof(struct lvl3_pd) != sizeof(struct lvl2_pd));
> +BUILD_BUG_ON(sizeof(struct lvl1_pd) > PAGE_SIZE);
> +BUILD_BUG_ON(sizeof(struct lvl2_pd) > PAGE_SIZE);
> +BUILD_BUG_ON(sizeof(struct lvl3_pd) > PAGE_SIZE);
> +BUILD_BUG_ON(sizeof(struct lvl4_pt) > PAGE_SIZE);
> 
> -return (struct lvl3_pd *) lvl2_pd_pool_alloc();
> +return __va(mfn_to_maddr(initial_page_alloc(1, 1)));
>  }
> 
> -static __init struct lvl4_pt *lvl4_pt_pool_alloc(void)
> +static void map_page_initial(struct lvl1_pd *lvl1, vaddr_t virt, paddr_t 
> phys,
> + unsigned long flags)
>  {
> -if ( initial_lvl4_pt_pool_used >= INITIAL_LVL4_PT_COUNT )
> +struct lvl2_pd *lvl2;
> +struct lvl3_pd *lvl3;
> +struct lvl4_pt *lvl4;
> +pde_t *pde;
> +pte_t *pte;
> +
> +/* Allocate LVL 2 PD if necessary */
> +pde = 

Re: [PATCH 3/4] x86/paging: vCPU host mode is always set

2024-04-24 Thread Jan Beulich
On 24.04.2024 11:34, Roger Pau Monné wrote:
> On Tue, Apr 23, 2024 at 04:32:32PM +0200, Jan Beulich wrote:
>> ... thanks to paging_vcpu_init() being part of vCPU creation. Further
>> if paging is enabled on a domain, it's also guaranteed to be either HAP
>> or shadow. Drop respective unnecessary (parts of) conditionals.
> 
> Is there some commit that changed when arch.paging.mode gets set, so
> that this is actually safe to do now, but not when the code in
> paging_dump_vcpu_info() was introduced?
> 
> I get the feeling we want to reference some change here in order to
> explain why is now always guaranteed to be set.

I was indeed meaning to, but when I found the same even in 3.2, I stopped
searching further.

>> Signed-off-by: Jan Beulich 
> 
> Acked-by: Roger Pau Monné 

Thanks.

Jan



Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2024-04-24 Thread Thomas Huth

On 24/04/2024 12.41, Prasad Pandit wrote:

On Wednesday, 24 April, 2024 at 03:36:01 pm IST, Philippe Mathieu-Daudé wrote:

On 1/6/23 05:18, Akihiko Odaki wrote:

Recently MemReentrancyGuard was added to DeviceState to record that the
device is engaging in I/O. The network device backend needs to update it
when delivering a packet to a device.
  
In preparation for such a change, add MemReentrancyGuard * as a

parameter of qemu_new_nic().


An user on IRC asked if this patch is related/fixing CVE-2021-20255,
any clue?


* CVE-2021-20255 bug: infinite recursion is pointing at a different fix patch.
   -> https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2021-20255

* And the this patch below has different issue tagged
   -> https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg08312.html
   Fixes: CVE-2023-3019


* They look different, former is an infinite recursion issue and the latter is 
a use-after-free one.


I assume the eepro reentrancy issue has been fixed with:

 https://gitlab.com/qemu-project/qemu/-/issues/556
 i.e.:
 https://gitlab.com/qemu-project/qemu/-/commit/c40ca2301c7603524eaddb5308a3

 HTH,
  Thomas





Re: [PATCH v2 1/2] net: Provide MemReentrancyGuard * to qemu_new_nic()

2024-04-24 Thread Prasad Pandit
On Wednesday, 24 April, 2024 at 03:36:01 pm IST, Philippe Mathieu-Daudé wrote:
>On 1/6/23 05:18, Akihiko Odaki wrote:
>> Recently MemReentrancyGuard was added to DeviceState to record that the
>> device is engaging in I/O. The network device backend needs to update it
>> when delivering a packet to a device.
>> 
>> In preparation for such a change, add MemReentrancyGuard * as a
>> parameter of qemu_new_nic().
>
>An user on IRC asked if this patch is related/fixing CVE-2021-20255,
>any clue?

* CVE-2021-20255 bug: infinite recursion is pointing at a different fix patch.
  -> https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2021-20255

* And the this patch below has different issue tagged
  -> https://lists.nongnu.org/archive/html/qemu-devel/2023-05/msg08312.html
  Fixes: CVE-2023-3019


* They look different, former is an infinite recursion issue and the latter is 
a use-after-free one.


Thank you.
---
  -Prasad



[XEN PATCH] automation/eclair: add deviation of MISRA C:2012 Rule 14.4

2024-04-24 Thread Federico Serafini
Update ECLAIR configuration to take into account the deviations
agreed during MISRA meetings.

Amend an existing entry of Rule 14.4 in deviations.rst:
it is not a project-wide deviation.

Signed-off-by: Federico Serafini 
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 4 
 docs/misra/deviations.rst| 8 +++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
b/automation/eclair_analysis/ECLAIR/deviations.ecl
index d21f112a9b..f1a29389fd 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -349,6 +349,10 @@ therefore have the same behavior of a boolean"
 -config=MC3R1.R14.4,etypes+={deliberate, 
"stmt(child(cond,child(expr,ref(^?::is_dying$","src_type(enum)"}
 -doc_end
 
+-doc_begin="A controlling expression of 'if' and iteration statements having 
integer, character or pointer type has a semantics that is well-known to all 
Xen developers."
+-config=MC3R1.R14.4,etypes+={deliberate, "any()", 
"src_type(integer||character)||src_expr(type(desugar(pointer(any()"}
+-doc_end
+
 #
 # Series 16.
 #
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index ed0c1e8ed0..a9d9cca04d 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -294,7 +294,13 @@ Deviations related to MISRA C:2012 Rules:
  - The XEN team relies on the fact that the enum is_dying has the
constant with assigned value 0 act as false and the other ones as true,
therefore have the same behavior of a boolean.
- - Project-wide deviation; tagged as `deliberate` for ECLAIR.
+ - Tagged as `deliberate` for ECLAIR.
+
+   * - R14.4
+ - A controlling expression of 'if' and iteration statements having
+   integer, character or pointer type has a semantics that is well-known to
+   all Xen developers.
+ - Tagged as `deliberate` for ECLAIR.
 
* - R16.2
  - Complying with the Rule would entail a lot of code duplication in the
-- 
2.34.1




Re: [PATCH v3 12/12] xen/arm: List static shared memory regions as /memory nodes

2024-04-24 Thread Julien Grall

Hi,

On 22/04/2024 11:24, Julien Grall wrote:

Hi,

On 22/04/2024 10:26, Michal Orzel wrote:



On 22/04/2024 10:07, Luca Fancellu wrote:



Hi Michal,

+    for ( cells = reg, i = 0; cells < reg + nr_cells; i++, cells 
+= reg_size )

+    {
+    u64 start = dt_read_number(cells, addrcells);

We should no longer use Linux derived types like u64. Use uint64_t.


+    u64 size = dt_read_number(cells + addrcells, sizecells);
+
+    dt_dprintk("  Bank %d: %#"PRIx64"->%#"PRIx64"\n",
+   i, start, start + size);

i is unsigned so the correct format specifier should be %u


Right, should have been more careful when copying the code from above



+void __init shm_mem_node_fill_reg_range(const struct kernel_info 
*kinfo,

+    __be32 *reg, int *nr_cells,
+    int addrcells, int sizecells)
+{
+    const struct membanks *mem = >shm_mem.common;
+    unsigned int i;
+    __be32 *cells;
+
+    BUG_ON(!nr_cells || !reg);
+
+    cells = [*nr_cells];
+    for ( i = 0; i < mem->nr_banks; i++ )
+    {
+    u64 start = mem->bank[i].start;

ditto


Will fix, here paddr_t should be ok isn’t it?

yes





Rest LGTM:
Reviewed-by: Michal Orzel 


Thanks, I will send the next one shortly.
I don't think there is a need to respin the whole series just for 
these fixes.

You should wait for the committers opinion.


AFAICT, there are multiple changes requested in various line. So I would 
rather prefer if this is respinned.


If this is the only patch that requires to change. You could send a new 
one in reply-to this patch. I think b4 is clever enough to pick up the 
new version in that case.


I was wrong. b4 didn't picked up the new version. Anyway, I have applied 
the new patch and send to gitlab for testing. I will merge it once it 
passes.


Cheers,

--
Julien Grall



[linux-linus test] 185779: regressions - FAIL

2024-04-24 Thread osstest service owner
flight 185779 linux-linus real [real]
flight 185784 linux-linus real-retest [real]
http://logs.test-lab.xenproject.org/osstest/logs/185779/
http://logs.test-lab.xenproject.org/osstest/logs/185784/

Regressions :-(

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

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

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-xl-credit1 15 migrate-support-check fail in 185784 never pass
 test-armhf-armhf-xl-credit1 16 saverestore-support-check fail in 185784 never 
pass
 test-armhf-armhf-xl-raw 14 migrate-support-check fail in 185784 never pass
 test-armhf-armhf-xl-raw 15 saverestore-support-check fail in 185784 never pass
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 185768
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 185768
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 185768
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 185768
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 185768
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 185768
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail 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-thunderx 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 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-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  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-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  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl  16 saverestore-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-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-qcow214 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-qcow215 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-rtds 16 saverestore-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  15 migrate-support-checkfail   never pass
 test-armhf-armhf-xl-arndale  16 saverestore-support-checkfail   never pass

version targeted for testing:
 linux9d1ddab261f3e2af7c384dc02238784ce0cf9f98
baseline version:
 linux71b1543c83d65af8215d7558d70fc2ecbee77dcf

Last test of basis   185768  2024-04-23 07:46:10 Z1 days
Testing same since   185779  2024-04-24 00:13:50 Z0 days1 attempts


People who touched revisions under test:
  David Howells 
  Linus Torvalds 
  Paulo Alcantara (Red Hat) 
  Paulo Alcantara 
  Ronnie Sahlberg 
  Steve French 
  Takayuki Nagata 
  Tom Talpey 

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

Re: [XEN PATCH v5 3/3] x86/iommu: Disable intrerrupt remapping if cx16 is not supported

2024-04-24 Thread Jan Beulich
On 18.04.2024 13:57, Teddy Astie wrote:
> All hardware with VT-d/AMD-Vi has CMPXCHG16B support.  Check this at
> initialisation time, and remove the effectively-dead logic for the non-cx16
> case.
> 
> Suggested-by: Andrew Cooper 
> Signed-off-by: Teddy Astie 

Hmm, so this looks to be the code that I commented on as missing in patch 1.
I don't think this can be two separate patches. If you want to keep things
in small chunks, move the removal of "the effectively-dead logic" to a
separate, follow-on patch.

Jan



[PATCH] arm/vpci: make prefetchable mem 64 bit

2024-04-24 Thread Stewart Hildebrand
The vPCI prefetchable memory range is >= 4GB, so the memory space flags
should be set to 64-bit. See IEEE Std 1275-1994 [1] for a definition of
the field.

[1] https://www.devicetree.org/open-firmware/bindings/pci/pci2_1.pdf

Signed-off-by: Stewart Hildebrand 
---
 xen/include/public/arch-arm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
index e167e14f8df9..289af81bd69d 100644
--- a/xen/include/public/arch-arm.h
+++ b/xen/include/public/arch-arm.h
@@ -487,7 +487,7 @@ typedef uint64_t xen_callback_t;
 #define GUEST_RAM0_SIZE   xen_mk_ullong(0xc000)
 
 /* 4GB @ 4GB Prefetch Memory for VPCI */
-#define GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM   xen_mk_ullong(0x4200)
+#define GUEST_VPCI_ADDR_TYPE_PREFETCH_MEM   xen_mk_ullong(0x4300)
 #define GUEST_VPCI_PREFETCH_MEM_ADDRxen_mk_ullong(0x1)
 #define GUEST_VPCI_PREFETCH_MEM_SIZExen_mk_ullong(0x1)
 

base-commit: 410ef3343924b5a3928bbe8e392491992b322cf0
-- 
2.43.2




[PATCH v3 1/8] gzip: clean up comments and fix code alignment

2024-04-24 Thread Daniel P. Smith
This commit cleans up the comments and fixes the code alignment using Xen
coding style. This is done to make the code more legible before refactoring.

Signed-off-by: Daniel P. Smith 
---
 xen/common/gzip/gunzip.c  |  14 +-
 xen/common/gzip/inflate.c | 787 +++---
 2 files changed, 406 insertions(+), 395 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index 53cee9ee178a..d07c451cd875 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -54,11 +54,10 @@ static __init void error(const char *x)
 
 static __init int fill_inbuf(void)
 {
-error("ran out of input data");
-return 0;
+error("ran out of input data");
+return 0;
 }
 
-
 #include "inflate.c"
 
 static __init void flush_window(void)
@@ -122,3 +121,12 @@ __init int perform_gunzip(char *output, char *image, 
unsigned long image_len)
 
 return rc;
 }
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index a03903f73116..220d2ff4d9d9 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -1,11 +1,13 @@
 #define DEBG(x)
 #define DEBG1(x)
-/* inflate.c -- Not copyrighted 1992 by Mark Adler
-   version c10p1, 10 January 1993 */
+/*
+ * inflate.c -- Not copyrighted 1992 by Mark Adler
+ * version c10p1, 10 January 1993
+ */
 
-/* 
+/*
  * Adapted for booting Linux by Hannu Savolainen 1993
- * based on gzip-1.0.3 
+ * based on gzip-1.0.3
  *
  * Nicolas Pitre , 1999/04/14 :
  *   Little mods for all variable to reside either into rodata or bss segments
@@ -15,92 +17,91 @@
  */
 
 /*
-   Inflate deflated (PKZIP's method 8 compressed) data.  The compression
-   method searches for as much of the current string of bytes (up to a
-   length of 258) in the previous 32 K bytes.  If it doesn't find any
-   matches (of at least length 3), it codes the next byte.  Otherwise, it
-   codes the length of the matched string and its distance backwards from
-   the current position.  There is a single Huffman code that codes both
-   single bytes (called "literals") and match lengths.  A second Huffman
-   code codes the distance information, which follows a length code.  Each
-   length or distance code actually represents a base value and a number
-   of "extra" (sometimes zero) bits to get to add to the base value.  At
-   the end of each deflated block is a special end-of-block (EOB) literal/
-   length code.  The decoding process is basically: get a literal/length
-   code; if EOB then done; if a literal, emit the decoded byte; if a
-   length then get the distance and emit the referred-to bytes from the
-   sliding window of previously emitted data.
-
-   There are (currently) three kinds of inflate blocks: stored, fixed, and
-   dynamic.  The compressor deals with some chunk of data at a time, and
-   decides which method to use on a chunk-by-chunk basis.  A chunk might
-   typically be 32 K or 64 K.  If the chunk is incompressible, then the
-   "stored" method is used.  In this case, the bytes are simply stored as
-   is, eight bits per byte, with none of the above coding.  The bytes are
-   preceded by a count, since there is no longer an EOB code.
-
-   If the data is compressible, then either the fixed or dynamic methods
-   are used.  In the dynamic method, the compressed data is preceded by
-   an encoding of the literal/length and distance Huffman codes that are
-   to be used to decode this block.  The representation is itself Huffman
-   coded, and so is preceded by a description of that code.  These code
-   descriptions take up a little space, and so for small blocks, there is
-   a predefined set of codes, called the fixed codes.  The fixed method is
-   used if the block codes up smaller that way (usually for quite small
-   chunks), otherwise the dynamic method is used.  In the latter case, the
-   codes are customized to the probabilities in the current block, and so
-   can code it much better than the pre-determined fixed codes.
- 
-   The Huffman codes themselves are decoded using a multi-level table
-   lookup, in order to maximize the speed of decoding plus the speed of
-   building the decoding tables.  See the comments below that precede the
-   lbits and dbits tuning parameters.
+ * Inflate deflated (PKZIP's method 8 compressed) data.  The compression
+ * method searches for as much of the current string of bytes (up to a
+ * length of 258) in the previous 32 K bytes.  If it doesn't find any
+ * matches (of at least length 3), it codes the next byte.  Otherwise, it
+ * codes the length of the matched string and its distance backwards from
+ * the current position.  There is a single Huffman code that codes both
+ * single bytes (called "literals") and match lengths.  A second Huffman
+ * code codes the distance information, which follows a length code.  Each
+ * length or distance 

[PATCH v3 4/8] gzip: move window pointer into gunzip state

2024-04-24 Thread Daniel P. Smith
Move the window pointer, outcnt/wp, into struct gunzip_data. It was erroneously
labeled as outcnt and then define aliased to wp, this eliminates the aliasing
and only refers to as wp, the window pointer.

Signed-off-by: Daniel P. Smith 
---
 xen/common/gzip/gunzip.c  | 11 +--
 xen/common/gzip/inflate.c | 17 -
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index e47f10ae19ad..11bfe45d8222 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -8,6 +8,8 @@
 
 struct gunzip_state {
 unsigned char *window;
+/* window pointer: */
+unsigned int wp;
 };
 
 static unsigned char *__initdata inbuf;
@@ -16,9 +18,6 @@ static unsigned int __initdata insize;
 /* Index of next byte to be processed in inbuf: */
 static unsigned int __initdata inptr;
 
-/* Bytes in output buffer: */
-static unsigned int __initdata outcnt;
-
 #define malloc(a)   xmalloc_bytes(a)
 #define free(a) xfree(a)
 #define memzero(s, n)   memset((s), 0, (n))
@@ -75,15 +74,15 @@ static __init void flush_window(struct gunzip_state *s)
 unsigned char *in, ch;
 
 in = s->window;
-for ( n = 0; n < outcnt; n++ )
+for ( n = 0; n < s->wp; n++ )
 {
 ch = *in++;
 c = crc_32_tab[((int)c ^ ch) & 0xff] ^ (c >> 8);
 }
 crc = c;
 
-bytes_out += (unsigned long)outcnt;
-outcnt = 0;
+bytes_out += (unsigned long)s->wp;
+s->wp = 0;
 }
 
 __init int gzip_check(char *image, unsigned long image_len)
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index 5fa5c039c6d1..78b2f20a97ba 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -130,7 +130,6 @@ static char rcsid[] = "#Id: inflate.c,v 0.14 1993/06/10 
13:27:04 jloup Exp #";
  * "uch *window;" and then malloc'ed in the latter case.  The definition
  * must be in unzip.h, included above.
  */
-#define wp outcnt
 
 /*
  * Huffman code lookup table entry--this entry is four bytes for machines
@@ -556,7 +555,7 @@ static int __init inflate_codes(
 /* make local copies of globals */
 b = bb;   /* initialize bit buffer */
 k = bk;
-w = wp;   /* initialize window position */
+w = s->wp;/* initialize window position */
 
 /* inflate the coded data */
 ml = mask_bits[bl];   /* precompute masks for speed */
@@ -579,7 +578,7 @@ static int __init inflate_codes(
 Tracevv((stderr, "%c", s->window[w-1]));
 if (w == WSIZE)
 {
-wp = w;
+s->wp = w;
 flush_window(s);
 w = 0;
 }
@@ -627,7 +626,7 @@ static int __init inflate_codes(
 } while (--e);
 if (w == WSIZE)
 {
-wp = w;
+s->wp = w;
 flush_window(s);
 w = 0;
 }
@@ -636,7 +635,7 @@ static int __init inflate_codes(
 }
 
 /* restore the globals from the locals */
-wp = w;   /* restore global window pointer */
+s->wp = w;/* restore global window pointer */
 bb = b;   /* restore global bit buffer */
 bk = k;
 
@@ -657,7 +656,7 @@ static int __init inflate_stored(struct gunzip_state *s)
 /* make local copies of globals */
 b = bb;   /* initialize bit buffer */
 k = bk;
-w = wp;   /* initialize window position */
+w = s->wp;/* initialize window position */
 
 
 /* go to byte boundary */
@@ -681,7 +680,7 @@ static int __init inflate_stored(struct gunzip_state *s)
 s->window[w++] = (uch)b;
 if (w == WSIZE)
 {
-wp = w;
+s->wp = w;
 flush_window(s);
 w = 0;
 }
@@ -689,7 +688,7 @@ static int __init inflate_stored(struct gunzip_state *s)
 }
 
 /* restore the globals from the locals */
-wp = w;   /* restore global window pointer */
+s->wp = w;/* restore global window pointer */
 bb = b;   /* restore global bit buffer */
 bk = k;
 
@@ -1004,7 +1003,7 @@ static int __init inflate(struct gunzip_state *s)
 int r;/* result code */
 
 /* initialize window, bit buffer */
-wp = 0;
+s->wp = 0;
 bk = 0;
 bb = 0;
 
-- 
2.30.2




[PATCH v3 3/8] gzip: refactor the gunzip window into common state

2024-04-24 Thread Daniel P. Smith
Begin moving core state, in this case the gunzip window, into struct
gunzip_state to allow a per decompression instance. In doing so, drop the
define aliasing of window to slide.

Signed-off-by: Daniel P. Smith 
---
 xen/common/gzip/gunzip.c  | 21 
 xen/common/gzip/inflate.c | 68 +++
 2 files changed, 48 insertions(+), 41 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index b7cadadcca8b..e47f10ae19ad 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -4,10 +4,12 @@
 #include 
 #include 
 
-static unsigned char *__initdata window;
-
 #define WSIZE   0x8000U
 
+struct gunzip_state {
+unsigned char *window;
+};
+
 static unsigned char *__initdata inbuf;
 static unsigned int __initdata insize;
 
@@ -43,7 +45,7 @@ typedef unsigned long   ulg;
 #endif
 
 static long __initdata bytes_out;
-static void flush_window(void);
+static void flush_window(struct gunzip_state *s);
 
 static __init void error(const char *x)
 {
@@ -62,7 +64,7 @@ static __init uch get_byte(void) {
 
 #include "inflate.c"
 
-static __init void flush_window(void)
+static __init void flush_window(struct gunzip_state *s)
 {
 /*
  * The window is equal to the output buffer therefore only need to
@@ -72,7 +74,7 @@ static __init void flush_window(void)
 unsigned int n;
 unsigned char *in, ch;
 
-in = window;
+in = s->window;
 for ( n = 0; n < outcnt; n++ )
 {
 ch = *in++;
@@ -99,12 +101,17 @@ __init int gzip_check(char *image, unsigned long image_len)
 
 __init int perform_gunzip(char *output, char *image, unsigned long image_len)
 {
+struct gunzip_state *s;
 int rc;
 
 if ( !gzip_check(image, image_len) )
 return 1;
 
-window = (unsigned char *)output;
+s = (struct gunzip_state *)malloc(sizeof(struct gunzip_state));
+if ( !s )
+return -ENOMEM;
+
+s->window = (unsigned char *)output;
 inbuf = (unsigned char *)image;
 insize = image_len;
 inptr = 0;
@@ -112,7 +119,7 @@ __init int perform_gunzip(char *output, char *image, 
unsigned long image_len)
 
 makecrc();
 
-if ( gunzip() < 0 )
+if ( gunzip(s) < 0 )
 {
 rc = -EINVAL;
 }
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index 02a395aeb86a..5fa5c039c6d1 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -126,12 +126,11 @@ static char rcsid[] = "#Id: inflate.c,v 0.14 1993/06/10 
13:27:04 jloup Exp #";
  * ANDing with 0x7fff (32K-1).
  *
  * It is left to other modules to supply the 32 K area.  It is assumed
- * to be usable as if it were declared "uch slide[32768];" or as just
- * "uch *slide;" and then malloc'ed in the latter case.  The definition
+ * to be usable as if it were declared "uch window[32768];" or as just
+ * "uch *window;" and then malloc'ed in the latter case.  The definition
  * must be in unzip.h, included above.
  */
 #define wp outcnt
-#define slide window
 
 /*
  * Huffman code lookup table entry--this entry is four bytes for machines
@@ -155,12 +154,13 @@ struct huft {
 static int huft_build(unsigned *, unsigned, unsigned,
   const ush *, const ush *, struct huft **, int *);
 static int huft_free(struct huft *);
-static int inflate_codes(struct huft *, struct huft *, int, int);
-static int inflate_stored(void);
-static int inflate_fixed(void);
-static int inflate_dynamic(void);
-static int inflate_block(int *);
-static int inflate(void);
+static int inflate_codes(
+struct gunzip_state *, struct huft *, struct huft *, int, int);
+static int inflate_stored(struct gunzip_state *s);
+static int inflate_fixed(struct gunzip_state *s);
+static int inflate_dynamic(struct gunzip_state *s);
+static int inflate_block(struct gunzip_state *, int *);
+static int inflate(struct gunzip_state *s);
 
 /* Tables for deflate from PKZIP's appnote.txt. */
 static const unsigned border[] = {/* Order of the bit length code lengths 
*/
@@ -542,7 +542,7 @@ static int __init huft_free(struct huft *t)
  * @param bd  Number of bits decoded by td[]
  */
 static int __init inflate_codes(
-struct huft *tl, struct huft *td, int bl, int bd)
+struct gunzip_state *s, struct huft *tl, struct huft *td, int bl, int bd)
 {
 register unsigned e;  /* table entry flag/number of extra bits */
 unsigned n, d;/* length and index for copy */
@@ -575,12 +575,12 @@ static int __init inflate_codes(
 DUMPBITS(t->b);
 if (e == 16)/* then it's a literal */
 {
-slide[w++] = (uch)t->v.n;
-Tracevv((stderr, "%c", slide[w-1]));
+s->window[w++] = (uch)t->v.n;
+Tracevv((stderr, "%c", s->window[w-1]));
 if (w == WSIZE)
 {
 wp = w;
-flush_window();
+flush_window(s);
 w = 0;
 }
 }
@@ -616,19 +616,19 @@ static int 

[PATCH v3 5/8] gzip: move input buffer handling into gunzip state

2024-04-24 Thread Daniel P. Smith
Move the input buffer handling, buffer pointer(inbuf), size(insize), and
index(inptr), into gunzip state. Adjust functions and macros that consumed the
input buffer to accept a struct gunzip_state reference.

Signed-off-by: Daniel P. Smith 
---
 xen/common/gzip/gunzip.c  | 23 +-
 xen/common/gzip/inflate.c | 92 +++
 2 files changed, 57 insertions(+), 58 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index 11bfe45d8222..3fb9589b069e 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -10,13 +10,12 @@ struct gunzip_state {
 unsigned char *window;
 /* window pointer: */
 unsigned int wp;
-};
-
-static unsigned char *__initdata inbuf;
-static unsigned int __initdata insize;
 
-/* Index of next byte to be processed in inbuf: */
-static unsigned int __initdata inptr;
+unsigned char *inbuf;
+size_t insize;
+/* Index of next byte to be processed in inbuf: */
+unsigned int inptr;
+};
 
 #define malloc(a)   xmalloc_bytes(a)
 #define free(a) xfree(a)
@@ -51,14 +50,14 @@ static __init void error(const char *x)
 panic("%s\n", x);
 }
 
-static __init uch get_byte(void) {
-if ( inptr >= insize )
+static __init uch get_byte(struct gunzip_state *s) {
+if ( s->inptr >= s->insize )
 {
 error("ran out of input data");
 return 0; /* should never reach */
 }
 
-return inbuf[inptr++];
+return s->inbuf[s->inptr++];
 }
 
 #include "inflate.c"
@@ -111,9 +110,9 @@ __init int perform_gunzip(char *output, char *image, 
unsigned long image_len)
 return -ENOMEM;
 
 s->window = (unsigned char *)output;
-inbuf = (unsigned char *)image;
-insize = image_len;
-inptr = 0;
+s->inbuf = (unsigned char *)image;
+s->insize = image_len;
+s->inptr = 0;
 bytes_out = 0;
 
 makecrc();
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index 78b2f20a97ba..f1a3b04cef8f 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -220,8 +220,8 @@ static const ush mask_bits[] = {
 0x01ff, 0x03ff, 0x07ff, 0x0fff, 0x1fff, 0x3fff, 0x7fff, 0x
 };
 
-#define NEXTBYTE()  (get_byte()) /* get_byte will panic on failure */
-#define NEEDBITS(n) {while(k<(n)){b|=((ulg)NEXTBYTE())<>=(n);k-=(n);}
 
 /*
@@ -562,14 +562,14 @@ static int __init inflate_codes(
 md = mask_bits[bd];
 for (;;)  /* do until end of block */
 {
-NEEDBITS((unsigned)bl);
+NEEDBITS(s, (unsigned)bl);
 if ((e = (t = tl + ((unsigned)b & ml))->e) > 16)
 do {
 if (e == 99)
 return 1;
 DUMPBITS(t->b);
 e -= 16;
-NEEDBITS(e);
+NEEDBITS(s, e);
 } while ((e = (t = t->v.t + ((unsigned)b & mask_bits[e]))->e) > 
16);
 DUMPBITS(t->b);
 if (e == 16)/* then it's a literal */
@@ -590,22 +590,22 @@ static int __init inflate_codes(
 break;
 
 /* get length of block to copy */
-NEEDBITS(e);
+NEEDBITS(s, e);
 n = t->v.n + ((unsigned)b & mask_bits[e]);
 DUMPBITS(e);
 
 /* decode distance of block to copy */
-NEEDBITS((unsigned)bd);
+NEEDBITS(s, (unsigned)bd);
 if ((e = (t = td + ((unsigned)b & md))->e) > 16)
 do {
 if (e == 99)
 return 1;
 DUMPBITS(t->b);
 e -= 16;
-NEEDBITS(e);
+NEEDBITS(s, e);
 } while ((e = (t = t->v.t + ((unsigned)b & mask_bits[e]))->e) 
> 16);
 DUMPBITS(t->b);
-NEEDBITS(e);
+NEEDBITS(s, e);
 d = w - t->v.n - ((unsigned)b & mask_bits[e]);
 DUMPBITS(e);
 Tracevv((stderr,"\\[%d,%d]", w-d, n));
@@ -665,10 +665,10 @@ static int __init inflate_stored(struct gunzip_state *s)
 
 
 /* get the length and its complement */
-NEEDBITS(16);
+NEEDBITS(s, 16);
 n = ((unsigned)b & 0x);
 DUMPBITS(16);
-NEEDBITS(16);
+NEEDBITS(s, 16);
 if (n != (unsigned)((~b) & 0x))
 return 1;   /* error in compressed data */
 DUMPBITS(16);
@@ -676,7 +676,7 @@ static int __init inflate_stored(struct gunzip_state *s)
 /* read and output the compressed data */
 while (n--)
 {
-NEEDBITS(8);
+NEEDBITS(s, 8);
 s->window[w++] = (uch)b;
 if (w == WSIZE)
 {
@@ -798,13 +798,13 @@ static int noinline __init inflate_dynamic(struct 
gunzip_state *s)
 k = bk;
 
 /* read in table lengths */
-NEEDBITS(5);
+NEEDBITS(s, 5);
 nl = 257 + ((unsigned)b & 0x1f);  /* number of literal/length codes */
 DUMPBITS(5);
-NEEDBITS(5);
+NEEDBITS(s, 5);
 nd = 1 + ((unsigned)b & 0x1f);/* 

[PATCH v3 6/8] gzip: move output buffer into gunzip state

2024-04-24 Thread Daniel P. Smith
Signed-off-by: Daniel P. Smith 
---
 xen/common/gzip/gunzip.c  | 7 ---
 xen/common/gzip/inflate.c | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index 3fb9589b069e..95d924d36726 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -15,6 +15,8 @@ struct gunzip_state {
 size_t insize;
 /* Index of next byte to be processed in inbuf: */
 unsigned int inptr;
+
+unsigned long bytes_out;
 };
 
 #define malloc(a)   xmalloc_bytes(a)
@@ -42,7 +44,6 @@ typedef unsigned long   ulg;
 #  define Tracecv(c, x)
 #endif
 
-static long __initdata bytes_out;
 static void flush_window(struct gunzip_state *s);
 
 static __init void error(const char *x)
@@ -80,7 +81,7 @@ static __init void flush_window(struct gunzip_state *s)
 }
 crc = c;
 
-bytes_out += (unsigned long)s->wp;
+s->bytes_out += (unsigned long)s->wp;
 s->wp = 0;
 }
 
@@ -113,7 +114,7 @@ __init int perform_gunzip(char *output, char *image, 
unsigned long image_len)
 s->inbuf = (unsigned char *)image;
 s->insize = image_len;
 s->inptr = 0;
-bytes_out = 0;
+s->bytes_out = 0;
 
 makecrc();
 
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index f1a3b04cef8f..bec8801df487 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -1196,7 +1196,7 @@ static int __init gunzip(struct gunzip_state *s)
 error("crc error");
 return -1;
 }
-if (orig_len != bytes_out) {
+if (orig_len != s->bytes_out) {
 error("length error");
 return -1;
 }
-- 
2.30.2




[PATCH v3 2/8] gzip: refactor and expand macros

2024-04-24 Thread Daniel P. Smith
This commit refactors macros into proper static functions. It in-place expands
the `flush_output` macro, allowing the clear removal of the dead code
underneath the `underrun` label.

Signed-off-by: Daniel P. Smith 
---
 xen/common/gzip/gunzip.c  | 14 +
 xen/common/gzip/inflate.c | 61 ++-
 2 files changed, 30 insertions(+), 45 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index d07c451cd875..b7cadadcca8b 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -25,8 +25,6 @@ typedef unsigned char   uch;
 typedef unsigned short  ush;
 typedef unsigned long   ulg;
 
-#define get_byte()  (inptr < insize ? inbuf[inptr++] : fill_inbuf())
-
 /* Diagnostic functions */
 #ifdef DEBUG
 #  define Assert(cond, msg) do { if (!(cond)) error(msg); } while (0)
@@ -52,10 +50,14 @@ static __init void error(const char *x)
 panic("%s\n", x);
 }
 
-static __init int fill_inbuf(void)
-{
-error("ran out of input data");
-return 0;
+static __init uch get_byte(void) {
+if ( inptr >= insize )
+{
+error("ran out of input data");
+return 0; /* should never reach */
+}
+
+return inbuf[inptr++];
 }
 
 #include "inflate.c"
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index 220d2ff4d9d9..02a395aeb86a 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -119,6 +119,18 @@ static char rcsid[] = "#Id: inflate.c,v 0.14 1993/06/10 
13:27:04 jloup Exp #";
 
 #endif /* !__XEN__ */
 
+/*
+ * The inflate algorithm uses a sliding 32 K byte window on the uncompressed
+ * stream to find repeated byte strings.  This is implemented here as a
+ * circular buffer.  The index is updated simply by incrementing and then
+ * ANDing with 0x7fff (32K-1).
+ *
+ * It is left to other modules to supply the 32 K area.  It is assumed
+ * to be usable as if it were declared "uch slide[32768];" or as just
+ * "uch *slide;" and then malloc'ed in the latter case.  The definition
+ * must be in unzip.h, included above.
+ */
+#define wp outcnt
 #define slide window
 
 /*
@@ -150,21 +162,6 @@ static int inflate_dynamic(void);
 static int inflate_block(int *);
 static int inflate(void);
 
-/*
- * The inflate algorithm uses a sliding 32 K byte window on the uncompressed
- * stream to find repeated byte strings.  This is implemented here as a
- * circular buffer.  The index is updated simply by incrementing and then
- * ANDing with 0x7fff (32K-1).
- *
- * It is left to other modules to supply the 32 K area.  It is assumed
- * to be usable as if it were declared "uch slide[32768];" or as just
- * "uch *slide;" and then malloc'ed in the latter case.  The definition
- * must be in unzip.h, included above.
- */
-/* unsigned wp; current position in slide */
-#define wp outcnt
-#define flush_output(w) (wp=(w),flush_window())
-
 /* Tables for deflate from PKZIP's appnote.txt. */
 static const unsigned border[] = {/* Order of the bit length code lengths 
*/
 16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15};
@@ -224,7 +221,7 @@ static const ush mask_bits[] = {
 0x01ff, 0x03ff, 0x07ff, 0x0fff, 0x1fff, 0x3fff, 0x7fff, 0x
 };
 
-#define NEXTBYTE()  ({ int v = get_byte(); if (v < 0) goto underrun; (uch)v; })
+#define NEXTBYTE()  (get_byte()) /* get_byte will panic on failure */
 #define NEEDBITS(n) {while(k<(n)){b|=((ulg)NEXTBYTE())<>=(n);k-=(n);}
 
@@ -582,7 +579,8 @@ static int __init inflate_codes(
 Tracevv((stderr, "%c", slide[w-1]));
 if (w == WSIZE)
 {
-flush_output(w);
+wp = w;
+flush_window();
 w = 0;
 }
 }
@@ -629,7 +627,8 @@ static int __init inflate_codes(
 } while (--e);
 if (w == WSIZE)
 {
-flush_output(w);
+wp = w;
+flush_window();
 w = 0;
 }
 } while (n);
@@ -643,9 +642,6 @@ static int __init inflate_codes(
 
 /* done */
 return 0;
-
- underrun:
-return 4;   /* Input underrun */
 }
 
 /* "decompress" an inflated type 0 (stored) block. */
@@ -685,7 +681,8 @@ static int __init inflate_stored(void)
 slide[w++] = (uch)b;
 if (w == WSIZE)
 {
-flush_output(w);
+wp = w;
+flush_window();
 w = 0;
 }
 DUMPBITS(8);
@@ -698,9 +695,6 @@ static int __init inflate_stored(void)
 
 DEBG(">");
 return 0;
-
- underrun:
-return 4;   /* Input underrun */
 }
 
 
@@ -956,10 +950,6 @@ static int noinline __init inflate_dynamic(void)
  out:
 free(ll);
 return ret;
-
- underrun:
-ret = 4;   /* Input underrun */
-goto out;
 }
 
 /*
@@ -1005,9 +995,6 @@ static int __init inflate_block(int *e)
 
 /* bad block type */
 return 2;
-
- underrun:
-return 4;   /* Input 

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

2024-04-24 Thread osstest service owner
flight 185787 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/185787/

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  6c04a0bf2c5824616c0bbc44bc4f07c64a5469be
baseline version:
 xen  410ef3343924b5a3928bbe8e392491992b322cf0

Last test of basis   185781  2024-04-24 02:00:26 Z0 days
Testing same since   185787  2024-04-24 16:02:12 Z0 days1 attempts


People who touched revisions under test:
  George Dunlap 
  Jan Beulich 
  Nick Rosbrook 
  Tobias Fitschen 

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
   410ef33439..6c04a0bf2c  6c04a0bf2c5824616c0bbc44bc4f07c64a5469be -> smoke



[PATCH 0/7] x86: Make MAX_ALTP2M configurable

2024-04-24 Thread Petr Beneš
From: Petr Beneš 

This series introduces the ability to configure the maximum number of altp2m
tables during domain creation. Previously, the limits were hardcoded to a
maximum of 10. This change allows for greater flexibility in environments that
require more or fewer altp2m views.

Adjustments include:
- "Prepare" commits with style changes.
- Adding a new `max_altp2m` parameter to xl.
- Adding a new `max_altp2m` parameter to the OCaml bindings.
- Adding a new `max_altp2m` parameter to the xl.cfg manual.
- Introducing `max_altp2m` into `xen_domctl_createdomain`, which, after sanity
  checks, is stored in newly introduced `max_altp2m` field of `struct domain` -
  leaving room for other architectures to implement the altp2m feature.
- Replacing MAX_ALTP2M macro occurrences with `domain->max_altp2m`.
- Finally, adjusting the initial allocation of pages in `hap_enable` from 256
  to 1024 pages to accommodate potentially larger `max_altp2m` values (i.e.,
  maximum of 512).

This enhancement is particularly relevant for users leveraging Xen's features
for virtual machine introspection.

Petr Beneš (7):
  x86/p2m: Add braces for better code clarity
  x86/hap: Refactor boolean field assignments
  tools/xl: Add max_altp2m parameter
  tools/ocaml: Add max_altp2m parameter
  docs/man: Add max_altp2m parameter to the xl.cfg manual
  x86: Make the maximum number of altp2m views configurable
  x86/hap: Increase the number of initial mempool_size to 1024 pages

 docs/man/xl.cfg.5.pod.in  | 14 +
 tools/golang/xenlight/helpers.gen.go  |  2 +
 tools/golang/xenlight/types.gen.go|  1 +
 tools/include/libxl.h |  8 +++
 tools/libs/light/libxl_create.c   |  9 
 tools/libs/light/libxl_types.idl  |  1 +
 tools/ocaml/libs/xc/xenctrl.ml|  1 +
 tools/ocaml/libs/xc/xenctrl.mli   |  1 +
 tools/ocaml/libs/xc/xenctrl_stubs.c   | 17 +++---
 .../paging-mempool/test-paging-mempool.c  |  2 +-
 tools/xl/xl_parse.c   |  4 ++
 xen/arch/x86/domain.c |  6 +++
 xen/arch/x86/hvm/hvm.c|  8 ++-
 xen/arch/x86/hvm/vmx/vmx.c|  2 +-
 xen/arch/x86/include/asm/domain.h |  7 ++-
 xen/arch/x86/include/asm/p2m.h|  4 +-
 xen/arch/x86/mm/altp2m.c  | 27 +-
 xen/arch/x86/mm/hap/hap.c | 12 ++---
 xen/arch/x86/mm/mem_access.c  | 14 ++---
 xen/arch/x86/mm/mem_sharing.c |  2 +-
 xen/arch/x86/mm/p2m-ept.c |  6 +--
 xen/arch/x86/mm/p2m.c | 54 ++-
 xen/common/domain.c   |  7 +++
 xen/include/public/domctl.h   |  1 +
 xen/include/xen/sched.h   |  2 +
 25 files changed, 152 insertions(+), 60 deletions(-)

-- 
2.34.1




[PATCH 5/7] docs/man: Add max_altp2m parameter to the xl.cfg manual

2024-04-24 Thread Petr Beneš
From: Petr Beneš 

Update manual pages to include detailed information about the max_altp2m
configuration parameter.

Signed-off-by: Petr Beneš 
---
 docs/man/xl.cfg.5.pod.in | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 8f2b375ce9..2d4ea35736 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -2039,6 +2039,20 @@ a single guest HVM domain. B: While the option "altp2mhvm" is deprecated, legacy applications for
 x86 systems will continue to work using it.
 
+=item B
+
+Specifies the maximum number of alternate-p2m views available to the guest.
+This setting is crucial in domain introspection scenarios that require
+multiple physical-to-machine (p2m) memory mappings to be established
+simultaneously.
+
+Enabling multiple p2m views may increase memory usage. It is advisable to
+review and adjust the B setting as necessary to accommodate
+the additional memory requirements.
+
+B: This option is ignored if B is disabled. The default value
+is 10.
+
 =item B
 
 Enable or disables guest access to hardware virtualisation features,
-- 
2.34.1




[PATCH 4/7] tools/ocaml: Add max_altp2m parameter

2024-04-24 Thread Petr Beneš
From: Petr Beneš 

Allow developers using the OCaml bindings to set the max_altp2m parameter.

Signed-off-by: Petr Beneš 
---
 tools/ocaml/libs/xc/xenctrl.ml  |  1 +
 tools/ocaml/libs/xc/xenctrl.mli |  1 +
 tools/ocaml/libs/xc/xenctrl_stubs.c | 17 ++---
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/tools/ocaml/libs/xc/xenctrl.ml b/tools/ocaml/libs/xc/xenctrl.ml
index 55923857ec..ed851bb071 100644
--- a/tools/ocaml/libs/xc/xenctrl.ml
+++ b/tools/ocaml/libs/xc/xenctrl.ml
@@ -82,6 +82,7 @@ type domctl_create_config =
 iommu_opts: domain_create_iommu_opts list;
 max_vcpus: int;
 max_evtchn_port: int;
+max_altp2m: int;
 max_grant_frames: int;
 max_maptrack_frames: int;
 max_grant_version: int;
diff --git a/tools/ocaml/libs/xc/xenctrl.mli b/tools/ocaml/libs/xc/xenctrl.mli
index 9b4b45db3a..971b269d85 100644
--- a/tools/ocaml/libs/xc/xenctrl.mli
+++ b/tools/ocaml/libs/xc/xenctrl.mli
@@ -74,6 +74,7 @@ type domctl_create_config = {
   iommu_opts: domain_create_iommu_opts list;
   max_vcpus: int;
   max_evtchn_port: int;
+  max_altp2m: int;
   max_grant_frames: int;
   max_maptrack_frames: int;
   max_grant_version: int;
diff --git a/tools/ocaml/libs/xc/xenctrl_stubs.c 
b/tools/ocaml/libs/xc/xenctrl_stubs.c
index 2b6d3c09df..0b70cc9b08 100644
--- a/tools/ocaml/libs/xc/xenctrl_stubs.c
+++ b/tools/ocaml/libs/xc/xenctrl_stubs.c
@@ -207,12 +207,13 @@ CAMLprim value stub_xc_domain_create(value xch_val, value 
wanted_domid, value co
 #define VAL_IOMMU_OPTS  Field(config, 3)
 #define VAL_MAX_VCPUS   Field(config, 4)
 #define VAL_MAX_EVTCHN_PORT Field(config, 5)
-#define VAL_MAX_GRANT_FRAMESField(config, 6)
-#define VAL_MAX_MAPTRACK_FRAMES Field(config, 7)
-#define VAL_MAX_GRANT_VERSION   Field(config, 8)
-#define VAL_VMTRACE_BUF_KB  Field(config, 9)
-#define VAL_CPUPOOL_ID  Field(config, 10)
-#define VAL_ARCHField(config, 11)
+#define VAL_MAX_ALTP2M  Field(config, 6)
+#define VAL_MAX_GRANT_FRAMESField(config, 7)
+#define VAL_MAX_MAPTRACK_FRAMES Field(config, 8)
+#define VAL_MAX_GRANT_VERSION   Field(config, 9)
+#define VAL_VMTRACE_BUF_KB  Field(config, 10)
+#define VAL_CPUPOOL_ID  Field(config, 11)
+#define VAL_ARCHField(config, 12)
 
uint32_t domid = Int_val(wanted_domid);
uint64_t vmtrace_size = Int32_val(VAL_VMTRACE_BUF_KB);
@@ -226,6 +227,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value 
wanted_domid, value co
.ssidref = Int32_val(VAL_SSIDREF),
.max_vcpus = Int_val(VAL_MAX_VCPUS),
.max_evtchn_port = Int_val(VAL_MAX_EVTCHN_PORT),
+   .max_altp2m = Int_val(VAL_MAX_ALTP2M),
.max_grant_frames = Int_val(VAL_MAX_GRANT_FRAMES),
.max_maptrack_frames = Int_val(VAL_MAX_MAPTRACK_FRAMES),
.grant_opts =
@@ -257,7 +259,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value 
wanted_domid, value co
 #if defined(__i386__) || defined(__x86_64__)
 
/* Quick & dirty check for ABI changes. */
-   BUILD_BUG_ON(sizeof(cfg) != 64);
+   BUILD_BUG_ON(sizeof(cfg) != 68);
 
 /* Mnemonics for the named fields inside xen_x86_arch_domainconfig */
 #define VAL_EMUL_FLAGS  Field(arch_domconfig, 0)
@@ -291,6 +293,7 @@ CAMLprim value stub_xc_domain_create(value xch_val, value 
wanted_domid, value co
 #undef VAL_MAX_GRANT_VERSION
 #undef VAL_MAX_MAPTRACK_FRAMES
 #undef VAL_MAX_GRANT_FRAMES
+#undef VAL_MAX_ALTP2M
 #undef VAL_MAX_EVTCHN_PORT
 #undef VAL_MAX_VCPUS
 #undef VAL_IOMMU_OPTS
-- 
2.34.1




[PATCH 1/7] x86/p2m: Add braces for better code clarity

2024-04-24 Thread Petr Beneš
From: Petr Beneš 

No functional change.

Signed-off-by: Petr Beneš 
---
 xen/arch/x86/mm/p2m.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index ce742c12e0..eb7996170d 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -106,6 +106,7 @@ void p2m_change_entry_type_global(struct domain *d,
 unsigned int i;
 
 for ( i = 0; i < MAX_ALTP2M; i++ )
+{
 if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
 {
 struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
@@ -114,6 +115,7 @@ void p2m_change_entry_type_global(struct domain *d,
 change_entry_type_global(altp2m, ot, nt);
 p2m_unlock(altp2m);
 }
+}
 }
 
 p2m_unlock(hostp2m);
@@ -139,6 +141,7 @@ void p2m_memory_type_changed(struct domain *d)
 unsigned int i;
 
 for ( i = 0; i < MAX_ALTP2M; i++ )
+{
 if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
 {
 struct p2m_domain *altp2m = d->arch.altp2m_p2m[i];
@@ -147,6 +150,7 @@ void p2m_memory_type_changed(struct domain *d)
 _memory_type_changed(altp2m);
 p2m_unlock(altp2m);
 }
+}
 }
 
 p2m_unlock(hostp2m);
-- 
2.34.1




[PATCH 7/7] x86/hap: Increase the number of initial mempool_size to 1024 pages

2024-04-24 Thread Petr Beneš
From: Petr Beneš 

This change anticipates scenarios where `max_altp2m` is set to its maximum
supported value (i.e., 512), ensuring sufficient memory is allocated upfront
to accommodate all altp2m tables without initialization failure.

Signed-off-by: Petr Beneš 
---
 tools/tests/paging-mempool/test-paging-mempool.c | 2 +-
 xen/arch/x86/mm/hap/hap.c| 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/tests/paging-mempool/test-paging-mempool.c 
b/tools/tests/paging-mempool/test-paging-mempool.c
index 1ebc13455a..91b06fa0cf 100644
--- a/tools/tests/paging-mempool/test-paging-mempool.c
+++ b/tools/tests/paging-mempool/test-paging-mempool.c
@@ -35,7 +35,7 @@ static struct xen_domctl_createdomain create = {
 
 static uint64_t default_mempool_size_bytes =
 #if defined(__x86_64__) || defined(__i386__)
-256 << 12; /* Only x86 HAP for now.  x86 Shadow needs more work. */
+1024 << 12; /* Only x86 HAP for now.  x86 Shadow needs more work. */
 #elif defined (__arm__) || defined(__aarch64__)
 16 << 12;
 #endif
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 7aff5fa664..fab7e256a4 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -468,7 +468,7 @@ int hap_enable(struct domain *d, u32 mode)
 if ( old_pages == 0 )
 {
 paging_lock(d);
-rv = hap_set_allocation(d, 256, NULL);
+rv = hap_set_allocation(d, 1024, NULL);
 if ( rv != 0 )
 {
 hap_set_allocation(d, 0, NULL);
-- 
2.34.1




[PATCH 2/7] x86/hap: Refactor boolean field assignments

2024-04-24 Thread Petr Beneš
From: Petr Beneš 

No functional change.

Signed-off-by: Petr Beneš 
---
 xen/arch/x86/mm/hap/hap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c
index 9f964c1d87..d2011fde24 100644
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -522,7 +522,7 @@ int hap_enable(struct domain *d, u32 mode)
goto out;
 }
 
-d->arch.altp2m_active = 0;
+d->arch.altp2m_active = false;
 }
 
 /* Now let other users see the new mode */
@@ -585,7 +585,7 @@ void hap_teardown(struct domain *d, bool *preempted)
 for_each_vcpu ( d, v )
 altp2m_vcpu_disable_ve(v);
 
-d->arch.altp2m_active = 0;
+d->arch.altp2m_active = false;
 
 FREE_XENHEAP_PAGE(d->arch.altp2m_eptp);
 FREE_XENHEAP_PAGE(d->arch.altp2m_visible_eptp);
-- 
2.34.1




[PATCH v3 7/8] gzip: move bitbuffer into gunzip state

2024-04-24 Thread Daniel P. Smith
Signed-off-by: Daniel P. Smith 
---
 xen/common/gzip/gunzip.c  |  3 +++
 xen/common/gzip/inflate.c | 43 ++-
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index 95d924d36726..0043ff8ac886 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -17,6 +17,9 @@ struct gunzip_state {
 unsigned int inptr;
 
 unsigned long bytes_out;
+
+unsigned long bb;  /* bit buffer */
+unsigned int  bk;  /* bits in bit buffer */
 };
 
 #define malloc(a)   xmalloc_bytes(a)
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index bec8801df487..8da14880cfbe 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -211,9 +211,6 @@ static const ush cpdext[] = { /* Extra bits for 
distance codes */
  * the stream.
  */
 
-static ulg __initdata bb;/* bit buffer */
-static unsigned __initdata bk;   /* bits in bit buffer */
-
 static const ush mask_bits[] = {
 0x,
 0x0001, 0x0003, 0x0007, 0x000f, 0x001f, 0x003f, 0x007f, 0x00ff,
@@ -553,8 +550,8 @@ static int __init inflate_codes(
 
 
 /* make local copies of globals */
-b = bb;   /* initialize bit buffer */
-k = bk;
+b = s->bb;/* initialize bit buffer */
+k = s->bk;
 w = s->wp;/* initialize window position */
 
 /* inflate the coded data */
@@ -636,8 +633,8 @@ static int __init inflate_codes(
 
 /* restore the globals from the locals */
 s->wp = w;/* restore global window pointer */
-bb = b;   /* restore global bit buffer */
-bk = k;
+s->bb = b;/* restore global bit buffer */
+s->bk = k;
 
 /* done */
 return 0;
@@ -654,8 +651,8 @@ static int __init inflate_stored(struct gunzip_state *s)
 DEBG("bb;/* initialize bit buffer */
+k = s->bk;
 w = s->wp;/* initialize window position */
 
 
@@ -689,8 +686,8 @@ static int __init inflate_stored(struct gunzip_state *s)
 
 /* restore the globals from the locals */
 s->wp = w;/* restore global window pointer */
-bb = b;   /* restore global bit buffer */
-bk = k;
+s->bb = b;/* restore global bit buffer */
+s->bk = k;
 
 DEBG(">");
 return 0;
@@ -794,8 +791,8 @@ static int noinline __init inflate_dynamic(struct 
gunzip_state *s)
 return 1;
 
 /* make local bit buffer */
-b = bb;
-k = bk;
+b = s->bb;
+k = s->bk;
 
 /* read in table lengths */
 NEEDBITS(s, 5);
@@ -899,8 +896,8 @@ static int noinline __init inflate_dynamic(struct 
gunzip_state *s)
 DEBG("dyn5 ");
 
 /* restore the global bit buffer */
-bb = b;
-bk = k;
+s->bb = b;
+s->bk = k;
 
 DEBG("dyn5a ");
 
@@ -965,8 +962,8 @@ static int __init inflate_block(struct gunzip_state *s, int 
*e)
 DEBG("bb;
+k = s->bk;
 
 /* read in last block bit */
 NEEDBITS(s, 1);
@@ -979,8 +976,8 @@ static int __init inflate_block(struct gunzip_state *s, int 
*e)
 DUMPBITS(2);
 
 /* restore the global bit buffer */
-bb = b;
-bk = k;
+s->bb = b;
+s->bk = k;
 
 /* inflate that block type */
 if (t == 2)
@@ -1004,8 +1001,8 @@ static int __init inflate(struct gunzip_state *s)
 
 /* initialize window, bit buffer */
 s->wp = 0;
-bk = 0;
-bb = 0;
+s->bk = 0;
+s->bb = 0;
 
 /* decompress until the last block */
 do {
@@ -1017,8 +1014,8 @@ static int __init inflate(struct gunzip_state *s)
 /* Undo too much lookahead. The next read will be byte aligned so we
  * can discard unused bits in the last meaningful byte.
  */
-while (bk >= 8) {
-bk -= 8;
+while (s->bk >= 8) {
+s->bk -= 8;
 s->inptr--;
 }
 
-- 
2.30.2




[PATCH v3 8/8] gzip: move crc state into gunzip state

2024-04-24 Thread Daniel P. Smith
Move the crc and its state into struct gunzip_state. In the process, expand the
only use of CRC_VALUE as it is hides what is being compared.

Signed-off-by: Daniel P. Smith 
---
 xen/common/gzip/gunzip.c  | 11 +++
 xen/common/gzip/inflate.c | 14 +-
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index 0043ff8ac886..26d2a4aa9d36 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -20,6 +20,9 @@ struct gunzip_state {
 
 unsigned long bb;  /* bit buffer */
 unsigned int  bk;  /* bits in bit buffer */
+
+uint32_t crc_32_tab[256];
+uint32_t crc;
 };
 
 #define malloc(a)   xmalloc_bytes(a)
@@ -72,7 +75,7 @@ static __init void flush_window(struct gunzip_state *s)
  * The window is equal to the output buffer therefore only need to
  * compute the crc.
  */
-unsigned long c = crc;
+unsigned int c = s->crc;
 unsigned int n;
 unsigned char *in, ch;
 
@@ -80,9 +83,9 @@ static __init void flush_window(struct gunzip_state *s)
 for ( n = 0; n < s->wp; n++ )
 {
 ch = *in++;
-c = crc_32_tab[((int)c ^ ch) & 0xff] ^ (c >> 8);
+c = s->crc_32_tab[((int)c ^ ch) & 0xff] ^ (c >> 8);
 }
-crc = c;
+s->crc = c;
 
 s->bytes_out += (unsigned long)s->wp;
 s->wp = 0;
@@ -119,7 +122,7 @@ __init int perform_gunzip(char *output, char *image, 
unsigned long image_len)
 s->inptr = 0;
 s->bytes_out = 0;
 
-makecrc();
+makecrc(s);
 
 if ( gunzip(s) < 0 )
 {
diff --git a/xen/common/gzip/inflate.c b/xen/common/gzip/inflate.c
index 8da14880cfbe..dc940e59d853 100644
--- a/xen/common/gzip/inflate.c
+++ b/xen/common/gzip/inflate.c
@@ -1032,16 +1032,12 @@ static int __init inflate(struct gunzip_state *s)
  *
  **/
 
-static ulg __initdata crc_32_tab[256];
-static ulg __initdata crc;  /* initialized in makecrc() so it'll reside in bss 
*/
-#define CRC_VALUE (crc ^ 0xUL)
-
 /*
  * Code to compute the CRC-32 table. Borrowed from
  * gzip-1.0.3/makecrc.c.
  */
 
-static void __init makecrc(void)
+static void __init makecrc(struct gunzip_state *s)
 {
 /* Not copyrighted 1990 Mark Adler */
 
@@ -1058,7 +1054,7 @@ static void __init makecrc(void)
 for (i = 0; i < sizeof(p)/sizeof(int); i++)
 e |= 1L << (31 - p[i]);
 
-crc_32_tab[0] = 0;
+s->crc_32_tab[0] = 0;
 
 for (i = 1; i < 256; i++)
 {
@@ -1069,11 +1065,11 @@ static void __init makecrc(void)
 if (k & 1)
 c ^= e;
 }
-crc_32_tab[i] = c;
+s->crc_32_tab[i] = c;
 }
 
 /* this is initialized here so this code could reside in ROM */
-crc = (ulg)0xUL; /* shift register contents */
+s->crc = (ulg)~0u; /* shift register contents */
 }
 
 /* gzip flag byte */
@@ -1189,7 +1185,7 @@ static int __init gunzip(struct gunzip_state *s)
 orig_len |= (ulg) NEXTBYTE(s) << 24;
 
 /* Validate decompression */
-if (orig_crc != CRC_VALUE) {
+if (orig_crc != (s->crc ^ ~0u)) {
 error("crc error");
 return -1;
 }
-- 
2.30.2




Re: [PATCH v4 4/6] xen/ppc: Enable bootfdt and boot allocator

2024-04-24 Thread Julien Grall

Hi Shawn,

On 12/04/2024 04:55, Shawn Anastasio wrote:

Enable usage of bootfdt for populating the boot info struct from the
firmware-provided device tree.  Also enable the Xen boot page allocator.

Additionally, modify bootfdt.c's boot_fdt_info() to tolerate the
scenario in which the FDT overlaps a reserved memory region, as is the
case on PPC when booted directly from skiboot. Since this means that Xen
can now boot without a BOOTMOD_FDT present in bootinfo, clarify this
fact in a comment above BOOTMOD_FDT's definition.

Signed-off-by: Shawn Anastasio 
---
Changes in v4:
   - drop unnecessary libfdt.h include in setup.c
   - make boot_fdt and xen_bootmodule const
   - more clearly document that BOOTMOD_FDT is now optional
   - add explicit (void) cast to BOOTMOD_FDT creation

  xen/arch/ppc/setup.c | 22 +-
  xen/common/device-tree/bootfdt.c | 11 +--
  xen/include/xen/bootfdt.h|  7 +++
  3 files changed, 37 insertions(+), 3 deletions(-)


The changes look overall good. I have a few remark belows. But you can 
have my acked-by with or without the NIT addressed:


Acked-by: Julien Grall 



diff --git a/xen/arch/ppc/setup.c b/xen/arch/ppc/setup.c
index 101bdd8bb6..47e997969f 100644
--- a/xen/arch/ppc/setup.c
+++ b/xen/arch/ppc/setup.c
@@ -1,12 +1,15 @@
  /* SPDX-License-Identifier: GPL-2.0-or-later */
  #include 
  #include 
+#include 
+#include 


We are tryying to keep the header order alphabetically. It looks like 
the existing code is respecting that. So can this be done for the two 
new headers?



  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
+#include 

  /* Xen stack for bringing up the first CPU. */
  unsigned char __initdata cpu0_boot_stack[STACK_SIZE] __aligned(STACK_SIZE);
@@ -24,6 +27,9 @@ void __init noreturn start_xen(unsigned long r3, unsigned 
long r4,
 unsigned long r5, unsigned long r6,
 unsigned long r7)
  {
+const void *boot_fdt;
+const struct bootmodule *xen_bootmodule;
+
  if ( r5 )
  {
  /* Unsupported OpenFirmware boot protocol */
@@ -32,11 +38,25 @@ void __init noreturn start_xen(unsigned long r3, unsigned 
long r4,
  else
  {
  /* kexec boot protocol */
-boot_opal_init((void *)r3);
+boot_fdt = (void *)r3;


NIT: Sorry I should have noticed it earlier. boot_fdt is only used to ..


+boot_opal_init(boot_fdt);
  }

  setup_exceptions();

+device_tree_flattened = boot_fdt;


... set device_tree_flattened and ...


+boot_fdt_info(boot_fdt, r3);



... used here. Is there any plan to have device_tree_flattened != 
boot_fdt? If not, then I would suggest to simply drop boot_fdt.



+
+/*
+ * Xen relocates itself at the ppc64 entrypoint, so we need to manually 
mark
+ * the kernel module.
+ */
+xen_bootmodule = add_boot_module(BOOTMOD_XEN, __pa(_start),
+ PAGE_ALIGN(__pa(_end)), false);
+BUG_ON(!xen_bootmodule);
+
+populate_boot_allocator();
+
  setup_initial_pagetables();

  early_printk("Hello, ppc64le!\n");
diff --git a/xen/common/device-tree/bootfdt.c b/xen/common/device-tree/bootfdt.c
index f01a5b5d76..76d0f72ef9 100644
--- a/xen/common/device-tree/bootfdt.c
+++ b/xen/common/device-tree/bootfdt.c
@@ -542,12 +542,19 @@ size_t __init boot_fdt_info(const void *fdt, paddr_t 
paddr)
  if ( ret < 0 )
  panic("No valid device tree\n");

-add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);
-
  ret = device_tree_for_each_node(fdt, 0, early_scan_node, NULL);
  if ( ret )
  panic("Early FDT parsing failed (%d)\n", ret);

+/*
+ * Add module for the FDT itself after the device tree has been parsed. 
This
+ * is required on ppc64le where the device tree passed to Xen may have been
+ * allocated by skiboot, in which case it will exist within a reserved
+ * region and this call will fail. This is fine, however, since either way
+ * the allocator will know not to step on the device tree.
+ */
+(void)add_boot_module(BOOTMOD_FDT, paddr, fdt_totalsize(fdt), false);
+
  /*
   * On Arm64 setup_directmap_mappings() expects to be called with the 
lowest
   * bank in memory first. There is no requirement that the DT will provide
diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
index 577618da16..ea3ad96bb9 100644
--- a/xen/include/xen/bootfdt.h
+++ b/xen/include/xen/bootfdt.h
@@ -13,7 +13,14 @@

  typedef enum {
  BOOTMOD_XEN,
+
+/*
+ * The BOOTMOD_FDT type will only be present when the firmware-provided FDT
+ * blob exists outside of a reserved memory region which is platform-
+ * dependent and may not be relied upon.
+ */
  BOOTMOD_FDT,
+
  BOOTMOD_KERNEL,
  BOOTMOD_RAMDISK,
  BOOTMOD_XSM,
--
2.30.2



Cheers,

--
Julien Grall



[PATCH v3] xen/x86/pvh: handle ACPI RSDT table in PVH Dom0 build

2024-04-24 Thread Daniel P. Smith
From: Stefano Stabellini 

Xen always generates as XSDT table even if the firmware provided an RSDT table.
Copy the RSDT header from the firmware table, adjusting the signature, for the
XSDT table when not provided by the firmware.

Fixes: 1d74282c455f ('x86: setup PVHv2 Dom0 ACPI tables')
Suggested-by: Roger Pau Monné 
Signed-off-by: Stefano Stabellini 
Signed-off-by: Daniel P. Smith 
---

This patch is used for development and testing of hyperlaunch using the QEMU
emulator. After dicussiong with Stefano, he was okay with me addressing Jan's
comment regarding the table signature and reposting for acceptance.

Changes in v3:
- ensure the constructed XSDT table always has the correct signature

---
 xen/arch/x86/hvm/dom0_build.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index ac71d43a6b3f..781aac00fe72 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -1077,7 +1077,16 @@ static int __init pvh_setup_acpi_xsdt(struct domain *d, 
paddr_t madt_addr,
 rc = -EINVAL;
 goto out;
 }
-xsdt_paddr = rsdp->xsdt_physical_address;
+/*
+ * Note the header is the same for both RSDT and XSDT, so it's fine to
+ * copy the native RSDT header to the Xen crafted XSDT if no native
+ * XSDT is available.
+ */
+if ( rsdp->revision > 1 && rsdp->xsdt_physical_address )
+xsdt_paddr = rsdp->xsdt_physical_address;
+else
+xsdt_paddr = rsdp->rsdt_physical_address;
+
 acpi_os_unmap_memory(rsdp, sizeof(*rsdp));
 table = acpi_os_map_memory(xsdt_paddr, sizeof(*table));
 if ( !table )
@@ -1089,6 +1098,9 @@ static int __init pvh_setup_acpi_xsdt(struct domain *d, 
paddr_t madt_addr,
 xsdt->header = *table;
 acpi_os_unmap_memory(table, sizeof(*table));
 
+/* In case the header is an RSDT copy, blindly ensure it has an XSDT sig */
+xsdt->header.signature[0] = 'X';
+
 /* Add the custom MADT. */
 xsdt->table_offset_entry[0] = madt_addr;
 
-- 
2.30.2




[libvirt test] 185783: tolerable all pass - PUSHED

2024-04-24 Thread osstest service owner
flight 185783 libvirt real [real]
http://logs.test-lab.xenproject.org/osstest/logs/185783/

Failures :-/ but no regressions.

Tests which did not succeed, but are not blocking:
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 185743
 test-amd64-amd64-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm 13 migrate-support-check 
fail never pass
 test-arm64-arm64-libvirt-xsm 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt 15 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-xsm 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt 16 saverestore-support-checkfail   never pass
 test-amd64-amd64-libvirt-raw 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt 15 migrate-support-checkfail   never pass
 test-amd64-amd64-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-arm64-arm64-libvirt-qcow2 14 migrate-support-checkfail never pass
 test-arm64-arm64-libvirt-qcow2 15 saverestore-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-armhf-armhf-libvirt-vhd 14 migrate-support-checkfail   never pass
 test-armhf-armhf-libvirt-vhd 15 saverestore-support-checkfail   never pass

version targeted for testing:
 libvirt  595b95cdde39a78528e584fdfd71a1d562ba9e45
baseline version:
 libvirt  c38720b337f74337ec94c0fe2e97a7c2c57188ae

Last test of basis   185743  2024-04-20 04:20:26 Z4 days
Testing same since   185783  2024-04-24 04:20:30 Z0 days1 attempts


People who touched revisions under test:
  Andi Chandler 
  Göran Uddeborg 

jobs:
 build-amd64-xsm  pass
 build-arm64-xsm  pass
 build-i386-xsm   pass
 build-amd64  pass
 build-arm64  pass
 build-armhf  pass
 build-i386   pass
 build-amd64-libvirt  pass
 build-arm64-libvirt  pass
 build-armhf-libvirt  pass
 build-i386-libvirt   pass
 build-amd64-pvopspass
 build-arm64-pvopspass
 build-armhf-pvopspass
 build-i386-pvops pass
 test-amd64-amd64-libvirt-qemuu-debianhvm-amd64-xsm   pass
 test-amd64-amd64-libvirt-xsm pass
 test-arm64-arm64-libvirt-xsm pass
 test-amd64-amd64-libvirt pass
 test-arm64-arm64-libvirt pass
 test-armhf-armhf-libvirt pass
 test-amd64-amd64-libvirt-pairpass
 test-amd64-amd64-libvirt-qcow2   pass
 test-arm64-arm64-libvirt-qcow2   pass
 test-amd64-amd64-libvirt-raw pass
 test-arm64-arm64-libvirt-raw pass
 test-amd64-amd64-libvirt-vhd pass
 test-armhf-armhf-libvirt-vhd pass



sg-report-flight on osstest.test-lab.xenproject.org
logs: /home/logs/logs
images: /home/logs/images

Logs, config files, etc. are available at
http://logs.test-lab.xenproject.org/osstest/logs

Explanation of these reports, and of osstest in general, is at
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README.email;hb=master
http://xenbits.xen.org/gitweb/?p=osstest.git;a=blob;f=README;hb=master

Test harness code can be found at
http://xenbits.xen.org/gitweb?p=osstest.git;a=summary


Pushing revision :

To xenbits.xen.org:/home/xen/git/libvirt.git
   c38720b337..595b95cdde  595b95cdde39a78528e584fdfd71a1d562ba9e45 -> 
xen-tested-master



[PATCH 6/7] x86: Make the maximum number of altp2m views configurable

2024-04-24 Thread Petr Beneš
From: Petr Beneš 

This commit introduces the ability to configure the maximum number of altp2m
tables during domain creation. Previously, the limits were hardcoded to a
maximum of 10. This change allows for greater flexibility in environments that
require more or fewer altp2m views.

Signed-off-by: Petr Beneš 
---
 xen/arch/x86/domain.c |  6 
 xen/arch/x86/hvm/hvm.c|  8 -
 xen/arch/x86/hvm/vmx/vmx.c|  2 +-
 xen/arch/x86/include/asm/domain.h |  7 ++---
 xen/arch/x86/include/asm/p2m.h|  4 +--
 xen/arch/x86/mm/altp2m.c  | 27 +++--
 xen/arch/x86/mm/hap/hap.c |  6 ++--
 xen/arch/x86/mm/mem_access.c  | 14 -
 xen/arch/x86/mm/mem_sharing.c |  2 +-
 xen/arch/x86/mm/p2m-ept.c |  6 ++--
 xen/arch/x86/mm/p2m.c | 50 +++
 xen/common/domain.c   |  7 +
 xen/include/xen/sched.h   |  2 ++
 13 files changed, 92 insertions(+), 49 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 4f851aa81f..95ae675ad0 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -679,6 +679,12 @@ int arch_sanitise_domain_config(struct 
xen_domctl_createdomain *config)
 return -EINVAL;
 }
 
+if ( config->max_altp2m > MAX_EPTP )
+{
+dprintk(XENLOG_INFO, "max_altp2m must be <= %u\n", MAX_EPTP);
+return -EINVAL;
+}
+
 if ( config->vmtrace_size )
 {
 unsigned int size = config->vmtrace_size;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 0ce45b177c..9b70fe7cfc 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4633,6 +4633,12 @@ static int do_altp2m_op(
 goto out;
 }
 
+if ( d->max_altp2m == 0 )
+{
+rc = -EINVAL;
+goto out;
+}
+
 if ( (rc = xsm_hvm_altp2mhvm_op(XSM_OTHER, d, mode, a.cmd)) )
 goto out;
 
@@ -5222,7 +5228,7 @@ void hvm_fast_singlestep(struct vcpu *v, uint16_t p2midx)
 if ( !hvm_is_singlestep_supported() )
 return;
 
-if ( p2midx >= MAX_ALTP2M )
+if ( p2midx >= v->domain->max_altp2m )
 return;
 
 v->arch.hvm.single_step = true;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 0935762378..eadde4dbcb 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4888,7 +4888,7 @@ bool asmlinkage vmx_vmenter_helper(const struct 
cpu_user_regs *regs)
 {
 unsigned int i;
 
-for ( i = 0; i < MAX_ALTP2M; ++i )
+for ( i = 0; i < currd->max_altp2m; ++i )
 {
 if ( currd->arch.altp2m_eptp[i] == mfn_x(INVALID_MFN) )
 continue;
diff --git a/xen/arch/x86/include/asm/domain.h 
b/xen/arch/x86/include/asm/domain.h
index f5daeb182b..5bb0bcae81 100644
--- a/xen/arch/x86/include/asm/domain.h
+++ b/xen/arch/x86/include/asm/domain.h
@@ -258,11 +258,10 @@ struct paging_vcpu {
 struct shadow_vcpu shadow;
 };
 
+#define INVALID_ALTP2M  0x
+#define MAX_EPTP((unsigned int)(PAGE_SIZE / sizeof(uint64_t)))
 #define MAX_NESTEDP2M 10
 
-#define MAX_ALTP2M  10 /* arbitrary */
-#define INVALID_ALTP2M  0x
-#define MAX_EPTP(PAGE_SIZE / sizeof(uint64_t))
 struct p2m_domain;
 struct time_scale {
 int shift;
@@ -353,7 +352,7 @@ struct arch_domain
 
 /* altp2m: allow multiple copies of host p2m */
 bool altp2m_active;
-struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
+struct p2m_domain **altp2m_p2m;
 mm_lock_t altp2m_list_lock;
 uint64_t *altp2m_eptp;
 uint64_t *altp2m_visible_eptp;
diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
index 111badf89a..2086bcb633 100644
--- a/xen/arch/x86/include/asm/p2m.h
+++ b/xen/arch/x86/include/asm/p2m.h
@@ -881,7 +881,7 @@ static inline struct p2m_domain *p2m_get_altp2m(struct vcpu 
*v)
 if ( index == INVALID_ALTP2M )
 return NULL;
 
-BUG_ON(index >= MAX_ALTP2M);
+BUG_ON(index >= v->domain->max_altp2m);
 
 return v->domain->arch.altp2m_p2m[index];
 }
@@ -891,7 +891,7 @@ static inline bool p2m_set_altp2m(struct vcpu *v, unsigned 
int idx)
 {
 struct p2m_domain *orig;
 
-BUG_ON(idx >= MAX_ALTP2M);
+BUG_ON(idx >= v->domain->max_altp2m);
 
 if ( idx == vcpu_altp2m(v).p2midx )
 return false;
diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
index a04297b646..c91e0fbfd1 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -13,6 +13,12 @@
 void
 altp2m_vcpu_initialise(struct vcpu *v)
 {
+struct domain *d = v->domain;
+
+/* Skip initialisation if no altp2m will be used. */
+if ( d->max_altp2m == 0 )
+return;
+
 if ( v != current )
 vcpu_pause(v);
 
@@ -28,8 +34,13 @@ altp2m_vcpu_initialise(struct vcpu *v)
 void
 altp2m_vcpu_destroy(struct vcpu *v)
 {
+struct domain *d = v->domain;
 struct p2m_domain *p2m;
 
+/* Skip destruction if no altp2m 

[PATCH 3/7] tools/xl: Add max_altp2m parameter

2024-04-24 Thread Petr Beneš
From: Petr Beneš 

Introduce a new max_altp2m parameter to control the maximum number of altp2m
views a domain can use. By default, if max_altp2m is unspecified and altp2m is
enabled, the value is set to 10, reflecting the legacy behavior.

Signed-off-by: Petr Beneš 
---
 tools/golang/xenlight/helpers.gen.go | 2 ++
 tools/golang/xenlight/types.gen.go   | 1 +
 tools/include/libxl.h| 8 
 tools/libs/light/libxl_create.c  | 9 +
 tools/libs/light/libxl_types.idl | 1 +
 tools/xl/xl_parse.c  | 4 
 xen/include/public/domctl.h  | 1 +
 7 files changed, 26 insertions(+)

diff --git a/tools/golang/xenlight/helpers.gen.go 
b/tools/golang/xenlight/helpers.gen.go
index 78bdb08b15..4458d5bcbb 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1158,6 +1158,7 @@ if err := 
x.ArchX86.MsrRelaxed.fromC(_x86.msr_relaxed);err != nil {
 return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
 }
 x.Altp2M = Altp2MMode(xc.altp2m)
+x.MaxAltp2M = uint32(xc.max_altp2m)
 x.VmtraceBufKb = int(xc.vmtrace_buf_kb)
 if err := x.Vpmu.fromC();err != nil {
 return fmt.Errorf("converting field Vpmu: %v", err)
@@ -1674,6 +1675,7 @@ if err := 
x.ArchX86.MsrRelaxed.toC(_x86.msr_relaxed); err != nil {
 return fmt.Errorf("converting field ArchX86.MsrRelaxed: %v", err)
 }
 xc.altp2m = C.libxl_altp2m_mode(x.Altp2M)
+xc.max_altp2m = C.uint32_t(x.MaxAltp2M)
 xc.vmtrace_buf_kb = C.int(x.VmtraceBufKb)
 if err := x.Vpmu.toC(); err != nil {
 return fmt.Errorf("converting field Vpmu: %v", err)
diff --git a/tools/golang/xenlight/types.gen.go 
b/tools/golang/xenlight/types.gen.go
index ccfe18019e..7139bcf324 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -602,6 +602,7 @@ ArchX86 struct {
 MsrRelaxed Defbool
 }
 Altp2M Altp2MMode
+MaxAltp2M uint32
 VmtraceBufKb int
 Vpmu Defbool
 }
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index 62cb07dea6..c73d9f2ff1 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -1239,6 +1239,14 @@ typedef struct libxl__ctx libxl_ctx;
  */
 #define LIBXL_HAVE_ALTP2M 1
 
+/*
+ * LIBXL_HAVE_MAX_ALTP2M
+ * If this is defined, then libxl supports setting the maximum number of
+ * alternate p2m tables.
+ */
+#define LIBXL_HAVE_MAX_ALTP2M 1
+#define LIBXL_MAX_ALTP2M_DEFAULT (~(uint32_t)0)
+
 /*
  * LIBXL_HAVE_REMUS
  * If this is defined, then libxl supports remus.
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 41252ec553..6ccc1fa158 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -483,6 +483,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
 return -ERROR_INVAL;
 }
 
+if (b_info->max_altp2m == LIBXL_MAX_ALTP2M_DEFAULT) {
+if ((libxl_defbool_val(b_info->u.hvm.altp2m) ||
+b_info->altp2m != LIBXL_ALTP2M_MODE_DISABLED))
+b_info->max_altp2m = 10;
+else
+b_info->max_altp2m = 0;
+}
+
 /* Assume that providing a bootloader user implies enabling restrict. */
 libxl_defbool_setdefault(_info->bootloader_restrict,
  !!b_info->bootloader_user);
@@ -645,6 +653,7 @@ int libxl__domain_make(libxl__gc *gc, libxl_domain_config 
*d_config,
 .ssidref = info->ssidref,
 .max_vcpus = b_info->max_vcpus,
 .max_evtchn_port = b_info->event_channels,
+.max_altp2m = b_info->max_altp2m,
 .max_grant_frames = b_info->max_grant_frames,
 .max_maptrack_frames = b_info->max_maptrack_frames,
 .grant_opts = XEN_DOMCTL_GRANT_version(b_info->max_grant_version),
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index 470122e768..c887d8ea8c 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -728,6 +728,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
 # Alternate p2m is not bound to any architecture or guest type, as it is
 # supported by x86 HVM and ARM support is planned.
 ("altp2m", libxl_altp2m_mode),
+("max_altp2m", uint32, {'init_val': 'LIBXL_MAX_ALTP2M_DEFAULT'}),
 
 # Size of preallocated vmtrace trace buffers (in KBYTES).
 # Use zero value to disable this feature.
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index ab09d0288b..741668e10a 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -2061,6 +2061,10 @@ void parse_config_data(const char *config_source,
 }
 }
 
+if (!xlu_cfg_get_long(config, "max_altp2m", , 1)) {
+b_info->max_altp2m = l;
+}
+
 if (!xlu_cfg_get_long(config, "vmtrace_buf_kb", , 1) && l) {
 b_info->vmtrace_buf_kb = l;
 }
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index a33f9ec32b..7a34465c21 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -77,6 +77,7 @@ struct 

[linux-5.4 test] 185782: regressions - FAIL

2024-04-24 Thread osstest service owner
flight 185782 linux-5.4 real [real]
http://logs.test-lab.xenproject.org/osstest/logs/185782/

Regressions :-(

Tests which did not succeed and are blocking,
including tests which could not be run:
 build-arm64-pvopsbroken  in 185433
 build-arm64-xsm  broken  in 185433
 build-arm64-libvirt  broken  in 185433
 test-arm64-arm64-libvirt-raw 17 guest-start/debian.repeat fail REGR. vs. 185168
 build-arm64-libvirt  5 host-build-prep fail in 185433 REGR. vs. 185168
 build-arm64-pvops5 host-build-prep fail in 185433 REGR. vs. 185168
 build-arm64-xsm  5 host-build-prep fail in 185433 REGR. vs. 185168

Tests which are failing intermittently (not blocking):
 test-amd64-i386-qemut-rhel6hvm-amd 14 guest-start/redhat.repeat fail in 185433 
pass in 185782
 test-armhf-armhf-examine  8 reboot   fail in 185769 pass in 185782
 test-arm64-arm64-libvirt-raw 13 guest-start  fail in 185769 pass in 185782
 test-amd64-amd64-xl-qcow2 21 guest-start/debian.repeat fail in 185769 pass in 
185782
 test-amd64-i386-libvirt-raw  12 debian-di-install  fail pass in 185433
 test-amd64-i386-libvirt-qcow2 12 debian-di-install fail pass in 185433
 test-amd64-i386-xl-vhd   12 debian-di-install  fail pass in 185433
 test-armhf-armhf-xl-multivcpu 14 guest-start   fail pass in 185769

Tests which did not succeed, but are not blocking:
 test-arm64-arm64-libvirt-xsm  1 build-check(1)   blocked in 185433 n/a
 test-arm64-arm64-xl   1 build-check(1)   blocked in 185433 n/a
 test-arm64-arm64-xl-xsm   1 build-check(1)   blocked in 185433 n/a
 test-arm64-arm64-libvirt-raw  1 build-check(1)   blocked in 185433 n/a
 test-arm64-arm64-xl-credit1   1 build-check(1)   blocked in 185433 n/a
 test-arm64-arm64-examine  1 build-check(1)   blocked in 185433 n/a
 test-arm64-arm64-xl-thunderx  1 build-check(1)   blocked in 185433 n/a
 test-arm64-arm64-xl-vhd   1 build-check(1)   blocked in 185433 n/a
 test-arm64-arm64-xl-credit2   1 build-check(1)   blocked in 185433 n/a
 test-armhf-armhf-xl-credit1 18 guest-start/debian.repeat fail blocked in 185168
 test-armhf-armhf-xl-credit2  19 guest-start.2 fail in 185433 blocked in 185168
 test-armhf-armhf-xl-credit1  14 guest-start fail in 185433 like 185168
 test-amd64-i386-libvirt-raw 14 migrate-support-check fail in 185433 never pass
 test-armhf-armhf-xl-multivcpu 15 migrate-support-check fail in 185433 never 
pass
 test-armhf-armhf-xl-multivcpu 16 saverestore-support-check fail in 185433 
never pass
 test-amd64-i386-libvirt-qcow2 14 migrate-support-check fail in 185433 never 
pass
 test-armhf-armhf-xl-credit1  19 guest-start.2 fail in 185769 blocked in 185168
 test-armhf-armhf-xl-multivcpu 18 guest-start/debian.repeat fail in 185769 like 
185168
 test-armhf-armhf-libvirt 16 saverestore-support-checkfail  like 185168
 test-amd64-amd64-xl-qemuu-win7-amd64 19 guest-stopfail like 185168
 test-amd64-i386-xl-qemut-win7-amd64 19 guest-stop fail like 185168
 test-amd64-amd64-xl-qemut-win7-amd64 19 guest-stopfail like 185168
 test-amd64-i386-xl-qemuu-win7-amd64 19 guest-stop fail like 185168
 test-amd64-amd64-qemuu-nested-amd 20 debian-hvm-install/l1/l2 fail like 185168
 test-amd64-amd64-xl-qemuu-ws16-amd64 19 guest-stopfail like 185168
 test-amd64-amd64-xl-qemut-ws16-amd64 19 guest-stopfail like 185168
 test-amd64-i386-xl-qemut-ws16-amd64 19 guest-stop fail like 185168
 test-armhf-armhf-xl-credit2  18 guest-start/debian.repeatfail  like 185168
 test-amd64-i386-xl-qemuu-ws16-amd64 19 guest-stop fail like 185168
 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-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-arm64-arm64-xl  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit2  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  15 migrate-support-checkfail   never pass
 test-arm64-arm64-xl-credit1  16 saverestore-support-checkfail   never pass
 test-arm64-arm64-xl-thunderx 15 migrate-support-checkfail   never pass
 

[PATCH v3 0/8] Clean up of gzip decompressor

2024-04-24 Thread Daniel P. Smith
An issue ran into by hyperlaunch was the need to use the gzip decompressor
multiple times. The current implementation fails when reused due to tainting of
decompressor state from a previous usage. This series seeks to colocate the
gzip unit files under a single directory similar to the other decompression
algorithms.  To enable the refactoring of the state tracking, the code is then
cleaned up in line with Xen coding style.

Changes in v3:
- patch "xen/gzip: Drop huffman code table tracking" was merged
- patch "xen/gzip: Remove custom memory allocator" was merged
- patch "xen/gzip: Drop unused define checks" was merged
- move of the decompressor state into struct was broken up to ease review
- expanded macros that were either only used once or obsfucated the logic
- adjusted variable types as needed to be more appropriate for their usage

Changes in v2:
- patch "xen/gzip: Colocate gunzip code files" was merged
- dropped ifdef chunks that are never enabled
- addressed formatting changes missed in v1
- replaced custom memory allocator with xalloc_bytes()
- renamed gzip_data to gzip_state
- moved gzip_state to being dynamically allocated
- changed crc table to the explicit size of uint32_t 
- instead of moving huffman tracking into state, dropped altogether


Daniel P. Smith (8):
  gzip: clean up comments and fix code alignment
  gzip: refactor and expand macros
  gzip: refactor the gunzip window into common state
  gzip: move window pointer into gunzip state
  gzip: move input buffer handling into gunzip state
  gzip: move output buffer into gunzip state
  gzip: move bitbuffer into gunzip state
  gzip: move crc state into gunzip state

 xen/common/gzip/gunzip.c  |  84 ++--
 xen/common/gzip/inflate.c | 948 +++---
 2 files changed, 516 insertions(+), 516 deletions(-)

-- 
2.30.2




Re: [PATCH v4 1/4] xen-livepatch: fix parameter name parsing

2024-04-24 Thread Anthony PERARD
On Wed, Apr 24, 2024 at 10:19:54AM +0200, Roger Pau Monne wrote:
> It's incorrect to restrict strncmp to the length of the command line input
> parameter, as then a user passing a rune like:
> 
> % xen-livepatch up foo.livepatch
> 
> Would match against the "upload" command, because the string comparison has
> been truncated to the length of the input argument.  Use strcmp instead which
> doesn't truncate.  Otherwise in order to keep using strncmp we would need to
> also check strings are of the same length before doing the comparison.
> 
> Fixes: 05bb8afedede ('xen-xsplice: Tool to manipulate xsplice payloads')
> Signed-off-by: Roger Pau Monné 

Acked-by: Anthony PERARD 

Thanks,

-- 
Anthony PERARD



Re: [XEN PATCH 2/2] x86/msr: add suffix 'U' to MSR_AMD_CSTATE_CFG macro.

2024-04-24 Thread Alessandro Zucchelli

On 2024-04-24 14:53, Teddy Astie wrote:

Le 24/04/2024 à 14:11, Alessandro Zucchelli a écrit :

This addresses violations of MISRA C:2012 Rule 7.2 which states as
following: A “u” or “U” suffix shall be applied to all integer 
constants

that are represented in an unsigned type.

No functional change.

Signed-off-by: Alessandro Zucchelli 
---
  xen/arch/x86/include/asm/msr-index.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/msr-index.h 
b/xen/arch/x86/include/asm/msr-index.h

index 92dd9fa496..9cdb5b2625 100644
--- a/xen/arch/x86/include/asm/msr-index.h
+++ b/xen/arch/x86/include/asm/msr-index.h
@@ -236,7 +236,7 @@

  #define MSR_VIRT_SPEC_CTRL  _AC(0xc001011f, U) /* 
Layout matches MSR_SPEC_CTRL */


-#define MSR_AMD_CSTATE_CFG  0xc0010296
+#define MSR_AMD_CSTATE_CFG  0xc0010296U

  /*
   * Legacy MSR constants in need of cleanup.  No new MSRs below this 
comment.


Hello, thanks for the patches

I wonder if the same approach should be also taken for all the other 
MSR

constants of this file that are similar ?


Hello,

from a (strict) point of view of Rule 7.2, the suffix is needed if the
numeric constant cannot be represented using an int and it is therefore
represented using an unsigned integer.
Every violation in the MSR file has already been addressed.
--
Alessandro Zucchelli, B.Sc.

Software Engineer, BUGSENG (https://bugseng.com)



Re: [PATCH v3 0/8] Clean up of gzip decompressor

2024-04-24 Thread Daniel P. Smith

On 4/24/24 12:34, Daniel P. Smith wrote:

An issue ran into by hyperlaunch was the need to use the gzip decompressor
multiple times. The current implementation fails when reused due to tainting of
decompressor state from a previous usage. This series seeks to colocate the
gzip unit files under a single directory similar to the other decompression
algorithms.  To enable the refactoring of the state tracking, the code is then
cleaned up in line with Xen coding style.



Forgot to add this comment to the cover letter.

Concern was raised about taking updates from original source to the 
code. In this case the original source is from the Linux kernel, which 
can be found in lib/inflate.c, and added to Xen in 2009. The last time 
there was a logic change to that code in the Linux kernel was in 2008, 
before it was added to Xen. Since then, it has only been updated for 
changes made to included headers. If fact, as far as I can see, while 
the file is still in place, nothing uses it. For zlib decompression, 
three is a new lib/zlib_deflate code base that is used. The scope of 
this work is not to completely fix/replace zlib decompression for Xen, 
but to stabilize it to allow hyperlaunch to decompress more than one 
zlib compressed kernel.


V/r,
Daniel P. Smith



Re: [PATCH v4 2/6] xen/device-tree: Move Arm's setup.c bootinfo functions to common

2024-04-24 Thread Julien Grall

Hi Shawn,

On 12/04/2024 04:55, Shawn Anastasio wrote:

diff --git a/xen/common/device-tree/Makefile b/xen/common/device-tree/Makefile
new file mode 100644
index 00..947bad979c
--- /dev/null
+++ b/xen/common/device-tree/Makefile
@@ -0,0 +1 @@
+obj-y += bootinfo.init.o
diff --git a/xen/common/device-tree/bootinfo.c 
b/xen/common/device-tree/bootinfo.c
new file mode 100644
index 00..914f876d29
--- /dev/null
+++ b/xen/common/device-tree/bootinfo.c
@@ -0,0 +1,446 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Derived from $xen/arch/arm/setup.c.
+ *
+ * Early device tree parsing and bookkeeping routines.


Based on the discussion in v3, I think this is only containing 
bookkepping routines. So you probably want to remove "Early device tree 
parsing".


This would also make a bit clearer the difference bettwen bootfdt.c and 
bootinfo.c.


[...]


+/*
+ * Populate the boot allocator.
+ * If a static heap was not provided by the admin, all the RAM but the
+ * following regions will be added:
+ *  - Modules (e.g., Xen, Kernel)
+ *  - Reserved regions
+ *  - Xenheap (CONFIG_SEPARATE_XENHEAP only)
+ * If a static heap was provided by the admin, populate the boot
+ * allocator with the corresponding regions only, but with Xenheap excluded
+ * on arm32.


Now this is moved in common code, I think we want to 
s/arm32/CONFIG_SEPARATE_XENHEAP/. This could be done on a follow-up commit.


[...]


diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h
new file mode 100644
index 00..b0487bdbbd
--- /dev/null
+++ b/xen/include/xen/bootfdt.h
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __XEN_BOOTFDT_H__
+#define __XEN_BOOTFDT_H__
+
+#include 
+
+#define MIN_FDT_ALIGN 8
+#define MAX_FDT_SIZE SZ_2M
+
+#define NR_MEM_BANKS 256
+
+#define MAX_MODULES 32 /* Current maximum useful modules */
+
+typedef enum {
+BOOTMOD_XEN,
+BOOTMOD_FDT,
+BOOTMOD_KERNEL,
+BOOTMOD_RAMDISK,
+BOOTMOD_XSM,
+BOOTMOD_GUEST_DTB,
+BOOTMOD_UNKNOWN
+} bootmodule_kind;
+
+enum membank_type {
+/*
+ * The MEMBANK_DEFAULT type refers to either reserved memory for the
+ * device/firmware (when the bank is in 'reserved_mem') or any RAM (when
+ * the bank is in 'mem').
+ */
+MEMBANK_DEFAULT,
+/*
+ * The MEMBANK_STATIC_DOMAIN type is used to indicate whether the memory
+ * bank is bound to a static Xen domain. It is only valid when the bank
+ * is in reserved_mem.
+ */
+MEMBANK_STATIC_DOMAIN,
+/*
+ * The MEMBANK_STATIC_HEAP type is used to indicate whether the memory
+ * bank is reserved as static heap. It is only valid when the bank is
+ * in reserved_mem.
+ */
+MEMBANK_STATIC_HEAP,
+};
+
+/* Indicates the maximum number of characters(\0 included) for shm_id */
+#define MAX_SHM_ID_LENGTH 16
+
+struct membank {
+paddr_t start;
+paddr_t size;
+enum membank_type type;
+#ifdef CONFIG_STATIC_SHM
+char shm_id[MAX_SHM_ID_LENGTH];
+unsigned int nr_shm_borrowers;
+#endif
+};
+
+struct meminfo {
+unsigned int nr_banks;
+struct membank bank[NR_MEM_BANKS];
+};


I have just committed some code which is modifying membank/meminfo. I 
think this may need to be rebased. I have CCed the author of the series 
(Luca) who may be able to advise/help.


Cheers,

--
Julien Grall



Re: [PATCH v4 1/6] xen/ppc: Introduce stub asm/static-shmem.h

2024-04-24 Thread Julien Grall

Hi Shawn,

On 12/04/2024 04:55, Shawn Anastasio wrote:

Required for bootfdt.c to build.


AFAIU, this patch is only necessary in #4. So I would consider to fold 
it there as it doesn't seem to add any value alone.




Signed-off-by: Shawn Anastasio 
---
Changes in v4: none

  xen/arch/ppc/include/asm/static-shmem.h | 12 
  1 file changed, 12 insertions(+)
  create mode 100644 xen/arch/ppc/include/asm/static-shmem.h

diff --git a/xen/arch/ppc/include/asm/static-shmem.h 
b/xen/arch/ppc/include/asm/static-shmem.h
new file mode 100644
index 00..84370d6e6c
--- /dev/null
+++ b/xen/arch/ppc/include/asm/static-shmem.h


Just an idea to avoid introducing static-shmem.h for PPC (and I guess 
RISC-V). Could we instead ifdef the inclusion and in bootfdt.c provide a 
stub for...



@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier:  GPL-2.0-only */
+
+#ifndef __ASM_PPC_STATIC_SHMEM_H__
+#define __ASM_PPC_STATIC_SHMEM_H__
+
+static inline int process_shm_node(const void *fdt, int node,
+   uint32_t address_cells, uint32_t size_cells)
+{
+return -EINVAL;
+}


... this helper? Something like [1].

Cheers,


[1]
diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index 4d708442a19e..26dada1e3a1e 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -15,7 +15,9 @@
 #include 
 #include 
 #include 
+#ifdef CONFIG_STATIC_SHM
 #include 
+#endif

 static void __init __maybe_unused build_assertions(void)
 {
@@ -436,6 +438,15 @@ static int __init process_domain_node(const void 
*fdt, int node,

MEMBANK_STATIC_DOMAIN);
 }

+#ifndef CONFIG_STATIC_SHM
+static inline int process_shm_node(const void *fdt, int node,
+   uint32_t address_cells, uint32_t 
size_cells)

+{
+printk("CONFIG_STATIC_SHM must be enabled for parsing static shared 
memory nodes\n");

+return -EINVAL;
+}
+#endif
+
 static int __init early_scan_node(const void *fdt,
   int node, const char *name, int depth,
   u32 address_cells, u32 size_cells,
diff --git a/xen/arch/arm/include/asm/static-shmem.h 
b/xen/arch/arm/include/asm/static-shmem.h

index 3b6569e5703f..39b5881d24aa 100644
--- a/xen/arch/arm/include/asm/static-shmem.h
+++ b/xen/arch/arm/include/asm/static-shmem.h
@@ -68,13 +68,6 @@ static inline int process_shm_chosen(struct domain *d,
 return 0;
 }

-static inline int process_shm_node(const void *fdt, int node,
-   uint32_t address_cells, uint32_t 
size_cells)

-{
-printk("CONFIG_STATIC_SHM must be enabled for parsing static shared 
memory nodes\n");

-return -EINVAL;
-}
-
 static inline void early_print_info_shmem(void) {};

 static inline void init_sharedmem_pages(void) {};


Cheers,

--
Julien Grall



Re: [PATCH 1/3] automation: Drop some of the non-debug variants of the same Arm jobs

2024-04-24 Thread Stefano Stabellini
On Tue, 23 Apr 2024, Michal Orzel wrote:
> To save some bandwith that can be later on used to increase the test
> coverage by adding new tests, drop the following non-debug test/build
> jobs existing in both debug and non-debug variants:
>  - static memory (arm64, arm32)
>  - static shared memory (arm64)
>  - static heap (arm64)
>  - boot cpupools (arm64)
>  - gzip (arm32)
> 
> More generic tests existing in both variants were left unmodified.
> 
> Signed-off-by: Michal Orzel 

Acked-by: Stefano Stabellini 


> ---
>  automation/gitlab-ci/build.yaml | 38 --
>  automation/gitlab-ci/test.yaml  | 48 -
>  2 files changed, 86 deletions(-)
> 
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index aac29ee13ab6..f3c934471f32 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -392,16 +392,6 @@ debian-bookworm-gcc-arm32-debug-randconfig:
>  HYPERVISOR_ONLY: y
>  RANDCONFIG: y
>  
> -debian-bookworm-gcc-arm32-staticmem:
> -  extends: .gcc-arm32-cross-build
> -  variables:
> -CONTAINER: debian:bookworm-arm64v8-arm32-gcc
> -HYPERVISOR_ONLY: y
> -EXTRA_XEN_CONFIG: |
> -  CONFIG_EXPERT=y
> -  CONFIG_UNSUPPORTED=y
> -  CONFIG_STATIC_MEMORY=y
> -
>  debian-bookworm-gcc-arm32-debug-staticmem:
>extends: .gcc-arm32-cross-build-debug
>variables:
> @@ -458,15 +448,6 @@ alpine-3.18-gcc-debug-arm64-randconfig:
>  CONTAINER: alpine:3.18-arm64v8
>  RANDCONFIG: y
>  
> -alpine-3.18-gcc-arm64-staticmem:
> -  extends: .gcc-arm64-build
> -  variables:
> -CONTAINER: alpine:3.18-arm64v8
> -EXTRA_XEN_CONFIG: |
> -  CONFIG_EXPERT=y
> -  CONFIG_UNSUPPORTED=y
> -  CONFIG_STATIC_MEMORY=y
> -
>  alpine-3.18-gcc-debug-arm64-staticmem:
>extends: .gcc-arm64-build-debug
>variables:
> @@ -476,15 +457,6 @@ alpine-3.18-gcc-debug-arm64-staticmem:
>CONFIG_UNSUPPORTED=y
>CONFIG_STATIC_MEMORY=y
>  
> -alpine-3.18-gcc-arm64-static-shared-mem:
> -  extends: .gcc-arm64-build
> -  variables:
> -CONTAINER: alpine:3.18-arm64v8
> -EXTRA_XEN_CONFIG: |
> -  CONFIG_UNSUPPORTED=y
> -  CONFIG_STATIC_MEMORY=y
> -  CONFIG_STATIC_SHM=y
> -
>  alpine-3.18-gcc-debug-arm64-static-shared-mem:
>extends: .gcc-arm64-build-debug
>variables:
> @@ -494,16 +466,6 @@ alpine-3.18-gcc-debug-arm64-static-shared-mem:
>CONFIG_STATIC_MEMORY=y
>CONFIG_STATIC_SHM=y
>  
> -alpine-3.18-gcc-arm64-boot-cpupools:
> -  extends: .gcc-arm64-build
> -  variables:
> -CONTAINER: alpine:3.18-arm64v8
> -EXTRA_XEN_CONFIG: |
> -  CONFIG_EXPERT=y
> -  CONFIG_UNSUPPORTED=y
> -  CONFIG_SCHED_NULL=y
> -  CONFIG_BOOT_TIME_CPUPOOLS=y
> -
>  alpine-3.18-gcc-debug-arm64-boot-cpupools:
>extends: .gcc-arm64-build-debug
>variables:
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 8b7b2e4da92d..55a7831ad292 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -255,14 +255,6 @@ qemu-smoke-dom0less-arm64-gcc-debug:
>  - *arm64-test-needs
>  - alpine-3.18-gcc-debug-arm64
>  
> -qemu-smoke-dom0less-arm64-gcc-staticmem:
> -  extends: .qemu-arm64
> -  script:
> -- ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-mem 2>&1 | 
> tee ${LOGFILE}
> -  needs:
> -- *arm64-test-needs
> -- alpine-3.18-gcc-arm64-staticmem
> -
>  qemu-smoke-dom0less-arm64-gcc-debug-staticmem:
>extends: .qemu-arm64
>script:
> @@ -271,14 +263,6 @@ qemu-smoke-dom0less-arm64-gcc-debug-staticmem:
>  - *arm64-test-needs
>  - alpine-3.18-gcc-debug-arm64-staticmem
>  
> -qemu-smoke-dom0less-arm64-gcc-staticheap:
> - extends: .qemu-arm64
> - script:
> -   - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-heap 2>&1 | 
> tee ${LOGFILE}
> - needs:
> -   - *arm64-test-needs
> -   - alpine-3.18-gcc-arm64
> -
>  qemu-smoke-dom0less-arm64-gcc-debug-staticheap:
>   extends: .qemu-arm64
>   script:
> @@ -287,14 +271,6 @@ qemu-smoke-dom0less-arm64-gcc-debug-staticheap:
> - *arm64-test-needs
> - alpine-3.18-gcc-debug-arm64
>  
> -qemu-smoke-dom0less-arm64-gcc-static-shared-mem:
> -  extends: .qemu-arm64
> -  script:
> -- ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-shared-mem 
> 2>&1 | tee ${LOGFILE}
> -  needs:
> -- *arm64-test-needs
> -- alpine-3.18-gcc-arm64-static-shared-mem
> -
>  qemu-smoke-dom0less-arm64-gcc-debug-static-shared-mem:
>extends: .qemu-arm64
>script:
> @@ -303,14 +279,6 @@ qemu-smoke-dom0less-arm64-gcc-debug-static-shared-mem:
>  - *arm64-test-needs
>  - alpine-3.18-gcc-debug-arm64-static-shared-mem
>  
> -qemu-smoke-dom0less-arm64-gcc-boot-cpupools:
> -  extends: .qemu-arm64
> -  script:
> -- ./automation/scripts/qemu-smoke-dom0less-arm64.sh boot-cpupools 2>&1 | 
> tee ${LOGFILE}
> -  needs:
> -- *arm64-test-needs
> -- alpine-3.18-gcc-arm64-boot-cpupools
> -
>  

Re: [PATCH 11/15] tools/helpers: Add get_overlay

2024-04-24 Thread Henry Wang

Hi Jan,

On 4/24/2024 2:08 PM, Jan Beulich wrote:

On 24.04.2024 05:34, Henry Wang wrote:

From: Vikram Garhwal 

This user level application copies the overlay dtbo shared by dom0 while doing
overlay node assignment operation. It uses xenstore to communicate with dom0.
More information on the protocol is writtien in docs/misc/overlay.txt file.

Signed-off-by: Vikram Garhwal 
Signed-off-by: Stefano Stabellini 
Signed-off-by: Henry Wang 
---
  tools/helpers/Makefile  |   8 +
  tools/helpers/get_overlay.c | 393 
  2 files changed, 401 insertions(+)
  create mode 100644 tools/helpers/get_overlay.c

As mentioned before on various occasions - new files preferably use dashes as
separators in preference to underscores. You not doing so is particularly
puzzling seeing ...


--- a/tools/helpers/Makefile
+++ b/tools/helpers/Makefile
@@ -12,6 +12,7 @@ TARGETS += init-xenstore-domain
  endif
  ifeq ($(CONFIG_ARM),y)
  TARGETS += init-dom0less
+TARGETS += get_overlay

... patch context here (demonstrating a whopping 3 dashes used in similar
cases).


I am not very sure why Vikram used "_" in the original patch. However I 
agree you are correct. Since I am currently doing the follow up of this 
series, I will use "-" in v2 as suggested. Thanks.


Kind regards,
Henry



Jan





[linux-linus test] 185785: regressions - FAIL

2024-04-24 Thread osstest service owner
flight 185785 linux-linus real [real]
http://logs.test-lab.xenproject.org/osstest/logs/185785/

Regressions :-(

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

Tests which are failing intermittently (not blocking):
 test-armhf-armhf-xl-credit2   8 xen-boot fail in 185779 pass in 185785
 test-armhf-armhf-xl-credit1   8 xen-boot fail in 185779 pass in 185785
 test-armhf-armhf-xl   8 xen-boot   fail pass in 185779
 test-amd64-amd64-qemuu-freebsd12-amd64 21 guest-start/freebsd.repeat fail pass 
in 185779

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

version targeted for testing:
 linux9d1ddab261f3e2af7c384dc02238784ce0cf9f98
baseline version:
 linux71b1543c83d65af8215d7558d70fc2ecbee77dcf

Last test of basis   185768  2024-04-23 07:46:10 Z1 days
Testing same since   185779  2024-04-24 00:13:50 Z1 days2 attempts


People who touched revisions under test:
  David Howells 
  Linus Torvalds 
  Paulo Alcantara (Red Hat) 
  Paulo Alcantara 
  Ronnie Sahlberg 
  Steve French 
  Takayuki Nagata 
  Tom Talpey 

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

Re: [PATCH v3 3/8] gzip: refactor the gunzip window into common state

2024-04-24 Thread Daniel P. Smith

On 4/24/24 12:34, Daniel P. Smith wrote:

Begin moving core state, in this case the gunzip window, into struct
gunzip_state to allow a per decompression instance. In doing so, drop the
define aliasing of window to slide.

Signed-off-by: Daniel P. Smith 
---
  xen/common/gzip/gunzip.c  | 21 
  xen/common/gzip/inflate.c | 68 +++
  2 files changed, 48 insertions(+), 41 deletions(-)

diff --git a/xen/common/gzip/gunzip.c b/xen/common/gzip/gunzip.c
index b7cadadcca8b..e47f10ae19ad 100644
--- a/xen/common/gzip/gunzip.c
+++ b/xen/common/gzip/gunzip.c
@@ -4,10 +4,12 @@
  #include 
  #include 
  
-static unsigned char *__initdata window;

-
  #define WSIZE   0x8000U
  
+struct gunzip_state {

+unsigned char *window;
+};
+
  static unsigned char *__initdata inbuf;
  static unsigned int __initdata insize;
  
@@ -43,7 +45,7 @@ typedef unsigned long   ulg;

  #endif
  
  static long __initdata bytes_out;

-static void flush_window(void);
+static void flush_window(struct gunzip_state *s);
  
  static __init void error(const char *x)

  {
@@ -62,7 +64,7 @@ static __init uch get_byte(void) {
  
  #include "inflate.c"
  
-static __init void flush_window(void)

+static __init void flush_window(struct gunzip_state *s)
  {
  /*
   * The window is equal to the output buffer therefore only need to
@@ -72,7 +74,7 @@ static __init void flush_window(void)
  unsigned int n;
  unsigned char *in, ch;
  
-in = window;

+in = s->window;
  for ( n = 0; n < outcnt; n++ )
  {
  ch = *in++;
@@ -99,12 +101,17 @@ __init int gzip_check(char *image, unsigned long image_len)
  
  __init int perform_gunzip(char *output, char *image, unsigned long image_len)

  {
+struct gunzip_state *s;
  int rc;
  
  if ( !gzip_check(image, image_len) )

  return 1;
  
-window = (unsigned char *)output;

+s = (struct gunzip_state *)malloc(sizeof(struct gunzip_state));


Looks like I inadvertently dropped the corresponding free when breaking 
up the monolithic patch.


v/r,
dps



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

2024-04-24 Thread osstest service owner
flight 185788 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/185788/

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  7846f7699fea25502061a05ea847e942ea624f12
baseline version:
 xen  6c04a0bf2c5824616c0bbc44bc4f07c64a5469be

Last test of basis   185787  2024-04-24 16:02:12 Z0 days
Testing same since   185788  2024-04-24 19:04:02 Z0 days1 attempts


People who touched revisions under test:
  Luca Fancellu 
  Penny Zheng 

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
   6c04a0bf2c..7846f7699f  7846f7699fea25502061a05ea847e942ea624f12 -> smoke



Re: [PATCH 2/3] automation: Add arm{64,32} earlyprintk jobs

2024-04-24 Thread Stefano Stabellini
On Tue, 23 Apr 2024, Michal Orzel wrote:
> Introduce qemu based Arm earlyprintk test and build jobs to cover this
> feature in debug variant. The tests simply check for the presence of the
> last message printed by the bootstrap code before entering the C world.
> 
> Signed-off-by: Michal Orzel 

Reviewed-by: Stefano Stabellini 


> ---
>  automation/gitlab-ci/build.yaml | 17 +
>  automation/gitlab-ci/test.yaml  | 16 
>  automation/scripts/qemu-smoke-dom0less-arm32.sh |  7 +++
>  automation/scripts/qemu-smoke-dom0less-arm64.sh |  5 +
>  4 files changed, 45 insertions(+)
> 
> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
> index f3c934471f32..49d6265ad5b4 100644
> --- a/automation/gitlab-ci/build.yaml
> +++ b/automation/gitlab-ci/build.yaml
> @@ -402,6 +402,15 @@ debian-bookworm-gcc-arm32-debug-staticmem:
>CONFIG_UNSUPPORTED=y
>CONFIG_STATIC_MEMORY=y
>  
> +debian-bookworm-gcc-arm32-debug-earlyprintk:
> +  extends: .gcc-arm32-cross-build-debug
> +  variables:
> +CONTAINER: debian:bookworm-arm64v8-arm32-gcc
> +HYPERVISOR_ONLY: y
> +EXTRA_XEN_CONFIG: |
> +  CONFIG_EARLY_UART_CHOICE_PL011=y
> +  CONFIG_EARLY_UART_BASE_ADDRESS=0x900
> +
>  # Arm builds
>  
>  debian-bookworm-gcc-arm64:
> @@ -473,6 +482,14 @@ alpine-3.18-gcc-debug-arm64-boot-cpupools:
>  EXTRA_XEN_CONFIG: |
>CONFIG_BOOT_TIME_CPUPOOLS=y
>  
> +alpine-3.18-gcc-debug-arm64-earlyprintk:
> +  extends: .gcc-arm64-build-debug
> +  variables:
> +CONTAINER: alpine:3.18-arm64v8
> +EXTRA_XEN_CONFIG: |
> +  CONFIG_EARLY_UART_CHOICE_PL011=y
> +  CONFIG_EARLY_UART_BASE_ADDRESS=0x900
> +
>  # RISC-V 64 cross-build
>  .riscv-fixed-randconfig:
>variables: 
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 55a7831ad292..1e5d86763f6c 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -287,6 +287,14 @@ qemu-smoke-dom0less-arm64-gcc-debug-boot-cpupools:
>  - *arm64-test-needs
>  - alpine-3.18-gcc-debug-arm64-boot-cpupools
>  
> +qemu-smoke-dom0less-arm64-gcc-debug-earlyprintk:
> +  extends: .qemu-arm64
> +  script:
> +- ./automation/scripts/qemu-smoke-dom0less-arm64.sh earlyprintk 2>&1 | 
> tee ${LOGFILE}
> +  needs:
> +- *arm64-test-needs
> +- alpine-3.18-gcc-debug-arm64-earlyprintk
> +
>  qemu-xtf-dom0less-arm64-gcc-hyp-xen-version:
>extends: .qemu-arm64
>script:
> @@ -359,6 +367,14 @@ qemu-smoke-dom0less-arm32-gcc-debug-without-dom0:
>  - *arm32-test-needs
>  - debian-bookworm-gcc-arm32-debug
>  
> +qemu-smoke-dom0less-arm32-gcc-debug-earlyprintk:
> +  extends: .qemu-arm32
> +  script:
> +- ./automation/scripts/qemu-smoke-dom0less-arm32.sh earlyprintk 2>&1 | 
> tee ${LOGFILE}
> +  needs:
> +- *arm32-test-needs
> +- debian-bookworm-gcc-arm32-debug-earlyprintk
> +
>  qemu-alpine-x86_64-gcc:
>extends: .qemu-x86-64
>script:
> diff --git a/automation/scripts/qemu-smoke-dom0less-arm32.sh 
> b/automation/scripts/qemu-smoke-dom0less-arm32.sh
> index e31b6b9014e1..1e2b939aadf7 100755
> --- a/automation/scripts/qemu-smoke-dom0less-arm32.sh
> +++ b/automation/scripts/qemu-smoke-dom0less-arm32.sh
> @@ -53,6 +53,13 @@ echo \"${passed}\"
>  "
>  fi
>  
> +if [[ "${test_variant}" == "earlyprintk" ]]; then
> +# Clear dom0 prompt
> +dom0_prompt=""
> +# Last early printk message before entering C world
> +passed="\- Ready \-"
> +fi
> +
>  # dom0/domU rootfs
>  # We are using the same rootfs for dom0 and domU. The only difference is
>  # that for the former, we set explictly rdinit to /bin/sh, whereas for the
> diff --git a/automation/scripts/qemu-smoke-dom0less-arm64.sh 
> b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> index e748b8ef1699..fc943a1a622c 100755
> --- a/automation/scripts/qemu-smoke-dom0less-arm64.sh
> +++ b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> @@ -61,6 +61,11 @@ fi
>  "
>  fi
>  
> +if [[ "${test_variant}" == "earlyprintk" ]]; then
> +# Last early printk message before entering C world
> +passed="\- Ready \-"
> +fi
> +
>  # XXX QEMU looks for "efi-virtio.rom" even if it is unneeded
>  curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
>  ./binaries/qemu-system-aarch64 \
> -- 
> 2.25.1
> 



Re: [PATCH 06/15] rangeset: Move struct range and struct rangeset to headerfile

2024-04-24 Thread Henry Wang

Hi Jan,

On 4/24/2024 2:22 PM, Jan Beulich wrote:

On 24.04.2024 05:34, Henry Wang wrote:

From: Vikram Garhwal 

Move struct range, rangeset and removed static from first_range and 
next_range().

NAK, for going against what we do elsewhere (limiting exposure of internals).
At least as long as the justification isn't any better than ...


IRQs and IOMEMs for nodes are stored as rangeset in the dynamic node addition
part. While removing the nodes we need to access every IRQ and IOMEM ranges to
unmap IRQ and IOMEM from the domain.

... this. You're aware of rangeset_report_ranges() and 
rangeset_consume_ranges(),
aren't you? If neither is suitable for your purpose, can you explain what you
need in addition?


I understand your concern. I will check if I can refactor this patch 
using the suggested helpers. Thanks!


Kind regards,
Henry




Jan





Re: [PATCH 03/15] xen/arm: Always enable IOMMU

2024-04-24 Thread Henry Wang

Hi Julien,

On 4/24/2024 9:03 PM, Julien Grall wrote:

Hi Henry,

On 24/04/2024 04:34, Henry Wang wrote:

From: Vikram Garhwal 

For overlay with iommu functionality to work with running VMs, we 
need to enable

IOMMU by default for the domains.

Signed-off-by: Vikram Garhwal 
Signed-off-by: Stefano Stabellini 
Signed-off-by: Henry Wang 
---
  xen/arch/arm/dom0less-build.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/dom0less-build.c 
b/xen/arch/arm/dom0less-build.c

index fb63ec6fd1..2d1fd1e214 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -894,7 +894,8 @@ void __init create_domUs(void)
  panic("Missing property 'cpus' for domain %s\n",
    dt_node_name(node));
  -    if ( dt_find_compatible_node(node, NULL, 
"multiboot,device-tree") &&

+    if ( (IS_ENABLED(CONFIG_OVERLAY_DTB) ||


Similar to the first patch, building Xen with the DTB overlay doesn't 
mean the user will want to use it (think of distros that may want to 
provide a generic Xen).


Instead, we should introduce a new DT property "passthrough" that 
would indicate whether the IOMMU should be used.


To be futureproof, I would match the values used by xl.cfg (see 
docs/man/xl.cfg.5.pod.in).


That sounds good. I can introduce a new DT property as suggested. Thanks 
for the suggestion!


Kind regards,
Henry




+ dt_find_compatible_node(node, NULL, "multiboot,device-tree")) &&
   iommu_enabled )
  d_cfg.flags |= XEN_DOMCTL_CDF_iommu;


Cheers,






Re: [XEN PATCH 03/10] automation/eclair_analysis: deviate macro count_args_ for MISRA Rule 20.7

2024-04-24 Thread Stefano Stabellini
On Tue, 23 Apr 2024, Nicola Vetrini wrote:
> The count_args_ macro violates Rule 20.7, but it can't be made
> compliant with Rule 20.7 without breaking its functionality. Since
> it's very unlikely for this macro to be misused, it is deviated.

That is OK but can't we use the SAF- framework to do it, given that it
is just one macro?

If not, this is also OK.


> No functional change.
> 
> Signed-off-by: Nicola Vetrini 
> ---
>  automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++
>  docs/misra/deviations.rst| 6 ++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl 
> b/automation/eclair_analysis/ECLAIR/deviations.ecl
> index d21f112a9b94..c17e2f5a0886 100644
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -445,6 +445,12 @@ not to parenthesize their arguments."
>  -config=MC3R1.R20.7,reports+={safe, 
> "any_area(any_loc(any_exp(macro(^alternative_(v)?call[0-9]$"}
>  -doc_end
>  
> +-doc_begin="The argument 'x' of the count_args_ macro can't be parenthesized 
> as
> +the rule would require, without breaking the functionality of the macro. The 
> uses
> +of this macro do not lead to developer confusion, and can thus be deviated."
> +-config=MC3R1.R20.7,reports+={safe, 
> "any_area(any_loc(any_exp(macro(^count_args_$"}
> +-doc_end
> +
>  -doc_begin="Uses of variadic macros that have one of their arguments defined 
> as
>  a macro and used within the body for both ordinary parameter expansion and 
> as an
>  operand to the # or ## operators have a behavior that is well-understood and
> diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> index ed0c1e8ed0bf..e228ae2e715f 100644
> --- a/docs/misra/deviations.rst
> +++ b/docs/misra/deviations.rst
> @@ -371,6 +371,12 @@ Deviations related to MISRA C:2012 Rules:
> sanity checks in place.
>   - Tagged as `safe` for ECLAIR.
>  
> +   * - R20.7
> + - The macro `count_args_` is not compliant with the rule, but is not 
> likely
> +   to incur in the risk of being misused or lead to developer confusion, 
> and
> +   refactoring it to add parentheses breaks its functionality.
> + - Tagged as `safe` for ECLAIR.
> +
> * - R20.12
>   - Variadic macros that use token pasting often employ the gcc extension
> `ext_paste_comma`, as detailed in `C-language-toolchain.rst`, which is
> -- 
> 2.34.1
> 



[ovmf test] 185789: all pass - PUSHED

2024-04-24 Thread osstest service owner
flight 185789 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/185789/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 90b6725562c82ec630d9e0cb19078f4b507db10b
baseline version:
 ovmf d97f964f7ce063f9861f4d21cc6352f6861f95a8

Last test of basis   185776  2024-04-23 21:41:06 Z1 days
Testing same since   185789  2024-04-24 23:42:57 Z0 days1 attempts


People who touched revisions under test:
  Michael Kubacki 

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



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
   d97f964f7c..90b6725562  90b6725562c82ec630d9e0cb19078f4b507db10b -> 
xen-tested-master



[ovmf test] 185792: all pass - PUSHED

2024-04-24 Thread osstest service owner
flight 185792 ovmf real [real]
http://logs.test-lab.xenproject.org/osstest/logs/185792/

Perfect :-)
All tests in this flight passed as required
version targeted for testing:
 ovmf 66c24219ade92b85b24f3ce29b988d187a9f6517
baseline version:
 ovmf 90b6725562c82ec630d9e0cb19078f4b507db10b

Last test of basis   185789  2024-04-24 23:42:57 Z0 days
Testing same since   185792  2024-04-25 01:43:46 Z0 days1 attempts


People who touched revisions under test:
  Gerd Hoffmann 
  Srikanth Aithal 

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



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
   90b6725562..66c24219ad  66c24219ade92b85b24f3ce29b988d187a9f6517 -> 
xen-tested-master



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

2024-04-24 Thread osstest service owner
flight 185790 xen-unstable-smoke real [real]
http://logs.test-lab.xenproject.org/osstest/logs/185790/

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  31d6b5b4a7c84818294dacca7a07e92f52393c9d
baseline version:
 xen  7846f7699fea25502061a05ea847e942ea624f12

Last test of basis   185788  2024-04-24 19:04:02 Z0 days
Testing same since   185790  2024-04-25 01:00:22 Z0 days1 attempts


People who touched revisions under test:
  Michal Orzel 
  Stefano Stabellini 

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
   7846f7699f..31d6b5b4a7  31d6b5b4a7c84818294dacca7a07e92f52393c9d -> smoke



Re: [PATCH 3/3] automation: Add arm64 test for running Xen with GICv3

2024-04-24 Thread Stefano Stabellini
On Tue, 23 Apr 2024, Michal Orzel wrote:
> At the moment, all the Arm64 Qemu tests use GICv2 which is the default
> GIC version used by Qemu. Improve the coverage by adding a new test in
> which Qemu will be configured to have GICv3.
> 
> Rename host device tree name to "virt.dtb" to be GIC version agnostic.
> Use "gic-version" Qemu option to select the version to use. Unless the
> test variant is set to "gicv3", version 2 will be used.
> 
> Signed-off-by: Michal Orzel 

Reviewed-by: Stefano Stabellini 


> ---
>  automation/gitlab-ci/test.yaml|  8 
>  .../scripts/qemu-smoke-dom0less-arm64.sh  | 19 ++-
>  2 files changed, 22 insertions(+), 5 deletions(-)
> 
> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> index 1e5d86763f6c..ad249fa0a5d9 100644
> --- a/automation/gitlab-ci/test.yaml
> +++ b/automation/gitlab-ci/test.yaml
> @@ -255,6 +255,14 @@ qemu-smoke-dom0less-arm64-gcc-debug:
>  - *arm64-test-needs
>  - alpine-3.18-gcc-debug-arm64
>  
> +qemu-smoke-dom0less-arm64-gcc-debug-gicv3:
> +  extends: .qemu-arm64
> +  script:
> +- ./automation/scripts/qemu-smoke-dom0less-arm64.sh gicv3 2>&1 | tee 
> ${LOGFILE}
> +  needs:
> +- *arm64-test-needs
> +- alpine-3.18-gcc-debug-arm64
> +
>  qemu-smoke-dom0less-arm64-gcc-debug-staticmem:
>extends: .qemu-arm64
>script:
> diff --git a/automation/scripts/qemu-smoke-dom0less-arm64.sh 
> b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> index fc943a1a622c..292c38a56147 100755
> --- a/automation/scripts/qemu-smoke-dom0less-arm64.sh
> +++ b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> @@ -4,6 +4,9 @@ set -ex
>  
>  test_variant=$1
>  
> +# Default GIC version
> +gic_version="2"
> +
>  if [ -z "${test_variant}" ]; then
>  passed="ping test passed"
>  domU_check="
> @@ -66,16 +69,22 @@ if [[ "${test_variant}" == "earlyprintk" ]]; then
>  passed="\- Ready \-"
>  fi
>  
> +if [[ "${test_variant}" == "gicv3" ]]; then
> +gic_version=3
> +passed="${test_variant} test passed"
> +domU_check="echo \"${passed}\""
> +fi
> +
>  # XXX QEMU looks for "efi-virtio.rom" even if it is unneeded
>  curl -fsSLO https://github.com/qemu/qemu/raw/v5.2.0/pc-bios/efi-virtio.rom
>  ./binaries/qemu-system-aarch64 \
> -machine virtualization=true \
> -   -cpu cortex-a57 -machine type=virt \
> +   -cpu cortex-a57 -machine type=virt,gic-version=$gic_version \
> -m 2048 -smp 2 -display none \
> -   -machine dumpdtb=binaries/virt-gicv2.dtb
> +   -machine dumpdtb=binaries/virt.dtb
>  
>  # XXX disable pl061 to avoid Linux crash
> -fdtput binaries/virt-gicv2.dtb -p -t s /pl061@903 status disabled
> +fdtput binaries/virt.dtb -p -t s /pl061@903 status disabled
>  
>  # Busybox
>  mkdir -p initrd
> @@ -138,7 +147,7 @@ cd ..
>  echo 'MEMORY_START="0x4000"
>  MEMORY_END="0x5000"
>  
> -DEVICE_TREE="virt-gicv2.dtb"
> +DEVICE_TREE="virt.dtb"
>  XEN="xen"
>  DOM0_KERNEL="Image"
>  DOM0_RAMDISK="dom0-rootfs.cpio.gz"
> @@ -200,7 +209,7 @@ echo "  virtio scan; dhcp; tftpb 0x4000 boot.scr; 
> source 0x4000"| \
>  timeout -k 1 240 \
>  ./binaries/qemu-system-aarch64 \
>  -machine virtualization=true \
> --cpu cortex-a57 -machine type=virt \
> +-cpu cortex-a57 -machine type=virt,gic-version=$gic_version \
>  -m 2048 -monitor none -serial stdio \
>  -smp 2 \
>  -no-reboot \
> -- 
> 2.25.1
> 



Re: [XEN PATCH] automation/eclair: reorganize pipelines

2024-04-24 Thread Stefano Stabellini
On Tue, 23 Apr 2024, Federico Serafini wrote:
> From: Simone Ballarin 
> 
> Introduce accepted_guidelines.sh: a script to autogenerate the
> configuration file accepted.ecl from docs/misra/rules.rst which enables
> all accepted guidelines.
> 
> Introduce monitored.ecl: a manual selection of accepted guidelines
> which are clean or almost clean, it is intended to be used for the
> analyses triggered by commits.
> 
> Reorganize tagging.ecl:
>   -Remove "accepted" tags: keeping track of accepted guidelines tagging
>them as "accepted" in the configuration file tagging.ecl is no
>longer needed since docs/rules.rst is keeping track of them.
>   -Tag more guidelines as clean.
> 
> Reorganize eclair pipelines:
>   - Set1, Set2, Set3 are now obsolete: remove the corresponding
> pipelines and ecl files.
>   - Amend scheduled eclair pipeline to use accepted.ecl.
>   - Amend triggered eclair pipeline to use monitored.ecl.
> 
> Rename and improve action_check_clean_regressions.sh to print a
> diagnostic in case a commit introduces a violation of a clean guideline.
> 
> An example of diagnostic is the following:
> 
> Failure: 13 regressions found for clean guidelines
>   service MC3R1.R8.2: (required) Function types shall be in prototype form 
> with named parameters:
>violation: 13
> 
> Signed-off-by: Simone Ballarin 
> Signed-off-by: Federico Serafini 
> Signed-off-by: Alessandro Zucchelli 

Fantastic work, thank you!

Reviewed-by: Stefano Stabellini 

Is this patch safe to commit now? Or would it cause gitlab-ci breakage?

One question below.


> ---
>  automation/eclair_analysis/ECLAIR/Set2.ecl| 25 ---
>  automation/eclair_analysis/ECLAIR/Set3.ecl| 67 ---
>  .../ECLAIR/accepted_guidelines.sh | 13 
>  .../eclair_analysis/ECLAIR/action.helpers |  3 +-
>  .../eclair_analysis/ECLAIR/action.settings|  1 +
>  .../ECLAIR/action_check_clean_regressions.sh  | 38 +++
>  .../ECLAIR/action_clean_added.sh  | 36 --
>  automation/eclair_analysis/ECLAIR/analyze.sh  |  2 +-
>  .../eclair_analysis/ECLAIR/generate_ecl.sh|  4 ++
>  .../ECLAIR/{Set1.ecl => monitored.ecl}| 57 +++-
>  automation/eclair_analysis/ECLAIR/tagging.ecl | 15 +
>  automation/gitlab-ci/analyze.yaml | 48 ++---
>  automation/scripts/eclair |  8 +--
>  13 files changed, 108 insertions(+), 209 deletions(-)
>  delete mode 100644 automation/eclair_analysis/ECLAIR/Set2.ecl
>  delete mode 100644 automation/eclair_analysis/ECLAIR/Set3.ecl
>  create mode 100755 automation/eclair_analysis/ECLAIR/accepted_guidelines.sh
>  create mode 100755 
> automation/eclair_analysis/ECLAIR/action_check_clean_regressions.sh
>  delete mode 100755 automation/eclair_analysis/ECLAIR/action_clean_added.sh
>  rename automation/eclair_analysis/ECLAIR/{Set1.ecl => monitored.ecl} (67%)
> 
> diff --git a/automation/eclair_analysis/ECLAIR/Set2.ecl 
> b/automation/eclair_analysis/ECLAIR/Set2.ecl
> deleted file mode 100644
> index 7608335cf4..00
> --- a/automation/eclair_analysis/ECLAIR/Set2.ecl
> +++ /dev/null
> @@ -1,25 +0,0 @@
> --doc_begin="Set 2 of Xen MISRA C guidelines"
> --enable=MC3R1.R10.1
> --enable=MC3R1.R10.2
> --enable=MC3R1.R10.3
> --enable=MC3R1.R10.4
> --enable=MC3R1.R10.6
> --enable=MC3R1.R10.7
> --enable=MC3R1.R10.8
> --enable=MC3R1.R11.1
> --enable=MC3R1.R11.2
> --enable=MC3R1.R11.3
> --enable=MC3R1.R11.6
> --enable=MC3R1.R11.7
> --enable=MC3R1.R11.8
> --enable=MC3R1.R11.9
> --enable=MC3R1.R12.2
> --enable=MC3R1.R13.1
> --enable=MC3R1.R13.2
> --enable=MC3R1.R13.5
> --enable=MC3R1.R13.6
> --enable=MC3R1.R14.1
> --enable=MC3R1.R14.2
> --enable=MC3R1.R14.3
> --enable=MC3R1.R14.4
> --doc_end
> diff --git a/automation/eclair_analysis/ECLAIR/Set3.ecl 
> b/automation/eclair_analysis/ECLAIR/Set3.ecl
> deleted file mode 100644
> index d2c2c4b21f..00
> --- a/automation/eclair_analysis/ECLAIR/Set3.ecl
> +++ /dev/null
> @@ -1,67 +0,0 @@
> --doc_begin="Set 3 of Xen MISRA C guidelines"
> --enable=MC3R1.D4.12
> --enable=MC3R1.R5.5
> --enable=MC3R1.R5.7
> --enable=MC3R1.R5.8
> --enable=MC3R1.R15.2
> --enable=MC3R1.R15.3
> --enable=MC3R1.R15.6
> --enable=MC3R1.R15.7
> --enable=MC3R1.R16.1
> --enable=MC3R1.R16.2
> --enable=MC3R1.R16.3
> --enable=MC3R1.R16.4
> --enable=MC3R1.R16.5
> --enable=MC3R1.R16.6
> --enable=MC3R1.R16.7
> --enable=MC3R1.R17.1
> --enable=MC3R1.R17.2
> --enable=MC3R1.R17.5
> --enable=MC3R1.R17.7
> --enable=MC3R1.R18.1
> --enable=MC3R1.R18.2
> --enable=MC3R1.R18.3
> --enable=MC3R1.R18.6
> --enable=MC3R1.R18.7
> --enable=MC3R1.R18.8
> --enable=MC3R1.R20.2
> --enable=MC3R1.R20.3
> --enable=MC3R1.R20.4
> --enable=MC3R1.R20.6
> --enable=MC3R1.R20.7
> --enable=MC3R1.R20.8
> --enable=MC3R1.R20.9
> --enable=MC3R1.R20.11
> --enable=MC3R1.R20.12
> --enable=MC3R1.R20.13
> --enable=MC3R1.R20.14
> --enable=MC3R1.R21.1
> --enable=MC3R1.R21.2
> --enable=MC3R1.R21.3
> --enable=MC3R1.R21.4
> --enable=MC3R1.R21.5
> 

Re: docs/misra: add R21.6 R21.14 R21.15 R21.16

2024-04-24 Thread Stefano Stabellini
On Thu, 18 Apr 2024, Jan Beulich wrote:
> On 16.04.2024 21:27, Stefano Stabellini wrote:
> > Also add two specific project-wide deviations for R21.6 and R21.15.
> > 
> > Signed-off-by: Stefano Stabellini 
> > 
> > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
> > index 32b02905d1..9123c8edb5 100644
> > --- a/docs/misra/deviations.rst
> > +++ b/docs/misra/deviations.rst
> > @@ -387,6 +387,22 @@ Deviations related to MISRA C:2012 Rules:
> > of the Rule due to uses of this macro.
> >   - Tagged as `deliberate` for ECLAIR.
> >  
> > +   * - R21.6
> > + - The use of snprintf() and vsnprintf() is justifiable as, despite
> > +   the fact that such functions have the same names of the
> > +   corresponding standard library functions, each configuration of
> > +   Xen has a unique implementation for them; the code implementing
> > +   such functions is subject to the analysis, so that any undefined
> > +   or unspecified behavior associated to them falls under the
> > +   responsibility of other MISRA guidelines
> 
> Checking the Misra spec, I'm actually surprised a deviation is needed. The
> rule's rationale talks about streams and file I/O only. Why would the string
> formatting functions be covered then at all? They also don't have, afaik,
> any undefined or implementation defined behavior.

As discussed during the call, I'll add an explanatory note to rules.rst


> > + - Tagged as `safe` for ECLAIR.
> > +
> > +   * - R21.15
> > + - The use of void* arguments is justifiable as the rationale for
> > +   the rule is to indicate possible mistakes, and void* is
> > +   frequently used in Xen to represent virtual memory addresses
> 
> But that doesn't rule out mistakes. Are there actually examples in the
> code base?

If you are asking if there are any violations or bugs, I'll defer to the
Bugseng team.

 
> Additionally I wonder (a) whether the rule actually needs an exception

Yes my understanding is that a deviation is necessary from MISRA point
of view, and if nothing else it will serve as extra clarification.


> and thus (b) whether the deviation isn't instead for 21.16. As to (a) I
> understand the rule is worded slightly differently than what would
> strictly be needed to permit void*, but the general rule in C is that
> void* is compatible with all other pointers (suitably qualified as
> needed, of course) anyway.

Roberto and others, can you please confirm whether we need a deviation
on 21.16 as well for similar reasons to 21.15? I am asking because I
don't have any notes about requiring a deviation for 21.16 but I would
like to check with you.



Re: [PATCH 14/15] add a domU script to fetch overlays and applying them to linux

2024-04-24 Thread Henry Wang

Hi Jan,

On 4/24/2024 2:16 PM, Jan Beulich wrote:

On 24.04.2024 05:34, Henry Wang wrote:

From: Vikram Garhwal 

Introduce a shell script that runs in the background and calls
get_overlay to retrive overlays and add them (or remove them) to Linux
device tree (running as a domU).

Signed-off-by: Vikram Garhwal 
Signed-off-by: Stefano Stabellini 
Signed-off-by: Henry Wang 
---
  tools/helpers/Makefile   |  2 +-
  tools/helpers/get_overlay.sh | 81 
  2 files changed, 82 insertions(+), 1 deletion(-)
  create mode 100755 tools/helpers/get_overlay.sh

Besides the same naming issue as in the earlier patch, the script also
looks very Linux-ish. Yet ...


I will fix the naming issue in v2. Would you mind elaborating a bit more 
about the "Linux-ish" concern? I guess this is because the original use 
case is on Linux, should I do anything about this?



--- a/tools/helpers/Makefile
+++ b/tools/helpers/Makefile
@@ -58,7 +58,6 @@ init-dom0less: $(INIT_DOM0LESS_OBJS)
  get_overlay: $(SHARE_OVERLAY_OBJS)
$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenvchan) $(LDLIBS_libxenstore) 
$(LDLIBS_libxenctrl) $(LDLIBS_libxengnttab) $(APPEND_LDFLAGS)
  
-

  .PHONY: install
  install: all
$(INSTALL_DIR) $(DESTDIR)$(LIBEXEC_BIN)
@@ -67,6 +66,7 @@ install: all
  .PHONY: uninstall
  uninstall:
for i in $(TARGETS); do rm -f $(DESTDIR)$(LIBEXEC_BIN)/$$i; done
+   $(RM) $(DESTDIR)$(LIBEXEC_BIN)/get_overlay.sh
  
  .PHONY: clean

  clean:

... you touching only the uninstall target, it's not even clear to me
how (and under what conditions) the script is going to make it into
$(DESTDIR)$(LIBEXEC_BIN)/. Did you mean to add to $(TARGETS), perhaps,
alongside the earlier added get-overlay binary?


You are right, I think the get-overlay binary and this script should be 
installed if DTB overlay is supported. Checking the code, I found 
LIBXL_HAVE_DT_OVERLAY which can indicate if we have this feature 
supported in libxl. Do you think it is a good idea to use it to install 
these two files in Makefile? Thanks.


Kind regards,
Henry



Jan





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

2024-04-24 Thread osstest service owner
flight 185786 xen-unstable real [real]
http://logs.test-lab.xenproject.org/osstest/logs/185786/

Failures :-/ but no regressions.

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

version targeted for testing:
 xen  410ef3343924b5a3928bbe8e392491992b322cf0
baseline version:
 xen  77e25f0e30ddd11e043e6fce84bf108ce7de5b6f

Last test of basis   185780  2024-04-24 01:37:24 Z1 days
Testing same since   185786  2024-04-24 13:05:04 Z0 days1 attempts


People who touched revisions under test:
  Petr Beneš 
  Tamas K Lengyel 

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

Re: [PATCH 1/4] x86/P2M: write_p2m_entry() is HVM-only anyway

2024-04-24 Thread Jan Beulich
On 24.04.2024 08:36, Jan Beulich wrote:
> On 23.04.2024 21:29, Andrew Cooper wrote:
>> On 23/04/2024 3:31 pm, Jan Beulich wrote:
>>> The latest as of e2b2ff677958 ("x86/P2M: split out init/teardown
>>> functions") the function is obviously unreachable for PV guests.
>>
>> This doesn't parse.  Do you mean "Since e2b2ff677958 ..." ?
> 
> Well. I'm sure you at least get the point of "the lastest as of", even
> if that may not be proper English. I specifically didn't use "since"
> because the fact mentioned may have been true before (more or less
> obviously). I'd therefore appreciate a wording suggestion which gets
> this across.
> 
>>>  Hence
>>> the paging_mode_enabled(d) check is pointless.
>>>
>>> Further host mode of a vCPU is always set, by virtue of
>>> paging_vcpu_init() being part of vCPU creation. Hence the
>>> paging_get_hostmode() check is pointless.
>>>
>>> With that the v local variable is unnecessary too. Drop the "if()"
>>> conditional and its corresponding "else".
>>>
>>> Signed-off-by: Jan Beulich 
>>> ---
>>> I have to confess that this if() has been puzzling me before.
>>
>> Puzzling yes, but it can't blindly be dropped.
> 
> And I'm not doing so "blindly". Every part of what is being dropped is
> being explained.
> 
>> This is the "did the toolstack initiate this update" check.  i.e. I
>> think it's "bypass the normal side effects of making this update".
> 
> Why would we want to bypass side effects?
> 
>> I suspect it exists because of improper abstraction between the guest
>> physmap and the shadow pagetables as-were - which were/are tighly
>> coupled to vCPUs even for aspects where they shouldn't have been.
>>
>> For better or worse, the toolstack can add_to_physmap() before it
>> creates vCPUs, and it will take this path you're trying to delete. 
>> There may be other cases too; I could see foreign mapping ending up
>> ticking this too.
>>
>> Whether we ought to permit a toolstack to do this is a different
>> question, but seeing as we explicitly intend (eventually for AMX) have a
>> set_policy call between domain_create() and vcpu_create(), I don't think
>> we can reasably restrict other hypercalls too in this period.
> 
> None of which explains what's wrong with the provided justification.
> The P2M isn't per-vCPU. Presence of vCPU-s therefore shouldn't matter
> for alterations of the P2M.

I've gone and checked further: The "side effects" are what the
write_p2m_entry_{pre,post}() hooks would do. Prior to the VM being
started that is a little bit of extra code which all ends up doing
nothing: There's nothing to flush, and there are no shadows to drop.
There's in particular no use of a vCPU anywhere, afaics. Plus, just
to mention it explicitly, the full path was forced anyway for nested
P2Ms, so there's no behavioral change there at all.

In fact I question the correctness of the plain safe_write_pte(),
without p2m_entry_modify(), if that path would have been taken (when
the domain has no vCPU-s yet).

Jan



Re: [XEN PATCH v2] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

2024-04-24 Thread Federico Serafini

On 24/04/24 10:30, Jan Beulich wrote:

On 24.04.2024 10:25, Federico Serafini wrote:

Update ECLAIR configuration to take into account the deviations
agreed during MISRA meetings for Rule 16.4.

Signed-off-by: Federico Serafini 
---
  automation/eclair_analysis/ECLAIR/deviations.ecl |  8 
  docs/misra/deviations.rst| 13 +
  2 files changed, 21 insertions(+)



So what has changed here from v1? It looks all the same to me, with it still
remaining unclear what exactly ...


--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -334,6 +334,19 @@ Deviations related to MISRA C:2012 Rules:
   - /\* Fallthrough \*/
   - /\* Fallthrough. \*/
  
+   * - R16.4

+ - Switch statements having a controlling expression of enum type
+   deliberately do not have a default case: gcc -Wall enables -Wswitch
+   which warns (and breaks the build as we use -Werror) if one of the enum
+   labels is missing from the switch.
+ - Tagged as `deliberate` for ECLAIR.
+
+   * - R16.4
+ - A switch statement with a single switch clause and no default label may
+   be used in place of an equivalent if statement if it is considered to
+   improve readability.
+ - Tagged as `deliberate` for ECLAIR.
+
 * - R16.6
   - A switch statement with a single switch clause and no default label may
 be used in place of an equivalent if statement if it is considered to


... a "switch clause" is.


I would define a switch clause as:
"the non-empy list of statements which follows a non-empty list of
case/default labels".
If you agree, I will place it near the occurrences of the term
"switch clause".

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)



Re: [PATCH 2/4] x86/P2M: un-indent write_p2m_entry()

2024-04-24 Thread Roger Pau Monné
On Tue, Apr 23, 2024 at 04:32:14PM +0200, Jan Beulich wrote:
> Drop the inner scope that was left from earlier if/else removal. Take
> the opportunity and make the paging_unlock() invocation common to
> success and error paths, though.

TBH I'm not sure I prefer the fact to continue function execution
after an error is found, I specially dislike that you have to add a
!rc check to the nestedhvm conditional block, and because anything
that we further add to the function would also need a !rc check.

> 
> Signed-off-by: Jan Beulich 

Acked-by: Roger Pau Monné 

Albeit I do prefer the extra call to paging_unlock() and early return
from the function in case of error.

Thanks, Roger.



Re: [PATCH 3/4] x86/paging: vCPU host mode is always set

2024-04-24 Thread Roger Pau Monné
On Tue, Apr 23, 2024 at 04:32:32PM +0200, Jan Beulich wrote:
> ... thanks to paging_vcpu_init() being part of vCPU creation. Further
> if paging is enabled on a domain, it's also guaranteed to be either HAP
> or shadow. Drop respective unnecessary (parts of) conditionals.

Is there some commit that changed when arch.paging.mode gets set, so
that this is actually safe to do now, but not when the code in
paging_dump_vcpu_info() was introduced?

I get the feeling we want to reference some change here in order to
explain why is now always guaranteed to be set.

> 
> Signed-off-by: Jan Beulich 

Acked-by: Roger Pau Monné 

Thanks, Roger.



Re: [XEN PATCH 01/10] libelf: address violations of MISRA C Rule 20.7

2024-04-24 Thread Jan Beulich
On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini 

Acked-by: Jan Beulich 





Re: [XEN PATCH 04/10] drivers: char: address violation of MISRA C Rule 20.7

2024-04-24 Thread Jan Beulich
On 23.04.2024 17:12, Nicola Vetrini wrote:
> --- a/xen/drivers/char/omap-uart.c
> +++ b/xen/drivers/char/omap-uart.c
> @@ -48,8 +48,9 @@
>  /* System configuration register */
>  #define UART_OMAP_SYSC_DEF_CONF   0x0d   /* autoidle mode, wakeup is enabled 
> */
>  
> -#define omap_read(uart, off)   readl((uart)->regs + (off< -#define omap_write(uart, off, val) writel((val), (uart)->regs + 
> (off< +#define omap_read(uart, off)   readl((uart)->regs + ((off) << REG_SHIFT))
> +#define omap_write(uart, off, val) writel((val), (uart)->regs + \

Would have been nice to drop the excess parentheses at the same time.

Jan



Re: [XEN PATCH 08/10] x86/hvm: hpet: address violations of MISRA C Rule 20.7

2024-04-24 Thread Jan Beulich
On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini 

Acked-by: Jan Beulich 





Re: [XEN PATCH 09/10] x86/debugreg: address violation of MISRA C Rule 20.7

2024-04-24 Thread Jan Beulich
On 23.04.2024 17:12, Nicola Vetrini wrote:
> MISRA C Rule 20.7 states: "Expressions resulting from the expansion
> of macro parameters shall be enclosed in parentheses". Therefore, some
> macro definitions should gain additional parentheses to ensure that all
> current and future users will be safe with respect to expansions that
> can possibly alter the semantics of the passed-in macro parameter.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini 

Acked-by: Jan Beulich 





Re: [XEN PATCH] automation/eclair: add deviations for MISRA C:2012 Rule 16.4

2024-04-24 Thread Federico Serafini

On 23/04/24 18:06, Jan Beulich wrote:

On 23.04.2024 17:52, Federico Serafini wrote:

On 23/04/24 12:26, Jan Beulich wrote:

On 23.04.2024 12:02, Federico Serafini wrote:

+
+   * - R16.4
+ - A switch statement with a single switch clause and no default label may
+   be used in place of an equivalent if statement if it is considered to
+   improve readability."


No, I don't think there should be examples in those documents. But those
documents should also not (blindly) rely on terminology in the Misra
spec, as not everyone has access to that (licensed copies had to be
obtained for quite a few of us).


In deviations.rst there is an identical deviation for Rule 16.6
("Every switch statement shall have at least two switch-clauses").
I think we should remain consistent.

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)



[PATCH v4 1/4] xen-livepatch: fix parameter name parsing

2024-04-24 Thread Roger Pau Monne
It's incorrect to restrict strncmp to the length of the command line input
parameter, as then a user passing a rune like:

% xen-livepatch up foo.livepatch

Would match against the "upload" command, because the string comparison has
been truncated to the length of the input argument.  Use strcmp instead which
doesn't truncate.  Otherwise in order to keep using strncmp we would need to
also check strings are of the same length before doing the comparison.

Fixes: 05bb8afedede ('xen-xsplice: Tool to manipulate xsplice payloads')
Signed-off-by: Roger Pau Monné 
---
Changes since v3:
 - Use strcmp.

Changes since v2:
 - New in this version.
---
 tools/misc/xen-livepatch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 5bf9d9a32b65..2c4f69e596fa 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -572,13 +572,13 @@ int main(int argc, char *argv[])
 return 0;
 }
 for ( i = 0; i < ARRAY_SIZE(main_options); i++ )
-if (!strncmp(main_options[i].name, argv[1], strlen(argv[1])))
+if (!strcmp(main_options[i].name, argv[1]))
 break;
 
 if ( i == ARRAY_SIZE(main_options) )
 {
 for ( j = 0; j < ARRAY_SIZE(action_options); j++ )
-if (!strncmp(action_options[j].name, argv[1], strlen(argv[1])))
+if (!strcmp(action_options[j].name, argv[1]))
 break;
 
 if ( j == ARRAY_SIZE(action_options) )
-- 
2.44.0




[PATCH v4 3/4] livepatch: refuse to resolve symbols that belong to init sections

2024-04-24 Thread Roger Pau Monne
Livepatch payloads containing symbols that belong to init sections can only
lead to page faults later on, as by the time the livepatch is loaded init
sections have already been freed.

Refuse to resolve such symbols and return an error instead.

Note such resolutions are only relevant for symbols that point to undefined
sections (SHN_UNDEF), as that implies the symbol is not in the current payload
and hence must either be a Xen or a different livepatch payload symbol.

Do not allow to resolve symbols that point to __init_begin, as that address is
also unmapped.  On the other hand, __init_end is not unmapped, and hence allow
resolutions against it.

Since __init_begin can alias other symbols (like _erodata for example)
allow the force flag to override the check and resolve the symbol anyway.

Signed-off-by: Roger Pau Monné 
---
Changes since v3:
 - Print warning message even when using the force option.

Changes since v2:
 - Allow bypassing added check with the force flag.

Changes since v1:
 - Fix off-by-one in range checking.
---
 xen/common/livepatch.c  | 13 -
 xen/common/livepatch_elf.c  | 22 +-
 xen/include/xen/livepatch_elf.h |  2 +-
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 502e264bc6fe..1afde0281402 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1080,7 +1080,8 @@ static void free_payload(struct payload *data)
 xfree(data);
 }
 
-static int load_payload_data(struct payload *payload, void *raw, size_t len)
+static int load_payload_data(struct payload *payload, void *raw, size_t len,
+ bool force)
 {
 struct livepatch_elf elf = { .name = payload->name, .len = len };
 int rc = 0;
@@ -1093,7 +1094,7 @@ static int load_payload_data(struct payload *payload, 
void *raw, size_t len)
 if ( rc )
 goto out;
 
-rc = livepatch_elf_resolve_symbols();
+rc = livepatch_elf_resolve_symbols(, force);
 if ( rc )
 goto out;
 
@@ -1133,7 +1134,8 @@ static int load_payload_data(struct payload *payload, 
void *raw, size_t len)
 return rc;
 }
 
-static int livepatch_upload(struct xen_sysctl_livepatch_upload *upload)
+static int livepatch_upload(struct xen_sysctl_livepatch_upload *upload,
+bool force)
 {
 struct payload *data, *found;
 char n[XEN_LIVEPATCH_NAME_SIZE];
@@ -1162,7 +1164,7 @@ static int livepatch_upload(struct 
xen_sysctl_livepatch_upload *upload)
 {
 memcpy(data->name, n, strlen(n));
 
-rc = load_payload_data(data, raw_data, upload->size);
+rc = load_payload_data(data, raw_data, upload->size, force);
 if ( rc )
 goto out;
 
@@ -2132,7 +2134,8 @@ int livepatch_op(struct xen_sysctl_livepatch_op 
*livepatch)
 switch ( livepatch->cmd )
 {
 case XEN_SYSCTL_LIVEPATCH_UPLOAD:
-rc = livepatch_upload(>u.upload);
+rc = livepatch_upload(>u.upload,
+  livepatch->flags & LIVEPATCH_FLAG_FORCE);
 break;
 
 case XEN_SYSCTL_LIVEPATCH_GET:
diff --git a/xen/common/livepatch_elf.c b/xen/common/livepatch_elf.c
index 45d73912a3cd..1d0ed8f99bcc 100644
--- a/xen/common/livepatch_elf.c
+++ b/xen/common/livepatch_elf.c
@@ -4,6 +4,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -276,7 +277,7 @@ static int elf_get_sym(struct livepatch_elf *elf, const 
void *data)
 return 0;
 }
 
-int livepatch_elf_resolve_symbols(struct livepatch_elf *elf)
+int livepatch_elf_resolve_symbols(struct livepatch_elf *elf, bool force)
 {
 unsigned int i;
 int rc = 0;
@@ -310,6 +311,25 @@ int livepatch_elf_resolve_symbols(struct livepatch_elf 
*elf)
 break;
 }
 }
+
+/*
+ * Ensure not an init symbol.  Only applicable to Xen symbols, as
+ * livepatch payloads don't have init sections or equivalent.
+ */
+else if ( st_value >= (uintptr_t)&__init_begin &&
+  st_value <  (uintptr_t)&__init_end )
+{
+printk("%s" LIVEPATCH "%s: symbol %s is in init section%s\n",
+   force ? XENLOG_WARNING : XENLOG_ERR,
+   elf->name, elf->sym[i].name,
+   force ? "" : ", not resolving");
+if ( !force )
+{
+rc = -ENXIO;
+break;
+}
+}
+
 dprintk(XENLOG_DEBUG, LIVEPATCH "%s: Undefined symbol resolved: %s 
=> %#"PRIxElfAddr"\n",
 elf->name, elf->sym[i].name, st_value);
 break;
diff --git a/xen/include/xen/livepatch_elf.h b/xen/include/xen/livepatch_elf.h
index 7116deaddc28..84e9c5eb7be5 100644
--- a/xen/include/xen/livepatch_elf.h
+++ b/xen/include/xen/livepatch_elf.h
@@ -44,7 +44,7 @@ livepatch_elf_sec_by_name(const struct 

[PATCH v4 4/4] x86/livepatch: perform sanity checks on the payload exception table contents

2024-04-24 Thread Roger Pau Monne
Ensure the entries of a payload exception table only apply to text regions in
the payload itself.  Since the payload exception table needs to be loaded and
active even before a patch is applied (because hooks might already rely on it),
make sure the exception table (if any) only contains fixups for the payload
text section.

Signed-off-by: Roger Pau Monné 
Reviewed-by: Jan Beulich 
---
Changes since v3:
 - Rename to extable_in_bounds().
 - Use _p() instead of casting to void *.

Changes since v2:
 - New in this version.
---
 xen/arch/x86/extable.c | 18 ++
 xen/arch/x86/include/asm/uaccess.h |  3 +++
 xen/common/livepatch.c |  9 +
 3 files changed, 30 insertions(+)

diff --git a/xen/arch/x86/extable.c b/xen/arch/x86/extable.c
index 8415cd1fa249..2be090b92df8 100644
--- a/xen/arch/x86/extable.c
+++ b/xen/arch/x86/extable.c
@@ -228,3 +228,21 @@ unsigned long asmlinkage search_pre_exception_table(struct 
cpu_user_regs *regs)
 }
 return fixup;
 }
+
+#ifdef CONFIG_LIVEPATCH
+bool extable_in_bounds(const struct exception_table_entry *ex_start,
+   const struct exception_table_entry *ex_end,
+   const void *start, const void *end)
+{
+for ( ; ex_start < ex_end; ex_start++ )
+{
+const void *addr = _p(ex_addr(ex_start));
+const void *cont = _p(ex_cont(ex_start));
+
+if ( addr < start || addr >= end || cont < start || cont >= end )
+return false;
+}
+
+return true;
+}
+#endif
diff --git a/xen/arch/x86/include/asm/uaccess.h 
b/xen/arch/x86/include/asm/uaccess.h
index 48b684c19d44..4995edfdd8db 100644
--- a/xen/arch/x86/include/asm/uaccess.h
+++ b/xen/arch/x86/include/asm/uaccess.h
@@ -426,5 +426,8 @@ extern unsigned long search_exception_table(const struct 
cpu_user_regs *regs,
 extern void sort_exception_tables(void);
 extern void sort_exception_table(struct exception_table_entry *start,
  const struct exception_table_entry *stop);
+extern bool extable_in_bounds(const struct exception_table_entry *ex_start,
+  const struct exception_table_entry *ex_end,
+  const void *start, const void *end);
 
 #endif /* __X86_UACCESS_H__ */
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 1afde0281402..4702c06902a9 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -912,6 +912,15 @@ static int prepare_payload(struct payload *payload,
 s = sec->load_addr;
 e = sec->load_addr + sec->sec->sh_size;
 
+if ( !extable_in_bounds(s, e, payload->text_addr,
+payload->text_addr + payload->text_size) )
+{
+printk(XENLOG_ERR LIVEPATCH
+   "%s: Invalid exception table with out of bounds entries\n",
+   elf->name);
+return -EINVAL;
+}
+
 sort_exception_table(s ,e);
 
 region->ex = s;
-- 
2.44.0




[PATCH v4 2/4] livepatch: introduce --force option

2024-04-24 Thread Roger Pau Monne
Introduce a xen-livepatch tool --force option, that's propagated into the
hyerpvisor for livepatch operations.  The intention is for the option to be
used to bypass some checks that would otherwise prevent the patch from being
loaded.

Re purpose the pad field in xen_sysctl_livepatch_op to be a flags field that
applies to all livepatch operations.  The flag is currently only set by the
hypercall wrappers for the XEN_SYSCTL_LIVEPATCH_UPLOAD operation, as that's so
far the only one where it will be used initially.  Other uses can be added as
required.

Note that helpers would set the .pad field to 0, that's been removed since the
structure is already zero initialized at definition.

No functional usages of the new flag introduced in this patch.

Signed-off-by: Roger Pau Monné 
Acked-by: Jan Beulich 
Acked-by: Anthony PERARD 
---
Changes since v3:
 - Use strcmp instead of strncmp.

Changes since v2:
 - New in this version.
---
 tools/include/xenctrl.h |  3 ++-
 tools/libs/ctrl/xc_misc.c   |  7 +++
 tools/misc/xen-livepatch.c  | 21 +++--
 xen/common/livepatch.c  |  3 ++-
 xen/include/public/sysctl.h |  4 +++-
 5 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 2ef8b4e05422..499685594427 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2555,7 +2555,8 @@ int xc_psr_get_hw_info(xc_interface *xch, uint32_t socket,
 #endif
 
 int xc_livepatch_upload(xc_interface *xch,
-char *name, unsigned char *payload, uint32_t size);
+char *name, unsigned char *payload, uint32_t size,
+bool force);
 
 int xc_livepatch_get(xc_interface *xch,
  char *name,
diff --git a/tools/libs/ctrl/xc_misc.c b/tools/libs/ctrl/xc_misc.c
index 5ecdfa2c7934..50282fd60dcc 100644
--- a/tools/libs/ctrl/xc_misc.c
+++ b/tools/libs/ctrl/xc_misc.c
@@ -576,7 +576,8 @@ int xc_getcpuinfo(xc_interface *xch, int max_cpus,
 int xc_livepatch_upload(xc_interface *xch,
 char *name,
 unsigned char *payload,
-uint32_t size)
+uint32_t size,
+bool force)
 {
 int rc;
 struct xen_sysctl sysctl = {};
@@ -612,7 +613,7 @@ int xc_livepatch_upload(xc_interface *xch,
 
 sysctl.cmd = XEN_SYSCTL_livepatch_op;
 sysctl.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_UPLOAD;
-sysctl.u.livepatch.pad = 0;
+sysctl.u.livepatch.flags = force ? LIVEPATCH_FLAG_FORCE : 0;
 sysctl.u.livepatch.u.upload.size = size;
 set_xen_guest_handle(sysctl.u.livepatch.u.upload.payload, local);
 
@@ -656,7 +657,6 @@ int xc_livepatch_get(xc_interface *xch,
 
 sysctl.cmd = XEN_SYSCTL_livepatch_op;
 sysctl.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_GET;
-sysctl.u.livepatch.pad = 0;
 
 sysctl.u.livepatch.u.get.status.state = 0;
 sysctl.u.livepatch.u.get.status.rc = 0;
@@ -985,7 +985,6 @@ static int _xc_livepatch_action(xc_interface *xch,
 
 sysctl.cmd = XEN_SYSCTL_livepatch_op;
 sysctl.u.livepatch.cmd = XEN_SYSCTL_LIVEPATCH_ACTION;
-sysctl.u.livepatch.pad = 0;
 sysctl.u.livepatch.u.action.cmd = action;
 sysctl.u.livepatch.u.action.timeout = timeout;
 sysctl.u.livepatch.u.action.flags = flags;
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index 2c4f69e596fa..c16fb6862d6c 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -19,11 +19,15 @@
 
 static xc_interface *xch;
 
+/* Global option to disable checks. */
+static bool force;
+
 void show_help(void)
 {
 fprintf(stderr,
 "xen-livepatch: live patching tool\n"
-"Usage: xen-livepatch  [args] [command-flags]\n"
+"Usage: xen-livepatch [--force]  [args] [command-flags]\n"
+" Use --force option to bypass some checks.\n"
 "  An unique name of payload. Up to %d characters.\n"
 "Commands:\n"
 "  help   display this help\n"
@@ -240,7 +244,7 @@ static int upload_func(int argc, char *argv[])
 return saved_errno;
 }
 printf("Uploading %s... ", filename);
-rc = xc_livepatch_upload(xch, name, fbuf, len);
+rc = xc_livepatch_upload(xch, name, fbuf, len, force);
 if ( rc )
 {
 rc = errno;
@@ -571,6 +575,19 @@ int main(int argc, char *argv[])
 show_help();
 return 0;
 }
+
+if ( strcmp("--force", argv[1]) )
+{
+if ( argc <= 2 )
+{
+show_help();
+return EXIT_FAILURE;
+}
+force = true;
+argv++;
+argc--;
+}
+
 for ( i = 0; i < ARRAY_SIZE(main_options); i++ )
 if (!strcmp(main_options[i].name, argv[1]))
 break;
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 351a3e0b9a60..502e264bc6fe 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -2125,7 

[PATCH v4 0/4] livepatch: minor bug fixes and improvements

2024-04-24 Thread Roger Pau Monne
Hello,

Following series contain some minor bugfixes and improvements for
livepatch logic, both inside the hypervisor and on the command line
tool.

Thanks, Roger.

Roger Pau Monne (4):
  xen-livepatch: fix parameter name parsing
  livepatch: introduce --force option
  livepatch: refuse to resolve symbols that belong to init sections
  x86/livepatch: perform sanity checks on the payload exception table
contents

 tools/include/xenctrl.h|  3 ++-
 tools/libs/ctrl/xc_misc.c  |  7 +++
 tools/misc/xen-livepatch.c | 25 +
 xen/arch/x86/extable.c | 18 ++
 xen/arch/x86/include/asm/uaccess.h |  3 +++
 xen/common/livepatch.c | 25 +++--
 xen/common/livepatch_elf.c | 22 +-
 xen/include/public/sysctl.h|  4 +++-
 xen/include/xen/livepatch_elf.h|  2 +-
 9 files changed, 91 insertions(+), 18 deletions(-)

-- 
2.44.0




  1   2   >