diff mbox series

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

Message ID 20221122054644.1092173-1-jiamei.xie@arm.com (mailing list archive)
State New, archived
Headers show
Series [v2] xen/arm: vpl011: Make access to DMACR write-ignore | expand

Commit Message

Jiamei Xie Nov. 22, 2022, 5:46 a.m. UTC 
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 0xffffffc00944d048
Mem abort info:
  ESR = 0x96000000
  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 = 0x00000000
  CM = 0, WnR = 0
swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000020e2e000
[ffffffc00944d048] pgd=100000003ffff803, p4d=100000003ffff803, pud=100000003ffff803, pmd=100000003fffa803, pte=006800009c090f13
Internal error: ttbr address size fault: 96000000 [#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: b9000083 b901f001 794038a0 8b000042 (b9000041)
---[ 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 <jiamei.xie@arm.com>
---
 xen/arch/arm/vpl011.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Julien Grall Nov. 22, 2022, 12:25 p.m. UTC | #1 
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 0xffffffc00944d048
> Mem abort info:
>    ESR = 0x96000000
>    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 = 0x00000000
>    CM = 0, WnR = 0
> swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000020e2e000
> [ffffffc00944d048] pgd=100000003ffff803, p4d=100000003ffff803, pud=100000003ffff803, pmd=100000003fffa803, pte=006800009c090f13
> Internal error: ttbr address size fault: 96000000 [#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: b9000083 b901f001 794038a0 8b000042 (b9000041)
> ---[ 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.

> 
> [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 <jiamei.xie@arm.com>
> ---
>   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,
Michal Orzel Nov. 22, 2022, 3:16 p.m. UTC | #2 
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 0xffffffc00944d048
>> Mem abort info:
>>    ESR = 0x96000000
>>    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 = 0x00000000
>>    CM = 0, WnR = 0
>> swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000020e2e000
>> [ffffffc00944d048] pgd=100000003ffff803, p4d=100000003ffff803, pud=100000003ffff803, pmd=100000003fffa803, pte=006800009c090f13
>> Internal error: ttbr address size fault: 96000000 [#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: b9000083 b901f001 794038a0 8b000042 (b9000041)
>> ---[ 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.

> 
>>
>> [1] https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.arm.com%2Fdocumentation%2Fden0094%2Fc%2F%3Flang%3Den&amp;data=05%7C01%7Cmichal.orzel%40amd.com%7C1065702b4fd2457cdbf808dacc84b45a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638047167600786580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=W1dbakw6lkGkv4ydElIi%2Ba7uT7e7Pt5dB3vDtYpP%2FqQ%3D&amp;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&amp;data=05%7C01%7Cmichal.orzel%40amd.com%7C1065702b4fd2457cdbf808dacc84b45a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638047167600786580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=O4zxuI3HqRA1bdraGcVVY8vV0HGbqOI3nFa%2FciC1cGQ%3D&amp;reserved=0
>>
>> Signed-off-by: Jiamei Xie <jiamei.xie@arm.com>
>> ---
>>   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
Jiamei Xie Nov. 23, 2022, 7:27 a.m. UTC | #3 
Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Sent: Tuesday, November 22, 2022 8:26 PM
> To: Jiamei Xie <Jiamei.Xie@arm.com>; xen-devel@lists.xenproject.org
> Cc: Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> 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 0xffffffc00944d048
> > Mem abort info:
> >    ESR = 0x96000000
> >    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 = 0x00000000
> >    CM = 0, WnR = 0
> > swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000020e2e000
> > [ffffffc00944d048] pgd=100000003ffff803, p4d=100000003ffff803,
> pud=100000003ffff803, pmd=100000003fffa803, pte=006800009c090f13
> > Internal error: ttbr address size fault: 96000000 [#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: b9000083 b901f001 794038a0 8b000042 (b9000041)
> > ---[ 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 <jiamei.xie@arm.com>
> > ---
> >   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
Jiamei Xie Nov. 23, 2022, 7:29 a.m. UTC | #4 
Hi Michal,

> -----Original Message-----
> From: Michal Orzel <michal.orzel@amd.com>
> Sent: Tuesday, November 22, 2022 11:17 PM
> To: Julien Grall <julien@xen.org>; Jiamei Xie <Jiamei.Xie@arm.com>; xen-
> devel@lists.xenproject.org
> Cc: Wei Chen <Wei.Chen@arm.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Bertrand Marquis <Bertrand.Marquis@arm.com>;
> Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
> 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 0xffffffc00944d048
> >> Mem abort info:
> >>    ESR = 0x96000000
> >>    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 = 0x00000000
> >>    CM = 0, WnR = 0
> >> swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000020e2e000
> >> [ffffffc00944d048] pgd=100000003ffff803, p4d=100000003ffff803,
> pud=100000003ffff803, pmd=100000003fffa803, pte=006800009c090f13
> >> Internal error: ttbr address size fault: 96000000 [#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: b9000083 b901f001 794038a0 8b000042 (b9000041)
> >> ---[ 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&amp;data=05%7C01%7Cmichal.orzel%40amd.com%7C1065702
> b4fd2457cdbf808dacc84b45a%7C3dd8961fe4884e608e11a82d994e183d%7C
> 0%7C0%7C638047167600786580%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> 00%7C%7C%7C&amp;sdata=O4zxuI3HqRA1bdraGcVVY8vV0HGbqOI3nFa%2F
> ciC1cGQ%3D&amp;reserved=0
> >>
> >> Signed-off-by: Jiamei Xie <jiamei.xie@arm.com>
> >> ---
> >>   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
diff mbox series

Patch

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: