On 31.08.21 10:48, Z.Q. Hou wrote: > > >> -----Original Message----- >> From: Jan Kiszka <[email protected]> >> Sent: 2021年8月31日 15:11 >> 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 31.08.21 08:55, Z.Q. Hou wrote: >>> >>> >>>> -----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. >>> >> >> I do worry: We trap the access to EL2 because the memory region is not >> backed by memory in the first stage translation. And then EL2 will issue that >> request and panic. > > You're correct, I did some experiments, the byte-width access to > non-byte-accessible register is also trapped to EL2. > But what I'm not understanding is the byte-width write access finally trigger > a SError (ESR_EL1: 0xbf000002) to the Guest instead of a exception to EL2, > and the Jailhouse does not crash. Do you know the reason of the result? >
No, not really. I'm architecturally not fluent in that details, and in that state I would have expected a crash at EL2 as well. Maybe someone else is listening with more detailed knowledge. Or we are in an implementation-specific behavior region here, and your mileage can vary from SoC to SoC. 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/2b88eb53-8258-2d0b-d0d1-71c24bf372cb%40siemens.com.
