Hi Lucas Am 26.06.22 um 23:01 schrieb Lucas De Marchi:
On Fri, Jun 17, 2022 at 01:52:04AM -0700, Lucas De Marchi wrote:Like was done for read, provide the equivalent for write. Even if current users are not in the hot path, this should future-proof it. v2: - Remove default from _Generic() - callers wanting to write more than u64 should use iosys_map_memcpy_to() - Add WRITE_ONCE() cases dereferencing the pointer when using system memoryThomas, do you have any additional concern on this patch regarding your previous review?
Sorry, your patches simply fell through the cracks. For the patchset: Reviewed-by: Thomas Zimmermann <tzimmermann@xxxxxxx> Thanks for the effort you put into this. Best regards Thomas
thanks Lucas De MarchiSigned-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx> Reviewed-by: Reviewed-by: Christian König <christian.koenig@xxxxxxx> # v1 --- include/linux/iosys-map.h | 42 ++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h index f59dd00ed202..580e14cd360c 100644 --- a/include/linux/iosys-map.h +++ b/include/linux/iosys-map.h@@ -337,9 +337,13 @@ static inline void iosys_map_memset(struct iosys_map *dst, size_t offset,#ifdef CONFIG_64BIT #define __iosys_map_rd_io_u64_case(val_, vaddr_iomem_) \ u64: val_ = readq(vaddr_iomem_) +#define __iosys_map_wr_io_u64_case(val_, vaddr_iomem_) \ + u64: writeq(val_, vaddr_iomem_) #else #define __iosys_map_rd_io_u64_case(val_, vaddr_iomem_) \ u64: memcpy_fromio(&(val_), vaddr_iomem__, sizeof(u64)) +#define __iosys_map_wr_io_u64_case(val_, vaddr_iomem_) \ + u64: memcpy_toio(vaddr_iomem_, &(val_), sizeof(u64)) #endif#define __iosys_map_rd_io(val__, vaddr_iomem__, type__) _Generic(val__, \ @@ -354,6 +358,19 @@ static inline void iosys_map_memset(struct iosys_map *dst, size_t offset,val__ = READ_ONCE(*((type__ *)vaddr__)); \ })+#define __iosys_map_wr_io(val__, vaddr_iomem__, type__) _Generic(val__, \+ u8: writeb(val__, vaddr_iomem__), \ + u16: writew(val__, vaddr_iomem__), \ + u32: writel(val__, vaddr_iomem__), \ + __iosys_map_wr_io_u64_case(val__, vaddr_iomem__)) + +#define __iosys_map_wr_sys(val__, vaddr__, type__) ({ \ + compiletime_assert(sizeof(type__) <= sizeof(u64), \ + "Unsupported access size for __iosys_map_wr_sys()"); \ + WRITE_ONCE(*((type__ *)vaddr__), val__); \ +}) + + /** * iosys_map_rd - Read a C-type value from the iosys_map *@@ -386,12 +403,17 @@ static inline void iosys_map_memset(struct iosys_map *dst, size_t offset,* @type__: Type of the value being written * @val__: Value to write *- * Write a C-type value to the iosys_map, handling possible un-aligned accesses- * to the mapping.+ * Write a C type value (u8, u16, u32 and u64) to the iosys_map. For other types+ * or if pointer may be unaligned (and problematic for the architecture + * supported), use iosys_map_memcpy_to() */ -#define iosys_map_wr(map__, offset__, type__, val__) ({ \ - type__ val = (val__); \ - iosys_map_memcpy_to(map__, offset__, &val, sizeof(val)); \ +#define iosys_map_wr(map__, offset__, type__, val__) ({ \ + type__ val = (val__); \ + if ((map__)->is_iomem) { \+ __iosys_map_wr_io(val, (map__)->vaddr_iomem + (offset__), type__);\+ } else { \+ __iosys_map_wr_sys(val, (map__)->vaddr + (offset__), type__); \+ } \ }) /**@@ -472,10 +494,12 @@ static inline void iosys_map_memset(struct iosys_map *dst, size_t offset,* @field__: Member of the struct to read * @val__: Value to write *- * Write a value to the iosys_map considering its layout is described by a C struct - * starting at @struct_offset__. The field offset and size is calculated and the - * @val__ is written handling possible un-aligned memory accesses. Refer to- * iosys_map_rd_field() for expected usage and memory layout.+ * Write a value to the iosys_map considering its layout is described by a C + * struct starting at @struct_offset__. The field offset and size is calculated + * and the @val__ is written. If the field access would incur in un-aligned+ * access, then either iosys_map_memcpy_to() needs to be used or the+ * architecture must support it. Refer to iosys_map_rd_field() for expected+ * usage and memory layout. */#define iosys_map_wr_field(map__, struct_offset__, struct_type__, field__, val__) ({ \struct_type__ *s; \ -- 2.36.1
-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
