lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <b0b2ae77-3311-34c8-d1a2-c6f30eca3f1e@intel.com> Date: Thu, 16 Feb 2023 18:51:08 +0100 From: Alexander Lobakin <aleksander.lobakin@...el.com> To: Alex Elder <elder@...aro.org> CC: <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>, <caleb.connolly@...aro.org>, <mka@...omium.org>, <evgreen@...omium.org>, <andersson@...nel.org>, <quic_cpratapa@...cinc.com>, <quic_avuyyuru@...cinc.com>, <quic_jponduru@...cinc.com>, <quic_subashab@...cinc.com>, <elder@...nel.org>, <netdev@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>, <linux-kernel@...r.kernel.org> Subject: Re: [PATCH net-next 2/6] net: ipa: kill gsi->virt_raw From: Alex Elder <elder@...aro.org> Date: Wed, 15 Feb 2023 13:53:48 -0600 > Starting at IPA v4.5, almost all GSI registers had their offsets > changed by a fixed amount (shifted downward by 0xd000). Rather than > defining offsets for all those registers dependent on version, an > adjustment was applied for most register accesses. This was > implemented in commit cdeee49f3ef7f ("net: ipa: adjust GSI register > addresses"). It was later modified to be a bit more obvious about > the adjusment, in commit 571b1e7e58ad3 ("net: ipa: use a separate > pointer for adjusted GSI memory"). [...] > @@ -142,27 +127,17 @@ int gsi_reg_init(struct gsi *gsi, struct platform_device *pdev) > return -EINVAL; > } > > - /* Make sure we can make our pointer adjustment if necessary */ > - adjust = gsi->version < IPA_VERSION_4_5 ? 0 : GSI_EE_REG_ADJUST; > - if (res->start < adjust) { > - dev_err(dev, "DT memory resource \"gsi\" too low (< %u)\n", > - adjust); > - return -EINVAL; > - } > - > gsi->regs = gsi_regs(gsi); > if (!gsi->regs) { > dev_err(dev, "unsupported IPA version %u (?)\n", gsi->version); > return -EINVAL; > } > > - gsi->virt_raw = ioremap(res->start, size); > - if (!gsi->virt_raw) { > + gsi->virt = ioremap(res->start, size); Now that at least one check above went away and the second one might be or be not correct (I thought ioremap core takes care of this), can't just devm_platform_ioremap_resource_byname() be used here for simplicity? > + if (!gsi->virt) { > dev_err(dev, "unable to remap \"gsi\" memory\n"); > return -ENOMEM; > } > - /* Most registers are accessed using an adjusted register range */ > - gsi->virt = gsi->virt_raw - adjust; > > return 0; > } > @@ -170,7 +145,7 @@ int gsi_reg_init(struct gsi *gsi, struct platform_device *pdev) > /* Inverse of gsi_reg_init() */ > void gsi_reg_exit(struct gsi *gsi) > { > + iounmap(gsi->virt); (don't forget to remove this unmap if you decide to switch to devm_) > gsi->virt = NULL; > - iounmap(gsi->virt_raw); > - gsi->virt_raw = NULL; > + gsi->regs = NULL; > } [...] > diff --git a/drivers/net/ipa/reg/gsi_reg-v3.1.c b/drivers/net/ipa/reg/gsi_reg-v3.1.c > index 651c8a7ed6116..8451d3f8e421e 100644 > --- a/drivers/net/ipa/reg/gsi_reg-v3.1.c > +++ b/drivers/net/ipa/reg/gsi_reg-v3.1.c > @@ -8,16 +8,12 @@ > #include "../reg.h" > #include "../gsi_reg.h" > > -/* The inter-EE IRQ registers are relative to gsi->virt_raw (IPA v3.5+) */ > - > REG(INTER_EE_SRC_CH_IRQ_MSK, inter_ee_src_ch_irq_msk, > 0x0000c020 + 0x1000 * GSI_EE_AP); > > REG(INTER_EE_SRC_EV_CH_IRQ_MSK, inter_ee_src_ev_ch_irq_msk, > 0x0000c024 + 0x1000 * GSI_EE_AP); > > -/* All other register offsets are relative to gsi->virt */ > - > static const u32 reg_ch_c_cntxt_0_fmask[] = { > [CHTYPE_PROTOCOL] = GENMASK(2, 0), > [CHTYPE_DIR] = BIT(3), > @@ -66,10 +62,6 @@ static const u32 reg_error_log_fmask[] = { > [ERR_EE] = GENMASK(31, 28), > }; > > -REG_FIELDS(ERROR_LOG, error_log, 0x0001f200 + 0x4000 * GSI_EE_AP); > - > -REG(ERROR_LOG_CLR, error_log_clr, 0x0001f210 + 0x4000 * GSI_EE_AP); > - > REG_STRIDE(CH_C_SCRATCH_0, ch_c_scratch_0, > 0x0001c060 + 0x4000 * GSI_EE_AP, 0x80); > > @@ -152,6 +144,7 @@ REG_FIELDS(GSI_STATUS, gsi_status, 0x0001f000 + 0x4000 * GSI_EE_AP); > > static const u32 reg_ch_cmd_fmask[] = { > [CH_CHID] = GENMASK(7, 0), > + /* Bits 8-23 reserved */ > [CH_OPCODE] = GENMASK(31, 24), > }; > > @@ -159,6 +152,7 @@ REG_FIELDS(CH_CMD, ch_cmd, 0x0001f008 + 0x4000 * GSI_EE_AP); > > static const u32 reg_ev_ch_cmd_fmask[] = { > [EV_CHID] = GENMASK(7, 0), > + /* Bits 8-23 reserved */ > [EV_OPCODE] = GENMASK(31, 24), > }; > [...] (offtopic) I hope all those gsi_reg-v*.c are autogenerated? They look pretty scary to be written and edited manually each time :D Thanks, Olek
Powered by blists - more mailing lists