[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: Return type of clean_and_invalidate_dcache_va_range
On 12.02.2024 19:38, Julien Grall wrote: > An alternative would be to introduced arch_grant_cache_flush() and move > the if/else logic there. Something like: > > diff --git a/xen/arch/arm/include/asm/page.h > b/xen/arch/arm/include/asm/page.h > index 69f817d1e68a..4a3de49762a1 100644 > --- a/xen/arch/arm/include/asm/page.h > +++ b/xen/arch/arm/include/asm/page.h > @@ -281,6 +281,19 @@ static inline void write_pte(lpae_t *p, lpae_t pte) > dsb(sy); > } > > +static inline arch_grant_cache_flush(unsigned int op, const void *p, > unsigned long size) > +{ > + unsigned int order = get_order_from_bytes(size); > + > + if ( (cflush->op & GNTTAB_CACHE_INVAL) && (cflush->op & > GNTTAB_CACHE_CLEAN) ) > + clean_and_invalidate_dcache_va_range(v, cflush->length); > + else if ( cflush->op & GNTTAB_CACHE_INVAL ) > + invalidate_dcache_va_range(v, cflush->length); > + else if ( cflush->op & GNTTAB_CACHE_CLEAN ) > + clean_dcache_va_range(v, cflush->length); > + > + return 0; > +} > > /* Flush the dcache for an entire page. */ > void flush_page_to_ram(unsigned long mfn, bool sync_icache); > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c > index 424744ad5e1a..647e1522466d 100644 > --- a/xen/arch/arm/setup.c > +++ b/xen/arch/arm/setup.c > @@ -735,8 +735,7 @@ void asmlinkage __init start_xen(unsigned long > boot_phys_offset, > fdt_paddr); > > /* Register Xen's load address as a boot module. */ > - xen_bootmodule = add_boot_module(BOOTMOD_XEN, > - virt_to_maddr(_start), > + xen_bootmodule = add_boot_module(BOOTMOD_XEN, virt_to_maddr(_start), > (paddr_t)(uintptr_t)(_end - _start), false); > BUG_ON(!xen_bootmodule); > > diff --git a/xen/arch/x86/include/asm/flushtlb.h > b/xen/arch/x86/include/asm/flushtlb.h > index bb0ad58db49b..dfe51cddde90 100644 > --- a/xen/arch/x86/include/asm/flushtlb.h > +++ b/xen/arch/x86/include/asm/flushtlb.h > @@ -182,23 +182,22 @@ void flush_area_mask(const cpumask_t *mask, const > void *va, > } > > static inline void flush_page_to_ram(unsigned long mfn, bool > sync_icache) {} > -static inline int invalidate_dcache_va_range(const void *p, > - unsigned long size) > -{ return -EOPNOTSUPP; } > -static inline int clean_and_invalidate_dcache_va_range(const void *p, > - unsigned long size) > + > +unsigned int guest_flush_tlb_flags(const struct domain *d); > +void guest_flush_tlb_mask(const struct domain *d, const cpumask_t *mask); > + > +static inline arch_grant_cache_flush(unsigned int op, const void *p, > unsigned long size) > { > - unsigned int order = get_order_from_bytes(size); > + unsigned int order; > + > + if ( !(cflush->op & GNTTAB_CACHE_CLEAN) ) > + return -EOPNOTSUPP; > + > + order = get_order_from_bytes(size); > /* sub-page granularity support needs to be added if necessary */ > flush_area_local(p, FLUSH_CACHE|FLUSH_ORDER(order)); > + > return 0; > } > -static inline int clean_dcache_va_range(const void *p, unsigned long size) > -{ > - return clean_and_invalidate_dcache_va_range(p, size); > -} > - > -unsigned int guest_flush_tlb_flags(const struct domain *d); > -void guest_flush_tlb_mask(const struct domain *d, const cpumask_t *mask); > > #endif /* __FLUSHTLB_H__ */ > > I have a slight preference for the latter. I would like to hear the > opinion of the others. I would prefer this 2nd form, too, assuming the setup.c change wasn't really meant to be there. The one thing that doesn't become clear: In the sketch above arch_grant_cache_flush() has no return type, yet has "return 0". This raises a question towards the one that's at the root of this thread: Do you mean the function to have a return value, and if so will it be (sensibly) used? Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |