Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 1 May 2024 11:55:08 -0500 | Subject | Re: [PATCH] wifi: ath11k: fix remapped ce accessing issue on 64bit OS | From | Larry Finger <> |
| |
On 5/1/24 11:14 AM, Ziyang Huang wrote: > On 64bit OS, when ab->mem_ce is lower than or 4G far away from ab->mem, > u32 is not enough to store the offsets, which makes ath11k_ahb_read32() > and ath11k_ahb_write32() access incorrect address and causes Data Abort > Exception. > > Let's use the high bits of offsets to decide where to access, which is > similar as ath11k_pci_get_window_start() done. In the future, we can merge > these functions for unified regs accessing. > > Signed-off-by: Ziyang Huang <hzyitc@outlook.com> > --- > drivers/net/wireless/ath/ath11k/ahb.c | 34 ++++++++++++++++++++------- > drivers/net/wireless/ath/ath11k/hal.c | 17 +++++--------- > drivers/net/wireless/ath/ath11k/hw.c | 14 +++++------ > drivers/net/wireless/ath/ath11k/hw.h | 7 +++++- > 4 files changed, 45 insertions(+), 27 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath11k/ahb.c b/drivers/net/wireless/ath/ath11k/ahb.c > index 7c0a23517949..9e59b4de93a9 100644 > --- a/drivers/net/wireless/ath/ath11k/ahb.c > +++ b/drivers/net/wireless/ath/ath11k/ahb.c > @@ -198,12 +198,30 @@ static const struct ath11k_pci_ops ath11k_ahb_pci_ops_wcn6750 = { > > static inline u32 ath11k_ahb_read32(struct ath11k_base *ab, u32 offset) > { > - return ioread32(ab->mem + offset); > + switch (offset & ATH11K_REG_TYPE_MASK) { > + case ATH11K_REG_TYPE_NORMAL: > + return ioread32(ab->mem + FIELD_GET(ATH11K_REG_OFFSET_MASK, offset)); > + case ATH11K_REG_TYPE_CE: > + return ioread32(ab->mem_ce + FIELD_GET(ATH11K_REG_OFFSET_MASK, offset)); > + default: > + BUG();
Do you really want to crash the system here? A dev_warn() or something similar would log the situation. I suspect this case is never taken, but a system crash is not a good response if it is.
> + return 0; > + } > } > > static inline void ath11k_ahb_write32(struct ath11k_base *ab, u32 offset, u32 value) > { > - iowrite32(value, ab->mem + offset); > + switch (offset & ATH11K_REG_TYPE_MASK) { > + case ATH11K_REG_TYPE_NORMAL: > + iowrite32(value, ab->mem + FIELD_GET(ATH11K_REG_OFFSET_MASK, offset)); > + break; > + case ATH11K_REG_TYPE_CE: > + iowrite32(value, ab->mem_ce + FIELD_GET(ATH11K_REG_OFFSET_MASK, offset)); > + break; > + default: > + BUG();
Ditto.
Larry
| ![\](/images/icornerr.gif) |