from:"Jiamei Xie"

Re: [PATCH v1 16/29] xen/asm-generic: introduce stub header flushtlb.h

2023-09-14 Thread Jiamei Xie


Hi Oleksii

On 2023/9/14 22:56, Oleksii Kurochko wrote:

The patch introduces header stub necessry for full Xen build.

Signed-off-by: Oleksii Kurochko 
---
  xen/include/asm-generic/flushtlb.h | 42 ++
  1 file changed, 42 insertions(+)
  create mode 100644 xen/include/asm-generic/flushtlb.h

diff --git a/xen/include/asm-generic/flushtlb.h 
b/xen/include/asm-generic/flushtlb.h
new file mode 100644
index 00..79e4773179
--- /dev/null
+++ b/xen/include/asm-generic/flushtlb.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __ASM_GENERIC_FLUSHTLB_H__
+#define __ASM_GENERIC_FLUSHTLB_H__
+
+#include 
+
+/*
+ * Filter the given set of CPUs, removing those that definitely flushed their
+ * TLB since @page_timestamp.
+ */
+/* XXX lazy implementation just doesn't clear anything */
+static inline void tlbflush_filter(cpumask_t *mask, uint32_t page_timestamp) {}
+
+#define tlbflush_current_time() (0)
+
+static inline void page_set_tlbflush_timestamp(struct page_info *page)
+{
+BUG();
+}
+
+/* Flush specified CPUs' TLBs */
+void arch_flush_tlb_mask(const cpumask_t *mask);
+
+#endif /* __ASM_GENERIC_FLUSHTLB_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
+
+

It's duplicated.

+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: BSD
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */

Re: [PATCH v5 4/4] automation: Add smoke test for ppc64le

2023-07-23 Thread Jiamei Xie


On 2023/7/22 01:02, Shawn Anastasio wrote:

Add an initial smoke test that boots xen on a ppc64/pseries machine and

checks for a magic string. Based on the riscv smoke test.



Eventually the powernv9 (POWER9 bare metal) machine type will want to be

tested as well, but for now we only boot on pseries.



Signed-off-by: Shawn Anastasio 
<mailto:sanasta...@raptorengineering.com>

Reviewed-by: Stefano Stabellini 
<mailto:sstabell...@kernel.org>
Reviewed-by: Jiamei Xie mailto:jiamei@arm.com>>

Re: [PATCH v5 4/4] automation: Add smoke test for ppc64le

2023-07-23 Thread Jiamei Xie




On 2023/7/22 01:02, Shawn Anastasio wrote:

Add an initial smoke test that boots xen on a ppc64/pseries machine and
checks for a magic string. Based on the riscv smoke test.

Eventually the powernv9 (POWER9 bare metal) machine type will want to be
tested as well, but for now we only boot on pseries.

Signed-off-by: Shawn Anastasio 
Reviewed-by: Stefano Stabellini 

Reviewed-by: Jiamei Xie

Re: [PATCH] build: Drop CONFIG_$ARCH_$(XEN_OS) definitions

2023-06-20 Thread Jiamei Xie


Hi Andrew，

On 2023/6/20 02:06, Andrew Cooper wrote:

These aren't used, and are not obvious useful either.

tools/ does have some logic which works on $(XEN_OS) directly, and some on
CONFIG_$(XEN_OS) too, but this isn't how we typically refer to things.

The only user ever of this scheme was introduced in c0fd920e987 (2006) and
deleted in fa2244104b4 (2010).

No functional change.

Signed-off-by: Andrew Cooper 



I run it on fvp_base_revc. It works fine.

Tested-by: Jiamei Xie mailto:jiamei@arm.com>

Best wishes,

Jiamei

Re: [PATCH 1/4] automation: make console options configurable via variables

2023-05-15 Thread Jiamei Xie



On 2023/5/13 10:12, Marek Marczykowski-Górecki wrote:

This makes the test script easier reusable for different runners, where
console may be connected differently. Include both console= option and
configuration for specific chosen console too (like com1= here) in the
'CONSOLE_OPTS' variable.

Signed-off-by: Marek Marczykowski-Górecki


Reviewed-by: Jiamei Xie

RE: [PATCH 1/2] automation: arm64: Create a test job for testing static heap on qemu

2023-03-03 Thread Jiamei Xie

Hi Michal,

> -Original Message-
> From: Michal Orzel 
> Sent: Friday, March 3, 2023 3:47 PM
> To: Jiamei Xie ; Stefano Stabellini
> 
> Cc: xen-devel@lists.xenproject.org; Wei Chen ;
> Bertrand Marquis ; Doug Goldstein
> 
> Subject: Re: [PATCH 1/2] automation: arm64: Create a test job for testing
> static heap on qemu
> 
> Hi Jiamei,
> 
> On 03/03/2023 07:49, Jiamei Xie wrote:
> >
> >
> > Hi Stefano,
> >
> >> -Original Message-
> >> From: Stefano Stabellini 
> >> Sent: Friday, March 3, 2023 9:51 AM
> >> To: Jiamei Xie 
> >> Cc: xen-devel@lists.xenproject.org; Wei Chen ;
> >> sstabell...@kernel.org; Bertrand Marquis ;
> >> Doug Goldstein 
> >> Subject: Re: [PATCH 1/2] automation: arm64: Create a test job for testing
> >> static heap on qemu
> >>
> >> On Thu, 2 Mar 2023, jiamei.xie wrote:
> >>> From: Jiamei Xie 
> >>>
> >>> Create a new test job, called qemu-smoke-dom0less-arm64-gcc-
> staticheap.
> >>>
> >>> Add property "xen,static-heap" under /chosen node to enable static-heap.
> >>> If the domU can start successfully with static-heap enabled, then this
> >>> test pass.
> >>>
> >>> Signed-off-by: Jiamei Xie 
> >>
> >> Hi Jiamei, thanks for the patch!
> >>
> >>
> >>> ---
> >>>  automation/gitlab-ci/test.yaml | 16 
> >>>  .../scripts/qemu-smoke-dom0less-arm64.sh   | 18
> >> ++
> >>>  2 files changed, 34 insertions(+)
> >>>
> >>> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-
> ci/test.yaml
> >>> index 1c5f400b68..5a9b88477a 100644
> >>> --- a/automation/gitlab-ci/test.yaml
> >>> +++ b/automation/gitlab-ci/test.yaml
> >>> @@ -133,6 +133,22 @@ qemu-smoke-dom0less-arm64-gcc-debug-
> >> staticmem:
> >>>  - *arm64-test-needs
> >>>  - alpine-3.12-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.12-gcc-arm64
> >>> +
> >>> +qemu-smoke-dom0less-arm64-gcc-debug-staticheap:
> >>> + extends: .qemu-arm64
> >>> + script:
> >>> +   - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-heap
> >> 2>&1 | tee ${LOGFILE}
> >>> + needs:
> >>> +   - *arm64-test-needs
> >>> +   - alpine-3.12-gcc-debug-arm64
> >>> +
> >>>  qemu-smoke-dom0less-arm64-gcc-boot-cpupools:
> >>>extends: .qemu-arm64
> >>>script:
> >>> diff --git a/automation/scripts/qemu-smoke-dom0less-arm64.sh
> >> b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> >>> index 182a4b6c18..4e73857199 100755
> >>> --- a/automation/scripts/qemu-smoke-dom0less-arm64.sh
> >>> +++ b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> >>> @@ -27,6 +27,11 @@ fi
> >>>  "
> >>>  fi
> >>>
> >>> +if [[ "${test_variant}" == "static-heap" ]]; then
> >>> +passed="${test_variant} test passed"
> >>> +domU_check="echo \"${passed}\""
> >>> +fi
> >>> +
> >>>  if [[ "${test_variant}" == "boot-cpupools" ]]; then
> >>>  # Check if domU0 (id=1) is assigned to Pool-1 with null scheduler
> >>>  passed="${test_variant} test passed"
> >>> @@ -128,6 +133,19 @@ if [[ "${test_variant}" == "static-mem" ]]; then
> >>>  echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base}
> ${domu_size}\"" >>
> >> binaries/config
> >>>  fi
> >>>
> >>> +if [[ "${test_variant}" == "static-heap" ]]; then
> >>> +# ImageBuilder uses the config file to create the uboot script.
> Devicetree
> >>> +# will be set via the generated uboot script.
> >>> +# The valid memory range is 0x4000 to 0x8000 as defined
> >> before.
> >>> +# ImageBuillder sets the kernel and ramdisk range based on the file
> size.
> >>> +#

RE: [ImageBuilder][PATCH 1/2] uboot-script-gen: Add support for static heap

2023-03-02 Thread Jiamei Xie

Hi Stefano and Michal,

