On 9/19/23 09:14, Julien Grall wrote: > Hi, > > On 19/09/2023 14:10, Julien Grall wrote: >> On 19/09/2023 12:28, Volodymyr Babchuk wrote: >>> ITS manages Device Tables and Interrupt Translation Tables on its own, >>> so generally we are not interested which shareability and cacheability >>> attributes it uses. But there is one exception: ITS requires that DT >>> and ITT must be initialized with zeroes. If ITS belongs to the Inner >>> Cacheability domain there is no problem at all. >>> >>> But in all other cases we need to do clean CPU caches manually, or >>> otherwise CPU can overwrite DT and ITT entries. From user perspective >>> this looks like interrupts are not delivered from a device. >>> >>> Also, we will rename HOST_ITS_FLUSH_CMD_QUEUE flag to >>> HOST_ITS_FLUSH_BUFFERS because now this flag controls not only command >>> queue. >> >> Reading the specification, CBASER will indicate the cacheability for the >> command queue. But I couldn't find any reference saying this will apply >> to the ITT as well. >> >> If such reference doesn't exist then... >> >>> >>> Signed-off-by: Volodymyr Babchuk <[email protected]> >>> --- >>> xen/arch/arm/gic-v3-its.c | 7 +++++-- >>> xen/arch/arm/include/asm/gic_v3_its.h | 2 +- >>> 2 files changed, 6 insertions(+), 3 deletions(-) >>> >>> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c >>> index 72cf318810..63e28a7706 100644 >>> --- a/xen/arch/arm/gic-v3-its.c >>> +++ b/xen/arch/arm/gic-v3-its.c >>> @@ -107,7 +107,7 @@ static int its_send_command(struct host_its >>> *hw_its, const void *its_cmd) >>> } >>> memcpy(hw_its->cmd_buf + writep, its_cmd, ITS_CMD_SIZE); >>> - if ( hw_its->flags & HOST_ITS_FLUSH_CMD_QUEUE ) >>> + if ( hw_its->flags & HOST_ITS_FLUSH_BUFFERS ) >>> clean_dcache_va_range(hw_its->cmd_buf + writep, ITS_CMD_SIZE); >>> else >>> dsb(ishst); >>> @@ -335,7 +335,7 @@ static void *its_map_cbaser(struct host_its *its) >>> */ >>> if ( !(reg & GITS_BASER_INNER_CACHEABILITY_MASK) ) >>> { >>> - its->flags |= HOST_ITS_FLUSH_CMD_QUEUE; >>> + its->flags |= HOST_ITS_FLUSH_BUFFERS; >>> printk(XENLOG_WARNING "using non-cacheable ITS command >>> queue\n"); >>> } >>> @@ -699,6 +699,9 @@ int gicv3_its_map_guest_device(struct domain *d, >>> if ( !itt_addr ) >>> goto out_unlock; >>> + if ( hw_its->flags & HOST_ITS_FLUSH_BUFFERS ) >>> + clean_dcache_va_range(itt_addr, nr_events * hw_its->itte_size); >> >> ... I think we need to have this flush unconditional like Linux does. > > I forgot to mention that this likely want to be a > clean_dcache_and_invalidate_va_range() (see my comment in patch #2).
I turned it into an unconditional clean_and_invalidate_dcache_va_range() and did a quick test, and it still fixes my test case on VCK190. So feel free to add: Tested-by: Stewart Hildebrand <[email protected]>
