Re: [trivial PATCH] treewide: Convert switch/case fallthrough; to break;

2020-09-09 Thread Gustavo A. R. Silva



On 9/9/20 15:06, Joe Perches wrote:
> fallthrough to a separate case/default label break; isn't very readable.
> 
> Convert pseudo-keyword fallthrough; statements to a simple break; when
> the next label is case or default and the only statement in the next
> label block is break;
> 
> Found using:
> 
> $ grep-2.5.4 -rP --include=*.[ch] -n 
> "fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" *
> 
> Miscellanea:
> 
> o Move or coalesce a couple label blocks above a default: block.
> 
> Signed-off-by: Joe Perches 

Acked-by: Gustavo A. R. Silva 

Thanks
--
Gustavo

> ---
> 
> Compiled allyesconfig x86-64 only.
> A few files for other arches were not compiled.
> 
>  arch/arm/mach-mmp/pm-pxa910.c |  2 +-
>  arch/arm64/kvm/handle_exit.c  |  2 +-
>  arch/mips/kernel/cpu-probe.c  |  2 +-
>  arch/mips/math-emu/cp1emu.c   |  2 +-
>  arch/s390/pci/pci.c   |  2 +-
>  crypto/tcrypt.c   |  4 ++--
>  drivers/ata/sata_mv.c |  2 +-
>  drivers/atm/lanai.c   |  2 +-
>  drivers/gpu/drm/i915/display/intel_sprite.c   |  2 +-
>  drivers/gpu/drm/nouveau/nvkm/engine/disp/hdmi.c   |  2 +-
>  drivers/hid/wacom_wac.c   |  2 +-
>  drivers/i2c/busses/i2c-i801.c |  2 +-
>  drivers/infiniband/ulp/rtrs/rtrs-clt.c| 14 +++---
>  drivers/infiniband/ulp/rtrs/rtrs-srv.c|  6 +++---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  2 +-
>  drivers/irqchip/irq-vic.c |  4 ++--
>  drivers/md/dm.c   |  2 +-
>  drivers/media/dvb-frontends/drxd_hard.c   |  2 +-
>  drivers/media/i2c/ov5640.c|  2 +-
>  drivers/media/i2c/ov6650.c|  5 ++---
>  drivers/media/i2c/smiapp/smiapp-core.c|  2 +-
>  drivers/media/i2c/tvp5150.c   |  2 +-
>  drivers/media/pci/ddbridge/ddbridge-core.c|  2 +-
>  drivers/media/usb/cpia2/cpia2_core.c  |  2 +-
>  drivers/mfd/iqs62x.c  |  3 +--
>  drivers/mmc/host/atmel-mci.c  |  2 +-
>  drivers/mtd/nand/raw/nandsim.c|  2 +-
>  drivers/net/ethernet/intel/e1000e/phy.c   |  2 +-
>  drivers/net/ethernet/intel/fm10k/fm10k_pf.c   |  2 +-
>  drivers/net/ethernet/intel/i40e/i40e_adminq.c |  2 +-
>  drivers/net/ethernet/intel/i40e/i40e_txrx.c   |  2 +-
>  drivers/net/ethernet/intel/iavf/iavf_txrx.c   |  2 +-
>  drivers/net/ethernet/intel/igb/e1000_phy.c|  2 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c|  2 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  2 +-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c|  2 +-
>  drivers/net/ethernet/intel/ixgbevf/vf.c   |  2 +-
>  drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c |  2 +-
>  drivers/net/ethernet/qlogic/qed/qed_mcp.c |  2 +-
>  drivers/net/ethernet/sfc/falcon/farch.c   |  2 +-
>  drivers/net/ethernet/sfc/farch.c  |  2 +-
>  drivers/net/phy/adin.c|  3 +--
>  drivers/net/usb/pegasus.c |  4 ++--
>  drivers/net/usb/usbnet.c  |  2 +-
>  drivers/net/wireless/ath/ath5k/eeprom.c   |  2 +-
>  drivers/net/wireless/mediatek/mt7601u/dma.c   |  8 
>  drivers/nvme/host/core.c  | 12 ++--
>  drivers/pcmcia/db1xxx_ss.c|  4 ++--
>  drivers/power/supply/abx500_chargalg.c|  2 +-
>  drivers/power/supply/charger-manager.c|  2 +-
>  drivers/rtc/rtc-pcf85063.c|  2 +-
>  drivers/s390/scsi/zfcp_fsf.c  |  2 +-
>  drivers/scsi/aic7xxx/aic79xx_core.c   |  4 ++--
>  drivers/scsi/aic94xx/aic94xx_tmf.c|  2 +-
>  drivers/scsi/lpfc/lpfc_sli.c  |  2 +-
>  drivers/scsi/smartpqi/smartpqi_init.c |  2 +-
>  drivers/scsi/sr.c |  2 +-
>  drivers/tty/serial/sunsu.c|  2

