| Message ID | 20210527072047.2572741-1-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: remove unused SBI helpers and exports | expand |
On Thu, May 27, 2021 at 12:51 PM Christoph Hellwig <hch@lst.de> wrote: > > Except for sbi_console_getchar and sbi_console_putchar none of > the SBI helpers are used in modular code, and many are not used at all > or only inside of sbi.c. Remove the dead wood. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > arch/riscv/include/asm/sbi.h | 17 ------ > arch/riscv/kernel/sbi.c | 103 +---------------------------------- > 2 files changed, 2 insertions(+), 118 deletions(-) > > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h > index 0d42693cb65e..c051556d5bbc 100644 > --- a/arch/riscv/include/asm/sbi.h > +++ b/arch/riscv/include/asm/sbi.h > @@ -101,9 +101,6 @@ long sbi_get_mvendorid(void); > long sbi_get_marchid(void); > long sbi_get_mimpid(void); > void sbi_set_timer(uint64_t stime_value); > -void sbi_shutdown(void); > -void sbi_clear_ipi(void); > -int sbi_send_ipi(const unsigned long *hart_mask); > int sbi_remote_fence_i(const unsigned long *hart_mask); > int sbi_remote_sfence_vma(const unsigned long *hart_mask, > unsigned long start, > @@ -113,20 +110,6 @@ int sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > unsigned long start, > unsigned long size, > unsigned long asid); > -int sbi_remote_hfence_gvma(const unsigned long *hart_mask, > - unsigned long start, > - unsigned long size); > -int sbi_remote_hfence_gvma_vmid(const unsigned long *hart_mask, > - unsigned long start, > - unsigned long size, > - unsigned long vmid); > -int sbi_remote_hfence_vvma(const unsigned long *hart_mask, > - unsigned long start, > - unsigned long size); > -int sbi_remote_hfence_vvma_asid(const unsigned long *hart_mask, > - unsigned long start, > - unsigned long size, > - unsigned long asid); > int sbi_probe_extension(int ext); > > /* Check if current SBI specification version is 0.1 or not */ > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c > index 7402a417f38e..c152c0c43358 100644 > --- a/arch/riscv/kernel/sbi.c > +++ b/arch/riscv/kernel/sbi.c > @@ -12,7 +12,6 @@ > > /* default SBI version is 0.1 */ > unsigned long sbi_spec_version __ro_after_init = SBI_SPEC_VERSION_DEFAULT; > -EXPORT_SYMBOL(sbi_spec_version); > > static void (*__sbi_set_timer)(uint64_t stime) __ro_after_init; > static int (*__sbi_send_ipi)(const unsigned long *hart_mask) __ro_after_init; > @@ -44,7 +43,6 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0, > > return ret; > } > -EXPORT_SYMBOL(sbi_ecall); This is used by the SBI HSM based CPU Idle driver which is under review. Atish's SBI PMU based driver also uses sbi_ecall(). > > int sbi_err_map_linux_errno(int err) > { > @@ -63,7 +61,6 @@ int sbi_err_map_linux_errno(int err) > return -ENOTSUPP; > }; > } > -EXPORT_SYMBOL(sbi_err_map_linux_errno); Even this is used by the SBI HSM based CPU Idle driver and SBI PMU based PMU driver which are under review. > > #ifdef CONFIG_RISCV_SBI_V01 > /** > @@ -98,22 +95,10 @@ EXPORT_SYMBOL(sbi_console_getchar); > * > * Return: None > */ > -void sbi_shutdown(void) > +static void sbi_shutdown(void) > { > sbi_ecall(SBI_EXT_0_1_SHUTDOWN, 0, 0, 0, 0, 0, 0, 0); > } > -EXPORT_SYMBOL(sbi_shutdown); This one is fine. > - > -/** > - * sbi_clear_ipi() - Clear any pending IPIs for the calling hart. > - * > - * Return: None > - */ > -void sbi_clear_ipi(void) > -{ > - sbi_ecall(SBI_EXT_0_1_CLEAR_IPI, 0, 0, 0, 0, 0, 0, 0); > -} > -EXPORT_SYMBOL(sbi_clear_ipi); Even this one is fine. > > /** > * __sbi_set_timer_v01() - Program the timer for next timer event. > @@ -364,11 +349,10 @@ void sbi_set_timer(uint64_t stime_value) > * > * Return: 0 on success, appropriate linux error code otherwise. > */ > -int sbi_send_ipi(const unsigned long *hart_mask) > +static int sbi_send_ipi(const unsigned long *hart_mask) > { > return __sbi_send_ipi(hart_mask); > } > -EXPORT_SYMBOL(sbi_send_ipi); This is fine as well. > > /** > * sbi_remote_fence_i() - Execute FENCE.I instruction on given remote harts. > @@ -381,7 +365,6 @@ int sbi_remote_fence_i(const unsigned long *hart_mask) > return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_FENCE_I, > hart_mask, 0, 0, 0, 0); > } > -EXPORT_SYMBOL(sbi_remote_fence_i); > > /** > * sbi_remote_sfence_vma() - Execute SFENCE.VMA instructions on given remote > @@ -399,7 +382,6 @@ int sbi_remote_sfence_vma(const unsigned long *hart_mask, > return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_SFENCE_VMA, > hart_mask, start, size, 0, 0); > } > -EXPORT_SYMBOL(sbi_remote_sfence_vma); > > /** > * sbi_remote_sfence_vma_asid() - Execute SFENCE.VMA instructions on given > @@ -420,86 +402,6 @@ int sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, > return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID, > hart_mask, start, size, asid, 0); > } > -EXPORT_SYMBOL(sbi_remote_sfence_vma_asid); > - > -/** > - * sbi_remote_hfence_gvma() - Execute HFENCE.GVMA instructions on given remote > - * harts for the specified guest physical address range. > - * @hart_mask: A cpu mask containing all the target harts. > - * @start: Start of the guest physical address > - * @size: Total size of the guest physical address range. > - * > - * Return: None > - */ > -int sbi_remote_hfence_gvma(const unsigned long *hart_mask, > - unsigned long start, > - unsigned long size) > -{ > - return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA, > - hart_mask, start, size, 0, 0); > -} > -EXPORT_SYMBOL_GPL(sbi_remote_hfence_gvma); > - > -/** > - * sbi_remote_hfence_gvma_vmid() - Execute HFENCE.GVMA instructions on given > - * remote harts for a guest physical address range belonging to a specific VMID. > - * > - * @hart_mask: A cpu mask containing all the target harts. > - * @start: Start of the guest physical address > - * @size: Total size of the guest physical address range. > - * @vmid: The value of guest ID (VMID). > - * > - * Return: 0 if success, Error otherwise. > - */ > -int sbi_remote_hfence_gvma_vmid(const unsigned long *hart_mask, > - unsigned long start, > - unsigned long size, > - unsigned long vmid) > -{ > - return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID, > - hart_mask, start, size, vmid, 0); > -} > -EXPORT_SYMBOL(sbi_remote_hfence_gvma_vmid); > - > -/** > - * sbi_remote_hfence_vvma() - Execute HFENCE.VVMA instructions on given remote > - * harts for the current guest virtual address range. > - * @hart_mask: A cpu mask containing all the target harts. > - * @start: Start of the current guest virtual address > - * @size: Total size of the current guest virtual address range. > - * > - * Return: None > - */ > -int sbi_remote_hfence_vvma(const unsigned long *hart_mask, > - unsigned long start, > - unsigned long size) > -{ > - return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA, > - hart_mask, start, size, 0, 0); > -} > -EXPORT_SYMBOL(sbi_remote_hfence_vvma); > - > -/** > - * sbi_remote_hfence_vvma_asid() - Execute HFENCE.VVMA instructions on given > - * remote harts for current guest virtual address range belonging to a specific > - * ASID. > - * > - * @hart_mask: A cpu mask containing all the target harts. > - * @start: Start of the current guest virtual address > - * @size: Total size of the current guest virtual address range. > - * @asid: The value of address space identifier (ASID). > - * > - * Return: None > - */ > -int sbi_remote_hfence_vvma_asid(const unsigned long *hart_mask, > - unsigned long start, > - unsigned long size, > - unsigned long asid) > -{ > - return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID, > - hart_mask, start, size, asid, 0); > -} > -EXPORT_SYMBOL(sbi_remote_hfence_vvma_asid); All sbi_remote_hfence_xyz() calls are used by KVM RISC-V module. > > /** > * sbi_probe_extension() - Check if an SBI extension ID is supported or not. > @@ -519,7 +421,6 @@ int sbi_probe_extension(int extid) > > return -ENOTSUPP; > } > -EXPORT_SYMBOL(sbi_probe_extension); This is used by SBI HSM based cpuidle driver. This is also used by SBI PMU based PMU driver. > > static long __sbi_base_ecall(int fid) > { > -- > 2.30.2 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv Regards, Anup
On 27/05/21 15:10, Anup Patel wrote: >> - >> -/** >> - * sbi_remote_hfence_vvma_asid() - Execute HFENCE.VVMA instructions on given >> - * remote harts for current guest virtual address range belonging to a specific >> - * ASID. >> - * >> - * @hart_mask: A cpu mask containing all the target harts. >> - * @start: Start of the current guest virtual address >> - * @size: Total size of the current guest virtual address range. >> - * @asid: The value of address space identifier (ASID). >> - * >> - * Return: None >> - */ >> -int sbi_remote_hfence_vvma_asid(const unsigned long *hart_mask, >> - unsigned long start, >> - unsigned long size, >> - unsigned long asid) >> -{ >> - return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID, >> - hart_mask, start, size, asid, 0); >> -} >> -EXPORT_SYMBOL(sbi_remote_hfence_vvma_asid); > All sbi_remote_hfence_xyz() calls are used by KVM RISC-V module. > Why aren't all these added by the series that add the module (not talking about KVM only)? Paolo
On Thu, May 27, 2021 at 03:15:09PM +0200, Paolo Bonzini wrote: >>> -EXPORT_SYMBOL(sbi_remote_hfence_vvma_asid); >> All sbi_remote_hfence_xyz() calls are used by KVM RISC-V module. >> > > Why aren't all these added by the series that add the module (not talking > about KVM only)? Yes, that is the way to do it.
On Thu, May 27, 2021 at 6:45 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 27/05/21 15:10, Anup Patel wrote: > >> - > >> -/** > >> - * sbi_remote_hfence_vvma_asid() - Execute HFENCE.VVMA instructions on given > >> - * remote harts for current guest virtual address range belonging to a specific > >> - * ASID. > >> - * > >> - * @hart_mask: A cpu mask containing all the target harts. > >> - * @start: Start of the current guest virtual address > >> - * @size: Total size of the current guest virtual address range. > >> - * @asid: The value of address space identifier (ASID). > >> - * > >> - * Return: None > >> - */ > >> -int sbi_remote_hfence_vvma_asid(const unsigned long *hart_mask, > >> - unsigned long start, > >> - unsigned long size, > >> - unsigned long asid) > >> -{ > >> - return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID, > >> - hart_mask, start, size, asid, 0); > >> -} > >> -EXPORT_SYMBOL(sbi_remote_hfence_vvma_asid); > > All sbi_remote_hfence_xyz() calls are used by KVM RISC-V module. > > > > Why aren't all these added by the series that add the module (not > talking about KVM only)? These calls are part of SBI v0.2 spec hence were added as part of SBI v0.2 support patches. Regards, Anup > > Paolo >
On Thu, May 27, 2021 at 6:47 PM Christoph Hellwig <hch@lst.de> wrote: > > On Thu, May 27, 2021 at 03:15:09PM +0200, Paolo Bonzini wrote: > >>> -EXPORT_SYMBOL(sbi_remote_hfence_vvma_asid); > >> All sbi_remote_hfence_xyz() calls are used by KVM RISC-V module. > >> > > > > Why aren't all these added by the series that add the module (not talking > > about KVM only)? > > Yes, that is the way to do it. Actually SBI v0.2 RFENCE extension defines all these calls hence Atish added it as part of SBI v0.2 patches. All sbi_remote_hfence_xyz() are primarily for hypervisors only. Regards, Anup
On Thu, 27 May 2021 06:24:10 PDT (-0700), anup@brainfault.org wrote: > On Thu, May 27, 2021 at 6:47 PM Christoph Hellwig <hch@lst.de> wrote: >> >> On Thu, May 27, 2021 at 03:15:09PM +0200, Paolo Bonzini wrote: >> >>> -EXPORT_SYMBOL(sbi_remote_hfence_vvma_asid); >> >> All sbi_remote_hfence_xyz() calls are used by KVM RISC-V module. >> >> >> > >> > Why aren't all these added by the series that add the module (not talking >> > about KVM only)? >> >> Yes, that is the way to do it. > > Actually SBI v0.2 RFENCE extension defines all these calls hence Atish > added it as part of SBI v0.2 patches. > > All sbi_remote_hfence_xyz() are primarily for hypervisors only. We took them early as a way to help make it easier to handle the KVM patches, as at the time these were the only things touching files outside of arch/riscv/kvm. I know the general rule is to only take code with callers, but these are defined by the SBI so I figured we'd make everyone's lives a bit easier by just keeping them around.
On Sat, May 29, 2021 at 04:48:56PM -0700, Palmer Dabbelt wrote: > I know the general rule is to only take code with callers, but these are > defined by the SBI so I figured we'd make everyone's lives a bit easier by > just keeping them around. Keeping unused code never helps anyone. Adding code without any known users like some of the others is even worse. And given that sbi_ecall is exported (and just a trivial inline asm wrapper) there is no point in having most of the helpers at all, but that is another story. So for now I'd suggest to merge my patch, and changing any new code to just use sbi_ecall instead of the remove wrappes as there is no point in bloating the core kernel with trivial wrappers only used by a single module.
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h index 0d42693cb65e..c051556d5bbc 100644 --- a/arch/riscv/include/asm/sbi.h +++ b/arch/riscv/include/asm/sbi.h @@ -101,9 +101,6 @@ long sbi_get_mvendorid(void); long sbi_get_marchid(void); long sbi_get_mimpid(void); void sbi_set_timer(uint64_t stime_value); -void sbi_shutdown(void); -void sbi_clear_ipi(void); -int sbi_send_ipi(const unsigned long *hart_mask); int sbi_remote_fence_i(const unsigned long *hart_mask); int sbi_remote_sfence_vma(const unsigned long *hart_mask, unsigned long start, @@ -113,20 +110,6 @@ int sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, unsigned long start, unsigned long size, unsigned long asid); -int sbi_remote_hfence_gvma(const unsigned long *hart_mask, - unsigned long start, - unsigned long size); -int sbi_remote_hfence_gvma_vmid(const unsigned long *hart_mask, - unsigned long start, - unsigned long size, - unsigned long vmid); -int sbi_remote_hfence_vvma(const unsigned long *hart_mask, - unsigned long start, - unsigned long size); -int sbi_remote_hfence_vvma_asid(const unsigned long *hart_mask, - unsigned long start, - unsigned long size, - unsigned long asid); int sbi_probe_extension(int ext); /* Check if current SBI specification version is 0.1 or not */ diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c index 7402a417f38e..c152c0c43358 100644 --- a/arch/riscv/kernel/sbi.c +++ b/arch/riscv/kernel/sbi.c @@ -12,7 +12,6 @@ /* default SBI version is 0.1 */ unsigned long sbi_spec_version __ro_after_init = SBI_SPEC_VERSION_DEFAULT; -EXPORT_SYMBOL(sbi_spec_version); static void (*__sbi_set_timer)(uint64_t stime) __ro_after_init; static int (*__sbi_send_ipi)(const unsigned long *hart_mask) __ro_after_init; @@ -44,7 +43,6 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0, return ret; } -EXPORT_SYMBOL(sbi_ecall); int sbi_err_map_linux_errno(int err) { @@ -63,7 +61,6 @@ int sbi_err_map_linux_errno(int err) return -ENOTSUPP; }; } -EXPORT_SYMBOL(sbi_err_map_linux_errno); #ifdef CONFIG_RISCV_SBI_V01 /** @@ -98,22 +95,10 @@ EXPORT_SYMBOL(sbi_console_getchar); * * Return: None */ -void sbi_shutdown(void) +static void sbi_shutdown(void) { sbi_ecall(SBI_EXT_0_1_SHUTDOWN, 0, 0, 0, 0, 0, 0, 0); } -EXPORT_SYMBOL(sbi_shutdown); - -/** - * sbi_clear_ipi() - Clear any pending IPIs for the calling hart. - * - * Return: None - */ -void sbi_clear_ipi(void) -{ - sbi_ecall(SBI_EXT_0_1_CLEAR_IPI, 0, 0, 0, 0, 0, 0, 0); -} -EXPORT_SYMBOL(sbi_clear_ipi); /** * __sbi_set_timer_v01() - Program the timer for next timer event. @@ -364,11 +349,10 @@ void sbi_set_timer(uint64_t stime_value) * * Return: 0 on success, appropriate linux error code otherwise. */ -int sbi_send_ipi(const unsigned long *hart_mask) +static int sbi_send_ipi(const unsigned long *hart_mask) { return __sbi_send_ipi(hart_mask); } -EXPORT_SYMBOL(sbi_send_ipi); /** * sbi_remote_fence_i() - Execute FENCE.I instruction on given remote harts. @@ -381,7 +365,6 @@ int sbi_remote_fence_i(const unsigned long *hart_mask) return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_FENCE_I, hart_mask, 0, 0, 0, 0); } -EXPORT_SYMBOL(sbi_remote_fence_i); /** * sbi_remote_sfence_vma() - Execute SFENCE.VMA instructions on given remote @@ -399,7 +382,6 @@ int sbi_remote_sfence_vma(const unsigned long *hart_mask, return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_SFENCE_VMA, hart_mask, start, size, 0, 0); } -EXPORT_SYMBOL(sbi_remote_sfence_vma); /** * sbi_remote_sfence_vma_asid() - Execute SFENCE.VMA instructions on given @@ -420,86 +402,6 @@ int sbi_remote_sfence_vma_asid(const unsigned long *hart_mask, return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_SFENCE_VMA_ASID, hart_mask, start, size, asid, 0); } -EXPORT_SYMBOL(sbi_remote_sfence_vma_asid); - -/** - * sbi_remote_hfence_gvma() - Execute HFENCE.GVMA instructions on given remote - * harts for the specified guest physical address range. - * @hart_mask: A cpu mask containing all the target harts. - * @start: Start of the guest physical address - * @size: Total size of the guest physical address range. - * - * Return: None - */ -int sbi_remote_hfence_gvma(const unsigned long *hart_mask, - unsigned long start, - unsigned long size) -{ - return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA, - hart_mask, start, size, 0, 0); -} -EXPORT_SYMBOL_GPL(sbi_remote_hfence_gvma); - -/** - * sbi_remote_hfence_gvma_vmid() - Execute HFENCE.GVMA instructions on given - * remote harts for a guest physical address range belonging to a specific VMID. - * - * @hart_mask: A cpu mask containing all the target harts. - * @start: Start of the guest physical address - * @size: Total size of the guest physical address range. - * @vmid: The value of guest ID (VMID). - * - * Return: 0 if success, Error otherwise. - */ -int sbi_remote_hfence_gvma_vmid(const unsigned long *hart_mask, - unsigned long start, - unsigned long size, - unsigned long vmid) -{ - return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_HFENCE_GVMA_VMID, - hart_mask, start, size, vmid, 0); -} -EXPORT_SYMBOL(sbi_remote_hfence_gvma_vmid); - -/** - * sbi_remote_hfence_vvma() - Execute HFENCE.VVMA instructions on given remote - * harts for the current guest virtual address range. - * @hart_mask: A cpu mask containing all the target harts. - * @start: Start of the current guest virtual address - * @size: Total size of the current guest virtual address range. - * - * Return: None - */ -int sbi_remote_hfence_vvma(const unsigned long *hart_mask, - unsigned long start, - unsigned long size) -{ - return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA, - hart_mask, start, size, 0, 0); -} -EXPORT_SYMBOL(sbi_remote_hfence_vvma); - -/** - * sbi_remote_hfence_vvma_asid() - Execute HFENCE.VVMA instructions on given - * remote harts for current guest virtual address range belonging to a specific - * ASID. - * - * @hart_mask: A cpu mask containing all the target harts. - * @start: Start of the current guest virtual address - * @size: Total size of the current guest virtual address range. - * @asid: The value of address space identifier (ASID). - * - * Return: None - */ -int sbi_remote_hfence_vvma_asid(const unsigned long *hart_mask, - unsigned long start, - unsigned long size, - unsigned long asid) -{ - return __sbi_rfence(SBI_EXT_RFENCE_REMOTE_HFENCE_VVMA_ASID, - hart_mask, start, size, asid, 0); -} -EXPORT_SYMBOL(sbi_remote_hfence_vvma_asid); /** * sbi_probe_extension() - Check if an SBI extension ID is supported or not. @@ -519,7 +421,6 @@ int sbi_probe_extension(int extid) return -ENOTSUPP; } -EXPORT_SYMBOL(sbi_probe_extension); static long __sbi_base_ecall(int fid) {
Except for sbi_console_getchar and sbi_console_putchar none of the SBI helpers are used in modular code, and many are not used at all or only inside of sbi.c. Remove the dead wood. Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/riscv/include/asm/sbi.h | 17 ------ arch/riscv/kernel/sbi.c | 103 +---------------------------------- 2 files changed, 2 insertions(+), 118 deletions(-)