> -Original Message-
> From: Stefano Stabellini 
> Sent: Friday, March 3, 2023 7:42 AM
> To: Michal Orzel 
> Cc: Jiamei Xie ; xen-devel@lists.xenproject.org; Wei
> Chen ; sstabell...@kernel.org
> Subject: Re: [ImageBuilder][PATCH 1/2] uboot-script-gen: Add support for
> static heap
> 
> On Thu, 2 Mar 2023, Michal Orzel wrote:
> > Hi Jiamei,
> >
> > Patch looks good apart from minor comments down below.
> 
> Just wanted to add that the patch looks OK to me too and don't have any
> further comments beyond the ones Michal's already made
> 
> 
> > On 02/03/2023 05:46, jiamei.xie wrote:
> > >
> > >
> > > From: jiamei Xie 
> > >
> > > Add a new config parameter to configure static heap.
> > > STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
> > > if specified, indicates the host physical address regions
> > > [baseaddr, baseaddr + size) to be reserved as static heap.
> > >
> > > For instance, STATIC_HEAP="0x5000 0x3000", if specified,
> > > indicates the host memory region starting from paddr 0x5000
> > > with a size of 0x3000 to be reserved as static heap.
> > >
> > > Signed-off-by: jiamei Xie 
> > > ---
> > >  README.md|  4 
> > >  scripts/uboot-script-gen | 20 
> > >  2 files changed, 24 insertions(+)
> > >
> > > diff --git a/README.md b/README.md
> > > index 814a004..787f413 100644
> > > --- a/README.md
> > > +++ b/README.md
> > > @@ -256,6 +256,10 @@ Where:
> > >
> > >  - NUM_CPUPOOLS specifies the number of boot-time cpupools to create.
> > >
> > > +- STATIC_HEAP="baseaddr1 size1 ... baseaddrN sizeN"
> > > +  if specified, indicates the host physical address regions
> > > +  [baseaddr, baseaddr + size) to be reserved as static heap.
> > As this option impacts Xen and not domUs, please call it XEN_STATIC_HEAP
> and move
> > it right after XEN_CMD documentation.
Thanks for your comments . Ack
> >
> > > +
> > >  Then you can invoke uboot-script-gen as follows:
> > >
> > >  ```
> > > diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> > > index f07e334..4775293 100755
> > > --- a/scripts/uboot-script-gen
> > > +++ b/scripts/uboot-script-gen
> > > @@ -189,6 +189,21 @@ function add_device_tree_static_mem()
> > >  dt_set "$path" "xen,static-mem" "hex" "${cells[*]}"
> > >  }
> > >
> > > +function add_device_tree_static_heap()
> > > +{
> > > +local path=$1
> > > +local regions=$2
> > > +local cells=()
> > > +local val
> > > +
> > > +for val in ${regions[@]}
> > > +do
> > > +cells+=("$(printf "0x%x 0x%x" $(($val >> 32)) $(($val & ((1 << 
> > > 32) -
> 1")
> > Please use split_value function instead of opencoding it.
> > It will then become:
> > cells+=("$(split_value $val)")

Thanks for your comments . Ack.
> >
> > ~Michal
> >

RE: [PATCH 1/2] automation: arm64: Create a test job for testing static heap on qemu

2023-03-02 Thread Jiamei Xie

Hi Stefano,

> -Original Message-
> From: Stefano Stabellini 
> Sent: Friday, March 3, 2023 9:51 AM
> To: Jiamei Xie 
> Cc: xen-devel@lists.xenproject.org; Wei Chen ;
> sstabell...@kernel.org; Bertrand Marquis ;
> Doug Goldstein 
> Subject: Re: [PATCH 1/2] automation: arm64: Create a test job for testing
> static heap on qemu
> 
> On Thu, 2 Mar 2023, jiamei.xie wrote:
> > From: Jiamei Xie 
> >
> > Create a new test job, called qemu-smoke-dom0less-arm64-gcc-staticheap.
> >
> > Add property "xen,static-heap" under /chosen node to enable static-heap.
> > If the domU can start successfully with static-heap enabled, then this
> > test pass.
> >
> > Signed-off-by: Jiamei Xie 
> 
> Hi Jiamei, thanks for the patch!
> 
> 
> > ---
> >  automation/gitlab-ci/test.yaml | 16 
> >  .../scripts/qemu-smoke-dom0less-arm64.sh   | 18
> ++
> >  2 files changed, 34 insertions(+)
> >
> > diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
> > index 1c5f400b68..5a9b88477a 100644
> > --- a/automation/gitlab-ci/test.yaml
> > +++ b/automation/gitlab-ci/test.yaml
> > @@ -133,6 +133,22 @@ qemu-smoke-dom0less-arm64-gcc-debug-
> staticmem:
> >  - *arm64-test-needs
> >  - alpine-3.12-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.12-gcc-arm64
> > +
> > +qemu-smoke-dom0less-arm64-gcc-debug-staticheap:
> > + extends: .qemu-arm64
> > + script:
> > +   - ./automation/scripts/qemu-smoke-dom0less-arm64.sh static-heap
> 2>&1 | tee ${LOGFILE}
> > + needs:
> > +   - *arm64-test-needs
> > +   - alpine-3.12-gcc-debug-arm64
> > +
> >  qemu-smoke-dom0less-arm64-gcc-boot-cpupools:
> >extends: .qemu-arm64
> >script:
> > diff --git a/automation/scripts/qemu-smoke-dom0less-arm64.sh
> b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> > index 182a4b6c18..4e73857199 100755
> > --- a/automation/scripts/qemu-smoke-dom0less-arm64.sh
> > +++ b/automation/scripts/qemu-smoke-dom0less-arm64.sh
> > @@ -27,6 +27,11 @@ fi
> >  "
> >  fi
> >
> > +if [[ "${test_variant}" == "static-heap" ]]; then
> > +passed="${test_variant} test passed"
> > +domU_check="echo \"${passed}\""
> > +fi
> > +
> >  if [[ "${test_variant}" == "boot-cpupools" ]]; then
> >  # Check if domU0 (id=1) is assigned to Pool-1 with null scheduler
> >  passed="${test_variant} test passed"
> > @@ -128,6 +133,19 @@ if [[ "${test_variant}" == "static-mem" ]]; then
> >  echo -e "\nDOMU_STATIC_MEM[0]=\"${domu_base} ${domu_size}\"" >>
> binaries/config
> >  fi
> >
> > +if [[ "${test_variant}" == "static-heap" ]]; then
> > +# ImageBuilder uses the config file to create the uboot script. 
> > Devicetree
> > +# will be set via the generated uboot script.
> > +# The valid memory range is 0x4000 to 0x8000 as defined
> before.
> > +# ImageBuillder sets the kernel and ramdisk range based on the file 
> > size.
> > +# It will use the memory range between 0x4560 to 0x47AED1E8, so
> set
> > +# memory range between 0x5000 and 0x8000 as static heap.
> 
> I think this is OK. One suggestion to make things more reliable would be
> to change MEMORY_END to be 0x5000 so that you can be sure that
> ImageBuilder won't go over the limit. You could do it just for this
> test, which would be safer, but to be honest you could limit MEMORY_END
> to 0x5000 for all tests in qemu-smoke-dom0less-arm64.sh because it
> shouldn't really cause any problems.
> 
[Jiamei Xie] 
Thanks for your comments. I am a little confused about " to change MEMORY_END 
to be 0x5000". 
 I set 0STATIC_HEAP="0x5000 0x3000" where is the start address. Why 
change MEMORY_END
 to be 0x5000?


Best wishes
Jiamei Xie


> 
> > +echo  '
> > +STATIC_HEAP="0x5000 0x3000"
> > +# The size of static heap should be greater than the guest memory
> > +DOMU_MEM[0]="128"' >> binaries/config
> > +fi
> > +
> >  if [[ "${test_variant}" == "boot-cpupools" ]]; then
> >  echo '
> >  CPUPOOL[0]="cpu@1 null"
> > --
> > 2.25.1
> >

[PATCH v5 2/2] xen/arm: vpl011: add ASSERT_UNREACHABLE in vpl011_mmio_read

2022-12-04 Thread Jiamei Xie

In vpl011_mmio_read switch block, all cases should have a return. Add
ASSERT_UNREACHABLE to catch case where the return is not added.

Signed-off-by: Jiamei Xie 
---
v4 -> v5
- don't remove "return 1" and add ASSERT_UNREACHABLE
v3 -> v4
- Don't consolidate check registers access
- Don't remove label read_as_zero
---
 xen/arch/arm/vpl011.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index f4a5621fab..c7f9dae76c 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -417,6 +417,7 @@ static int vpl011_mmio_read(struct vcpu *v,
 goto read_as_zero;
 }
 
+ASSERT_UNREACHABLE();
 return 1;
 
 read_as_zero:
-- 
2.25.1

[PATCH v5 1/2] xen/arm: vpl011: emulate non-SBSA registers as WI/RAZ

2022-12-04 Thread Jiamei Xie

When the guest kernel enables DMA engine with "CONFIG_DMA_ENGINE=y",
Linux SBSA PL011 driver will access PL011 DMACR register in some
functions. As chapter "B Generic UART" in "ARM Server Base System
Architecture"[1] documentation describes, SBSA UART doesn't support
DMA. In current code, when the kernel tries to access DMACR register,
Xen will inject a data abort:
Unhandled fault at 0xffc00944d048
Mem abort info:
  ESR = 0x9600
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
  FSC = 0x00: ttbr address size fault
Data abort info:
  ISV = 0, ISS = 0x
  CM = 0, WnR = 0
swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
[ffc00944d048] pgd=10003803, p4d=10003803, 
pud=10003803, pmd=10003fffa803, pte=00689c090f13
Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
...
Call trace:
 pl011_stop_rx+0x70/0x80
 tty_port_shutdown+0x7c/0xb4
 tty_port_close+0x60/0xcc
 uart_close+0x34/0x8c
 tty_release+0x144/0x4c0
 __fput+0x78/0x220
 fput+0x1c/0x30
 task_work_run+0x88/0xc0
 do_notify_resume+0x8d0/0x123c
 el0_svc+0xa8/0xc0
 el0t_64_sync_handler+0xa4/0x130
 el0t_64_sync+0x1a0/0x1a4
Code: b983 b901f001 794038a0 8b42 (b941)
---[ end trace 83dd93df15c3216f ]---
note: bootlogd[132] exited with preempt_count 1
/etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-daemon

As discussed in [2], this commit makes the access to non-SBSA registers
RAZ/WI as an improvement.

[1] https://developer.arm.com/documentation/den0094/c/?lang=en
[2] 
https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2211161552420.4020@ubuntu-linux-20-04-desktop/

Signed-off-by: Jiamei Xie 
---
v4 -> v5
- no change
v3 -> v4
- remove the size check for unknown registers in the SBSA UART
- remove lock in read_as_zero
v2 -> v3
- emulate non-SBSA registers as WI/RAZ in default case
- update commit message
v1 -> v2
- print a message using XENLOG_G_DEBUG when it's write-ignore
---
 xen/arch/arm/vpl011.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 43522d48fd..f4a5621fab 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -414,11 +414,15 @@ static int vpl011_mmio_read(struct vcpu *v,
 default:
 gprintk(XENLOG_ERR, "vpl011: unhandled read r%d offset %#08x\n",
 dabt.reg, vpl011_reg);
-return 0;
+goto read_as_zero;
 }
 
 return 1;
 
+read_as_zero:
+*r = 0;
+return 1;
+
 bad_width:
 gprintk(XENLOG_ERR, "vpl011: bad read width %d r%d offset %#08x\n",
 dabt.size, dabt.reg, vpl011_reg);
@@ -486,7 +490,7 @@ static int vpl011_mmio_write(struct vcpu *v,
 default:
 gprintk(XENLOG_ERR, "vpl011: unhandled write r%d offset %#08x\n",
 dabt.reg, vpl011_reg);
-return 0;
+goto write_ignore;
 }
 
 write_ignore:
-- 
2.25.1

[PATCH v5 0/2] xen/arm: refine vpl011

2022-12-04 Thread Jiamei Xie

This patch is the version 5 for "xen/arm: vpl011: Make access to DMACR
write-ignore" [1]. 

[1] 
https://patchwork.kernel.org/project/xen-devel/patch/20221122054644.1092173-1-jiamei@arm.com/

Thanks,
Jiamei Xie

v4 -> v5
- don't remove "return 1" and add ASSERT_UNREACHABLE
v3 -> v4
- remove the size check for unknown registers in the SBSA UART
- remove lock in read_as_zero
- Don't consolidate check registers access
- Don't remove label read_as_zero
v2 -> v3
- emulate non-SBSA registers as WI/RAZ in default case
- update commit message
v1 -> v2
- print a message using XENLOG_G_DEBUG when it's write-ignore

Jiamei Xie (2):
  xen/arm: vpl011: emulate non-SBSA registers as WI/RAZ
  xen/arm: vpl011: add ASSERT_UNREACHABLE in vpl011_mmio_read

 xen/arch/arm/vpl011.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH v4 0/2] xen/arm: refine vpl011

2022-12-01 Thread Jiamei Xie

This patch is the version 4 for "xen/arm: vpl011: Make access to DMACR
write-ignore" [1]. 

[1] 
https://patchwork.kernel.org/project/xen-devel/patch/20221122054644.1092173-1-jiamei@arm.com/

Thanks,
Jiamei Xie

v3 -> v4
- remove the size check for unknown registers in the SBSA UART
- remove lock in read_as_zero
- Don't consolidate check registers access
- Don't remove label read_as_zero
v2 -> v3
- emulate non-SBSA registers as WI/RAZ in default case
- update commit message
v1 -> v2
- print a message using XENLOG_G_DEBUG when it's write-ignore

Jiamei Xie (2):
  xen/arm: vpl011: emulate non-SBSA registers as WI/RAZ
  xen/arm: vpl011: drop extra return in vpl011_mmio_read

 xen/arch/arm/vpl011.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH v4 2/2] xen/arm: vpl011: drop extra return in vpl011_mmio_read

2022-12-01 Thread Jiamei Xie

In vpl011_mmio_read switch block, all cases have a return. So the
outside one can be removed.

Signed-off-by: Jiamei Xie 
---
v3 -> v4
- Don't consolidate check registers access
- Don't remove label read_as_zero
---
 xen/arch/arm/vpl011.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index f4a5621fab..35de50760c 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -417,8 +417,6 @@ static int vpl011_mmio_read(struct vcpu *v,
 goto read_as_zero;
 }
 
-return 1;
-
 read_as_zero:
 *r = 0;
 return 1;
-- 
2.25.1

[PATCH v4 1/2] xen/arm: vpl011: emulate non-SBSA registers as WI/RAZ

2022-12-01 Thread Jiamei Xie

When the guest kernel enables DMA engine with "CONFIG_DMA_ENGINE=y",
Linux SBSA PL011 driver will access PL011 DMACR register in some
functions. As chapter "B Generic UART" in "ARM Server Base System
Architecture"[1] documentation describes, SBSA UART doesn't support
DMA. In current code, when the kernel tries to access DMACR register,
Xen will inject a data abort:
Unhandled fault at 0xffc00944d048
Mem abort info:
  ESR = 0x9600
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
  FSC = 0x00: ttbr address size fault
Data abort info:
  ISV = 0, ISS = 0x
  CM = 0, WnR = 0
swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
[ffc00944d048] pgd=10003803, p4d=10003803, 
pud=10003803, pmd=10003fffa803, pte=00689c090f13
Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
...
Call trace:
 pl011_stop_rx+0x70/0x80
 tty_port_shutdown+0x7c/0xb4
 tty_port_close+0x60/0xcc
 uart_close+0x34/0x8c
 tty_release+0x144/0x4c0
 __fput+0x78/0x220
 fput+0x1c/0x30
 task_work_run+0x88/0xc0
 do_notify_resume+0x8d0/0x123c
 el0_svc+0xa8/0xc0
 el0t_64_sync_handler+0xa4/0x130
 el0t_64_sync+0x1a0/0x1a4
Code: b983 b901f001 794038a0 8b42 (b941)
---[ end trace 83dd93df15c3216f ]---
note: bootlogd[132] exited with preempt_count 1
/etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-daemon

As discussed in [2], this commit makes the access to non-SBSA registers
RAZ/WI as an improvement.

[1] https://developer.arm.com/documentation/den0094/c/?lang=en
[2] 
https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2211161552420.4020@ubuntu-linux-20-04-desktop/

Signed-off-by: Jiamei Xie 
---
v3 -> v4
- remove the size check for unknown registers in the SBSA UART
- remove lock in read_as_zero
v2 -> v3
- emulate non-SBSA registers as WI/RAZ in default case
- update commit message
v1 -> v2
- print a message using XENLOG_G_DEBUG when it's write-ignore
---
 xen/arch/arm/vpl011.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 43522d48fd..f4a5621fab 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -414,11 +414,15 @@ static int vpl011_mmio_read(struct vcpu *v,
 default:
 gprintk(XENLOG_ERR, "vpl011: unhandled read r%d offset %#08x\n",
 dabt.reg, vpl011_reg);
-return 0;
+goto read_as_zero;
 }
 
 return 1;
 
+read_as_zero:
+*r = 0;
+return 1;
+
 bad_width:
 gprintk(XENLOG_ERR, "vpl011: bad read width %d r%d offset %#08x\n",
 dabt.size, dabt.reg, vpl011_reg);
@@ -486,7 +490,7 @@ static int vpl011_mmio_write(struct vcpu *v,
 default:
 gprintk(XENLOG_ERR, "vpl011: unhandled write r%d offset %#08x\n",
 dabt.reg, vpl011_reg);
-return 0;
+goto write_ignore;
 }
 
 write_ignore:
-- 
2.25.1

[PATCH v3 2/2] xen/arm: vpl011: drop redundancy in mmio_write/read

2022-11-28 Thread Jiamei Xie

This commit is to drop redundancy in the function vpl011_mmio_write
and vpl011_mmio_read:
- In vpl011_mmio_read switch block, all cases have a return. So the
  outside one can be removed.
- Each switch case checks access by the same if statments. So we can
  just use one and put it before the switch block.
- The goto label bad_width and read_as_zero is used only once, put the
  code directly

Signed-off-by: Jiamei Xie 
---
 xen/arch/arm/vpl011.c | 66 +--
 1 file changed, 19 insertions(+), 47 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 1bf803fc1f..80b859baed 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -354,11 +354,15 @@ static int vpl011_mmio_read(struct vcpu *v,
 struct domain *d = v->domain;
 unsigned long flags;
 
+if ( !vpl011_reg32_check_access(dabt) ) {
+gprintk(XENLOG_ERR, "vpl011: bad read width %d r%d offset %#08x\n",
+dabt.size, dabt.reg, vpl011_reg);
+return 0;
+}
+
 switch ( vpl011_reg )
 {
 case DR:
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 if ( vpl011->backend_in_domain )
 *r = vreg_reg32_extract(vpl011_read_data(d), info);
 else
@@ -366,31 +370,23 @@ static int vpl011_mmio_read(struct vcpu *v,
 return 1;
 
 case RSR:
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 /* It always returns 0 as there are no physical errors. */
 *r = 0;
 return 1;
 
 case FR:
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 VPL011_LOCK(d, flags);
 *r = vreg_reg32_extract(vpl011->uartfr, info);
 VPL011_UNLOCK(d, flags);
 return 1;
 
 case RIS:
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 VPL011_LOCK(d, flags);
 *r = vreg_reg32_extract(vpl011->uartris, info);
 VPL011_UNLOCK(d, flags);
 return 1;
 
 case MIS:
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 VPL011_LOCK(d, flags);
 *r = vreg_reg32_extract(vpl011->uartris & vpl011->uartimsc,
 info);
@@ -398,40 +394,25 @@ static int vpl011_mmio_read(struct vcpu *v,
 return 1;
 
 case IMSC:
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 VPL011_LOCK(d, flags);
 *r = vreg_reg32_extract(vpl011->uartimsc, info);
 VPL011_UNLOCK(d, flags);
 return 1;
 
 case ICR:
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 /* Only write is valid. */
 return 0;
 
 default:
 gprintk(XENLOG_ERR, "vpl011: unhandled read r%d offset %#08x\n",
 dabt.reg, vpl011_reg);
-goto read_as_zero;
-}
-
-return 1;
-
-read_as_zero:
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
-VPL011_LOCK(d, flags);
-*r = 0;
-VPL011_UNLOCK(d, flags);
-return 1;
-
-bad_width:
-gprintk(XENLOG_ERR, "vpl011: bad read width %d r%d offset %#08x\n",
-dabt.size, dabt.reg, vpl011_reg);
-return 0;
 
+/* Read as zero */
+VPL011_LOCK(d, flags);
+*r = 0;
+VPL011_UNLOCK(d, flags);
+return 1;
+}
 }
 
 static int vpl011_mmio_write(struct vcpu *v,
@@ -446,14 +427,18 @@ static int vpl011_mmio_write(struct vcpu *v,
 struct domain *d = v->domain;
 unsigned long flags;
 
-switch ( vpl011_reg )
+   if ( !vpl011_reg32_check_access(dabt) ) {
+   gprintk(XENLOG_ERR, "vpl011: bad write width %d r%d offset %#08x\n",
+   dabt.size, dabt.reg, vpl011_reg);
+   return 0;
+   }
+
+   switch ( vpl011_reg )
 {
 case DR:
 {
 uint32_t data = 0;
 
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 vreg_reg32_update(&data, r, info);
 data &= 0xFF;
 if ( vpl011->backend_in_domain )
@@ -464,8 +449,6 @@ static int vpl011_mmio_write(struct vcpu *v,
 }
 
 case RSR: /* Nothing to clear. */
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 return 1;
 
 case FR:
@@ -474,8 +457,6 @@ static int vpl011_mmio_write(struct vcpu *v,
 goto write_ignore;
 
 case IMSC:
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 VPL011_LOCK(d, flags);
 vreg_reg32_update(&vpl011->uartimsc, r, info);
 vpl011_update_interrupt_status(v->domain);
@@ -483,8 +464,6 @@ static int vpl011_mmio_write(struct vcpu *v,
 return 1;
 
 case ICR:
-if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
-
 VPL011_LOCK(d, flags);
 vreg_reg32_clearbits(&vpl011->uartris, r, info);
 vpl011_update_interrupt_status(d);
@@ -498,14 +477,7 @@ static int vpl011_mmio_write(struct vcpu *v,
 }
 
 write_

[PATCH v3 1/2] xen/arm: vpl011: emulate non-SBSA registers as WI/RAZ

2022-11-28 Thread Jiamei Xie

When the guest kernel enables DMA engine with "CONFIG_DMA_ENGINE=y",
Linux SBSA PL011 driver will access PL011 DMACR register in some
functions. As chapter "B Generic UART" in "ARM Server Base System
Architecture"[1] documentation describes, SBSA UART doesn't support
DMA. In current code, when the kernel tries to access DMACR register,
Xen will inject a data abort:
Unhandled fault at 0xffc00944d048
Mem abort info:
  ESR = 0x9600
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
  FSC = 0x00: ttbr address size fault
Data abort info:
  ISV = 0, ISS = 0x
  CM = 0, WnR = 0
swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
[ffc00944d048] pgd=10003803, p4d=10003803, 
pud=10003803, pmd=10003fffa803, pte=00689c090f13
Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
...
Call trace:
 pl011_stop_rx+0x70/0x80
 tty_port_shutdown+0x7c/0xb4
 tty_port_close+0x60/0xcc
 uart_close+0x34/0x8c
 tty_release+0x144/0x4c0
 __fput+0x78/0x220
 fput+0x1c/0x30
 task_work_run+0x88/0xc0
 do_notify_resume+0x8d0/0x123c
 el0_svc+0xa8/0xc0
 el0t_64_sync_handler+0xa4/0x130
 el0t_64_sync+0x1a0/0x1a4
Code: b983 b901f001 794038a0 8b42 (b941)
---[ end trace 83dd93df15c3216f ]---
note: bootlogd[132] exited with preempt_count 1
/etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-daemon

As discussed in [2], this commit makes the access to non-SBSA registers
RAZ/WI as an improvement.

[1] https://developer.arm.com/documentation/den0094/c/?lang=en
[2] 
https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2211161552420.4020@ubuntu-linux-20-04-desktop/

Signed-off-by: Jiamei Xie 
---
v2 -> v3
- emulate non-SBSA registers as WI/RAZ in default case
- update commit message
v1 -> v2
- print a message using XENLOG_G_DEBUG when it's write-ignore
---
 xen/arch/arm/vpl011.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 43522d48fd..1bf803fc1f 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -414,11 +414,19 @@ static int vpl011_mmio_read(struct vcpu *v,
 default:
 gprintk(XENLOG_ERR, "vpl011: unhandled read r%d offset %#08x\n",
 dabt.reg, vpl011_reg);
-return 0;
+goto read_as_zero;
 }
 
 return 1;
 
+read_as_zero:
+if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
+
+VPL011_LOCK(d, flags);
+*r = 0;
+VPL011_UNLOCK(d, flags);
+return 1;
+
 bad_width:
 gprintk(XENLOG_ERR, "vpl011: bad read width %d r%d offset %#08x\n",
 dabt.size, dabt.reg, vpl011_reg);
@@ -486,10 +494,11 @@ static int vpl011_mmio_write(struct vcpu *v,
 default:
 gprintk(XENLOG_ERR, "vpl011: unhandled write r%d offset %#08x\n",
 dabt.reg, vpl011_reg);
-return 0;
+goto write_ignore;
 }
 
 write_ignore:
+if ( !vpl011_reg32_check_access(dabt) ) goto bad_width;
 return 1;
 
 bad_width:
-- 
2.25.1

[PATCH v3 0/2] xen/arm: refine vpl011

2022-11-28 Thread Jiamei Xie

Hi all,

This patch is the version 3 for "xen/arm: vpl011: Make access to DMACR
write-ignore" [1]. 

[1] 
https://patchwork.kernel.org/project/xen-devel/patch/20221122054644.1092173-1-jiamei@arm.com/

Thanks,
Jiamei Xie

v2 -> v3
- emulate non-SBSA registers as WI/RAZ in default case
- update commit message
- add a patch to drop redundancy in mmio_write/read
v1 -> v2
- print a message using XENLOG_G_DEBUG when it's write-ignore

Jiamei Xie (2):
  xen/arm: vpl011: emulate non-SBSA registers as WI/RAZ
  xen/arm: vpl011: drop redundancy in mmio_write/read

 xen/arch/arm/vpl011.c | 59 +++
 1 file changed, 20 insertions(+), 39 deletions(-)

-- 
2.25.1

RE: [PATCH v2] xen/arm: vpl011: Make access to DMACR write-ignore

2022-11-22 Thread Jiamei Xie

Hi Michal,

> -Original Message-
> From: Michal Orzel 
> Sent: Tuesday, November 22, 2022 11:17 PM
> To: Julien Grall ; Jiamei Xie ; xen-
> de...@lists.xenproject.org
> Cc: Wei Chen ; Stefano Stabellini
> ; Bertrand Marquis ;
> Volodymyr Babchuk 
> Subject: Re: [PATCH v2] xen/arm: vpl011: Make access to DMACR write-
> ignore
> 
> Hi,
> 
> On 22/11/2022 13:25, Julien Grall wrote:
> >
> >
> > Hi,
> >
> > On 22/11/2022 05:46, Jiamei Xie wrote:
> >> When the guest kernel enables DMA engine with
> "CONFIG_DMA_ENGINE=y",
> >> Linux SBSA PL011 driver will access PL011 DMACR register in some
> >> functions. As chapter "B Generic UART" in "ARM Server Base System
> >> Architecture"[1] documentation describes, SBSA UART doesn't support
> >> DMA. In current code, when the kernel tries to access DMACR register,
> >> Xen will inject a data abort:
> >> Unhandled fault at 0xffc00944d048
> >> Mem abort info:
> >>ESR = 0x9600
> >>EC = 0x25: DABT (current EL), IL = 32 bits
> >>SET = 0, FnV = 0
> >>EA = 0, S1PTW = 0
> >>FSC = 0x00: ttbr address size fault
> >> Data abort info:
> >>ISV = 0, ISS = 0x
> >>CM = 0, WnR = 0
> >> swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
> >> [ffc00944d048] pgd=10003803, p4d=10003803,
> pud=10003803, pmd=10003fffa803, pte=00689c090f13
> >> Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
> >> ...
> >> Call trace:
> >>   pl011_stop_rx+0x70/0x80
> >>   tty_port_shutdown+0x7c/0xb4
> >>   tty_port_close+0x60/0xcc
> >>   uart_close+0x34/0x8c
> >>   tty_release+0x144/0x4c0
> >>   __fput+0x78/0x220
> >>   fput+0x1c/0x30
> >>   task_work_run+0x88/0xc0
> >>   do_notify_resume+0x8d0/0x123c
> >>   el0_svc+0xa8/0xc0
> >>   el0t_64_sync_handler+0xa4/0x130
> >>   el0t_64_sync+0x1a0/0x1a4
> >> Code: b983 b901f001 794038a0 8b42 (b941)
> >> ---[ end trace 83dd93df15c3216f ]---
> >> note: bootlogd[132] exited with preempt_count 1
> >> /etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-
> daemon
> >>
> >> As discussed in [2], this commit makes the access to DMACR register
> >> write-ignore as an improvement.
> >
> > Didn't we agree to emulate all non-SBSA registers as WI? IOW, the
> > default case should contain a 'goto write_ignore' rather return 0.
> + we also agreed on emulating the reads to non spec compliant registers as
> RAZ.

Thanks for reminding me of this. I'll  emulating the reads to non spec 
compliant registers as
RAZ in patch v3.

Best wishes
Jiamei Xie

> 
> >
> >>
> >> [1]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeve
> loper.arm.com%2Fdocumentation%2Fden0094%2Fc%2F%3Flang%3Den&am
> p;data=05%7C01%7Cmichal.orzel%40amd.com%7C1065702b4fd2457cdbf80
> 8dacc84b45a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6380
> 47167600786580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi
> LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&
> amp;sdata=W1dbakw6lkGkv4ydElIi%2Ba7uT7e7Pt5dB3vDtYpP%2FqQ%3D&a
> mp;reserved=0
> >> [2]
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Fxen-
> devel%2Falpine.DEB.2.22.394.2211161552420.4020%40ubuntu-linux-20-04-
> desktop%2F&data=05%7C01%7Cmichal.orzel%40amd.com%7C1065702
> b4fd2457cdbf808dacc84b45a%7C3dd8961fe4884e608e11a82d994e183d%7C
> 0%7C0%7C638047167600786580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> 00%7C%7C%7C&sdata=O4zxuI3HqRA1bdraGcVVY8vV0HGbqOI3nFa%2F
> ciC1cGQ%3D&reserved=0
> >>
> >> Signed-off-by: Jiamei Xie 
> >> ---
> >>   xen/arch/arm/vpl011.c | 4 
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> >> index 43522d48fd..e97fe3ebe7 100644
> >> --- a/xen/arch/arm/vpl011.c
> >> +++ b/xen/arch/arm/vpl011.c
> >> @@ -463,6 +463,10 @@ static int vpl011_mmio_write(struct vcpu *v,
> >>   case FR:
> >>   case RIS:
> >>   case MIS:
> >> +case DMACR:
> >> +printk(XENLOG_G_DEBUG
> >> +   "vpl011: WI on register offset %#08x\n",
> >> +   vpl011_reg);
> >
> > IMHO, this message should be printed just after the write_ignore label.
> >
> >>   goto write_ignore;
> >>
> >>   case IMSC:
> >
> > Cheers,
> >
> > --
> > Julien Grall
> >
> 
> ~Michal

RE: [PATCH v2] xen/arm: vpl011: Make access to DMACR write-ignore

2022-11-22 Thread Jiamei Xie

Hi Julien,

> -Original Message-
> From: Julien Grall 
> Sent: Tuesday, November 22, 2022 8:26 PM
> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> Cc: Wei Chen ; Stefano Stabellini
> ; Bertrand Marquis ;
> Volodymyr Babchuk 
> Subject: Re: [PATCH v2] xen/arm: vpl011: Make access to DMACR write-
> ignore
> 
> Hi,
> 
> On 22/11/2022 05:46, Jiamei Xie wrote:
> > When the guest kernel enables DMA engine with
> "CONFIG_DMA_ENGINE=y",
> > Linux SBSA PL011 driver will access PL011 DMACR register in some
> > functions. As chapter "B Generic UART" in "ARM Server Base System
> > Architecture"[1] documentation describes, SBSA UART doesn't support
> > DMA. In current code, when the kernel tries to access DMACR register,
> > Xen will inject a data abort:
> > Unhandled fault at 0xffc00944d048
> > Mem abort info:
> >ESR = 0x9600
> >EC = 0x25: DABT (current EL), IL = 32 bits
> >SET = 0, FnV = 0
> >EA = 0, S1PTW = 0
> >FSC = 0x00: ttbr address size fault
> > Data abort info:
> >ISV = 0, ISS = 0x
> >CM = 0, WnR = 0
> > swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
> > [ffc00944d048] pgd=10003803, p4d=10003803,
> pud=10003803, pmd=10003fffa803, pte=00689c090f13
> > Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
> > ...
> > Call trace:
> >   pl011_stop_rx+0x70/0x80
> >   tty_port_shutdown+0x7c/0xb4
> >   tty_port_close+0x60/0xcc
> >   uart_close+0x34/0x8c
> >   tty_release+0x144/0x4c0
> >   __fput+0x78/0x220
> >   fput+0x1c/0x30
> >   task_work_run+0x88/0xc0
> >   do_notify_resume+0x8d0/0x123c
> >   el0_svc+0xa8/0xc0
> >   el0t_64_sync_handler+0xa4/0x130
> >   el0t_64_sync+0x1a0/0x1a4
> > Code: b983 b901f001 794038a0 8b42 (b941)
> > ---[ end trace 83dd93df15c3216f ]---
> > note: bootlogd[132] exited with preempt_count 1
> > /etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-
> daemon
> >
> > As discussed in [2], this commit makes the access to DMACR register
> > write-ignore as an improvement.
> 
> Didn't we agree to emulate all non-SBSA registers as WI? IOW, the
> default case should contain a 'goto write_ignore' rather return 0.

Thanks for your review.  I'll  emulate all non-SBSA registers as WI in patch v3.

> 
> >
> > [1] https://developer.arm.com/documentation/den0094/c/?lang=en
> > [2] https://lore.kernel.org/xen-
> devel/alpine.DEB.2.22.394.2211161552420.4020@ubuntu-linux-20-04-
> desktop/
> >
> > Signed-off-by: Jiamei Xie 
> > ---
> >   xen/arch/arm/vpl011.c | 4 
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> > index 43522d48fd..e97fe3ebe7 100644
> > --- a/xen/arch/arm/vpl011.c
> > +++ b/xen/arch/arm/vpl011.c
> > @@ -463,6 +463,10 @@ static int vpl011_mmio_write(struct vcpu *v,
> >   case FR:
> >   case RIS:
> >   case MIS:
> > +case DMACR:
> > +printk(XENLOG_G_DEBUG
> > +   "vpl011: WI on register offset %#08x\n",
> > +   vpl011_reg);
> 
> IMHO, this message should be printed just after the write_ignore label.

I'll put it after the write_ignore lable in patch v3.

Best wishes
Jiamei Xie


> 
> >   goto write_ignore;
> >
> >   case IMSC:
> 
> Cheers,
> 
> --
> Julien Grall

[PATCH v2] xen/arm: vpl011: Make access to DMACR write-ignore

2022-11-21 Thread Jiamei Xie

When the guest kernel enables DMA engine with "CONFIG_DMA_ENGINE=y",
Linux SBSA PL011 driver will access PL011 DMACR register in some
functions. As chapter "B Generic UART" in "ARM Server Base System
Architecture"[1] documentation describes, SBSA UART doesn't support
DMA. In current code, when the kernel tries to access DMACR register,
Xen will inject a data abort:
Unhandled fault at 0xffc00944d048
Mem abort info:
  ESR = 0x9600
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
  FSC = 0x00: ttbr address size fault
Data abort info:
  ISV = 0, ISS = 0x
  CM = 0, WnR = 0
swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
[ffc00944d048] pgd=10003803, p4d=10003803, 
pud=10003803, pmd=10003fffa803, pte=00689c090f13
Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
...
Call trace:
 pl011_stop_rx+0x70/0x80
 tty_port_shutdown+0x7c/0xb4
 tty_port_close+0x60/0xcc
 uart_close+0x34/0x8c
 tty_release+0x144/0x4c0
 __fput+0x78/0x220
 fput+0x1c/0x30
 task_work_run+0x88/0xc0
 do_notify_resume+0x8d0/0x123c
 el0_svc+0xa8/0xc0
 el0t_64_sync_handler+0xa4/0x130
 el0t_64_sync+0x1a0/0x1a4
Code: b983 b901f001 794038a0 8b42 (b941)
---[ end trace 83dd93df15c3216f ]---
note: bootlogd[132] exited with preempt_count 1
/etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-daemon

As discussed in [2], this commit makes the access to DMACR register
write-ignore as an improvement.

[1] https://developer.arm.com/documentation/den0094/c/?lang=en
[2] 
https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2211161552420.4020@ubuntu-linux-20-04-desktop/

Signed-off-by: Jiamei Xie 
---
 xen/arch/arm/vpl011.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 43522d48fd..e97fe3ebe7 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -463,6 +463,10 @@ static int vpl011_mmio_write(struct vcpu *v,
 case FR:
 case RIS:
 case MIS:
+case DMACR:
+printk(XENLOG_G_DEBUG
+   "vpl011: WI on register offset %#08x\n",
+   vpl011_reg);
 goto write_ignore;
 
 case IMSC:
-- 
2.25.1

RE: [PATCH] xen/arm: vpl011: Make access to DMACR write-ignore

2022-11-18 Thread Jiamei Xie

Hi Michal，

> -Original Message-
> From: Michal Orzel 
> Sent: Friday, November 18, 2022 3:42 PM
> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Julien Grall 
> ;
> Bertrand Marquis ; Volodymyr Babchuk
> ; Wei Chen 
> Subject: Re: [PATCH] xen/arm: vpl011: Make access to DMACR write-ignore
> 
> Hi Jiamei,
> 
> On 18/11/2022 03:18, Jiamei Xie wrote:
> >
> >
> > Hi
> >
> > Sorry there is no subject in the  last email. So add in this one.
> I would consider re-pushing this patch(although please wait for some
> comments).
> The reason being, a patch without a subject is not picked by patchwork/b4 or
> other
> tools used to grab a patch.
Got it, thanks!

Best wishes
Jiamei Xie


> 
> ~Michal

RE: [PATCH] xen/arm: vpl011: Make access to DMACR write-ignore

2022-11-17 Thread Jiamei Xie

Hi 

Sorry there is no subject in the  last email. So add in this one.

Best wishes
Jiamei Xie

> -Original Message-
> From: Jiamei Xie 
> Sent: Friday, November 18, 2022 10:00 AM
> To: xen-devel@lists.xenproject.org
> Cc: Jiamei Xie ; Stefano Stabellini
> ; Julien Grall ; Bertrand Marquis
> ; Volodymyr Babchuk
> ; Wei Chen 
> Subject:
> 
> Date: Thu, 17 Nov 2022 11:07:12 +0800
> Subject: [PATCH] xen/arm: vpl011: Make access to DMACR write-ignore
> 
> When the guest kernel enables DMA engine with
> "CONFIG_DMA_ENGINE=y",
> Linux SBSA PL011 driver will access PL011 DMACR register in some
> functions. As chapter "B Generic UART" in "ARM Server Base System
> Architecture"[1] documentation describes, SBSA UART doesn't support
> DMA. In current code, when the kernel tries to access DMACR register,
> Xen will inject a data abort:
> Unhandled fault at 0xffc00944d048
> Mem abort info:
>   ESR = 0x9600
>   EC = 0x25: DABT (current EL), IL = 32 bits
>   SET = 0, FnV = 0
>   EA = 0, S1PTW = 0
>   FSC = 0x00: ttbr address size fault
> Data abort info:
>   ISV = 0, ISS = 0x
>   CM = 0, WnR = 0
> swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
> [ffc00944d048] pgd=10003803, p4d=10003803,
> pud=10003803, pmd=10003fffa803, pte=00689c090f13
> Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
> ...
> Call trace:
>  pl011_stop_rx+0x70/0x80
>  tty_port_shutdown+0x7c/0xb4
>  tty_port_close+0x60/0xcc
>  uart_close+0x34/0x8c
>  tty_release+0x144/0x4c0
>  __fput+0x78/0x220
>  fput+0x1c/0x30
>  task_work_run+0x88/0xc0
>  do_notify_resume+0x8d0/0x123c
>  el0_svc+0xa8/0xc0
>  el0t_64_sync_handler+0xa4/0x130
>  el0t_64_sync+0x1a0/0x1a4
> Code: b983 b901f001 794038a0 8b42 (b941)
> ---[ end trace 83dd93df15c3216f ]---
> note: bootlogd[132] exited with preempt_count 1
> /etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-daemon
> 
> As discussed in [2], this commit makes the access to DMACR register
> write-ignore as an improvement.
> 
> [1] https://developer.arm.com/documentation/den0094/c/?lang=en
> [2] https://lore.kernel.org/xen-
> devel/alpine.DEB.2.22.394.2211161552420.4020@ubuntu-linux-20-04-
> desktop/
> 
> Signed-off-by: Jiamei Xie 
> ---
>  xen/arch/arm/vpl011.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
> index 43522d48fd..80d00b3052 100644
> --- a/xen/arch/arm/vpl011.c
> +++ b/xen/arch/arm/vpl011.c
> @@ -463,6 +463,7 @@ static int vpl011_mmio_write(struct vcpu *v,
>  case FR:
>  case RIS:
>  case MIS:
> +case DMACR:
>  goto write_ignore;
> 
>  case IMSC:
> --
> 2.25.1

[no subject]

2022-11-17 Thread Jiamei Xie

Date: Thu, 17 Nov 2022 11:07:12 +0800
Subject: [PATCH] xen/arm: vpl011: Make access to DMACR write-ignore

When the guest kernel enables DMA engine with "CONFIG_DMA_ENGINE=y",
Linux SBSA PL011 driver will access PL011 DMACR register in some
functions. As chapter "B Generic UART" in "ARM Server Base System
Architecture"[1] documentation describes, SBSA UART doesn't support
DMA. In current code, when the kernel tries to access DMACR register,
Xen will inject a data abort:
Unhandled fault at 0xffc00944d048
Mem abort info:
  ESR = 0x9600
  EC = 0x25: DABT (current EL), IL = 32 bits
  SET = 0, FnV = 0
  EA = 0, S1PTW = 0
  FSC = 0x00: ttbr address size fault
Data abort info:
  ISV = 0, ISS = 0x
  CM = 0, WnR = 0
swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
[ffc00944d048] pgd=10003803, p4d=10003803, 
pud=10003803, pmd=10003fffa803, pte=00689c090f13
Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
...
Call trace:
 pl011_stop_rx+0x70/0x80
 tty_port_shutdown+0x7c/0xb4
 tty_port_close+0x60/0xcc
 uart_close+0x34/0x8c
 tty_release+0x144/0x4c0
 __fput+0x78/0x220
 fput+0x1c/0x30
 task_work_run+0x88/0xc0
 do_notify_resume+0x8d0/0x123c
 el0_svc+0xa8/0xc0
 el0t_64_sync_handler+0xa4/0x130
 el0t_64_sync+0x1a0/0x1a4
Code: b983 b901f001 794038a0 8b42 (b941)
---[ end trace 83dd93df15c3216f ]---
note: bootlogd[132] exited with preempt_count 1
/etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-daemon

As discussed in [2], this commit makes the access to DMACR register
write-ignore as an improvement.

[1] https://developer.arm.com/documentation/den0094/c/?lang=en
[2] 
https://lore.kernel.org/xen-devel/alpine.DEB.2.22.394.2211161552420.4020@ubuntu-linux-20-04-desktop/

Signed-off-by: Jiamei Xie 
---
 xen/arch/arm/vpl011.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/arch/arm/vpl011.c b/xen/arch/arm/vpl011.c
index 43522d48fd..80d00b3052 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -463,6 +463,7 @@ static int vpl011_mmio_write(struct vcpu *v,
 case FR:
 case RIS:
 case MIS:
+case DMACR:
 goto write_ignore;
 
 case IMSC:
-- 
2.25.1

RE: Xen Arm vpl011 UART will cause segmentation fault in Linux guest

2022-11-16 Thread Jiamei Xie

Hi

 I also wrote a patch to make xen ignore the DMACR write and tested it. It also 
can fix this segmentation fault issue.

Best wishes
Jiamei Xie

> -Original Message-
> From: Jiamei Xie 
> Sent: Wednesday, November 16, 2022 8:46 PM
> To: Jiamei Xie ; Stefano Stabellini
> ; Michal Orzel 
> Cc: xen-devel@lists.xenproject.org; Wei Chen ;
> Bertrand Marquis ; jul...@xen.org
> Subject: RE: Xen Arm vpl011 UART will cause segmentation fault in Linux
> guest
> 
> Hi
> 
>  I have wrote a kernel patch to fix this. And I have tested. It will not have
> segmentation fault.
> 
> Best wishes
> Jiamei Xie
> 
> > -Original Message-
> > From: Xen-devel  On Behalf Of
> > Jiamei Xie
> > Sent: Friday, November 11, 2022 12:31 PM
> > To: Stefano Stabellini ; Michal Orzel
> > 
> > Cc: xen-devel@lists.xenproject.org; Wei Chen ;
> > Bertrand Marquis ; jul...@xen.org
> > Subject: RE: Xen Arm vpl011 UART will cause segmentation fault in Linux
> > guest
> >
> > Hi
> >
> > > -Original Message-
> > > From: Stefano Stabellini 
> > > Sent: Friday, November 11, 2022 4:33 AM
> > > To: Michal Orzel 
> > > Cc: Jiamei Xie ; xen-devel@lists.xenproject.org;
> Wei
> > > Chen ; Bertrand Marquis
> > > ; jul...@xen.org; sstabell...@kernel.org
> > > Subject: Re: Xen Arm vpl011 UART will cause segmentation fault in Linux
> > > guest
> > >
> > > On Wed, 9 Nov 2022, Michal Orzel wrote:
> > > > Hi Jiamei,
> > > >
> > > > On 09/11/2022 09:25, Jiamei Xie wrote:
> > > > >
> > > > >
> > > > > Hi Michal，
> > > > >
> > > > > Below log can be got when stating the linux guest. It says 9c09 is 
> > > > > sbsa.
> > > And 9c09 is also output
> > > > >  in bootlogd error message:
> > > > > Serial: AMBA PL011 UART driver
> > > > > 9c0b.uart: ttyAMA0 at MMIO 0x9c0b (irq = 12, base_baud = 0)
> > > is a PL011 rev2
> > > > > printk: console [ttyAMA0] enabled
> > > > > 9c09.sbsa-uart: ttyAMA1 at MMIO 0x9c09 (irq = 15,
> base_baud
> > > = 0) is a SBSA
> > > > >
> > > >
> > > > Xen behavior is correct and this would be Linux fault to try to write to
> > > DMACR for SBSA UART device.
> > > > DMACR is just an example. If you try to program e.g. the baudrate
> > (through
> > > LCR) for VPL011 it will
> > > > also result in injecting abort into the guest. Should Xen support it? 
> > > > No.
> > The
> > > reason why is that
> > > > it is not spec compliant operation. SBSA specification directly 
> > > > specifies
> > > what registers are exposed.
> > > > If Linux tries to write to some of the none-spec compliant registers - 
> > > > it is
> > its
> > > fault.
> > >
> > > Yeah, we need to fix Linux.
> > >
> > > FYI this is not the first bug in Linux affecting the sbsa-uart driver:
> > > the issue is that the pl011 driver and the sbsa-uart driver share the
> > > same code in Linux so it happens sometimes that a pl011-only feature
> > > creeps into the sbsa-uart driver by mistake.
> >
> > Thanks for your confirm about this. In that case, I will check the Linux 
> > code
> to
> > see why this happens and how to fix it.
> >
> > Best wishes
> > Jiamei Xie
> > >
> > >
> > > > >> -Original Message-
> > > > >> From: Michal Orzel 
> > > > >> Sent: Wednesday, November 9, 2022 3:40 PM
> > > > >> To: Jiamei Xie ; xen-
> de...@lists.xenproject.org
> > > > >> Cc: Wei Chen ; Bertrand Marquis
> > > > >> ; jul...@xen.org;
> > sstabell...@kernel.org
> > > > >> Subject: Re: Xen Arm vpl011 UART will cause segmentation fault in
> > Linux
> > > > >> guest
> > > > >>
> > > > >> Hi Jiamei,
> > > > >>
> > > > >> On 09/11/2022 08:20, Jiamei Xie wrote:
> > > > >>>
> > > > >>>
> > > > >>> Hi all,
> > > > >>>
> > > > >>> When the guest kernel enables DMA engine with
> > > > >> "CONFIG_DMA_ENGINE=y", Linux AMBA PL011 driver will access
> > PL011
> > > > >> DMACR register. But this register have not been supported

RE: Xen Arm vpl011 UART will cause segmentation fault in Linux guest

2022-11-16 Thread Jiamei Xie

Hi
 
 I have wrote a kernel patch to fix this. And I have tested. It will not have 
segmentation fault.

Best wishes
Jiamei Xie

> -Original Message-
> From: Xen-devel  On Behalf Of
> Jiamei Xie
> Sent: Friday, November 11, 2022 12:31 PM
> To: Stefano Stabellini ; Michal Orzel
> 
> Cc: xen-devel@lists.xenproject.org; Wei Chen ;
> Bertrand Marquis ; jul...@xen.org
> Subject: RE: Xen Arm vpl011 UART will cause segmentation fault in Linux
> guest
> 
> Hi
> 
> > -Original Message-
> > From: Stefano Stabellini 
> > Sent: Friday, November 11, 2022 4:33 AM
> > To: Michal Orzel 
> > Cc: Jiamei Xie ; xen-devel@lists.xenproject.org; Wei
> > Chen ; Bertrand Marquis
> > ; jul...@xen.org; sstabell...@kernel.org
> > Subject: Re: Xen Arm vpl011 UART will cause segmentation fault in Linux
> > guest
> >
> > On Wed, 9 Nov 2022, Michal Orzel wrote:
> > > Hi Jiamei,
> > >
> > > On 09/11/2022 09:25, Jiamei Xie wrote:
> > > >
> > > >
> > > > Hi Michal，
> > > >
> > > > Below log can be got when stating the linux guest. It says 9c09 is sbsa.
> > And 9c09 is also output
> > > >  in bootlogd error message:
> > > > Serial: AMBA PL011 UART driver
> > > > 9c0b.uart: ttyAMA0 at MMIO 0x9c0b (irq = 12, base_baud = 0)
> > is a PL011 rev2
> > > > printk: console [ttyAMA0] enabled
> > > > 9c09.sbsa-uart: ttyAMA1 at MMIO 0x9c09 (irq = 15, base_baud
> > = 0) is a SBSA
> > > >
> > >
> > > Xen behavior is correct and this would be Linux fault to try to write to
> > DMACR for SBSA UART device.
> > > DMACR is just an example. If you try to program e.g. the baudrate
> (through
> > LCR) for VPL011 it will
> > > also result in injecting abort into the guest. Should Xen support it? No.
> The
> > reason why is that
> > > it is not spec compliant operation. SBSA specification directly specifies
> > what registers are exposed.
> > > If Linux tries to write to some of the none-spec compliant registers - it 
> > > is
> its
> > fault.
> >
> > Yeah, we need to fix Linux.
> >
> > FYI this is not the first bug in Linux affecting the sbsa-uart driver:
> > the issue is that the pl011 driver and the sbsa-uart driver share the
> > same code in Linux so it happens sometimes that a pl011-only feature
> > creeps into the sbsa-uart driver by mistake.
> 
> Thanks for your confirm about this. In that case, I will check the Linux code 
> to
> see why this happens and how to fix it.
> 
> Best wishes
> Jiamei Xie
> >
> >
> > > >> -Original Message-
> > > >> From: Michal Orzel 
> > > >> Sent: Wednesday, November 9, 2022 3:40 PM
> > > >> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> > > >> Cc: Wei Chen ; Bertrand Marquis
> > > >> ; jul...@xen.org;
> sstabell...@kernel.org
> > > >> Subject: Re: Xen Arm vpl011 UART will cause segmentation fault in
> Linux
> > > >> guest
> > > >>
> > > >> Hi Jiamei,
> > > >>
> > > >> On 09/11/2022 08:20, Jiamei Xie wrote:
> > > >>>
> > > >>>
> > > >>> Hi all,
> > > >>>
> > > >>> When the guest kernel enables DMA engine with
> > > >> "CONFIG_DMA_ENGINE=y", Linux AMBA PL011 driver will access
> PL011
> > > >> DMACR register. But this register have not been supported by vpl011
> of
> > Xen.
> > > >> Xen will inject a data abort into guest, this will cause segmentation
> fault
> > of
> > > >> guest with the below message:
> > > >> I am quite confused.
> > > >> VPL011 implements SBSA UART which only implements some subset
> of
> > PL011
> > > >> operations (SBSA UART is not PL011).
> > > >> According to spec (SBSA ver. 6.0), the SBSA_UART does not support
> > DMA
> > > >> features so Xen code is fine.
> > > >> When Xen exposes vpl011 device to a guest, this device has
> "arm,sbsa-
> > uart"
> > > >> compatible and not "uart-pl011".
> > > >> Linux driver "amba-pl011.c" should see this compatible and assign
> > proper
> > > >> operations (sbsa_uart_pops instead of amba_pl011_pops) that do not
> > enable
> > > >> DMA.
> > > >> Maybe the issue is with your

RE: Xen Arm vpl011 UART will cause segmentation fault in Linux guest

2022-11-10 Thread Jiamei Xie

Hi

> -Original Message-
> From: Stefano Stabellini 
> Sent: Friday, November 11, 2022 4:33 AM
> To: Michal Orzel 
> Cc: Jiamei Xie ; xen-devel@lists.xenproject.org; Wei
> Chen ; Bertrand Marquis
> ; jul...@xen.org; sstabell...@kernel.org
> Subject: Re: Xen Arm vpl011 UART will cause segmentation fault in Linux
> guest
> 
> On Wed, 9 Nov 2022, Michal Orzel wrote:
> > Hi Jiamei,
> >
> > On 09/11/2022 09:25, Jiamei Xie wrote:
> > >
> > >
> > > Hi Michal，
> > >
> > > Below log can be got when stating the linux guest. It says 9c09 is sbsa.
> And 9c09 is also output
> > >  in bootlogd error message:
> > > Serial: AMBA PL011 UART driver
> > > 9c0b.uart: ttyAMA0 at MMIO 0x9c0b (irq = 12, base_baud = 0)
> is a PL011 rev2
> > > printk: console [ttyAMA0] enabled
> > > 9c09.sbsa-uart: ttyAMA1 at MMIO 0x9c09 (irq = 15, base_baud
> = 0) is a SBSA
> > >
> >
> > Xen behavior is correct and this would be Linux fault to try to write to
> DMACR for SBSA UART device.
> > DMACR is just an example. If you try to program e.g. the baudrate (through
> LCR) for VPL011 it will
> > also result in injecting abort into the guest. Should Xen support it? No. 
> > The
> reason why is that
> > it is not spec compliant operation. SBSA specification directly specifies
> what registers are exposed.
> > If Linux tries to write to some of the none-spec compliant registers - it 
> > is its
> fault.
> 
> Yeah, we need to fix Linux.
> 
> FYI this is not the first bug in Linux affecting the sbsa-uart driver:
> the issue is that the pl011 driver and the sbsa-uart driver share the
> same code in Linux so it happens sometimes that a pl011-only feature
> creeps into the sbsa-uart driver by mistake.

Thanks for your confirm about this. In that case, I will check the Linux code 
to see why this happens and how to fix it.

Best wishes
Jiamei Xie
> 
> 
> > >> -Original Message-
> > >> From: Michal Orzel 
> > >> Sent: Wednesday, November 9, 2022 3:40 PM
> > >> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> > >> Cc: Wei Chen ; Bertrand Marquis
> > >> ; jul...@xen.org; sstabell...@kernel.org
> > >> Subject: Re: Xen Arm vpl011 UART will cause segmentation fault in Linux
> > >> guest
> > >>
> > >> Hi Jiamei,
> > >>
> > >> On 09/11/2022 08:20, Jiamei Xie wrote:
> > >>>
> > >>>
> > >>> Hi all,
> > >>>
> > >>> When the guest kernel enables DMA engine with
> > >> "CONFIG_DMA_ENGINE=y", Linux AMBA PL011 driver will access PL011
> > >> DMACR register. But this register have not been supported by vpl011 of
> Xen.
> > >> Xen will inject a data abort into guest, this will cause segmentation 
> > >> fault
> of
> > >> guest with the below message:
> > >> I am quite confused.
> > >> VPL011 implements SBSA UART which only implements some subset of
> PL011
> > >> operations (SBSA UART is not PL011).
> > >> According to spec (SBSA ver. 6.0), the SBSA_UART does not support
> DMA
> > >> features so Xen code is fine.
> > >> When Xen exposes vpl011 device to a guest, this device has "arm,sbsa-
> uart"
> > >> compatible and not "uart-pl011".
> > >> Linux driver "amba-pl011.c" should see this compatible and assign
> proper
> > >> operations (sbsa_uart_pops instead of amba_pl011_pops) that do not
> enable
> > >> DMA.
> > >> Maybe the issue is with your configuration?
> > >>
> > >> ~Michal
> > >>
> > >>> Unhandled fault at 0xffc00944d048
> > >>> Mem abort info:
> > >>> ESR = 0x9600
> > >>> EC = 0x25: DABT (current EL), IL = 32 bits
> > >>> SET = 0, FnV = 0
> > >>> EA = 0, S1PTW = 0
> > >>> FSC = 0x00: ttbr address size fault
> > >>> Data abort info:
> > >>> ISV = 0, ISS = 0x
> > >>> CM = 0, WnR = 0
> > >>> swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
> > >>> [ffc00944d048] pgd=10003803, p4d=10003803,
> > >> pud=10003803, pmd=10003fffa803,
> pte=00689c090f13
> > >>> Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
> > >>> Modules linked in:
> > >>> CPU: 0 PID: 132 Comm: boot

RE: Xen Arm vpl011 UART will cause segmentation fault in Linux guest

2022-11-09 Thread Jiamei Xie

Hi Michal，

Below log can be got when stating the linux guest. It says 9c09 is sbsa. And 
9c09 is also output
 in bootlogd error message: 
Serial: AMBA PL011 UART driver
9c0b.uart: ttyAMA0 at MMIO 0x9c0b (irq = 12, base_baud = 0) is a PL011 
rev2
printk: console [ttyAMA0] enabled
9c09.sbsa-uart: ttyAMA1 at MMIO 0x9c09 (irq = 15, base_baud = 0) is a 
SBSA

Best wishes
Jiamei Xie



> -Original Message-
> From: Michal Orzel 
> Sent: Wednesday, November 9, 2022 3:40 PM
> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> Cc: Wei Chen ; Bertrand Marquis
> ; jul...@xen.org; sstabell...@kernel.org
> Subject: Re: Xen Arm vpl011 UART will cause segmentation fault in Linux
> guest
> 
> Hi Jiamei,
> 
> On 09/11/2022 08:20, Jiamei Xie wrote:
> >
> >
> > Hi all,
> >
> > When the guest kernel enables DMA engine with
> "CONFIG_DMA_ENGINE=y", Linux AMBA PL011 driver will access PL011
> DMACR register. But this register have not been supported by vpl011 of Xen.
> Xen will inject a data abort into guest, this will cause segmentation fault of
> guest with the below message:
> I am quite confused.
> VPL011 implements SBSA UART which only implements some subset of PL011
> operations (SBSA UART is not PL011).
> According to spec (SBSA ver. 6.0), the SBSA_UART does not support DMA
> features so Xen code is fine.
> When Xen exposes vpl011 device to a guest, this device has "arm,sbsa-uart"
> compatible and not "uart-pl011".
> Linux driver "amba-pl011.c" should see this compatible and assign proper
> operations (sbsa_uart_pops instead of amba_pl011_pops) that do not enable
> DMA.
> Maybe the issue is with your configuration?
> 
> ~Michal
> 
> > Unhandled fault at 0xffc00944d048
> > Mem abort info:
> > ESR = 0x9600
> > EC = 0x25: DABT (current EL), IL = 32 bits
> > SET = 0, FnV = 0
> > EA = 0, S1PTW = 0
> > FSC = 0x00: ttbr address size fault
> > Data abort info:
> > ISV = 0, ISS = 0x
> > CM = 0, WnR = 0
> > swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
> > [ffc00944d048] pgd=10003803, p4d=10003803,
> pud=10003803, pmd=10003fffa803, pte=00689c090f13
> > Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
> > Modules linked in:
> > CPU: 0 PID: 132 Comm: bootlogd Not tainted 5.15.44-yocto-standard #1
> > pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > pc : pl011_stop_rx+0x70/0x80
> > lr : uart_tty_port_shutdown+0x44/0x110
> > sp : ffc00999bba0
> > x29: ffc00999bba0 x28: ff80234ac380 x27: ff8022f5d000
> > x26:  x25: 45585401 x24: 
> > x23: ff8021ba4660 x22: 0001 x21: ff8021a0e2a0
> > x20: ff802198f880 x19: ff8021a0e1a0 x18: 
> > x17:  x16:  x15: 
> > x14:  x13:  x12: 
> > x11:  x10:  x9 : ffc00871ba14
> > x8 : ffc0099de260 x7 : ff8021a0e318 x6 : 0003
> > x5 : ffc009315f20 x4 : ffc00944d038 x3 : 
> > x2 : ffc00944d048 x1 :  x0 : 0048
> > Call trace:
> > pl011_stop_rx+0x70/0x80
> > tty_port_shutdown+0x7c/0xb4
> > tty_port_close+0x60/0xcc
> > uart_close+0x34/0x8c
> > tty_release+0x144/0x4c0
> > __fput+0x78/0x220
> > fput+0x1c/0x30
> > task_work_run+0x88/0xc0
> > do_notify_resume+0x8d0/0x123c
> > el0_svc+0xa8/0xc0
> > el0t_64_sync_handler+0xa4/0x130
> > el0t_64_sync+0x1a0/0x1a4
> > Code: b983 b901f001 794038a0 8b42 (b941)
> > ---[ end trace 83dd93df15c3216f ]---
> > note: bootlogd[132] exited with preempt_count 1
> > /etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-
> daemon
> > In Xen, vpl011_mmio_write doesn't handle DMACR . And kernel doesn't
> check if pl011_write executes sucessfully in pl011_dma_rx_stop . So such
> segmentation fault occurs.
> > static inline void pl011_dma_rx_stop(struct uart_amba_port *uap)
> > {
> > /* FIXME.  Just disable the DMA enable */
> > uap->dmacr &= ~UART011_RXDMAE;
> > pl011_write(uap->dmacr, uap, REG_DMACR);
> > }
> >
> > I think we should prevent such segmentation fault. We have checked the
> PL011 spec, it seems there is not any register bit can indicate DMA support
> status of PL011. We might have two options:
> > 1. Option#1 is to add DMA support for vpl011, but this is not trivial.
> > 2. Option#2 is to ignore the write to DMACR, and return 0 for DMACR read
> in vpl011. But this option need co-work with kernel, because current Linux
> PL011 driver assume the write operation will never be failed, and will not
> fallback to no-DMA mode, when Xen return 0 for DMA enabled bit in DMACR.
> >
> > How do you think about it?  Any suggestion about it is welcome. Thanks.
> >
> > Best wishes
> > Jiamei Xie
> >

Xen Arm vpl011 UART will cause segmentation fault in Linux guest

2022-11-08 Thread Jiamei Xie

Hi all,

When the guest kernel enables DMA engine with "CONFIG_DMA_ENGINE=y", Linux AMBA 
PL011 driver will access PL011 DMACR register. But this register have not been 
supported by vpl011 of Xen. Xen will inject a data abort into guest, this will 
cause segmentation fault of guest with the below message:
Unhandled fault at 0xffc00944d048
Mem abort info:
ESR = 0x9600
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x00: ttbr address size fault
Data abort info:
ISV = 0, ISS = 0x
CM = 0, WnR = 0
swapper pgtable: 4k pages, 39-bit VAs, pgdp=20e2e000
[ffc00944d048] pgd=10003803, p4d=10003803, 
pud=10003803, pmd=10003fffa803, pte=00689c090f13
Internal error: ttbr address size fault: 9600 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 132 Comm: bootlogd Not tainted 5.15.44-yocto-standard #1
pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : pl011_stop_rx+0x70/0x80
lr : uart_tty_port_shutdown+0x44/0x110
sp : ffc00999bba0
x29: ffc00999bba0 x28: ff80234ac380 x27: ff8022f5d000
x26:  x25: 45585401 x24: 
x23: ff8021ba4660 x22: 0001 x21: ff8021a0e2a0
x20: ff802198f880 x19: ff8021a0e1a0 x18: 
x17:  x16:  x15: 
x14:  x13:  x12: 
x11:  x10:  x9 : ffc00871ba14
x8 : ffc0099de260 x7 : ff8021a0e318 x6 : 0003
x5 : ffc009315f20 x4 : ffc00944d038 x3 : 
x2 : ffc00944d048 x1 :  x0 : 0048
Call trace:
pl011_stop_rx+0x70/0x80
tty_port_shutdown+0x7c/0xb4
tty_port_close+0x60/0xcc
uart_close+0x34/0x8c
tty_release+0x144/0x4c0
__fput+0x78/0x220
fput+0x1c/0x30
task_work_run+0x88/0xc0
do_notify_resume+0x8d0/0x123c
el0_svc+0xa8/0xc0
el0t_64_sync_handler+0xa4/0x130
el0t_64_sync+0x1a0/0x1a4
Code: b983 b901f001 794038a0 8b42 (b941)
---[ end trace 83dd93df15c3216f ]---
note: bootlogd[132] exited with preempt_count 1
/etc/rcS.d/S07bootlogd: line 47: 132 Segmentation fault start-stop-daemon
In Xen, vpl011_mmio_write doesn't handle DMACR . And kernel doesn't check if 
pl011_write executes sucessfully in pl011_dma_rx_stop . So such segmentation 
fault occurs.
static inline void pl011_dma_rx_stop(struct uart_amba_port *uap)
{
/* FIXME.  Just disable the DMA enable */
uap->dmacr &= ~UART011_RXDMAE;
pl011_write(uap->dmacr, uap, REG_DMACR);
}

I think we should prevent such segmentation fault. We have checked the PL011 
spec, it seems there is not any register bit can indicate DMA support status of 
PL011. We might have two options:
1. Option#1 is to add DMA support for vpl011, but this is not trivial.
2. Option#2 is to ignore the write to DMACR, and return 0 for DMACR read in 
vpl011. But this option need co-work with kernel, because current Linux PL011 
driver assume the write operation will never be failed, and will not fallback 
to no-DMA mode, when Xen return 0 for DMA enabled bit in DMACR.

How do you think about it?  Any suggestion about it is welcome. Thanks.

Best wishes
Jiamei Xie

RE: [PATCH v3 05/10] automation: Add Arm containers to containerize script

2022-10-20 Thread Jiamei Xie

Hi Michal,

> -Original Message-
> From: Michal Orzel 
> Sent: Thursday, October 20, 2022 3:52 PM
> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> Cc: Doug Goldstein ; Stefano Stabellini
> 
> Subject: Re: [PATCH v3 05/10] automation: Add Arm containers to
> containerize script
> 
> Hi Jiamei,
> 
> On 20/10/2022 09:13, Jiamei Xie wrote:
> >
> >
> > Hi Michal,
> >
> >> -Original Message-
> >> From: Michal Orzel 
> >> Sent: Thursday, October 20, 2022 2:59 PM
> >> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> >> Cc: Doug Goldstein ; Stefano Stabellini
> >> 
> >> Subject: Re: [PATCH v3 05/10] automation: Add Arm containers to
> >> containerize script
> >>
> >> Hi Jiamei,
> >>
> >> On 20/10/2022 05:00, Jiamei Xie wrote:
> >>>
> >>>
> >>> Hi Michal,
> >>>
> >>>> -Original Message-
> >>>> From: Xen-devel  On Behalf
> Of
> >>>> Michal Orzel
> >>>> Sent: Tuesday, September 27, 2022 5:47 PM
> >>>> To: xen-devel@lists.xenproject.org
> >>>> Cc: Michal Orzel ; Doug Goldstein
> >>>> ; Stefano Stabellini 
> >>>> Subject: [PATCH v3 05/10] automation: Add Arm containers to
> >> containerize
> >>>> script
> >>>>
> >>>> Script automation/scripts/containerize makes it easy to build Xen
> within
> >>>> predefined containers from gitlab container registry. This script is
> >>>> currently missing the helpers to select Arm containers, so populate the
> >>>> necessary entries.
> >>>>
> >>>> Signed-off-by: Michal Orzel 
> >>>> Acked-by: Stefano Stabellini 
> >>>> ---
> >>
> >>>
> >>> [Jiamei Xie]
> >>> I wonder if an default container for arm can be added. For example,  if
> >>>  "CONTAINER=arm64 automation/scripts/containerize bash",
> >>>  set the default CONTAINER as "registry.gitlab.com/xen-
> >> project/xen/alpine:3.12-arm64v8"
> >>>
> >>
> >> It can be added doing the following:
> >>
> >> diff --git a/automation/scripts/containerize
> >> b/automation/scripts/containerize
> >> index 0f4645c4cccb..b395bd359ecf 100755
> >> --- a/automation/scripts/containerize
> >> +++ b/automation/scripts/containerize
> >> @@ -25,7 +25,7 @@ die() {
> >>  BASE="registry.gitlab.com/xen-project/xen"
> >>  case "_${CONTAINER}" in
> >>  _alpine) CONTAINER="${BASE}/alpine:3.12" ;;
> >> -_alpine-arm64v8) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;;
> >> +_alpine-arm64v8|_arm64) CONTAINER="${BASE}/alpine:3.12-
> arm64v8" ;;
> >>  _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;;
> >>  _riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;;
> >>  _centos7) CONTAINER="${BASE}/centos:7" ;;
> >>
> >> The question is whether it would be beneficial. After all you would still
> need
> >> to
> >> type CONTAINER=arm64, whereas at the moment, you need to type
> >> CONTAINER=alpine-arm64v8.
> >> TBH I'm not sure it is improving anything (?).
> >>
> >> ~Michal
> > [Jiamei Xie]
> > I am not sure about this either.  I added something like below f to run it 
> > on
> arm64 machine.   But it  didn't take "running container for a different
> architecture" into consideration.
> >
> So your question is not about adding default container when selecting
> CONTAINER=arm64, but adding
> a default one when running on arm64 platform. Right now, the default one
> is debian:stretch
> (if you don't type CONTAINER= at all). Do I understand it right that you
> would like the same
> behavior when running on arm64 platform (currently, it would also select
> debian:stretch)?
> So that when executing:
> ./automation/scripts/containerize ...
> it would automatically select alpine-arm64v8?
> 
Yes, this is what I mean.
> 
> > --- a/automation/scripts/containerize
> > +++ b/automation/scripts/containerize
> > @@ -18,6 +18,12 @@ die() {
> >  exit 1
> >  }
> >
> > +# There are two containers that can run on aarch64, unstable and alpine.
> > +# Set the default container to alpine for aarch64
> > +if [[ $(uname -m) = "aarch64" && -z ${CONTAINER} ]]; then
> The out

RE: [PATCH v3 05/10] automation: Add Arm containers to containerize script

2022-10-20 Thread Jiamei Xie

Hi Michal,

> -Original Message-
> From: Michal Orzel 
> Sent: Thursday, October 20, 2022 2:59 PM
> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> Cc: Doug Goldstein ; Stefano Stabellini
> 
> Subject: Re: [PATCH v3 05/10] automation: Add Arm containers to
> containerize script
> 
> Hi Jiamei,
> 
> On 20/10/2022 05:00, Jiamei Xie wrote:
> >
> >
> > Hi Michal,
> >
> >> -Original Message-
> >> From: Xen-devel  On Behalf Of
> >> Michal Orzel
> >> Sent: Tuesday, September 27, 2022 5:47 PM
> >> To: xen-devel@lists.xenproject.org
> >> Cc: Michal Orzel ; Doug Goldstein
> >> ; Stefano Stabellini 
> >> Subject: [PATCH v3 05/10] automation: Add Arm containers to
> containerize
> >> script
> >>
> >> Script automation/scripts/containerize makes it easy to build Xen within
> >> predefined containers from gitlab container registry. This script is
> >> currently missing the helpers to select Arm containers, so populate the
> >> necessary entries.
> >>
> >> Signed-off-by: Michal Orzel 
> >> Acked-by: Stefano Stabellini 
> >> ---
> 
> >
> > [Jiamei Xie]
> > I wonder if an default container for arm can be added. For example,  if
> >  "CONTAINER=arm64 automation/scripts/containerize bash",
> >  set the default CONTAINER as "registry.gitlab.com/xen-
> project/xen/alpine:3.12-arm64v8"
> >
> 
> It can be added doing the following:
> 
> diff --git a/automation/scripts/containerize
> b/automation/scripts/containerize
> index 0f4645c4cccb..b395bd359ecf 100755
> --- a/automation/scripts/containerize
> +++ b/automation/scripts/containerize
> @@ -25,7 +25,7 @@ die() {
>  BASE="registry.gitlab.com/xen-project/xen"
>  case "_${CONTAINER}" in
>  _alpine) CONTAINER="${BASE}/alpine:3.12" ;;
> -_alpine-arm64v8) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;;
> +_alpine-arm64v8|_arm64) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;;
>  _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;;
>  _riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;;
>  _centos7) CONTAINER="${BASE}/centos:7" ;;
> 
> The question is whether it would be beneficial. After all you would still need
> to
> type CONTAINER=arm64, whereas at the moment, you need to type
> CONTAINER=alpine-arm64v8.
> TBH I'm not sure it is improving anything (?).
> 
> ~Michal
[Jiamei Xie] 
I am not sure about this either.  I added something like below f to run it on 
arm64 machine.   But it  didn't take "running container for a different 
architecture" into consideration.

--- a/automation/scripts/containerize
+++ b/automation/scripts/containerize
@@ -18,6 +18,12 @@ die() {
 exit 1
 }

+# There are two containers that can run on aarch64, unstable and alpine.
+# Set the default container to alpine for aarch64
+if [[ $(uname -m) = "aarch64" && -z ${CONTAINER} ]]; then
+CONTAINER="alpine"
+fi
+
 #
 # The caller is expected to override the CONTAINER environment
 # variable with the container they wish to launch.
@@ -41,6 +47,11 @@ case "_${CONTAINER}" in
 _opensuse-tumbleweed|_tumbleweed) 
CONTAINER="${BASE}/suse:opensuse-tumbleweed" ;;
 esac

+# Containers for aarch64 have a sufix "-arm64v8"
+if [[ $(uname -m) = "aarch64" ]]; then
+CONTAINER="${CONTAINER}-arm64v8"
+fi
+
 # Use this variable to control whether root should be used
 case "_${CONTAINER_UID0}" in
 _1)   userarg= ;;


Best wishes
Jiamei Xie

RE: [PATCH] automation: test.yaml: Introduce templates to reduce the overhead

2022-10-19 Thread Jiamei Xie

H Michal,

> -Original Message-
> From: Xen-devel  On Behalf Of
> Michal Orzel
> Sent: Thursday, October 20, 2022 12:43 AM
> To: xen-devel@lists.xenproject.org
> Cc: Michal Orzel ; Doug Goldstein
> ; Stefano Stabellini 
> Subject: [PATCH] automation: test.yaml: Introduce templates to reduce the
> overhead
> 
> At the moment, we define lots of test jobs in test.yaml, that make use
> of the same configuration sections like variables, tags, artifacts.
> Introduce templates (hidden jobs whose names start with a dot) to
> reduce the overhead and simplify the file (more than 100 lines saved).
> This way, the actual jobs can only specify sections that are unique
> to them.
> 
> Most of the test jobs specify the same set of prerequisite jobs under needs
> property with just one additional being unique to the job itself. Introduce
> YAML anchors for that purpose, because when using extends, the needs
> property
> is not being merged (the parent property overwrites the child one).
> 
> Signed-off-by: Michal Orzel 
> ---
> This patch is based on the CI next branch where we already have several
> patches (already acked) to be merged into staging after the release:
> https://gitlab.com/xen-project/people/sstabellini/xen/-/tree/next
> 
> Tested pipeline:
> https://gitlab.com/xen-project/people/morzel/xen-orzelmichal/-
> /pipelines/671114820

Looks good to me.

Reviewed-by: jiamei@arm.com

RE: [PATCH v3 05/10] automation: Add Arm containers to containerize script

2022-10-19 Thread Jiamei Xie

Hi Michal,

> -Original Message-
> From: Xen-devel  On Behalf Of
> Michal Orzel
> Sent: Tuesday, September 27, 2022 5:47 PM
> To: xen-devel@lists.xenproject.org
> Cc: Michal Orzel ; Doug Goldstein
> ; Stefano Stabellini 
> Subject: [PATCH v3 05/10] automation: Add Arm containers to containerize
> script
> 
> Script automation/scripts/containerize makes it easy to build Xen within
> predefined containers from gitlab container registry. This script is
> currently missing the helpers to select Arm containers, so populate the
> necessary entries.
> 
> Signed-off-by: Michal Orzel 
> Acked-by: Stefano Stabellini 
> ---
> Changes in v3:
> - none
> Changes in v2:
> - modify commit msg to reflect that we are missing helpers but in reality
>   it could be possible to use Arm containers by specifying the full path
>   to gitlab container registry. However, such usage is annoying.
> ---
>  automation/scripts/containerize | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/automation/scripts/containerize
> b/automation/scripts/containerize
> index 9d4beca4fa4b..0f4645c4cccb 100755
> --- a/automation/scripts/containerize
> +++ b/automation/scripts/containerize
> @@ -25,6 +25,7 @@ die() {
>  BASE="registry.gitlab.com/xen-project/xen"
>  case "_${CONTAINER}" in
>  _alpine) CONTAINER="${BASE}/alpine:3.12" ;;
> +_alpine-arm64v8) CONTAINER="${BASE}/alpine:3.12-arm64v8" ;;
>  _archlinux|_arch) CONTAINER="${BASE}/archlinux:current" ;;
>  _riscv64) CONTAINER="${BASE}/archlinux:riscv64" ;;
>  _centos7) CONTAINER="${BASE}/centos:7" ;;
> @@ -35,6 +36,8 @@ case "_${CONTAINER}" in
>  _stretch|_) CONTAINER="${BASE}/debian:stretch" ;;
>  _buster-gcc-ibt) CONTAINER="${BASE}/debian:buster-gcc-ibt" ;;
>  _unstable|_) CONTAINER="${BASE}/debian:unstable" ;;
> +_unstable-arm32-gcc) CONTAINER="${BASE}/debian:unstable-arm32-
> gcc" ;;
> +_unstable-arm64v8) CONTAINER="${BASE}/debian:unstable-arm64v8" ;;
>  _trusty) CONTAINER="${BASE}/ubuntu:trusty" ;;
>  _xenial) CONTAINER="${BASE}/ubuntu:xenial" ;;
>  _opensuse-leap|_leap) CONTAINER="${BASE}/suse:opensuse-leap" ;;
> --
> 2.25.1

[Jiamei Xie] 
I wonder if an default container for arm can be added. For example,  if 
 "CONTAINER=arm64 automation/scripts/containerize bash", 
 set the default CONTAINER as 
"registry.gitlab.com/xen-project/xen/alpine:3.12-arm64v8"

Best wishes
Jiamei Xie

RE: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2 extended input/output registers

2022-08-22 Thread Jiamei Xie



Hi
I build and run it on armv8a, and start dom0 with two cpus. Cpu off and on 
tests passed. It seems it don't break current cpu basic functions.

Best wishes
Jiamei Xie


> -Original Message-
> From: Xen-devel  On Behalf Of
> Jens Wiklander
> Sent: Thursday, August 18, 2022 6:56 PM
> To: xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Julien Grall 
> ;
> Volodymyr Babchuk ; Bertrand Marquis
> ; Anthony PERARD
> ; Juergen Gross ; Wei Liu
> ; Jens Wiklander ; Luca Fancellu
> 
> Subject: [PATCH v5 1/9] xen/arm: smccc: add support for SMCCCv1.2
> extended input/output registers
> 
> SMCCC v1.2 [1] AArch64 allows x0-x17 to be used as both parameter
> registers and result registers for the SMC and HVC instructions.
> 
> Arm Firmware Framework for Armv8-A specification makes use of x0-x7 as
> parameter and result registers.
> 
> Let us add new interface to support this extended set of input/output
> registers.
> 
> This is based on 3fdc0cb59d97 ("arm64: smccc: Add support for SMCCCv1.2
> extended input/output registers") by Sudeep Holla from the Linux kernel
> 
> The SMCCC version reported to the VM is bumped to 1.2 in order to support
> handling FF-A messages.
> 
> [1] https://developer.arm.com/documentation/den0028/c/?lang=en
> 
> Reviewed-by: Luca Fancellu 
> Signed-off-by: Jens Wiklander 
> ---
>  xen/arch/arm/arm64/asm-offsets.c |  9 +++
>  xen/arch/arm/arm64/smc.S | 43
> 
>  xen/arch/arm/include/asm/smccc.h | 40
> +
>  xen/arch/arm/vsmc.c  |  2 +-
>  4 files changed, 93 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/arm64/asm-offsets.c b/xen/arch/arm/arm64/asm-
> offsets.c
> index 280ddb55bfd4..1721e1ed26e1 100644
> --- a/xen/arch/arm/arm64/asm-offsets.c
> +++ b/xen/arch/arm/arm64/asm-offsets.c
> @@ -56,6 +56,15 @@ void __dummy__(void)
> BLANK();
> OFFSET(SMCCC_RES_a0, struct arm_smccc_res, a0);
> OFFSET(SMCCC_RES_a2, struct arm_smccc_res, a2);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X0_OFFS, struct arm_smccc_1_2_regs,
> a0);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X2_OFFS, struct arm_smccc_1_2_regs,
> a2);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X4_OFFS, struct arm_smccc_1_2_regs,
> a4);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X6_OFFS, struct arm_smccc_1_2_regs,
> a6);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X8_OFFS, struct arm_smccc_1_2_regs,
> a8);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X10_OFFS, struct arm_smccc_1_2_regs,
> a10);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X12_OFFS, struct arm_smccc_1_2_regs,
> a12);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X14_OFFS, struct arm_smccc_1_2_regs,
> a14);
> +   OFFSET(ARM_SMCCC_1_2_REGS_X16_OFFS, struct arm_smccc_1_2_regs,
> a16);
>  }
> 
>  /*
> diff --git a/xen/arch/arm/arm64/smc.S b/xen/arch/arm/arm64/smc.S
> index 91bae62dd4d2..c546192e7f2d 100644
> --- a/xen/arch/arm/arm64/smc.S
> +++ b/xen/arch/arm/arm64/smc.S
> @@ -27,3 +27,46 @@ ENTRY(__arm_smccc_1_0_smc)
>  stp x2, x3, [x4, #SMCCC_RES_a2]
>  1:
>  ret
> +
> +
> +/*
> + * void arm_smccc_1_2_smc(const struct arm_smccc_1_2_regs *args,
> + *struct arm_smccc_1_2_regs *res)
> + */
> +ENTRY(arm_smccc_1_2_smc)
> +/* Save `res` and free a GPR that won't be clobbered */
> +stp x1, x19, [sp, #-16]!
> +
> +/* Ensure `args` won't be clobbered while loading regs in next step */
> +mov  x19, x0
> +
> +/* Load the registers x0 - x17 from the struct arm_smccc_1_2_regs */
> +ldp  x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
> +ldp  x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
> +ldp  x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
> +ldp  x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
> +ldp  x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
> +ldp  x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
> +ldp  x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
> +ldp  x14, x15, [x19, #ARM_SMCCC_1_2_REGS_X14_OFFS]
> +ldp  x16, x17, [x19, #ARM_SMCCC_1_2_REGS_X16_OFFS]
> +
> +smc #0
> +
> +/* Load the `res` from the stack */
> +ldr  x19, [sp]
> +
> +/* Store the registers x0 - x17 into the result structure */
> +stp  x0, x1, [x19, #ARM_SMCCC_1_2_REGS_X0_OFFS]
> +stp  x2, x3, [x19, #ARM_SMCCC_1_2_REGS_X2_OFFS]
> +stp  x4, x5, [x19, #ARM_SMCCC_1_2_REGS_X4_OFFS]
> +stp  x6, x7, [x19, #ARM_SMCCC_1_2_REGS_X6_OFFS]
> +stp  x8, x9, [x19, #ARM_SMCCC_1_2_REGS_X8_OFFS]
> +stp  x10, x11, [x19, #ARM_SMCCC_1_2_REGS_X10_OFFS]
> +stp  x12, x13, [x19, #ARM_SMCCC_1_2_REGS_X12_OFFS]
> +stp  x14, x15, [x19, #A

RE: [PATCH 01/16] libxl: Add support for Virtio disk configuration

2022-08-17 Thread Jiamei Xie

Hi all
 Sorry for that, please ignore this too.  I sent the wrong email.

> -Original Message-
> From: jiaxie01 
> Sent: Wednesday, August 17, 2022 10:07 AM
> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> Cc: Oleksandr Tyshchenko ; Wei Liu
> ; Anthony PERARD ; George
> Dunlap ; Nick Rosbrook
> ; Juergen Gross 
> Subject: [PATCH 01/16] libxl: Add support for Virtio disk configuration
> 
> From: Oleksandr Tyshchenko 
> 
> This patch adds basic support for configuring and assisting virtio-mmio
> based virtio-disk backend (emulator) which is intended to run out of
> Qemu and could be run in any domain.
> Although the Virtio block device is quite different from traditional
> Xen PV block device (vbd) from the toolstack's point of view:
>  - as the frontend is virtio-blk which is not a Xenbus driver, nothing
>written to Xenstore are fetched by the frontend currently ("vdev"
>is not passed to the frontend). But this might need to be revised
>in future, so frontend data might be written to Xenstore in order to
>support hotplugging virtio devices or passing the backend domain id
>on arch where the device-tree is not available.
>  - the ring-ref/event-channel are not used for the backend<->frontend
>communication, the proposed IPC for Virtio is IOREQ/DM
> it is still a "block device" and ought to be integrated in existing
> "disk" handling. So, re-use (and adapt) "disk" parsing/configuration
> logic to deal with Virtio devices as well.
> 
> For the immediate purpose and an ability to extend that support for
> other use-cases in future (Qemu, virtio-pci, etc) perform the following
> actions:
> - Add new disk backend type (LIBXL_DISK_BACKEND_STANDALONE) and
> reflect
>   that in the configuration
> - Introduce new disk "specification" and "transport" fields to struct
>   libxl_device_disk. Both are written to the Xenstore. The transport
>   field is only used for the specification "virtio" and it assumes
>   only "mmio" value for now.
> - Introduce new "specification" option with "xen" communication
>   protocol being default value.
> - Add new device kind (LIBXL__DEVICE_KIND_VIRTIO_DISK) as current
>   one (LIBXL__DEVICE_KIND_VBD) doesn't fit into Virtio disk model
> 
> An example of domain configuration for Virtio disk:
> disk = [ 'phy:/dev/mmcblk0p3, xvda1, backendtype=standalone,
> specification=virtio']
> 
> Nothing has changed for default Xen disk configuration.
> 
> Please note, this patch is not enough for virtio-disk to work
> on Xen (Arm), as for every Virtio device (including disk) we need
> to allocate Virtio MMIO params (IRQ and memory region) and pass
> them to the backend, also update Guest device-tree. The subsequent
> patch will add these missing bits. For the current patch,
> the default "irq" and "base" are just written to the Xenstore.
> This is not an ideal splitting, but this way we avoid breaking
> the bisectability.
> 
> Signed-off-by: Oleksandr Tyshchenko 
> ---
> Changes RFC -> V1:
>- no changes
> 
> Changes V1 -> V2:
>- rebase according to the new location of libxl_virtio_disk.c
> 
> Changes V2 -> V3:
>- no changes
> 
> Changes V3 -> V4:
>- rebase according to the new argument for DEFINE_DEVICE_TYPE_STRUCT
> 
> Changes V4 -> V5:
>- split the changes, change the order of the patches
>- update patch description
>- don't introduce new "vdisk" configuration option with own parsing logic,
>  re-use Xen PV block "disk" parsing/configuration logic for the 
> virtio-disk
>- introduce "virtio" flag and document it's usage
>- add LIBXL_HAVE_DEVICE_DISK_VIRTIO
>- update libxlu_disk_l.[ch]
>- drop num_disks variable/MAX_VIRTIO_DISKS
>- drop Wei's T-b
> 
> Changes V5 -> V6:
>- rebase on current staging
>- use "%"PRIu64 instead of %lu for disk->base in device_disk_add()
>- update *.gen.go files
> 
> Changes V6 -> V7:
>- rebase on current staging
>- update *.gen.go files and libxlu_disk_l.[ch] files
>- update patch description
>- rework significantly to support more flexible configuration
>  and have more generic basic implementation for being able to extend
>  that for other use-cases (virtio-pci, qemu, etc).
> 
> Changes V7 -> V8:
>- update *.gen.go files and libxlu_disk_l.[ch] files
>- update patch description and comments in the code
>- use "specification" config option instead of "protocol"
>

RE: [PATCH 02/16] libxl: Introduce basic virtio-mmio support on Arm

2022-08-16 Thread Jiamei Xie

Hi all，
Sorry for that, please ignore this. I sent the wrong email.

> -Original Message-
> From: jiaxie01 
> Sent: Wednesday, August 17, 2022 10:07 AM
> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> Cc: Julien Grall ; Wei Liu ; Anthony
> PERARD ; Juergen Gross ;
> Stefano Stabellini ; Julien Grall ;
> Bertrand Marquis ; Volodymyr Babchuk
> ; Oleksandr Tyshchenko
> 
> Subject: [PATCH 02/16] libxl: Introduce basic virtio-mmio support on Arm
> 
> From: Julien Grall 
> 
> This patch introduces helpers to allocate Virtio MMIO params
> (IRQ and memory region) and create specific device node in
> the Guest device-tree with allocated params. In order to deal
> with multiple Virtio devices, reserve corresponding ranges.
> For now, we reserve 1MB for memory regions and 10 SPIs.
> 
> As these helpers should be used for every Virtio device attached
> to the Guest, call them for Virtio disk(s).
> 
> Please note, with statically allocated Virtio IRQs there is
> a risk of a clash with a physical IRQs of passthrough devices.
> For the first version, it's fine, but we should consider allocating
> the Virtio IRQs automatically. Thankfully, we know in advance which
> IRQs will be used for passthrough to be able to choose non-clashed
> ones.
> 
> Signed-off-by: Julien Grall 
> Signed-off-by: Oleksandr Tyshchenko 
> Reviewed-by: Stefano Stabellini 
> Reviewed-by: Anthony PERARD 
> ---
> Please note, this is a split/cleanup/hardening of Julien's PoC:
> "Add support for Guest IO forwarding to a device emulator"
> 
> Changes RFC -> V1:
>- was squashed with:
>  "[RFC PATCH V1 09/12] libxl: Handle virtio-mmio irq in more correct way"
>  "[RFC PATCH V1 11/12] libxl: Insert "dma-coherent" property into virtio-
> mmio device node"
>  "[RFC PATCH V1 12/12] libxl: Fix duplicate memory node in DT"
>- move VirtIO MMIO #define-s to xen/include/public/arch-arm.h
> 
> Changes V1 -> V2:
>- update the author of a patch
> 
> Changes V2 -> V3:
>- no changes
> 
> Changes V3 -> V4:
>- no changes
> 
> Changes V4 -> V5:
>- split the changes, change the order of the patches
>- drop an extra "virtio" configuration option
>- update patch description
>- use CONTAINER_OF instead of own implementation
>- reserve ranges for Virtio MMIO params and put them
>  in correct location
>- create helpers to allocate Virtio MMIO params, add
>  corresponding sanity-сhecks
>- add comment why MMIO size 0x200 is chosen
>- update debug print
>- drop Wei's T-b
> 
> Changes V5 -> V6:
>- rebase on current staging
> 
> Changes V6 -> V7:
>- rebase on current staging
>- add T-b and R-b tags
>- update according to the recent changes to
>  "libxl: Add support for Virtio disk configuration"
> 
> Changes V7 -> V8:
>- drop T-b and R-b tags
>- make virtio_mmio_base/irq global variables to be local in
>  libxl__arch_domain_prepare_config() and initialize them at
>  the beginning of the function, then rework alloc_virtio_mmio_base/irq()
>  to take a pointer to virtio_mmio_base/irq variables as an argument
>- update according to the recent changes to
>  "libxl: Add support for Virtio disk configuration"
> 
> Changes V8 -> V9:
>- Stefano already gave his Reviewed-by, I dropped it due to the changes
>- remove the second set of parentheses for check in
> alloc_virtio_mmio_base()
>- clarify the updating of "nr_spis" right after num_disks loop in
>  libxl__arch_domain_prepare_config() and add a comment on top of it
>- use GCSPRINTF() instead of using a buffer of a static size
>  calculated by hand in make_virtio_mmio_node()
> 
> Changes V9 -> V10:
>- add Stefano's and Anthony's R-b
> ---
> ---
>  tools/libs/light/libxl_arm.c  | 121 +-
>  xen/include/public/arch-arm.h |   7 ++
>  2 files changed, 126 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index eef1de0939..9be9b2a2d1 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -8,6 +8,46 @@
>  #include 
>  #include 
> 
> +/*
> + * There is no clear requirements for the total size of Virtio MMIO region.
> + * The size of control registers is 0x100 and device-specific configuration
> + * registers starts at the offset 0x100, however it's size depends on the
> device
> + * and the driver. Pick the biggest known size at the moment to cover most
> + * o

RE: [PATCH 5/7] xen/arm32: head: Move earlyprintk messages to .rodata.str

2022-08-14 Thread Jiamei Xie

Hi Julien,

> -Original Message-
> From: Xen-devel  On Behalf Of
> Julien Grall
> Sent: Saturday, August 13, 2022 3:25 AM
> To: xen-devel@lists.xenproject.org
> Cc: jul...@xen.org; Julien Grall ; Stefano Stabellini
> ; Bertrand Marquis ;
> Volodymyr Babchuk 
> Subject: [PATCH 5/7] xen/arm32: head: Move earlyprintk messages
> to .rodata.str
> 
> From: Julien Grall 
> 
> At the moment, the strings are in text right after each use because
> the instruction 'adr' has specific requirement on the location
> and the compiler will forbid cross section label.
> 
> The macro 'adr_l' was recently reworked so the caller doesn't need
> to know whether the MMU is on. This makes it easier to use where
> instructions can be run in both context.
> 
> This also means that the strings don't need to be part of .text
> anymore. So move them to .rodata.str.
> 
> Signed-off-by: Julien Grall 
> ---
>  xen/arch/arm/arm32/head.S | 12 
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 27d02ac8d68f..a558c2a6876e 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -93,13 +93,10 @@
>   */
>  #define PRINT(_s)   \
>  mov   r3, lr   ;\
> -adr   r0, 98f  ;\
> +adr_l r0, 98f  ;\
>  blputs ;\
>  mov   lr, r3   ;\
> -b 99f  ;\
> -98: .asciz _s  ;\
> -.align 2   ;\
> -99:
> +RODATA_STR(98, _s)
> 
>  /*
>   * Macro to print the value of register \rb
> @@ -736,7 +733,7 @@ ENDPROC(puts)
>   * Clobbers r0-r3
>   */
>  putn:
> -adr   r1, hex
> +adr_l r1, hex
>  mov   r3, #8
>  1:
>  early_uart_ready r11, r2
> @@ -749,8 +746,7 @@ putn:
>  mov   pc, lr
>  ENDPROC(putn)
> 
> -hex:    .ascii "0123456789abcdef"
> -.align 2
> +RODATA_STR(hex, "0123456789abcdef")
> 
>  #else  /* CONFIG_EARLY_PRINTK */
> 
> --
> 2.37.1
> 

That looks good to me.
Reviewed-by: Jiamei Xie 

Best wishes
Jiamei Xie

RE: [PATCH V11.1 1/3] libxl: Add support for Virtio disk configuration

2022-07-19 Thread Jiamei Xie

Hi Oleksandr,

We have tested it on arm64 with " disk = [ 'phy:/usr/share/guests/disk.img0, 
xvda1, backendtype=standalone, specification=virtio']". It works ok.

Tested-by: Jiamei Xie 
Tested-by: Henry Wang 

Best wishes
Jiamei Xie


> -Original Message-
> From: Xen-devel  On Behalf Of
> Oleksandr Tyshchenko
> Sent: Sunday, July 17, 2022 12:38 AM
> To: xen-devel@lists.xenproject.org
> Cc: Oleksandr Tyshchenko ; Wei Liu
> ; Anthony PERARD ; George
> Dunlap ; Nick Rosbrook
> ; Juergen Gross ; Stefano
> Stabellini ; Julien Grall ; Volodymyr
> Babchuk ; Bertrand Marquis
> 
> Subject: [PATCH V11.1 1/3] libxl: Add support for Virtio disk configuration
> 
> From: Oleksandr Tyshchenko 
> 
> This patch adds basic support for configuring and assisting virtio-mmio
> based virtio-disk backend (emulator) which is intended to run out of
> Qemu and could be run in any domain.
> Although the Virtio block device is quite different from traditional
> Xen PV block device (vbd) from the toolstack's point of view:
>  - as the frontend is virtio-blk which is not a Xenbus driver, nothing
>written to Xenstore are fetched by the frontend currently ("vdev"
>is not passed to the frontend). But this might need to be revised
>in future, so frontend data might be written to Xenstore in order to
>support hotplugging virtio devices or passing the backend domain id
>on arch where the device-tree is not available.
>  - the ring-ref/event-channel are not used for the backend<->frontend
>communication, the proposed IPC for Virtio is IOREQ/DM
> it is still a "block device" and ought to be integrated in existing
> "disk" handling. So, re-use (and adapt) "disk" parsing/configuration
> logic to deal with Virtio devices as well.
> 
> For the immediate purpose and an ability to extend that support for
> other use-cases in future (Qemu, virtio-pci, etc) perform the following
> actions:
> - Add new disk backend type (LIBXL_DISK_BACKEND_STANDALONE) and
> reflect
>   that in the configuration
> - Introduce new disk "specification" and "transport" fields to struct
>   libxl_device_disk. Both are written to the Xenstore. The transport
>   field is only used for the specification "virtio" and it assumes
>   only "mmio" value for now.
> - Introduce new "specification" option with "xen" communication
>   protocol being default value.
> - Add new device kind (LIBXL__DEVICE_KIND_VIRTIO_DISK) as current
>   one (LIBXL__DEVICE_KIND_VBD) doesn't fit into Virtio disk model
> 
> An example of domain configuration for Virtio disk:
> disk = [ 'phy:/dev/mmcblk0p3, xvda1, backendtype=standalone,
> specification=virtio']
> 
> Nothing has changed for default Xen disk configuration.
> 
> Please note, this patch is not enough for virtio-disk to work
> on Xen (Arm), as for every Virtio device (including disk) we need
> to allocate Virtio MMIO params (IRQ and memory region) and pass
> them to the backend, also update Guest device-tree. The subsequent
> patch will add these missing bits. For the current patch,
> the default "irq" and "base" are just written to the Xenstore.
> This is not an ideal splitting, but this way we avoid breaking
> the bisectability.
> 
> Signed-off-by: Oleksandr Tyshchenko 
> ---
> Changes RFC -> V1:
>- no changes
> 
> Changes V1 -> V2:
>- rebase according to the new location of libxl_virtio_disk.c
> 
> Changes V2 -> V3:
>- no changes
> 
> Changes V3 -> V4:
>- rebase according to the new argument for DEFINE_DEVICE_TYPE_STRUCT
> 
> Changes V4 -> V5:
>- split the changes, change the order of the patches
>- update patch description
>- don't introduce new "vdisk" configuration option with own parsing logic,
>  re-use Xen PV block "disk" parsing/configuration logic for the 
> virtio-disk
>- introduce "virtio" flag and document it's usage
>- add LIBXL_HAVE_DEVICE_DISK_VIRTIO
>- update libxlu_disk_l.[ch]
>- drop num_disks variable/MAX_VIRTIO_DISKS
>- drop Wei's T-b
> 
> Changes V5 -> V6:
>- rebase on current staging
>- use "%"PRIu64 instead of %lu for disk->base in device_disk_add()
>- update *.gen.go files
> 
> Changes V6 -> V7:
>- rebase on current staging
>- update *.gen.go files and libxlu_disk_l.[ch] files
>- update patch description
>- rework significantly to support more flexible configuration
>  and have more generic basic implementation for being able to extend
>  that for other use-c

RE: [PATCH] xen/char: pv_console: Fix MISRA C 2012 Rule 2.1 violation

2022-07-06 Thread Jiamei Xie

Hi Xenia,

> -Original Message-
> From: Xen-devel  On Behalf Of
> Xenia Ragiadakou
> Sent: Thursday, July 7, 2022 1:50 AM
> To: xen-devel@lists.xenproject.org
> Cc: Andrew Cooper ; George Dunlap
> ; Jan Beulich ; Julien Grall
> ; Stefano Stabellini ; Wei Liu
> 
> Subject: [PATCH] xen/char: pv_console: Fix MISRA C 2012 Rule 2.1 violation
> 
> Remove the definition of the function pv_console_evtchn(),
> when CONFIG_XEN_GUEST is not set, because the function is not used.
> 
> Signed-off-by: Xenia Ragiadakou 
> ---
>  xen/include/xen/pv_console.h | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/xen/include/xen/pv_console.h b/xen/include/xen/pv_console.h
> index 4745f46f2d..55b20323fb 100644
> --- a/xen/include/xen/pv_console.h
> +++ b/xen/include/xen/pv_console.h
> @@ -19,11 +19,6 @@ static inline void
> pv_console_set_rx_handler(serial_rx_fn fn) { }
>  static inline void pv_console_init_postirq(void) { }
>  static inline void pv_console_puts(const char *buf, size_t nr) { }
>  static inline size_t pv_console_rx(struct cpu_user_regs *regs) { return 0; }
> -evtchn_port_t pv_console_evtchn(void)
> -{
> -ASSERT_UNREACHABLE();
> -return 0;
> -}
> 
>  #endif /* !CONFIG_XEN_GUEST */
>  #endif /* __XEN_PV_CONSOLE_H__ */
> --
> 2.34.1
> 

I have run it on arm64, booting Xen+Dom0 and starting few guests, connecting 
consoles. It all works fine.
Tested-by: Jiamei Xie

[PATCH v4] xen/arm: avoid overflow when setting vtimer in context switch

2022-07-06 Thread Jiamei Xie

virt_vtimer_save() will calculate the next deadline when the vCPU is
scheduled out. At the moment, Xen will use the following equation:

  virt_timer.cval + virt_time_base.offset - boot_count

The three values are 64-bit and one (cval) is controlled by domain. In
theory, it would be possible that the domain has started a long time
after the system boot. So virt_time_base.offset - boot_count may be a
large numbers.

This means a domain may inadvertently set a cval so the result would
overflow. Consequently, the deadline would be set very far in the
future. This could result to loss of timer interrupts or the vCPU
getting block "forever".

One way to solve the problem, would be to separately
   1) compute when the domain was created in ns
   2) convert cval to ns
   3) Add 1 and 2 together

The first part of the equation never change (the value is set/known at
domain creation). So take the opportunity to store it in domain structure.

Signed-off-by: Jiamei Xie 
---
was "xen/arm: avoid vtimer flip-flop transition in context switch".
v4 changes:
- re-write commit message
---
v3 changes:
- re-write commit message
- store nanoseconds in virt_timer_base instead of adding a new structure
- assign to nanoseconds first, then seconds
---
 xen/arch/arm/include/asm/domain.h | 1 +
 xen/arch/arm/vtimer.c | 9 ++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/include/asm/domain.h 
b/xen/arch/arm/include/asm/domain.h
index ed63c2b6f9..cd9ce19b4b 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -71,6 +71,7 @@ struct arch_domain
 
 struct {
 uint64_t offset;
+s_time_t nanoseconds;
 } virt_timer_base;
 
 struct vgic_dist vgic;
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index 6b78fea77d..aeaea78e4c 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -63,7 +63,9 @@ static void virt_timer_expired(void *data)
 int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
 {
 d->arch.virt_timer_base.offset = get_cycles();
-d->time_offset.seconds = ticks_to_ns(d->arch.virt_timer_base.offset - 
boot_count);
+d->arch.virt_timer_base.nanoseconds =
+ticks_to_ns(d->arch.virt_timer_base.offset - boot_count);
+d->time_offset.seconds = d->arch.virt_timer_base.nanoseconds;
 do_div(d->time_offset.seconds, 10);
 
 config->clock_frequency = timer_dt_clock_frequency;
@@ -144,8 +146,9 @@ void virt_timer_save(struct vcpu *v)
 if ( (v->arch.virt_timer.ctl & CNTx_CTL_ENABLE) &&
  !(v->arch.virt_timer.ctl & CNTx_CTL_MASK))
 {
-set_timer(&v->arch.virt_timer.timer, 
ticks_to_ns(v->arch.virt_timer.cval +
-  v->domain->arch.virt_timer_base.offset - boot_count));
+set_timer(&v->arch.virt_timer.timer,
+  v->domain->arch.virt_timer_base.nanoseconds +
+  ticks_to_ns(v->arch.virt_timer.cval));
 }
 }
 
-- 
2.25.1

RE: [PATCH v3] xen/arm: avoid overflow when setting vtimer in context switch

2022-06-29 Thread Jiamei Xie

Hi,

> -Original Message-
> From: Jiamei Xie 
> Sent: 2022年6月30日 9:54
> To: xen-devel@lists.xenproject.org
> Cc: Jiamei Xie ; Stefano Stabellini
> ; Julien Grall ; Bertrand Marquis
> ; Volodymyr Babchuk
> ; Wei Chen 
> Subject: [PATCH v3] xen/arm: avoid overflow when setting vtimer in context
> switch
> 
> virt_vtimer_save is calculating the new time for the vtimer in:
> "v->arch.virt_timer.cval + v->domain->arch.virt_timer_base.offset
> - boot_count".
> In this formula, "cval + offset" might cause uint64_t overflow.
> Changing it to "ticks_to_ns(v->domain->arch.virt_timer_base.offset -
> boot_count) + ticks_to_ns(v->arch.virt_timer.cval)" can avoid overflow,
> and "ticks_to_ns(arch.virt_timer_base.offset - boot_count)" will be
> always the same, which has been caculated in domain_vtimer_init.
> Introduce a new field virt_timer_base.nanoseconds to store this value
> for arm in struct arch_domain, so we can use it directly.
> 
> Signed-off-by: Jiamei Xie 
> Change-Id: Ib80cee51eaf844661e6f92154a0339ad2a652f9b

I am sorry I forget to remove the Change-Id.

> ---
> was "xen/arm: avoid vtimer flip-flop transition in context switch".
> v3 changes:
> -re-write commit message
> -store nanoseconds in virt_timer_base instead of adding a new structure
> -assign to nanoseconds first, then seconds
> ---
>  xen/arch/arm/include/asm/domain.h | 1 +
>  xen/arch/arm/vtimer.c | 9 ++---
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/domain.h
> b/xen/arch/arm/include/asm/domain.h
> index ed63c2b6f9..cd9ce19b4b 100644
> --- a/xen/arch/arm/include/asm/domain.h
> +++ b/xen/arch/arm/include/asm/domain.h
> @@ -71,6 +71,7 @@ struct arch_domain
> 
>  struct {
>  uint64_t offset;
> +s_time_t nanoseconds;
>  } virt_timer_base;
> 
>  struct vgic_dist vgic;
> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> index 6b78fea77d..aeaea78e4c 100644
> --- a/xen/arch/arm/vtimer.c
> +++ b/xen/arch/arm/vtimer.c
> @@ -63,7 +63,9 @@ static void virt_timer_expired(void *data)
>  int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig
> *config)
>  {
>  d->arch.virt_timer_base.offset = get_cycles();
> -d->time_offset.seconds = ticks_to_ns(d->arch.virt_timer_base.offset -
> boot_count);
> +d->arch.virt_timer_base.nanoseconds =
> +ticks_to_ns(d->arch.virt_timer_base.offset - boot_count);
> +d->time_offset.seconds = d->arch.virt_timer_base.nanoseconds;
>  do_div(d->time_offset.seconds, 10);
> 
>  config->clock_frequency = timer_dt_clock_frequency;
> @@ -144,8 +146,9 @@ void virt_timer_save(struct vcpu *v)
>  if ( (v->arch.virt_timer.ctl & CNTx_CTL_ENABLE) &&
>   !(v->arch.virt_timer.ctl & CNTx_CTL_MASK))
>  {
> -set_timer(&v->arch.virt_timer.timer, 
> ticks_to_ns(v->arch.virt_timer.cval
> +
> -  v->domain->arch.virt_timer_base.offset - boot_count));
> +set_timer(&v->arch.virt_timer.timer,
> +  v->domain->arch.virt_timer_base.nanoseconds +
> +  ticks_to_ns(v->arch.virt_timer.cval));
>  }
>  }
> 
> --
> 2.25.1

[PATCH v3] xen/arm: avoid overflow when setting vtimer in context switch

2022-06-29 Thread Jiamei Xie

virt_vtimer_save is calculating the new time for the vtimer in:
"v->arch.virt_timer.cval + v->domain->arch.virt_timer_base.offset
- boot_count".
In this formula, "cval + offset" might cause uint64_t overflow.
Changing it to "ticks_to_ns(v->domain->arch.virt_timer_base.offset -
boot_count) + ticks_to_ns(v->arch.virt_timer.cval)" can avoid overflow,
and "ticks_to_ns(arch.virt_timer_base.offset - boot_count)" will be
always the same, which has been caculated in domain_vtimer_init.
Introduce a new field virt_timer_base.nanoseconds to store this value
for arm in struct arch_domain, so we can use it directly.

Signed-off-by: Jiamei Xie 
Change-Id: Ib80cee51eaf844661e6f92154a0339ad2a652f9b
---
was "xen/arm: avoid vtimer flip-flop transition in context switch".
v3 changes:
-re-write commit message
-store nanoseconds in virt_timer_base instead of adding a new structure
-assign to nanoseconds first, then seconds
---
 xen/arch/arm/include/asm/domain.h | 1 +
 xen/arch/arm/vtimer.c | 9 ++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/include/asm/domain.h 
b/xen/arch/arm/include/asm/domain.h
index ed63c2b6f9..cd9ce19b4b 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -71,6 +71,7 @@ struct arch_domain
 
 struct {
 uint64_t offset;
+s_time_t nanoseconds;
 } virt_timer_base;
 
 struct vgic_dist vgic;
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index 6b78fea77d..aeaea78e4c 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -63,7 +63,9 @@ static void virt_timer_expired(void *data)
 int domain_vtimer_init(struct domain *d, struct xen_arch_domainconfig *config)
 {
 d->arch.virt_timer_base.offset = get_cycles();
-d->time_offset.seconds = ticks_to_ns(d->arch.virt_timer_base.offset - 
boot_count);
+d->arch.virt_timer_base.nanoseconds =
+ticks_to_ns(d->arch.virt_timer_base.offset - boot_count);
+d->time_offset.seconds = d->arch.virt_timer_base.nanoseconds;
 do_div(d->time_offset.seconds, 10);
 
 config->clock_frequency = timer_dt_clock_frequency;
@@ -144,8 +146,9 @@ void virt_timer_save(struct vcpu *v)
 if ( (v->arch.virt_timer.ctl & CNTx_CTL_ENABLE) &&
  !(v->arch.virt_timer.ctl & CNTx_CTL_MASK))
 {
-set_timer(&v->arch.virt_timer.timer, 
ticks_to_ns(v->arch.virt_timer.cval +
-  v->domain->arch.virt_timer_base.offset - boot_count));
+set_timer(&v->arch.virt_timer.timer,
+  v->domain->arch.virt_timer_base.nanoseconds +
+  ticks_to_ns(v->arch.virt_timer.cval));
 }
 }
 
-- 
2.25.1

RE: [PATCH] xen/arm: avoid extra caclulations when setting vtimer in context switch

2022-06-28 Thread Jiamei Xie

Hi Bertrand,

> -Original Message-
> From: Bertrand Marquis 
> Sent: 2022年6月28日 15:29
> To: Jiamei Xie 
> Cc: Julien Grall ; xen-devel@lists.xenproject.org; Stefano
> Stabellini ; Volodymyr Babchuk
> ; Wei Chen 
> Subject: Re: [PATCH] xen/arm: avoid extra caclulations when setting vtimer
> in context switch
> 
> Hi Jiamei,
> 
> > On 28 Jun 2022, at 07:35, Jiamei Xie  wrote:
> >
> > Hi Julien,
> >
> >> -Original Message-
> >> From: Julien Grall 
> >> Sent: 2022年6月27日 18:36
> >> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> >> Cc: Stefano Stabellini ; Bertrand Marquis
> >> ; Volodymyr Babchuk
> >> ; Wei Chen 
> >> Subject: Re: [PATCH] xen/arm: avoid extra caclulations when setting
> vtimer
> >> in context switch
> >>
> >> Hi Jiami
> >>
> >> Title: s/caclulations/calculations/
> >>
> >> However, I think the title should mention the overflow rather than the
> >> extra calculations. The former is more important the latter.
> >>
> > I will change the title to " xen/arm: avoid overflow when setting vtimer in
> context switch"
> >
> >> On 27/06/2022 03:58, Jiamei Xie wrote:
> >>> virt_vtimer_save is calculating the new time for the vtimer in:
> >>> "v->arch.virt_timer.cval + v->domain->arch.virt_timer_base.offset
> >>> - boot_count".
> >>> In this formula, "cval + offset" might cause uint64_t overflow.
> >>> Changing it to "v->domain->arch.virt_timer_base.offset - boot_count +
> >>> v->arch.virt_timer.cval" can reduce the possibility of overflow
> >>
> >> This read strange to me. We want to remove the overflow completely not
> >> reducing it. The overflow is completely removed by converting the
> >> "offset - bount_count" to ns upfront.
> >>
> >> AFAICT, the commit message doesn't explain that.
> > Thanks for pointing out that. How about putting the commit message like
> the below:
> > xen/arm: avoid overflow when setting vtimer in context switch
> >
> > virt_vtimer_save is calculating the new time for the vtimer in:
> > "v->arch.virt_timer.cval + v->domain->arch.virt_timer_base.offset
> > - boot_count".
> > In this formula, "cval + offset" might cause uint64_t overflow.
> > Changing it to "ticks_to_ns(v->domain->arch.virt_timer_base.offset -
> > boot_count) + ticks_to_ns(v->arch.virt_timer.cval)" can avoid overflow,
> > and "ticks_to_ns(arch.virt_timer_base.offset - boot_count)" will be
> > always the same, which has been caculated in domain_vtimer_init.
> > Introduce a new field virt_timer_base.nanoseconds to store this value
> > for arm in struct arch_domain, so we can use it directly.
> >>
> >>> , and
> >>> "arch.virt_timer_base.offset - boot_count" will be always the same,
> >>> which has been caculated in domain_vtimer_init. Introduce a new field
> >>> vtimer_offset.nanoseconds to store this value for arm in struct
> >>> arch_domain, so we can use it directly and extra caclulations can be
> >>> avoided.
> >>>
> >>> This patch is enlightened from [1].
> >>>
> >>> Signed-off-by: Jiamei Xie 
> >>>
> >>> [1] https://www.mail-archive.com/xen-
> >> de...@lists.xenproject.org/msg123139.htm
> >>
> >> This link doesn't work. But I would personally remove it from the commit
> >> message (or add ---) because it doesn't bring value (this patch looks
> >> like a v2 to me).
> > Sorry, a 'l' is missing at the end of the link. The link is 
> > https://www.mail-
> archive.com/xen-devel@lists.xenproject.org/msg123139.html .
> > I will put it after --- in v3.
> >>
> >>> ---
> >>> xen/arch/arm/include/asm/domain.h | 4 
> >>> xen/arch/arm/vtimer.c | 6 --
> >>> 2 files changed, 8 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/xen/arch/arm/include/asm/domain.h
> >> b/xen/arch/arm/include/asm/domain.h
> >>> index ed63c2b6f9..94fe5b6444 100644
> >>> --- a/xen/arch/arm/include/asm/domain.h
> >>> +++ b/xen/arch/arm/include/asm/domain.h
> >>> @@ -73,6 +73,10 @@ struct arch_domain
> >>> uint64_t offset;
> >>> } virt_timer_base;
> >>>
> >>> + struct {
> >>> + int64_t nanoseconds;
> >>
&

RE: [PATCH] xen/arm: avoid extra caclulations when setting vtimer in context switch

2022-06-27 Thread Jiamei Xie

Hi Julien,

> -Original Message-
> From: Julien Grall 
> Sent: 2022年6月27日 18:36
> To: Jiamei Xie ; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Bertrand Marquis
> ; Volodymyr Babchuk
> ; Wei Chen 
> Subject: Re: [PATCH] xen/arm: avoid extra caclulations when setting vtimer
> in context switch
> 
> Hi Jiami
> 
> Title: s/caclulations/calculations/
> 
> However, I think the title should mention the overflow rather than the
> extra calculations. The former is more important the latter.
> 
I will change the title to " xen/arm: avoid overflow when setting vtimer in 
context switch"

> On 27/06/2022 03:58, Jiamei Xie wrote:
> > virt_vtimer_save is calculating the new time for the vtimer in:
> > "v->arch.virt_timer.cval + v->domain->arch.virt_timer_base.offset
> > - boot_count".
> > In this formula, "cval + offset" might cause uint64_t overflow.
> > Changing it to "v->domain->arch.virt_timer_base.offset - boot_count +
> > v->arch.virt_timer.cval" can reduce the possibility of overflow
> 
> This read strange to me. We want to remove the overflow completely not
> reducing it. The overflow is completely removed by converting the
> "offset - bount_count" to ns upfront.
> 
> AFAICT, the commit message doesn't explain that.
Thanks for pointing out that. How about putting the commit message like the 
below:
xen/arm: avoid overflow when setting vtimer in context switch

virt_vtimer_save is calculating the new time for the vtimer in:
"v->arch.virt_timer.cval + v->domain->arch.virt_timer_base.offset
- boot_count".
In this formula, "cval + offset" might cause uint64_t overflow.
Changing it to "ticks_to_ns(v->domain->arch.virt_timer_base.offset -
boot_count) + ticks_to_ns(v->arch.virt_timer.cval)" can avoid overflow,
and "ticks_to_ns(arch.virt_timer_base.offset - boot_count)" will be
always the same, which has been caculated in domain_vtimer_init.
Introduce a new field virt_timer_base.nanoseconds to store this value
for arm in struct arch_domain, so we can use it directly.
> 
> > , and
> > "arch.virt_timer_base.offset - boot_count" will be always the same,
> > which has been caculated in domain_vtimer_init. Introduce a new field
> > vtimer_offset.nanoseconds to store this value for arm in struct
> > arch_domain, so we can use it directly and extra caclulations can be
> > avoided.
> >
> > This patch is enlightened from [1].
> >
> > Signed-off-by: Jiamei Xie 
> >
> > [1] https://www.mail-archive.com/xen-
> de...@lists.xenproject.org/msg123139.htm
> 
> This link doesn't work. But I would personally remove it from the commit
> message (or add ---) because it doesn't bring value (this patch looks
> like a v2 to me).
Sorry, a 'l' is missing at the end of the link.  The link is  
https://www.mail-archive.com/xen-devel@lists.xenproject.org/msg123139.html .
I will put it after --- in v3.
> 
> > ---
> > xen/arch/arm/include/asm/domain.h | 4 
> >   xen/arch/arm/vtimer.c | 6 --
> >   2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/include/asm/domain.h
> b/xen/arch/arm/include/asm/domain.h
> > index ed63c2b6f9..94fe5b6444 100644
> > --- a/xen/arch/arm/include/asm/domain.h
> > +++ b/xen/arch/arm/include/asm/domain.h
> > @@ -73,6 +73,10 @@ struct arch_domain
> >   uint64_t offset;
> >   } virt_timer_base;
> >
> > +struct {
> > +int64_t nanoseconds;
> 
> This should be s_time_t to match the argument of set_timer() and return
> of ticks_to_ns().
> 
> > +} vtimer_offset;
> 
> Why are you adding a new structure rather than re-using virt_timer_base?
Sure, I'll add this field in virt_timer_base.
 struct {
 uint64_t offset;
 s_time_t nanoseconds;
 } virt_timer_base;
> 
> > +
> >   struct vgic_dist vgic;
> >
> >   struct vuart {
> > diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> > index 6b78fea77d..54161e5fea 100644
> > --- a/xen/arch/arm/vtimer.c
> > +++ b/xen/arch/arm/vtimer.c
> > @@ -64,6 +64,7 @@ int domain_vtimer_init(struct domain *d, struct
> xen_arch_domainconfig *config)
> >   {
> >   d->arch.virt_timer_base.offset = get_cycles();
> >   d->time_offset.seconds = ticks_to_ns(d->arch.virt_timer_base.offset -
> boot_count);
> > +d->arch.vtimer_offset.nanoseconds = d->time_offset.seconds;
> 
> Hmmm... I find odd to assign a field "nanosecond

[PATCH] xen/arm: avoid extra caclulations when setting vtimer in context switch

2022-06-26 Thread Jiamei Xie

virt_vtimer_save is calculating the new time for the vtimer in:
"v->arch.virt_timer.cval + v->domain->arch.virt_timer_base.offset
- boot_count".
In this formula, "cval + offset" might cause uint64_t overflow.
Changing it to "v->domain->arch.virt_timer_base.offset - boot_count +
v->arch.virt_timer.cval" can reduce the possibility of overflow, and
"arch.virt_timer_base.offset - boot_count" will be always the same,
which has been caculated in domain_vtimer_init. Introduce a new field
vtimer_offset.nanoseconds to store this value for arm in struct
arch_domain, so we can use it directly and extra caclulations can be
avoided.

This patch is enlightened from [1].

Signed-off-by: Jiamei Xie 

[1] https://www.mail-archive.com/xen-devel@lists.xenproject.org/msg123139.htm
---
xen/arch/arm/include/asm/domain.h | 4 
 xen/arch/arm/vtimer.c | 6 --
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/include/asm/domain.h 
b/xen/arch/arm/include/asm/domain.h
index ed63c2b6f9..94fe5b6444 100644
--- a/xen/arch/arm/include/asm/domain.h
+++ b/xen/arch/arm/include/asm/domain.h
@@ -73,6 +73,10 @@ struct arch_domain
 uint64_t offset;
 } virt_timer_base;
 
+struct {
+int64_t nanoseconds;
+} vtimer_offset;
+
 struct vgic_dist vgic;
 
 struct vuart {
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index 6b78fea77d..54161e5fea 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -64,6 +64,7 @@ int domain_vtimer_init(struct domain *d, struct 
xen_arch_domainconfig *config)
 {
 d->arch.virt_timer_base.offset = get_cycles();
 d->time_offset.seconds = ticks_to_ns(d->arch.virt_timer_base.offset - 
boot_count);
+d->arch.vtimer_offset.nanoseconds = d->time_offset.seconds;
 do_div(d->time_offset.seconds, 10);
 
 config->clock_frequency = timer_dt_clock_frequency;
@@ -144,8 +145,9 @@ void virt_timer_save(struct vcpu *v)
 if ( (v->arch.virt_timer.ctl & CNTx_CTL_ENABLE) &&
  !(v->arch.virt_timer.ctl & CNTx_CTL_MASK))
 {
-set_timer(&v->arch.virt_timer.timer, 
ticks_to_ns(v->arch.virt_timer.cval +
-  v->domain->arch.virt_timer_base.offset - boot_count));
+set_timer(&v->arch.virt_timer.timer,
+  v->domain->arch.vtimer_offset.nanoseconds +
+  ticks_to_ns(v->arch.virt_timer.cval));
 }
 }
 
-- 
2.25.1

RE: [PATCH v5 6/6] byteorder: Remove byteorder

2022-05-23 Thread Jiamei Xie

Hi Lin,
> -Original Message-
> From: Xen-devel  On Behalf Of Lin
> Liu
> Sent: 2022年5月23日 22:51
> To: xen-devel@lists.xenproject.org
> Cc: Lin Liu ; Andrew Cooper
> ; George Dunlap ;
> Jan Beulich ; Julien Grall ; Bertrand
> Marquis ; Stefano Stabellini
> ; Wei Liu 
> Subject: [PATCH v5 6/6] byteorder: Remove byteorder
> 
> include/xen/byteswap.h has simplify the interface, just clean
> the old interface
There is a  typo.   s/ simplify/simplified /.

Best wishes
Jiamei Xie
> 
> No functional change
> 
> Signed-off-by: Lin Liu 
> Reviewed-by: Andrew Cooper 
> ---
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Jan Beulich 
> Cc: Julien Grall 
> Cc: Bertrand Marquis 
> Cc: Stefano Stabellini 
> Cc: Wei Liu 
> ---
>  xen/include/xen/byteorder/big_endian.h| 102 
>  xen/include/xen/byteorder/generic.h   |  68 
>  xen/include/xen/byteorder/little_endian.h | 102 
>  xen/include/xen/byteorder/swab.h  | 183 --
>  4 files changed, 455 deletions(-)
>  delete mode 100644 xen/include/xen/byteorder/big_endian.h
>  delete mode 100644 xen/include/xen/byteorder/generic.h
>  delete mode 100644 xen/include/xen/byteorder/little_endian.h
>  delete mode 100644 xen/include/xen/byteorder/swab.h
> 
> diff --git a/xen/include/xen/byteorder/big_endian.h
> b/xen/include/xen/byteorder/big_endian.h
> deleted file mode 100644
> index 40eb80a390..00
> --- a/xen/include/xen/byteorder/big_endian.h
> +++ /dev/null
> @@ -1,102 +0,0 @@
> -#ifndef __XEN_BYTEORDER_BIG_ENDIAN_H__
> -#define __XEN_BYTEORDER_BIG_ENDIAN_H__
> -
> -#ifndef __BIG_ENDIAN
> -#define __BIG_ENDIAN 4321
> -#endif
> -#ifndef __BIG_ENDIAN_BITFIELD
> -#define __BIG_ENDIAN_BITFIELD
> -#endif
> -
> -#include 
> -#include 
> -
> -#define __constant_cpu_to_le64(x) ((__force
> __le64)___constant_swab64((x)))
> -#define __constant_le64_to_cpu(x) ___constant_swab64((__force
> __u64)(__le64)(x))
> -#define __constant_cpu_to_le32(x) ((__force
> __le32)___constant_swab32((x)))
> -#define __constant_le32_to_cpu(x) ___constant_swab32((__force
> __u32)(__le32)(x))
> -#define __constant_cpu_to_le16(x) ((__force
> __le16)___constant_swab16((x)))
> -#define __constant_le16_to_cpu(x) ___constant_swab16((__force
> __u16)(__le16)(x))
> -#define __constant_cpu_to_be64(x) ((__force __be64)(__u64)(x))
> -#define __constant_be64_to_cpu(x) ((__force __u64)(__be64)(x))
> -#define __constant_cpu_to_be32(x) ((__force __be32)(__u32)(x))
> -#define __constant_be32_to_cpu(x) ((__force __u32)(__be32)(x))
> -#define __constant_cpu_to_be16(x) ((__force __be16)(__u16)(x))
> -#define __constant_be16_to_cpu(x) ((__force __u16)(__be16)(x))
> -#define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
> -#define __le64_to_cpu(x) __swab64((__force __u64)(__le64)(x))
> -#define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
> -#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
> -#define __cpu_to_le16(x) ((__force __le16)__swab16((x)))
> -#define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
> -#define __cpu_to_be64(x) ((__force __be64)(__u64)(x))
> -#define __be64_to_cpu(x) ((__force __u64)(__be64)(x))
> -#define __cpu_to_be32(x) ((__force __be32)(__u32)(x))
> -#define __be32_to_cpu(x) ((__force __u32)(__be32)(x))
> -#define __cpu_to_be16(x) ((__force __be16)(__u16)(x))
> -#define __be16_to_cpu(x) ((__force __u16)(__be16)(x))
> -
> -static inline __le64 __cpu_to_le64p(const __u64 *p)
> -{
> -return (__force __le64)__swab64p(p);
> -}
> -static inline __u64 __le64_to_cpup(const __le64 *p)
> -{
> -return __swab64p((__u64 *)p);
> -}
> -static inline __le32 __cpu_to_le32p(const __u32 *p)
> -{
> -return (__force __le32)__swab32p(p);
> -}
> -static inline __u32 __le32_to_cpup(const __le32 *p)
> -{
> -return __swab32p((__u32 *)p);
> -}
> -static inline __le16 __cpu_to_le16p(const __u16 *p)
> -{
> -return (__force __le16)__swab16p(p);
> -}
> -static inline __u16 __le16_to_cpup(const __le16 *p)
> -{
> -return __swab16p((__u16 *)p);
> -}
> -static inline __be64 __cpu_to_be64p(const __u64 *p)
> -{
> -return (__force __be64)*p;
> -}
> -static inline __u64 __be64_to_cpup(const __be64 *p)
> -{
> -return (__force __u64)*p;
> -}
> -static inline __be32 __cpu_to_be32p(const __u32 *p)
> -{
> -return (__force __be32)*p;
> -}
> -static inline __u32 __be32_to_cpup(const __be32 *p)
> -{
> -return (__force __u32)*p;
> -}
> -static inline __be16 __cpu_to_be16p(const __u16 *p)
> -{
> -return (__force __be16)*p;
> -}
> -static inline __u16 __be16_to_cpup(const __be16 *p)
> -{
> -return (

RE: [PATCH V7 2/2] libxl: Introduce basic virtio-mmio support on Arm

2022-04-21 Thread Jiamei Xie

Hi Oleksandr,

 I am happy to keep my T-b tag.  I have tested this latest patch series and it 
works. 

Regards,
Jiamei Xie

> -Original Message-
> From: Xen-devel  On Behalf Of
> Oleksandr Tyshchenko
> Sent: 2022年4月9日 2:21
> To: xen-devel@lists.xenproject.org
> Cc: Julien Grall ; Wei Liu ; Anthony
> PERARD ; Juergen Gross ;
> Stefano Stabellini ; Julien Grall ;
> Bertrand Marquis ; Volodymyr Babchuk
> ; Jiamei Xie ;
> Henry Wang ; Oleksandr Tyshchenko
> 
> Subject: [PATCH V7 2/2] libxl: Introduce basic virtio-mmio support on Arm
> 
> From: Julien Grall 
> 
> This patch introduces helpers to allocate Virtio MMIO params
> (IRQ and memory region) and create specific device node in
> the Guest device-tree with allocated params. In order to deal
> with multiple Virtio devices, reserve corresponding ranges.
> For now, we reserve 1MB for memory regions and 10 SPIs.
> 
> As these helpers should be used for every Virtio device attached
> to the Guest, call them for Virtio disk(s).
> 
> Please note, with statically allocated Virtio IRQs there is
> a risk of a clash with a physical IRQs of passthrough devices.
> For the first version, it's fine, but we should consider allocating
> the Virtio IRQs automatically. Thankfully, we know in advance which
> IRQs will be used for passthrough to be able to choose non-clashed
> ones.
> 
> Signed-off-by: Julien Grall 
> Signed-off-by: Oleksandr Tyshchenko 
> Tested-by: Jiamei Xie 
> Reviewed-by: Henry Wang 
> ---
> @Jiamei, @Henry I decided to leave your T-b and R-b tags with the minor
> change I made, are you still happy with that?
> 
> s/if (disk->virtio)/if (disk->protocol == LIBXL_DISK_PROTOCOL_VIRTIO_MMIO)
> 
> Please note, this is a split/cleanup/hardening of Julien's PoC:
> "Add support for Guest IO forwarding to a device emulator"
> 
> Changes RFC -> V1:
>- was squashed with:
>  "[RFC PATCH V1 09/12] libxl: Handle virtio-mmio irq in more correct way"
>  "[RFC PATCH V1 11/12] libxl: Insert "dma-coherent" property into virtio-
> mmio device node"
>  "[RFC PATCH V1 12/12] libxl: Fix duplicate memory node in DT"
>- move VirtIO MMIO #define-s to xen/include/public/arch-arm.h
> 
> Changes V1 -> V2:
>- update the author of a patch
> 
> Changes V2 -> V3:
>- no changes
> 
> Changes V3 -> V4:
>- no changes
> 
> Changes V4 -> V5:
>- split the changes, change the order of the patches
>- drop an extra "virtio" configuration option
>- update patch description
>- use CONTAINER_OF instead of own implementation
>- reserve ranges for Virtio MMIO params and put them
>  in correct location
>- create helpers to allocate Virtio MMIO params, add
>  corresponding sanity-сhecks
>- add comment why MMIO size 0x200 is chosen
>- update debug print
>- drop Wei's T-b
> 
> Changes V5 -> V6:
>- rebase on current staging
> 
> Changes V6 -> V7:
>- rebase on current staging
>- add T-b and R-b tags
>- update according to the recent changes to
>  "libxl: Add support for Virtio disk configuration"
> ---
>  tools/libs/light/libxl_arm.c  | 131
> +-
>  xen/include/public/arch-arm.h |   7 +++
>  2 files changed, 136 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index eef1de0..8132a47 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -8,6 +8,56 @@
>  #include 
>  #include 
> 
> +/*
> + * There is no clear requirements for the total size of Virtio MMIO region.
> + * The size of control registers is 0x100 and device-specific configuration
> + * registers starts at the offset 0x100, however it's size depends on the
> device
> + * and the driver. Pick the biggest known size at the moment to cover most
> + * of the devices (also consider allowing the user to configure the size via
> + * config file for the one not conforming with the proposed value).
> + */
> +#define VIRTIO_MMIO_DEV_SIZE   xen_mk_ullong(0x200)
> +
> +static uint64_t virtio_mmio_base;
> +static uint32_t virtio_mmio_irq;
> +
> +static void init_virtio_mmio_params(void)
> +{
> +virtio_mmio_base = GUEST_VIRTIO_MMIO_BASE;
> +virtio_mmio_irq = GUEST_VIRTIO_MMIO_SPI_FIRST;
> +}
> +
> +static uint64_t alloc_virtio_mmio_base(libxl__gc *gc)
> +{
> +uint64_t base = virtio_mmio_base;
> +
> +/* Make sure we have enough reserved resources */
> +if ((virtio_mmio_base + VIRTIO_MMIO_DEV_SIZE >
> +GUEST_VIRT

RE: [PATCH v2 01/10] xen/arm: Print a 64-bit number in hex from early uart

2022-04-19 Thread Jiamei Xie

Hi Wei,

> -Original Message-
> From: Xen-devel  On Behalf Of
> Wei Chen
> Sent: 2022年4月18日 17:07
> To: --to=xen-devel@lists.xenproject.org; xen-devel@lists.xenproject.org
> Cc: nd ; Wei Chen ; Stefano Stabellini
> ; Julien Grall ; Bertrand Marquis
> ; Volodymyr Babchuk
> ; Julien Grall 
> Subject: [PATCH v2 01/10] xen/arm: Print a 64-bit number in hex from early
> uart
> 
> Current putn function that is using for early print
> only can print low 32-bit of AArch64 register. This
> will lose some important messages while debugging
> with early console. For example:
> (XEN) Bringing up CPU5
> - CPU 00010100 booting -
> Will be truncated to
> (XEN) Bringing up CPU5
> - CPU 0100 booting -
> 
> In this patch, we increased the print loops and shift
> bits to make putn print 64-bit number.
> 
> Signed-off-by: Wei Chen 
> Acked-by: Julien Grall 
> ---
>  xen/arch/arm/arm64/head.S | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index e62c48ec1c..2bb7906f69 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -866,17 +866,19 @@ puts:
>  ret
>  ENDPROC(puts)
> 
> -/* Print a 32-bit number in hex.  Specific to the PL011 UART.
> +/*
> + * Print a 64-bit number in hex.
>   * x0: Number to print.
>   * x23: Early UART base address
> - * Clobbers x0-x3 */
> + * Clobbers x0-x3
> + */
>  putn:
>  adr   x1, hex
> -mov   x3, #8
> +mov   x3, #16
>  1:
>  early_uart_ready x23, 2
> -and   x2, x0, #0xf000/* Mask off the top nybble */
> -lsr   x2, x2, #28
> +and   x2, x0, #(0xf<<60) /* Mask off the top nybble */
> +lsr   x2, x2, #60
>  ldrb  w2, [x1, x2]   /* Convert to a char */
>  early_uart_transmit x23, w2
>  lsl   x0, x0, #4 /* Roll it through one nybble at a time 
> */
> --
> 2.25.1
> 

I have tested the whole patch series on Armv8A(config without NUMA) and 
X86(config with NUMA), both can enter Dom0 successfully and the X86 NUMA works 
normally.

Tested-by: Jiamei Xie  

Regards,
Jiamei Xie

RE: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions

2022-03-17 Thread Jiamei Xie




> -Original Message-
> From: Xen-devel  On Behalf Of
> Jiamei Xie
> Sent: 2022年3月17日 17:17
> To: Ross Lagerwall ; Bjoern Doebel
> ; xen-devel@lists.xenproject.org
> Cc: Michael Kurth ; Martin Pohlack
> ; Roger Pau Monne ;
> Andrew Cooper ; Konrad Rzeszutek Wilk
> 
> Subject: RE: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
> enhanced functions
> 
> Hi  Bjoern,
> 
> > -Original Message-
> > From: Xen-devel  On Behalf Of
> > Ross Lagerwall
> > Sent: 2022年3月10日 1:12
> > To: Bjoern Doebel ; xen-devel@lists.xenproject.org
> > Cc: Michael Kurth ; Martin Pohlack
> > ; Roger Pau Monne ;
> > Andrew Cooper ; Konrad Rzeszutek Wilk
> > 
> > Subject: Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
> > enhanced functions
> >
> > > From: Bjoern Doebel 
> > > Sent: Wednesday, March 9, 2022 2:53 PM
> > > To: xen-devel@lists.xenproject.org 
> > > Cc: Michael Kurth ; Martin Pohlack
> > ; Roger Pau Monne ;
> > Andrew Cooper ; Bjoern Doebel
> > ; Konrad Rzeszutek Wilk ;
> > Ross Lagerwall 
> > > Subject: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
> > enhanced functions
> > >
> > > Xen enabled CET for supporting architectures. The control flow aspect of
> > > CET expects functions that can be called indirectly (i.e., via function
> > > pointers) to start with an ENDBR64 instruction. Otherwise a control flow
> > > exception is raised.
> > >
> > > This expectation breaks livepatching flows because we patch functions by
> > > overwriting their first 5 bytes with a JMP + , thus breaking the
> > > ENDBR64. We fix this by checking the start of a patched function for
> > > being ENDBR64. In the positive case we move the livepatch JMP to start
> > > behind the ENDBR64 instruction.
> > >
> > > To avoid having to guess the ENDBR64 offset again on patch reversal
> > > (which might race with other mechanisms adding/removing ENDBR
> > > dynamically), use the livepatch metadata to store the computed offset
> > > along with the saved bytes of the overwritten function.
> > >
> > > Signed-off-by: Bjoern Doebel 
> > > Acked-by: Konrad Rzeszutek Wilk 
> > > CC: Ross Lagerwall 
> >
> > Reviewed-by: Ross Lagerwall 
> 
> Tested-by: Jiamei xie 
> 
> Cheers,
> Jiamei
Sorry I forgot to add the scope I tested in last email. I tested it on armv8a. 
It worked fine and  didn't break arm.
Tested-by: Jiamei xie 
> Cheers,
> Jiamei

RE: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-enhanced functions

2022-03-17 Thread Jiamei Xie

Hi  Bjoern,

> -Original Message-
> From: Xen-devel  On Behalf Of
> Ross Lagerwall
> Sent: 2022年3月10日 1:12
> To: Bjoern Doebel ; xen-devel@lists.xenproject.org
> Cc: Michael Kurth ; Martin Pohlack
> ; Roger Pau Monne ;
> Andrew Cooper ; Konrad Rzeszutek Wilk
> 
> Subject: Re: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
> enhanced functions
> 
> > From: Bjoern Doebel 
> > Sent: Wednesday, March 9, 2022 2:53 PM
> > To: xen-devel@lists.xenproject.org 
> > Cc: Michael Kurth ; Martin Pohlack
> ; Roger Pau Monne ;
> Andrew Cooper ; Bjoern Doebel
> ; Konrad Rzeszutek Wilk ;
> Ross Lagerwall 
> > Subject: [PATCH v5 2/2] xen/x86: Livepatch: support patching CET-
> enhanced functions
> >
> > Xen enabled CET for supporting architectures. The control flow aspect of
> > CET expects functions that can be called indirectly (i.e., via function
> > pointers) to start with an ENDBR64 instruction. Otherwise a control flow
> > exception is raised.
> >
> > This expectation breaks livepatching flows because we patch functions by
> > overwriting their first 5 bytes with a JMP + , thus breaking the
> > ENDBR64. We fix this by checking the start of a patched function for
> > being ENDBR64. In the positive case we move the livepatch JMP to start
> > behind the ENDBR64 instruction.
> >
> > To avoid having to guess the ENDBR64 offset again on patch reversal
> > (which might race with other mechanisms adding/removing ENDBR
> > dynamically), use the livepatch metadata to store the computed offset
> > along with the saved bytes of the overwritten function.
> >
> > Signed-off-by: Bjoern Doebel 
> > Acked-by: Konrad Rzeszutek Wilk 
> > CC: Ross Lagerwall 
> 
> Reviewed-by: Ross Lagerwall 

Tested-by: Jiamei xie 

Cheers, 
Jiamei

RE: [XEN PATCH v3] xen/arm: introduce dummy iommu node for dom0

2022-01-14 Thread Jiamei Xie




> -Original Message-
> From: Xen-devel  On Behalf Of
> Rahul Singh
> Sent: 2022年1月14日 16:22
> To: Sergiy Kibrik 
> Cc: xen-devel ; Stefano Stabellini
> ; Julien Grall ; Oleksandr
> Tyshchenko ; Andrii Anisov
> 
> Subject: Re: [XEN PATCH v3] xen/arm: introduce dummy iommu node for
> dom0
> 
> Hi,
> 
> > On 11 Jan 2022, at 11:26 am, Sergiy Kibrik 
> wrote:
> >
> > Currently no IOMMU properties are exposed to dom0, thus kernel by
> default
> > assumes no protection and enables swiotlb-xen, which leads to costly and
> > unnecessary buffers bouncing.
> >
> > To let kernel know which device is behing IOMMU and hence needs no
[Jiamei Xie] 
behing?  Typo

Best wishes
Jiamei Xie


> swiotlb
> > services we introduce dummy xen-iommu node in FDT and link protected
> device
> > nodes to it, using here device tree iommu bindings.
> >
> > Signed-off-by: Sergiy Kibrik 
> 
> Reviewed-by: Rahul Singh 
> 
> Regards,
> Rahul
> > ---
> > Cc: Stefano Stabellini 
> > Cc: Julien Grall 
> > Cc: Oleksandr Tyshchenko 
> > Cc: Andrii Anisov 
> >
> >
> > Changelog:
> >
> > v3: rebased over staging & remove redundand phandle_iommu attribute,
> discussion:
> > https://lists.xenproject.org/archives/html/xen-devel/2021-
> 12/msg01753.html
> >
> > v2: re-use common iommu dt bindings to let guests know which devices
> are protected:
> > https://lists.xenproject.org/archives/html/xen-devel/2021-
> 10/msg00073.html
> >
> > xen/arch/arm/domain_build.c   | 42
> +++
> > xen/include/public/device_tree_defs.h |  1 +
> > 2 files changed, 43 insertions(+)
> >
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index 6931c022a2..b82ba72fac 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -845,6 +845,12 @@ static int __init write_properties(struct domain *d,
> struct kernel_info *kinfo,
> > }
> > }
> >
> > +if ( iommu_node && is_iommu_enabled(d) &&
> dt_device_is_protected(node) )
> > +{
> > +res = fdt_property_cell(kinfo->fdt, "iommus",
> GUEST_PHANDLE_IOMMU);
> > +if ( res )
> > +return res;
> > +}
> > return 0;
> > }
> >
> > @@ -1479,6 +1485,38 @@ static int __init make_cpus_node(const struct
> domain *d, void *fdt)
> > return res;
> > }
> >
> > +static int __init make_iommu_node(const struct domain *d,
> > +  const struct kernel_info *kinfo)
> > +{
> > +const char compat[] = "xen,iommu-el2-v1";
> > +int res;
> > +
> > +if ( !is_iommu_enabled(d) )
> > +return 0;
> > +
> > +dt_dprintk("Create iommu node\n");
> > +
> > +res = fdt_begin_node(kinfo->fdt, "xen-iommu");
> > +if ( res )
> > +return res;
> > +
> > +res = fdt_property(kinfo->fdt, "compatible", compat, sizeof(compat));
> > +if ( res )
> > +return res;
> > +
> > +res = fdt_property_cell(kinfo->fdt, "#iommu-cells", 0);
> > +if ( res )
> > +return res;
> > +
> > +res = fdt_property_cell(kinfo->fdt, "phandle",
> GUEST_PHANDLE_IOMMU);
> > +
> > +res = fdt_end_node(kinfo->fdt);
> > +if ( res )
> > +return res;
> > +
> > +return res;
> > +}
> > +
> > static int __init make_gic_node(const struct domain *d, void *fdt,
> > const struct dt_device_node *node)
> > {
> > @@ -2127,6 +2165,10 @@ static int __init handle_node(struct domain *d,
> struct kernel_info *kinfo,
> > if ( res )
> > return res;
> >
> > +res = make_iommu_node(d, kinfo);
> > +if ( res )
> > +return res;
> > +
> > res = make_memory_node(d, kinfo->fdt, addrcells, sizecells, &kinfo-
> >mem);
> > if ( res )
> > return res;
> > diff --git a/xen/include/public/device_tree_defs.h
> b/xen/include/public/device_tree_defs.h
> > index 209d43de3f..df58944bd0 100644
> > --- a/xen/include/public/device_tree_defs.h
> > +++ b/xen/include/public/device_tree_defs.h
> > @@ -7,6 +7,7 @@
> >  * onwards. Reserve a high value for the GIC phandle.
> >  */
> > #define GUEST_PHANDLE_GIC (65000)
> > +#define GUEST_PHANDLE_IOMMU (GUEST_PHANDLE_GIC + 1)
> >
> > #define GUEST_ROOT_ADDRESS_CELLS 2
> > #define GUEST_ROOT_SIZE_CELLS 2
> > --
> > 2.25.1
> >
> >
>

RE: [PATCH V6 1/2] libxl: Add support for Virtio disk configuration

2021-12-08 Thread Jiamei Xie




> -Original Message-
> From: Xen-devel  On Behalf Of
> Oleksandr Tyshchenko
> Sent: 2021年12月9日 1:00
> To: xen-devel@lists.xenproject.org
> Cc: Oleksandr Tyshchenko ; Wei Liu
> ; George Dunlap ; Nick Rosbrook
> ; Anthony PERARD ;
> Juergen Gross ; Stefano Stabellini
> ; Julien Grall ; Volodymyr Babchuk
> ; Bertrand Marquis
> 
> Subject: [PATCH V6 1/2] libxl: Add support for Virtio disk configuration
> 
> From: Oleksandr Tyshchenko 
> 
> This patch adds basic support for configuring and assisting virtio-disk
> backend (emualator) which is intended to run out of Qemu and could be
> run in any domain.
> Although the Virtio block device is quite different from traditional
> Xen PV block device (vbd) from the toolstack point of view:
>  - as the frontend is virtio-blk which is not a Xenbus driver, nothing
>written to Xenstore are fetched by the frontend (the vdev is not
>passed to the frontend)
>  - the ring-ref/event-channel are not used for the backend<->frontend
>communication, the proposed IPC for Virtio is IOREQ/DM
> it is still a "block device" and ought to be integrated in existing
> "disk" handling. So, re-use (and adapt) "disk" parsing/configuration
> logic to deal with Virtio devices as well.
> 
> Besides introducing new disk backend type (LIBXL_DISK_BACKEND_VIRTIO)
> introduce new device kind (LIBXL__DEVICE_KIND_VIRTIO_DISK) as current
> one (LIBXL__DEVICE_KIND_VBD) doesn't fit into Virtio disk model.
> 
> In order to inform the toolstack that Virtio disk needs to be used
> extend "disk" configuration by introducing new "virtio" flag.
> An example of domain configuration:
> disk = [ 'backend=DomD, phy:/dev/mmcblk1p3, xvda1, rw, virtio' ]
> 
> Please note, this patch is not enough for virtio-disk to work
> on Xen (Arm), as for every Virtio device (including disk) we need
> to allocate Virtio MMIO params (IRQ and memory region) and pass
> them to the backend, also update Guest device-tree. The subsequent
> patch will add these missing bits. For the current patch,
> the default "irq" and "base" are just written to the Xenstore.
> 
> Signed-off-by: Oleksandr Tyshchenko 
> 
> ---
> Changes RFC -> V1:
>- no changes
> 
> Changes V1 -> V2:
>- rebase according to the new location of libxl_virtio_disk.c
> 
> Changes V2 -> V3:
>- no changes
> 
> Changes V3 -> V4:
>- rebase according to the new argument for DEFINE_DEVICE_TYPE_STRUCT
> 
> Changes V4 -> V5:
>- split the changes, change the order of the patches
>- update patch description
>- don't introduce new "vdisk" configuration option with own parsing logic,
>  re-use Xen PV block "disk" parsing/configuration logic for the 
> virtio-disk
>- introduce "virtio" flag and document it's usage
>- add LIBXL_HAVE_DEVICE_DISK_VIRTIO
>- update libxlu_disk_l.[ch]
>- drop num_disks variable/MAX_VIRTIO_DISKS
>- drop Wei's T-b
> 
> Changes V5 -> V6:
>- rebase on current staging
>- use "%"PRIu64 instead of %lu for disk->base in device_disk_add()
>- update *.gen.go files
> ---
>  docs/man/xl-disk-configuration.5.pod.in   |  27 +
>  tools/golang/xenlight/helpers.gen.go  |   6 +
>  tools/golang/xenlight/types.gen.go|   4 +
>  tools/include/libxl.h |   6 +
>  tools/libs/light/libxl_device.c   |  38 +-
>  tools/libs/light/libxl_disk.c |  99 +++-
>  tools/libs/light/libxl_types.idl  |   4 +
>  tools/libs/light/libxl_types_internal.idl |   1 +
>  tools/libs/light/libxl_utils.c|   2 +
>  tools/libs/util/libxlu_disk_l.c   | 881 
> +++---
>  tools/libs/util/libxlu_disk_l.h   |   2 +-
>  tools/libs/util/libxlu_disk_l.l   |   1 +
>  tools/xl/xl_block.c   |  11 +
>  13 files changed, 636 insertions(+), 446 deletions(-)
> 
> diff --git a/docs/man/xl-disk-configuration.5.pod.in b/docs/man/xl-disk-
> configuration.5.pod.in
> index 71d0e86..9cc189f 100644
> --- a/docs/man/xl-disk-configuration.5.pod.in
> +++ b/docs/man/xl-disk-configuration.5.pod.in
> @@ -344,8 +344,35 @@ can be used to disable "hole punching" for file
> based backends which
>  were intentionally created non-sparse to avoid fragmentation of the
>  file.
> 
> +=item B
> +
> +=over 4
> +
> +=item Description
> +
> +Enables experimental Virtio support for disk
> +
> +=item Supported values
> +
> +absent, present
> +
> +=item Mandatory
> +
> +No
> +
> +=item Default value
> +
> +absent
> +
>  =back
> 
> +Besides forcing toolstack to use specific Xen Virtio backend implementation
> +(for example, virtio-disk), this also affects the guest's view of the device
> +and requires virtio-blk driver to be used.
> +Please note, the virtual device (vdev) is not passed to the guest in that 
> case,
> +but it still must be specified for the internal purposes.
> +
> +=back
> 
>  =head1 COLO Parameters
> 
> diff --git a/tools/golang/xenlight/helpers.gen.go
> b/tools/golang/xenlight/helpers.gen.go
> index b746ff1..787e3ba

RE: [PATCH V6 2/2] libxl: Introduce basic virtio-mmio support on Arm

2021-12-08 Thread Jiamei Xie

gt; +virtio_mmio_irq);
> +return 0;
> +}
> +virtio_mmio_irq++;
> +
> +return irq;
> +}
> +
>  static const char *gicv_to_string(libxl_gic_version gic_version)
>  {
>  switch (gic_version) {
> @@ -26,8 +76,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>  {
>  uint32_t nr_spis = 0;
>  unsigned int i;
> -uint32_t vuart_irq;
> -bool vuart_enabled = false;
> +uint32_t vuart_irq, virtio_irq = 0;
> +bool vuart_enabled = false, virtio_enabled = false;
> 
>  /*
>   * If pl011 vuart is enabled then increment the nr_spis to allow 
> allocation
> @@ -39,6 +89,35 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>  vuart_enabled = true;
>  }
> 
> +/*
> + * Virtio MMIO params are non-unique across the whole system and
> must be
> + * initialized for every new guest.
> + */
> +init_virtio_mmio_params();
> +for (i = 0; i < d_config->num_disks; i++) {
> +libxl_device_disk *disk = &d_config->disks[i];
> +
> +if (disk->virtio) {
> +disk->base = alloc_virtio_mmio_base(gc);
> +if (!disk->base)
> +return ERROR_FAIL;
> +
> +disk->irq = alloc_virtio_mmio_irq(gc);
> +if (!disk->irq)
> +return ERROR_FAIL;
> +
> +if (virtio_irq < disk->irq)
> +virtio_irq = disk->irq;
> +virtio_enabled = true;
> +
> +LOG(DEBUG, "Allocate Virtio MMIO params for Vdev %s: IRQ %u
> BASE 0x%"PRIx64,
> +disk->vdev, disk->irq, disk->base);
> +}
> +}
> +
> +if (virtio_enabled)
> +nr_spis += (virtio_irq - 32) + 1;
> +
>  for (i = 0; i < d_config->b_info.num_irqs; i++) {
>  uint32_t irq = d_config->b_info.irqs[i];
>  uint32_t spi;
> @@ -58,6 +137,13 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>  return ERROR_FAIL;
>  }
> 
> +/* The same check as for vpl011 */
> +if (virtio_enabled &&
> +   (irq >= GUEST_VIRTIO_MMIO_SPI_FIRST && irq <= virtio_irq)) {
> +LOG(ERROR, "Physical IRQ %u conflicting with Virtio MMIO IRQ
> range\n", irq);
> +return ERROR_FAIL;
> +}
> +
>  if (irq < 32)
>  continue;
> 
> @@ -787,6 +873,39 @@ static int make_vpci_node(libxl__gc *gc, void *fdt,
>  return 0;
>  }
> 
> +
> +static int make_virtio_mmio_node(libxl__gc *gc, void *fdt,
> + uint64_t base, uint32_t irq)
> +{
> +int res;
> +gic_interrupt intr;
> +/* Placeholder for virtio@ + a 64-bit number + \0 */
> +char buf[24];
> +
> +snprintf(buf, sizeof(buf), "virtio@%"PRIx64, base);
> +res = fdt_begin_node(fdt, buf);
> +if (res) return res;
> +
> +res = fdt_property_compat(gc, fdt, 1, "virtio,mmio");
> +if (res) return res;
> +
> +res = fdt_property_regs(gc, fdt, GUEST_ROOT_ADDRESS_CELLS,
> GUEST_ROOT_SIZE_CELLS,
> +1, base, VIRTIO_MMIO_DEV_SIZE);
> +if (res) return res;
> +
> +set_interrupt(intr, irq, 0xf, DT_IRQ_TYPE_EDGE_RISING);
> +res = fdt_property_interrupts(gc, fdt, &intr, 1);
> +if (res) return res;
> +
> +res = fdt_property(fdt, "dma-coherent", NULL, 0);
> +if (res) return res;
> +
> +res = fdt_end_node(fdt);
> +if (res) return res;
> +
> +return 0;
> +}
> +
>  static const struct arch_info *get_arch_info(libxl__gc *gc,
>   const struct xc_dom_image *dom)
>  {
> @@ -988,6 +1107,7 @@ static int libxl__prepare_dtb(libxl__gc *gc,
> libxl_domain_config *d_config,
>  size_t fdt_size = 0;
>  int pfdt_size = 0;
>  libxl_domain_build_info *const info = &d_config->b_info;
> +unsigned int i;
> 
>  const libxl_version_info *vers;
>  const struct arch_info *ainfo;
> @@ -1094,6 +1214,13 @@ next_resize:
>  if (d_config->num_pcidevs)
>  FDT( make_vpci_node(gc, fdt, ainfo, dom) );
> 
> +for (i = 0; i < d_config->num_disks; i++) {
> +libxl_device_disk *disk = &d_config->disks[i];
> +
> +if (disk->virtio)
> +        FDT( make_virtio_mmio_node(gc, fdt, disk->base, disk->irq) );
> +}
> +
>  if (pfdt)
>  FDT( copy_partial_fdt(gc, fdt, pfdt) );
> 
> diff --git a/xen/include/public/arch-arm.h b/xen/include/public/arch-arm.h
> index 94b3151..6dc55df 100644
> --- a/xen/include/public/arch-arm.h
> +++ b/xen/include/public/arch-arm.h
> @@ -398,6 +398,10 @@ typedef uint64_t xen_callback_t;
> 
>  /* Physical Address Space */
> 
> +/* Virtio MMIO mappings */
> +#define GUEST_VIRTIO_MMIO_BASE   xen_mk_ullong(0x0200)
> +#define GUEST_VIRTIO_MMIO_SIZE   xen_mk_ullong(0x0010)
> +
>  /*
>   * vGIC mappings: Only one set of mapping is used by the guest.
>   * Therefore they can overlap.
> @@ -484,6 +488,9 @@ typedef uint64_t xen_callback_t;
> 
>  #define GUEST_VPL011_SPI32
> 
> +#define GUEST_VIRTIO_MMIO_SPI_FIRST   33
> +#define GUEST_VIRTIO_MMIO_SPI_LAST43
> +
>  /* PSCI functions */
>  #define PSCI_cpu_suspend 0
>  #define PSCI_cpu_off 1
> --
> 2.7.4
> 

[Jiamei Xie] 
It works fine as before.

Tested-by: Jiamei Xie 

Best wishes
Jiamei Xie

RE: [PATCH v7 1/7] xen/arm: rename DEVICE_PCI to DEVICE_PCI_HOSTBRIDGE

2021-11-24 Thread Jiamei Xie




> -Original Message-
> From: Xen-devel  On Behalf Of
> Oleksandr Andrushchenko
> Sent: 2021年11月24日 16:00
> To: xen-devel@lists.xenproject.org
> Cc: jul...@xen.org; sstabell...@kernel.org;
> oleksandr_tyshche...@epam.com; volodymyr_babc...@epam.com;
> artem_myga...@epam.com; roger@citrix.com; jbeul...@suse.com;
> andrew.coop...@citrix.com; george.dun...@citrix.com; p...@xen.org;
> Bertrand Marquis ; Rahul Singh
> ; Oleksandr Andrushchenko
> ; Julien Grall 
> Subject: [PATCH v7 1/7] xen/arm: rename DEVICE_PCI to
> DEVICE_PCI_HOSTBRIDGE
> 
> From: Oleksandr Andrushchenko 
> 
> To better reflect the nature of the device type and not to make any
> confusion rename DEVICE_PCI to DEVICE_PCI_HOSTBRIDGE which it
> really is.
> 
> Suggested-by: Julien Grall 
> Signed-off-by: Oleksandr Andrushchenko
> 
> Reviewed-by: Julien Grall 
> ---
> New in v6
> ---
>  xen/arch/arm/pci/pci-host-generic.c | 2 +-
>  xen/arch/arm/pci/pci-host-zynqmp.c  | 2 +-
>  xen/arch/arm/pci/pci.c  | 2 +-
>  xen/include/asm-arm/device.h| 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/pci/pci-host-generic.c b/xen/arch/arm/pci/pci-
> host-generic.c
> index 33457fbe9615..46de6e43cc72 100644
> --- a/xen/arch/arm/pci/pci-host-generic.c
> +++ b/xen/arch/arm/pci/pci-host-generic.c
> @@ -32,7 +32,7 @@ static int __init pci_host_generic_probe(struct
> dt_device_node *dev,
>  return pci_host_common_probe(dev, &pci_generic_ecam_ops);
>  }
> 
> -DT_DEVICE_START(pci_gen, "PCI HOST GENERIC", DEVICE_PCI)
> +DT_DEVICE_START(pci_gen, "PCI HOST GENERIC", DEVICE_PCI_HOSTBRIDGE)
>  .dt_match = gen_pci_dt_match,
>  .init = pci_host_generic_probe,
>  DT_DEVICE_END
> diff --git a/xen/arch/arm/pci/pci-host-zynqmp.c b/xen/arch/arm/pci/pci-
> host-zynqmp.c
> index 61a9807d3d58..516982bca833 100644
> --- a/xen/arch/arm/pci/pci-host-zynqmp.c
> +++ b/xen/arch/arm/pci/pci-host-zynqmp.c
> @@ -49,7 +49,7 @@ static int __init pci_host_generic_probe(struct
> dt_device_node *dev,
>  return pci_host_common_probe(dev, &nwl_pcie_ops);
>  }
> 
> -DT_DEVICE_START(pci_gen, "PCI HOST ZYNQMP", DEVICE_PCI)
> +DT_DEVICE_START(pci_gen, "PCI HOST ZYNQMP", DEVICE_PCI_HOSTBRIDGE)
>  .dt_match = nwl_pcie_dt_match,
>  .init = pci_host_generic_probe,
>  DT_DEVICE_END
> diff --git a/xen/arch/arm/pci/pci.c b/xen/arch/arm/pci/pci.c
> index 082c14e127a8..78b97beaef12 100644
> --- a/xen/arch/arm/pci/pci.c
> +++ b/xen/arch/arm/pci/pci.c
> @@ -46,7 +46,7 @@ static int __init dt_pci_init(void)
> 
>  dt_for_each_device_node(dt_host, np)
>  {
> -rc = device_init(np, DEVICE_PCI, NULL);
> +rc = device_init(np, DEVICE_PCI_HOSTBRIDGE, NULL);
>  /*
>   * Ignore the following error codes:
>   *   - EBADF: Indicate the current device is not a pci device.
> diff --git a/xen/include/asm-arm/device.h b/xen/include/asm-arm/device.h
> index 3782660751b6..086dde13eb6b 100644
> --- a/xen/include/asm-arm/device.h
> +++ b/xen/include/asm-arm/device.h
> @@ -37,7 +37,7 @@ enum device_class
>  DEVICE_SERIAL,
>  DEVICE_IOMMU,
>  DEVICE_GIC,
> -DEVICE_PCI,
> +DEVICE_PCI_HOSTBRIDGE,
>  /* Use for error */
>  DEVICE_UNKNOWN,
>  };
> --
> 2.25.1
> 

[Jiamei Xie] 
LGTM.
Reviewed-by: Jiamei xie

RE: [PATCH v2] xen: Fix implicit type conversion

2021-10-26 Thread Jiamei Xie



> -Original Message-
> From: Xen-devel  On Behalf Of
> Juergen Gross
> Sent: 2021年10月26日 15:36
> To: Jiasheng Jiang ; boris.ostrov...@oracle.com;
> sstabell...@kernel.org
> Cc: xen-devel@lists.xenproject.org; linux-ker...@vger.kernel.org
> Subject: Re: [PATCH v2] xen: Fix implicit type conversion
> 
> On 26.10.21 09:32, Jiasheng Jiang wrote:
> > The variable 'i' is defined as UINT.
> > However in the for_each_possible_cpu, its value is assigned to -1.
> > That doesn't make sense and in the cpumask_next() it is implicitly
> > type conversed to INT.
> > It is universally accepted that the implicit type conversion is
> > terrible.
> > Also, having the good programming custom will set an example for
> > others.
> > Thus, it might be better to change the definition of 'i' from UINT
> > to INT.
> >
> > Fixes: 3fac101 ("xen: Re-upload processor PM data to hypervisor after S3
> resume (v2)")
> > Signed-off-by: Jiasheng Jiang 
> 
> Reviewed-by: Juergen Gross 
> 
> 
> Juergen

[Jiamei Xie] 
Reviewed-by: Jiamei Xie

RE: Some questions about virtio-net on Xen x86

2021-08-19 Thread Jiamei Xie

Hi Anthony,

-Original Message-
From: Anthony PERARD  
Sent: 2021年8月19日 22:16
To: Jiamei Xie 
Cc: xen-devel@lists.xenproject.org; Wei Chen ; nd 

Subject: Re: Some questions about virtio-net on Xen x86

On Thu, Aug 19, 2021 at 10:38:49AM +, Jiamei Xie wrote:
> Hi all,
> I tried to run virtio-net on X86 machine according to the Wiki page 
> https://wiki.xenproject.org/wiki/Virtio_On_Xen. 
> And I Encountered some confusing problems.
> 
> It seems eth0 is not virtio-net, properly a pv-net. I am really confused.
> 
>  I have the following questions:
> 1. Does Xen x86 still support virtio-net based on QEMU backend?

Well, we don't test it, and the libxl toolstack doesn't really help getting 
virtio devices.

> 2. If yes, is there anything wrong in my guest config file?

The wiki state that you should add 'xen_emul_unplug=never' to the guest 
kernel's command line. (That would be "extra" in your guest.cfg.) I think that 
will prevent the virtio-net device from been unplug, and so you will see the 
virtio-net device in the guest.

But since libxl create Xen PV device anyway, you'll still have the Xen PV NIC, 
so two NICs with the same MAC.

I hope that help,

--
Anthony PERARD

Your answer helped me a lot, thank you very much. After adding 
'xen_emul_unplug=never', virtio-net can work normally.
I can see the virtio net device with lspci command.
00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
00:03.0 SCSI storage controller: Broadcom / LSI 53c895a
00:04.0 VGA compatible controller: Cirrus Logic GD 5446
00:05.0 Ethernet controller: Red Hat, Inc. Virtio network device

Best wishes
Jiamei Xie

Some questions about virtio-net on Xen x86

2021-08-19 Thread Jiamei Xie

Hi all,
I tried to run virtio-net on X86 machine according to the Wiki page 
https://wiki.xenproject.org/wiki/Virtio_On_Xen. 
And I Encountered some confusing problems.

The Qemu version I used is QEMU emulator version 4.2.1 (Debian 
1:4.2-3ubuntu6.17)

The content of guest.cfg is as below:
type = "hvm"
name = "g1"
memory = 512
vcpus = 2
vif = [ 'model=virtio-net' ]
disk = [ 'file:xxx/rootfs.image,xvda1,rw,backendtype=qdisk' ] kernel = 
"/boot/vmlinuz-5.4.0-80-generic"
ramdisk = "/boot/initrd.img-5.4.0-80-generic"
root = "/dev/xvda1 ro"
extra = 'console=hvc0 xencons=tty'

I enter the guest shell. Use ps command to get the running qemu command as 
below:

/usr/local/lib/xen/bin/qemu-system-i386
-xen-domid 31
-no-shutdown
-chardev socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-31,server=on,wait=off
-mon chardev=libxl-cmd,mode=control
-chardev 
socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-31,server=on,wait=off
-mon chardev=libxenstat-cmd,mode=control
-nodefaults -no-user-config
-name g1
-vnc 127.0.0.1:0,to=99
-display none
-kernel /boot/vmlinuz-5.4.0-80-generic
-initrd /boot/initrd.img-5.4.0-80-generic -append root=/dev/xvda1 ro 
console=hvc0 xencons=tty -device cirrus-vga,vgamem_mb=8 -boot order=cda -smp 
2,maxcpus=2 -device virtio-net,id=nic0,netdev=net0,mac=00:16:3e:30:1a:6e
-netdev type=tap,id=net0,ifname=vif31.0-emu,br=xenbr0,script=no,downscript=no
-machine xenfv,suppress-vmdesc=on -m 504 -drive 
file=xxx/rootfs.image,if=ide,index=0,media=disk,format=raw,cache=writeback

The network-related parameters are
" 
-device virtio-net,id=nic0,netdev=net0,mac=00:16:3e:30:1a:6e
-netdev type=tap,id=net0,ifname=vif31.0-emu,br=xenbr0,script=no,downscript=no
"

Get the interface info as below using command 'ip a'. The mac address of eth0 is
00:16:3e:30:1a:6e, the same as in the qemu command, which made me think
 virto-net has run successfully.
2: eth0:  mtu 1500 qdisc mq state UP group 
default qlen 1000
link/ether 00:16:3e:30:1a:6e brd ff:ff:ff:ff:ff:ff
inet 192.168.200.3/24 scope global eth0
   valid_lft forever preferred_lft forever
inet6 fe80::216:3eff:fe30:1a6e/64 scope link
   valid_lft forever preferred_lft forever

I made further check. 
1. I run command 'lspci' and got below message. But there is no virtio devices.
00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE [Natoma/Triton II]
00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
00:03.0 VGA compatible controller: Cirrus Logic GD 5446

2. Run "ls /sys/class/net/ -l", what I got is as below:
lrwxrwxrwx  1 root root 0 Aug 19 08:15 eth0 -> ../../devices/vif-0/net/eth0 
lrwxrwxrwx  1 root root 0 Aug 19 08:15 lo -> ../../devices/virtual/net/lo

Run "ls /sys/bus/xen/devices/vif-0/net/ -l", what I got is as below:
total 0 drwxr-xr-x 5
root root 0 Aug 19 08:15 eth0

It seems eth0 is not virtio-net, properly a pv-net. I am really confused.

 I have the following questions:
1. Does Xen x86 still support virtio-net based on QEMU backend?
2. If yes, is there anything wrong in my guest config file?
3. Is my way to check virtio-net in guest is right?

It is highly appreciated if you kindly tell me about these.

Best wishes
Jiamei Xie

RE: [RESEND PATCH V5 1/2] libxl: Add support for Virtio disk configuration

2021-08-17 Thread Jiamei Xie

Hi Oleksandr,
I am sorry to resend it because the first one is wrong format.

-Original Message-
From: Wei Chen  
Sent: 2021年8月17日 15:11
To: Oleksandr Tyshchenko ; xen-devel@lists.xenproject.org
Cc: Oleksandr Tyshchenko ; Ian Jackson 
; Wei Liu ; Anthony PERARD 
; Stefano Stabellini ; 
Julien Grall ; Volodymyr Babchuk ; 
Bertrand Marquis ; Kaly Xin ; Alex 
Bennée ; Jiamei Xie ; Henry Wang 

Subject: RE: [RESEND PATCH V5 1/2] libxl: Add support for Virtio disk 
configuration

CC to Jiamei and Henry

> -Original Message-
> From: Oleksandr Tyshchenko 
> Sent: 2021年5月22日 3:46
> To: xen-devel@lists.xenproject.org
> Cc: Oleksandr Tyshchenko ; Ian Jackson
> ; Wei Liu ; Anthony PERARD
> ; Stefano Stabellini ;
> Julien Grall ; Volodymyr Babchuk
> ; Wei Chen ; Bertrand
> Marquis ; Kaly Xin ; Alex
> Bennée 
> Subject: [RESEND PATCH V5 1/2] libxl: Add support for Virtio disk
> configuration
> 
> From: Oleksandr Tyshchenko 
> 
> This patch adds basic support for configuring and assisting virtio-disk
> backend (emualator) which is intended to run out of Qemu and could be
> run in any domain.
> Although the Virtio block device is quite different from traditional
> Xen PV block device (vbd) from the toolstack point of view:
>  - the frontend is virtio-blk which is not a Xenbus driver, so nothing
>written to Xenstore are fetched by the frontend (the vdev is not
>passed to the frontend, etc)
>  - the ring-ref/event-channel are not used for the backend<->frontend
>communication, the proposed IPC for Virtio is IOREQ/DM
> it is still a "block device" and ought to be integrated in existing
> "disk" handling. So, re-use (and adapt) "disk" parsing/configuration
> logic to deal with Virtio devices as well.
> 
> Besides introducing new disk backend type (LIBXL_DISK_BACKEND_VIRTIO)
> introduce new device kind (LIBXL__DEVICE_KIND_VIRTIO_DISK) as current
> one (LIBXL__DEVICE_KIND_VBD) doesn't fit into Virtio disk model.
> 
> In order to inform the toolstack that Virtio disk needs to be used
> extend "disk" configuration by introducing new "virtio" flag.
> An example of domain configuration:
> disk = [ 'backend=DomD, phy:/dev/mmcblk1p3, xvda1, rw, virtio' ]
> 
> Please note, this patch is not enough for virtio-disk to work
> on Xen (Arm), as for every Virtio device (including disk) we need
> to allocate Virtio MMIO params (IRQ and memory region) and pass
> them to the backend, also update Guest device-tree with the allocated
> params. The subsequent patch will add these missing bits.
> For the current patch, the default "irq" and "base" are just written
> to the Xenstore.
> 
> Signed-off-by: Oleksandr Tyshchenko 
> 
> ---
> Changes RFC -> V1:
>- no changes
> 
> Changes V1 -> V2:
>- rebase according to the new location of libxl_virtio_disk.c
> 
> Changes V2 -> V3:
>- no changes
> 
> Changes V3 -> V4:
>- rebase according to the new argument for DEFINE_DEVICE_TYPE_STRUCT
> 
> Changes V4 -> V5:
>- split the changes, change the order of the patches
>- update patch description
>- don't introduce new "vdisk" configuration option with own parsing
> logic,
>  re-use Xen PV block "disk" parsing/configuration logic for the
> virtio-disk
>- introduce "virtio" flag and document it's usage
>- add LIBXL_HAVE_DEVICE_DISK_VIRTIO
>- update libxlu_disk_l.[ch]
>- drop num_disks variable/MAX_VIRTIO_DISKS
>- drop Wei's T-b
> ---
> ---
>  docs/man/xl-disk-configuration.5.pod.in   |  27 +
>  tools/include/libxl.h |   6 +
>  tools/libs/light/libxl_device.c   |  38 +-
>  tools/libs/light/libxl_disk.c |  99 +++-
>  tools/libs/light/libxl_types.idl  |   4 +
>  tools/libs/light/libxl_types_internal.idl |   1 +
>  tools/libs/light/libxl_utils.c|   2 +
>  tools/libs/util/libxlu_disk_l.c   | 881 +++--
> -
>  tools/libs/util/libxlu_disk_l.h   |   2 +-
>  tools/libs/util/libxlu_disk_l.l   |   1 +
>  tools/xl/xl_block.c   |  11 +
>  11 files changed, 626 insertions(+), 446 deletions(-)
> 
> diff --git a/docs/man/xl-disk-configuration.5.pod.in b/docs/man/xl-disk-
> configuration.5.pod.in
> index 71d0e86..9cc189f 100644
> --- a/docs/man/xl-disk-configuration.5.pod.in
> +++ b/docs/man/xl-disk-configuration.5.pod.in
> @@ -344,8 +344,35 @@ can be used to disable "hole punching" for file based
> backends which
>  were intentionally created non-sparse to avoid fragmentation of the
>  file.
> 
> +=item B
> +

RE: [RESEND PATCH V5 1/2] libxl: Add support for Virtio disk configuration

2021-08-17 Thread Jiamei Xie

Hi Oleksandr,
This code will fail when compiling it  on arm32.
> +flexarray_append_pair(back, "base", GCSPRINTF("%lu",
> disk->base));

Below is its error message.  Maybe PRIu64 should be used instead 'lu'.  Or use 
'll', and cast base type  to 'long long unsigned int'.
| make[6]: *** [libxl_disk.o] Error 1
| make[6]: *** Waiting for unfinished jobs
| In file included from libxl_disk.c:17:
| libxl_disk.c: In function 'device_disk_add':
| libxl_internal.h:4376:51: error: format '%lu' expects argument of type 'long 
unsigned int', but argument 3 has type 'uint64_t' {aka 'long long unsigned 
int'} [-Werror=format=]
|  4376 | #define GCSPRINTF(fmt, ...) (libxl__sprintf((gc), (fmt), __VA_ARGS__))
|   |   ^
| libxl_disk.c:353:53: note: in expansion of macro 'GCSPRINTF'
|   353 | flexarray_append_pair(back, "base", GCSPRINTF("%lu", 
disk->base));
|   | ^

-Original Message-
From: Wei Chen  
Sent: 2021年8月17日 15:11
To: Oleksandr Tyshchenko ; xen-devel@lists.xenproject.org
Cc: Oleksandr Tyshchenko ; Ian Jackson 
; Wei Liu ; Anthony PERARD 
; Stefano Stabellini ; 
Julien Grall ; Volodymyr Babchuk ; 
Bertrand Marquis ; Kaly Xin ; Alex 
Bennée ; Jiamei Xie ; Henry Wang 

Subject: RE: [RESEND PATCH V5 1/2] libxl: Add support for Virtio disk 
configuration

CC to Jiamei and Henry

> -Original Message-
> From: Oleksandr Tyshchenko 
> Sent: 2021年5月22日 3:46
> To: xen-devel@lists.xenproject.org
> Cc: Oleksandr Tyshchenko ; Ian Jackson
> ; Wei Liu ; Anthony PERARD
> ; Stefano Stabellini ;
> Julien Grall ; Volodymyr Babchuk
> ; Wei Chen ; Bertrand
> Marquis ; Kaly Xin ; Alex
> Bennée 
> Subject: [RESEND PATCH V5 1/2] libxl: Add support for Virtio disk
> configuration
> 
> From: Oleksandr Tyshchenko 
> 
> This patch adds basic support for configuring and assisting virtio-disk
> backend (emualator) which is intended to run out of Qemu and could be
> run in any domain.
> Although the Virtio block device is quite different from traditional
> Xen PV block device (vbd) from the toolstack point of view:
>  - the frontend is virtio-blk which is not a Xenbus driver, so nothing
>written to Xenstore are fetched by the frontend (the vdev is not
>passed to the frontend, etc)
>  - the ring-ref/event-channel are not used for the backend<->frontend
>communication, the proposed IPC for Virtio is IOREQ/DM
> it is still a "block device" and ought to be integrated in existing
> "disk" handling. So, re-use (and adapt) "disk" parsing/configuration
> logic to deal with Virtio devices as well.
> 
> Besides introducing new disk backend type (LIBXL_DISK_BACKEND_VIRTIO)
> introduce new device kind (LIBXL__DEVICE_KIND_VIRTIO_DISK) as current
> one (LIBXL__DEVICE_KIND_VBD) doesn't fit into Virtio disk model.
> 
> In order to inform the toolstack that Virtio disk needs to be used
> extend "disk" configuration by introducing new "virtio" flag.
> An example of domain configuration:
> disk = [ 'backend=DomD, phy:/dev/mmcblk1p3, xvda1, rw, virtio' ]
> 
> Please note, this patch is not enough for virtio-disk to work
> on Xen (Arm), as for every Virtio device (including disk) we need
> to allocate Virtio MMIO params (IRQ and memory region) and pass
> them to the backend, also update Guest device-tree with the allocated
> params. The subsequent patch will add these missing bits.
> For the current patch, the default "irq" and "base" are just written
> to the Xenstore.
> 
> Signed-off-by: Oleksandr Tyshchenko 
> 
> ---
> Changes RFC -> V1:
>- no changes
> 
> Changes V1 -> V2:
>- rebase according to the new location of libxl_virtio_disk.c
> 
> Changes V2 -> V3:
>- no changes
> 
> Changes V3 -> V4:
>- rebase according to the new argument for DEFINE_DEVICE_TYPE_STRUCT
> 
> Changes V4 -> V5:
>- split the changes, change the order of the patches
>- update patch description
>- don't introduce new "vdisk" configuration option with own parsing
> logic,
>  re-use Xen PV block "disk" parsing/configuration logic for the
> virtio-disk
>- introduce "virtio" flag and document it's usage
>- add LIBXL_HAVE_DEVICE_DISK_VIRTIO
>- update libxlu_disk_l.[ch]
>- drop num_disks variable/MAX_VIRTIO_DISKS
>- drop Wei's T-b
> ---
> ---
>  docs/man/xl-disk-configuration.5.pod.in   |  27 +
>  tools/include/libxl.h |   6

58 matches

Mail list logo