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. > > > > > Signed-off-by: Hou Zhiqiang <[email protected]> > > --- > > hypervisor/arch/arm-common/irqchip.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/hypervisor/arch/arm-common/irqchip.c > > b/hypervisor/arch/arm-common/irqchip.c > > index 256af114..daae5512 100644 > > --- a/hypervisor/arch/arm-common/irqchip.c > > +++ b/hypervisor/arch/arm-common/irqchip.c > > @@ -69,6 +69,9 @@ restrict_bitmask_access(struct mmio_access *mmio, > unsigned int reg_index, > > if (irqchip_irq_in_cell(cell, first_irq + irq)) > > access_mask |= irq_bits << (irq * bits_per_irq); > > > > + access_mask >>= 8 * (mmio->address & 0x3); > > + access_mask &= (1UL << (mmio->size * 8)) - 1; > > + > > Are we only talking about mitigating wrong results affecting the issuing > cell? Or > does the wrong mask have the potential to corrupt content that affects other > cells as well? In the latter case, we must not rely on the cell-provided > access size, > I strongly suspect. Correct, other cells' content also can be corrupted, and resolved by this fix. The cell-provided access size is not a problem as the final mask is 'mask of size field' AND 'mask calculated from the IRQ assignment'. Thanks, Zhiqiang > > Jan > > > if (!mmio->is_write) { > > /* Restrict the read value */ > > mmio_perform_access(gicd_base, mmio); > > > > -- > 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/HE1PR0402MB337191DE611BA6CC480C104D84CB9%40HE1PR0402MB3371.eurprd04.prod.outlook.com.
