> -----Original Message-----
> From: Jan Kiszka <[email protected]>
> Sent: 2021年8月30日 20:03
> To: Z.Q. Hou <[email protected]>; [email protected]
> Cc: Jiafei Pan <[email protected]>; Rui Sousa <[email protected]>
> Subject: Re: [PATCH] arm: irqchip: Fix the mask according to access address
> and size
>
> On 30.08.21 12:54, Z.Q. Hou wrote:
> >
> >
> >> -----Original Message-----
> >> From: Jan Kiszka [mailto:[email protected]]
> >> Sent: 2021年8月30日 13:43
> >> To: Z.Q. Hou <[email protected]>;
> [email protected]
> >> Subject: Re: [PATCH] arm: irqchip: Fix the mask according to access
> >> address and size
> >>
> >> On 30.08.21 05:02, Z.Q. Hou wrote:
> >>> Hi Jan,
> >>>
> >>>> -----Original Message-----
> >>>> From: Jan Kiszka [mailto:[email protected]]
> >>>> Sent: 2021年8月27日 15:53
> >>>> To: Z.Q. Hou <[email protected]>;
> [email protected]
> >>>> Subject: Re: [PATCH] arm: irqchip: Fix the mask according to access
> >>>> address and size
> >>>>
> >>>> On 27.08.21 09:51, Zhiqiang Hou wrote:
> >>>>> From: Hou Zhiqiang <[email protected]>
> >>>>>
> >>>>> In function restrict_bitmask_access(), the current access_mask is
> >>>>> implicitly assuming that it always access the whole register, but
> >>>>> some registers are byte-accessible, the Guest may get/set wrong
> >>>>> value when it issue a byte or halfword access to these registers.
> >>>>
> >>>> Can you be more specific in the affected scenarios? At least one
> >>>> example would be good.
> >>>
> >>> Let take GICD_IPRIORITYR as an example, which is a byte-accessible
> register.
> >>> Thinking the SPI INTID 33 is assigned to an inmate cell, and Guest
> >>> wants to set
> >> the priority of 33 by byte-accessing.
> >>>
> >>> Assuming:
> >>> GICD_IPRIORITY8 original value: 0x00000000 Guest set value: 0xa0
> >>>
> >>> Then the current logic like:
> >>> The access_mask is calculated as 0xff00, GICD_IPRIORITY8 read back
> >>> value is
> >> 0x00. Then 0x00 & ~(0xff00) get 0x00, 0x00 | (0xa0 & 0xff00) get
> >> 0x00, then finally 0x00 will be written back.
> >>>
> >>> With this fix, the access_mask will become (0xff00 >> 8) & 0xff =
> >>> 0xff, so finally
> >> the 0xa0 will be written back.
> >>>
> >>
> >> OK, indeed a good point. We missed the byte-accessability of
> GICD_IPRIORITYR.
> >>
> >> But this reveals more: We perform a read-modify-write cycle on behalf
> >> of the cell with the cell's idea of access width. That is potentially
> >> unsafe.
> >
> > Can you help understand why there is still potential unsafe with this fix
> patch?
> >
>
> The patch is fine for GICD_IPRIORITYR, so is accepting all accesses for that
> register. The problem is with other registers that do not support byte
> accesses.
Don't worry, according to the programmer guide the Guest code must not access
those registers with Byte-width; even if there is someone do that by mistake,
it will trigger an exception before the access is trapped to EL2.
Thanks,
Zhiqiang
>
> So, to make things simpler, we convert to always performing word accesses
> for all of them. That means no extra effort for the other regs, just
> filtering of
> size != 4, but GICD_IPRIORITYR now needs upfront work.
> And that provides the chance to address also the issue you found.
>
> >>
> >> Handling of GICD_ITARGETSR has to resolve the same issue, and it does
> >> that by converting the request unconditionally into a word-access. I
> >> think we should do the same here as well.
> >
> > I think this fix way is more simple than converting to word-access
> unconditionally, as this fix way only tweaks the access_mask, while the
> word-access way need to tweak both the addr and value.
> >
>
> We could stack the two changes. I'll take yours first, and then we only need
> to decide who does the second fix on top...
>
> Jan
>
> --
> Siemens AG, T RDA IOT
> Corporate Competence Center Embedded Linux
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/HE1PR0402MB337140548025A20E651BB4AA84CC9%40HE1PR0402MB3371.eurprd04.prod.outlook.com.
