On 01.09.21 05:47, Z.Q. Hou wrote: > > >> -----Original Message----- >> From: Jan Kiszka <[email protected]> >> Sent: 2021年8月31日 21:14 >> 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 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. > > I did the experiments on imx8mp with A53 cores and GIC-500, and looked into > the TRM of GIC-500, it says: " Byte or halfword accesses to registers that do > not permit those access sizes are not successful and return a SLVERR > response." According to A53 TRM, the SLVERR is consistent with experimental > results ESR 0xbf000002. > > The last point I'm wondering is, the actual access instruction is issued from > EL2, but finally this SError is taken to EL1? This doesn't adhere to the > armv8 exception model that an exception can never be taken to a lower EL. How > to explain this? > I did one more experiment that in restrict_bitmask_access() check the access > size, doesn't perform the access if it's not word-width. The result is the > byte-access to the not permitted registers doesn't cause SError, so it > confirms that the SError is indeed caused by the EL2 access instruction. >
Hmm, maybe try that with a different silicon as well? Or, to be vendor neutral, see what happens inside QEMU. Image available via jailhouse-images. 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/ab9e7f4c-c52a-4c4f-ee77-c8148763184a%40siemens.com.