Re: [PATCH] arm64: KVM: hyp: debug-sr: Mark expected switch fall-throughs

2019-08-07 Thread Gustavo A. R. Silva



On 8/7/19 10:11 AM, Marc Zyngier wrote:

> 
> Already fixed (together with all the other KVM/arm warnings/bugs), and
> pull request sent to Paolo.
> 

Awesome. :)

Thanks, Marc.
--
Gustavo


[PATCH] arm64: KVM: hyp: debug-sr: Mark expected switch fall-throughs

2019-08-07 Thread Gustavo A. R. Silva
Mark switch cases where we are expecting to fall through.

This patch fixes the following warnings (Building: allmodconfig arm64):

arch/arm64/kvm/hyp/debug-sr.c:20:19: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:21:19: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:22:19: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:23:19: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:24:19: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:25:19: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:26:18: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:27:18: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:28:18: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:29:18: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:30:18: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:31:18: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:32:18: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:33:18: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
arch/arm64/kvm/hyp/debug-sr.c:34:18: warning: this statement may fall through 
[-Wimplicit-fallthrough=]

Signed-off-by: Gustavo A. R. Silva 
---
 arch/arm64/kvm/hyp/debug-sr.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm64/kvm/hyp/debug-sr.c b/arch/arm64/kvm/hyp/debug-sr.c
index 26781da3ad3e..0fc9872a1467 100644
--- a/arch/arm64/kvm/hyp/debug-sr.c
+++ b/arch/arm64/kvm/hyp/debug-sr.c
@@ -18,40 +18,70 @@
 #define save_debug(ptr,reg,nr) \
switch (nr) {   \
case 15:ptr[15] = read_debug(reg, 15);  \
+   /* Fall through */  \
case 14:ptr[14] = read_debug(reg, 14);  \
+   /* Fall through */  \
case 13:ptr[13] = read_debug(reg, 13);  \
+   /* Fall through */  \
case 12:ptr[12] = read_debug(reg, 12);  \
+   /* Fall through */  \
case 11:ptr[11] = read_debug(reg, 11);  \
+   /* Fall through */  \
case 10:ptr[10] = read_debug(reg, 10);  \
+   /* Fall through */  \
case 9: ptr[9] = read_debug(reg, 9);\
+   /* Fall through */  \
case 8: ptr[8] = read_debug(reg, 8);\
+   /* Fall through */  \
case 7: ptr[7] = read_debug(reg, 7);\
+   /* Fall through */  \
case 6: ptr[6] = read_debug(reg, 6);\
+   /* Fall through */  \
case 5: ptr[5] = read_debug(reg, 5);\
+   /* Fall through */  \
case 4: ptr[4] = read_debug(reg, 4);\
+   /* Fall through */  \
case 3: ptr[3] = read_debug(reg, 3);\
+   /* Fall through */  \
case 2: ptr[2] = read_debug(reg, 2);\
+   /* Fall through */  \
case 1: ptr[1] = read_debug(reg, 1);\
+   /* Fall through */  \
default:ptr[0] = read_debug(reg, 0);\
}
 
 #define restore_debug(ptr,reg,nr)  \
switch (nr) {   \
case 15:write_debug(ptr[15], reg, 15);  \
+   /* Fall through */  \
case 14:write_debug(ptr[14], reg, 14);  \
+   /* Fall through */  \
case 13:write_debug(ptr[13

[WRONG] KVM: arm/arm64: vgic: fix off-by-one bug in vgic_get_irq()

2018-12-19 Thread Gustavo A. R. Silva
Hi Marc,

This is wrong: commit 6022fcc0e87a0eb5e9a72b15ed70dd29ebcb7343

The above is not my original patch and it should not be tagged for stable,
as it introduces the same kind of bug I intended to fix:

array_index_nospec() can now return kvm->arch.vgic.nr_spis + 
VGIC_NR_PRIVATE_IRQS
and this is not what you want. So, in this case the following line of code
is just fine as it is:

intid = array_index_nospec(intid, kvm->arch.vgic.nr_spis + 
VGIC_NR_PRIVATE_IRQS); 


As the commit log says, my patch fixes:

commit 41b87599c74300027f305d7b34368ec558978ff2

not both:

commit 41b87599c74300027f305d7b34368ec558978ff2

and 

commit bea2ef803ade3359026d5d357348842bca9edcf1

If you want to apply the fix on top of bea2ef803ade3359026d5d357348842bca9edcf1
then you should apply this instead:

diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index bb1a83345741..e607547c7bb0 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -103,7 +103,7 @@ struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
 {
/* SGIs and PPIs */
if (intid <= VGIC_MAX_PRIVATE) {
-   intid = array_index_nospec(intid, VGIC_MAX_PRIVATE);
+   intid = array_index_nospec(intid, VGIC_MAX_PRIVATE + 1);
return >arch.vgic_cpu.private_irqs[intid];
}
 

The commit log should remain the same.

Thanks
--
Gustavo
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH] KVM: arm/arm64: vgic: fix off-by-one bug in vgic_get_irq()

2018-12-13 Thread Gustavo A. R. Silva
When using the nospec API, it should be taken into account that:

"...if the CPU speculates past the bounds check then
 * array_index_nospec() will clamp the index within the range of [0,
 * size)."

The above is part of the header for macro array_index_nospec() in
linux/nospec.h

Now, in this particular case, if intid evaluates to exactly VGIC_MAX_SPI
or to exaclty VGIC_MAX_PRIVATE, the array_index_nospec() macro ends up
returning VGIC_MAX_SPI - 1 or VGIC_MAX_PRIVATE - 1 respectively, instead
of VGIC_MAX_SPI or VGIC_MAX_PRIVATE, which, based on the original logic:

/* SGIs and PPIs */
if (intid <= VGIC_MAX_PRIVATE)
return >arch.vgic_cpu.private_irqs[intid];

/* SPIs */
if (intid <= VGIC_MAX_SPI)
return >arch.vgic.spis[intid - VGIC_NR_PRIVATE_IRQS];

are valid values for intid.

Fix this by calling array_index_nospec() macro with VGIC_MAX_PRIVATE + 1
and VGIC_MAX_SPI + 1 as arguments for its parameter size.

Fixes: 41b87599c743 ("KVM: arm/arm64: vgic: fix possible spectre-v1 in 
vgic_get_irq()")
Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva 
---
 virt/kvm/arm/vgic/vgic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 50e25438fb3c..5db40696055a 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -103,13 +103,13 @@ struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct 
kvm_vcpu *vcpu,
 {
/* SGIs and PPIs */
if (intid <= VGIC_MAX_PRIVATE) {
-   intid = array_index_nospec(intid, VGIC_MAX_PRIVATE);
+   intid = array_index_nospec(intid, VGIC_MAX_PRIVATE + 1);
return >arch.vgic_cpu.private_irqs[intid];
}
 
/* SPIs */
if (intid <= VGIC_MAX_SPI) {
-   intid = array_index_nospec(intid, VGIC_MAX_SPI);
+   intid = array_index_nospec(intid, VGIC_MAX_SPI + 1);
return >arch.vgic.spis[intid - VGIC_NR_PRIVATE_IRQS];
}
 
-- 
2.17.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH] KVM: arm: Use true and false for boolean values

2018-08-08 Thread Gustavo A. R. Silva
Return statements in functions returning bool should use true or false
instead of an integer value.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
 arch/arm/include/asm/kvm_emulate.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/kvm_emulate.h 
b/arch/arm/include/asm/kvm_emulate.h
index b0517ad..77121b7 100644
--- a/arch/arm/include/asm/kvm_emulate.h
+++ b/arch/arm/include/asm/kvm_emulate.h
@@ -119,7 +119,7 @@ static inline void vcpu_set_wfe_traps(struct kvm_vcpu *vcpu)
 
 static inline bool vcpu_mode_is_32bit(const struct kvm_vcpu *vcpu)
 {
-   return 1;
+   return true;
 }
 
 static inline unsigned long *vcpu_pc(struct kvm_vcpu *vcpu)
-- 
2.7.4

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm