All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH net-next] net: remove redundant input checks in SIOCSIFTXQLEN case of dev_ifsioc
From: David Miller @ 2018-07-23 21:00 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: tariqt, netdev, eranbe
In-Reply-To: <CAM_iQpUo-Rp5OhbtLVGOG_HPYZYuPXeqVMZ3ip=VMUJ42Cg4PQ@mail.gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Mon, 23 Jul 2018 13:37:22 -0700

> On Sun, Jul 22, 2018 at 12:29 AM Tariq Toukan <tariqt@mellanox.com> wrote:
>>
>>
>>
>> On 19/07/2018 8:21 PM, Cong Wang wrote:
>> > On Thu, Jul 19, 2018 at 7:50 AM Tariq Toukan <tariqt@mellanox.com> wrote:
>> >> --- a/net/core/dev_ioctl.c
>> >> +++ b/net/core/dev_ioctl.c
>> >> @@ -282,14 +282,7 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
>> >>                  return dev_mc_del_global(dev, ifr->ifr_hwaddr.sa_data);
>> >>
>> >>          case SIOCSIFTXQLEN:
>> >> -               if (ifr->ifr_qlen < 0)
>> >> -                       return -EINVAL;
>> >
>> > Are you sure we can remove this if check too?
>> >
>> > The other one is safe to remove.
>> >
>>
>> Hmm, let's see:
>> dev_change_tx_queue_len gets unsigned long new_len, any negative value
>> passed is interpreted as a very large number, then we test:
>> if (new_len != (unsigned int)new_len)
>>
>> This test returns true if range of unsigned long is larger than range of
>> unsigned int. AFAIK these ranges are Arch dependent and there is no
>> guarantee this holds.
> 
> I am not sure either, you probably have to give it a test.
> And at least, explain it in changelog if you still want to remove it.

On 64-bit we will fail with -ERANGE.  The 32-bit int ifr_qlen will be sign
extended to 64-bits when it is passed into dev_change_tx_queue_len(). And
then for negative values this test triggers:

	if (new_len != (unsigned int)new_len)
		return -ERANGE;

because:
	if (0xffffffffWHATEVER != 0x00000000WHATEVER)

On 32-bit the signed value will be accepted, changing behavior.

I think, therefore, that the < 0 check should be retained.

Thank you.

^ permalink raw reply

* Re: [PATCH] x86/svm: Drop the suggestion of Long Mode Segment Limit support
From: Boris Ostrovsky @ 2018-07-23 22:04 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel
  Cc: Sergey Dyasli, Wei Liu, Jan Beulich, Suravee Suthikulpanit,
	Brian Woods, Roger Pau Monné
In-Reply-To: <c8e0c512-8dae-f7ad-81d0-4b6a33911e59@oracle.com>

On 07/23/2018 06:02 PM, Boris Ostrovsky wrote:
> On 07/23/2018 10:42 AM, Andrew Cooper wrote:
>> Because of a bug in 2010, LMSL support didn't functioned in Xen.
>>
>> c/s f2c608444 noticed but avoided fixing the issue for migration reasons.  In
>> addition to migration problems, changes to the segmentation logic for
>> emulation would be needed before the feature could be enabled.
>>
>> This feature is entirely unused by operating systems (probably owing to its
>> semantics which only cover half the segment registers), and no one has
>> commented on its absence from Xen.  As supporting it would involve a large
>> amount of effort, it seems better to remove the code entirely.
>>
>> If someone finds a valid usecase, we can resurrecting the code and
>> implementing the remaining parts, but I doubt anyone will.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> CC: Brian Woods <brian.woods@amd.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Sergey Dyasli <sergey.dyasli@citrix.com>
>> ---
>>  xen/arch/x86/hvm/hvm.c          |  3 ---
>>  xen/arch/x86/hvm/svm/svm.c      | 23 -----------------------
>>  xen/arch/x86/pv/emul-priv-op.c  |  2 +-
>>  xen/include/asm-x86/hvm/hvm.h   |  1 -
>>  xen/include/asm-x86/msr-index.h |  4 +---
>>  5 files changed, 2 insertions(+), 31 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index c099c61..67b99af 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -925,9 +925,6 @@ const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
>>      if ( (value & EFER_SVME) && (!p->extd.svm || !nestedhvm_enabled(d)) )
>>          return "SVME without nested virt";
>>  
>> -    if ( (value & EFER_LMSLE) && !cpu_has_lmsl )
>> -        return "LMSLE without support";
>> -
> So we are silently dropping the bit? Should we still warn?


Nevermind.

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>


>
> -boris
>
>
>>      if ( (value & EFER_FFXSE) && !p->extd.ffxsr )
>>          return "FFXSE without feature";
>>  


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply

* Re: bisected: 4.18-rc* regression: x86-32 troubles (with timers?)
From: Daniel Borkmann @ 2018-07-23 22:04 UTC (permalink / raw)
  To: Meelis Roos; +Cc: Arnd Bergmann, Linux Kernel list, Networking
In-Reply-To: <alpine.LRH.2.21.1807232322430.11834@math.ut.ee>

On 07/23/2018 10:34 PM, Meelis Roos wrote:
>>>> Now this seems more relevant:
>>>>
>>>> mroos@rx100s2:~/linux$ nice git bisect good
>>>> 24dea04767e6e5175f4750770281b0c17ac6a2fb is the first bad commit
>>>> commit 24dea04767e6e5175f4750770281b0c17ac6a2fb
>>>> Author: Daniel Borkmann <daniel@iogearbox.net>
>>>> Date:   Fri May 4 01:08:23 2018 +0200
>>>>
>>>>     bpf, x32: remove ld_abs/ld_ind
>>>>
>>>>     Since LD_ABS/LD_IND instructions are now removed from the core and
>>>>     reimplemented through a combination of inlined BPF instructions and
>>>>     a slow-path helper, we can get rid of the complexity from x32 JIT.
>>>
>>> This does seem much more likely than the previous bisection, given
>>> that you ended up in an x86-32 specific commit (the subject says x32,
>>> but that is a mistake). I also checked that systemd indeed does
>>> call into bpf in a number of places, possibly for the journald socket.
>>>
>>> OTOH, it's still hard to tell how that commit can have ended up
>>> corrupting the clock read function in systemd. To cross-check,
>>> could you try reverting that commit on the latest kernel and see
>>> if it still works?
>>
>> I would be curious as well about that whether revert would make it
>> work. What's the value of sysctl net.core.bpf_jit_enable ? Does it
>> change anything if you set it to 0 (only interpreter) or 1 (JIT
>> enabled). Seems a bit strange to me that bisect ended at this commit
>> given the issue you have. The JIT itself was also new in this window
>> fwiw. In any case some more debug info would be great to have.
> 
> net.core.bpf_jit_enable is 1.
> 
> Since it breaks bootup, I can not easily change the value at runtime (it 
> would be postfactum). Do you mean changing the 
> CONFIG_BPF_JIT_ALWAYS_ON=y option?

Yeah if you chance it to N, and don't have any fixed /etc/sysctl.conf
setting, then you'll boot up with interpreter first (net.core.bpf_jit_enable
as 0). Curious whether that works just fine for you.

> Anyway, I started compile of v4.18-rc5 that was the latest I tested, 
> with the commit in question reverted. Will see if I can test tomorrow 
> morning. But I will leave tomorrow for a week and can only test further 
> things if they happen to boot fine (no manual reboot possible for a 
> week).

Ok, thanks, please keep us posted on the outcome with the revert. Right
now I would doubt it's related resp. changes anything on the issue, but
lets see.

Thanks,
Daniel

^ permalink raw reply

* [Intel-wired-lan] [PATCH v2 00/13] Bug fixes for ice
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan

This patch set fixes multiple bugs for the ice driver.

Anirudh Venkataramanan (4):
  ice: Fix static analyser warning
  ice: Cleanup magic number
  ice: Fix bugs in control queue processing
  ice: Fix couple of null pointer dereference issues

Brett Creeley (2):
  ice: Don't explicitly set port_vlan_bits to 0
  ice: Set VLAN flags correctly

Bruce Allan (3):
  ice: Fix missing shift
  ice: Update to interrupts enabled in OICR
  ice: Change struct members from bool to u8

Jacob Keller (2):
  ice: Report stats for allocated queues via ethtool stats
  ice: Use order_base_2 to calculate higher power of 2

Jesse Brandeburg (1):
  ice: Fix potential return of uninitialized value

Preethi Banala (1):
  ice: Clean control queues only when they are initialized

 drivers/net/ethernet/intel/ice/ice.h            | 15 ++--
 drivers/net/ethernet/intel/ice/ice_adminq_cmd.h |  1 +
 drivers/net/ethernet/intel/ice/ice_controlq.c   | 29 +++++---
 drivers/net/ethernet/intel/ice/ice_ethtool.c    | 52 ++++++++++----
 drivers/net/ethernet/intel/ice/ice_hw_autogen.h |  8 ---
 drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h  |  1 +
 drivers/net/ethernet/intel/ice/ice_main.c       | 91 +++++++++++++++----------
 drivers/net/ethernet/intel/ice/ice_switch.c     |  3 +-
 drivers/net/ethernet/intel/ice/ice_switch.h     |  6 +-
 drivers/net/ethernet/intel/ice/ice_txrx.h       |  2 +-
 drivers/net/ethernet/intel/ice/ice_type.h       | 16 ++---
 11 files changed, 141 insertions(+), 83 deletions(-)

-- 
2.14.3


^ permalink raw reply

* [Intel-wired-lan] [PATCH v2 01/13] ice: Fix static analyser warning
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

This patch fixes one of the smatch (a static analysis tool) warnings
reported by Dan Carpenter. ICE_LG_ACT_GENERIC_OFFSET_M is the right mask
to use but it appears that ICE_LG_ACT_GENERIC_VALUE_M was used instead,
which resulted in the following smatch warning:

ice_add_marker_act() warn: odd binop '0x380000 & 0x7fff8'

Additionally, use a new define ICE_LG_ACT_GENERIC_OFF_RX_DESC_PROF_IDX
instead of magic number '7'.

Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_adminq_cmd.h | 1 +
 drivers/net/ethernet/intel/ice/ice_switch.c     | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
index 7541ec2270b3..6d3e11659ba5 100644
--- a/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
+++ b/drivers/net/ethernet/intel/ice/ice_adminq_cmd.h
@@ -594,6 +594,7 @@ struct ice_sw_rule_lg_act {
 #define ICE_LG_ACT_GENERIC_OFFSET_M	(0x7 << ICE_LG_ACT_GENERIC_OFFSET_S)
 #define ICE_LG_ACT_GENERIC_PRIORITY_S	22
 #define ICE_LG_ACT_GENERIC_PRIORITY_M	(0x7 << ICE_LG_ACT_GENERIC_PRIORITY_S)
+#define ICE_LG_ACT_GENERIC_OFF_RX_DESC_PROF_IDX	7
 
 	/* Action = 7 - Set Stat count */
 #define ICE_LG_ACT_STAT_COUNT		0x7
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.c b/drivers/net/ethernet/intel/ice/ice_switch.c
index 723d15f1e90b..750d2051a976 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.c
+++ b/drivers/net/ethernet/intel/ice/ice_switch.c
@@ -645,7 +645,8 @@ ice_add_marker_act(struct ice_hw *hw, struct ice_fltr_mgmt_list_entry *m_ent,
 	act |= (1 << ICE_LG_ACT_GENERIC_VALUE_S) & ICE_LG_ACT_GENERIC_VALUE_M;
 	lg_act->pdata.lg_act.act[1] = cpu_to_le32(act);
 
-	act = (7 << ICE_LG_ACT_GENERIC_OFFSET_S) & ICE_LG_ACT_GENERIC_VALUE_M;
+	act = (ICE_LG_ACT_GENERIC_OFF_RX_DESC_PROF_IDX <<
+	       ICE_LG_ACT_GENERIC_OFFSET_S) & ICE_LG_ACT_GENERIC_OFFSET_M;
 
 	/* Third action Marker value */
 	act |= ICE_LG_ACT_GENERIC;
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 02/13] ice: Cleanup magic number
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

Use define for the unit size shift of the Rx LAN context descriptor base
address instead of the magic number 7.

Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h | 1 +
 drivers/net/ethernet/intel/ice/ice_main.c      | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h b/drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h
index d23a91665b46..068dbc740b76 100644
--- a/drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h
+++ b/drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h
@@ -265,6 +265,7 @@ enum ice_rx_flex_desc_status_error_0_bits {
 struct ice_rlan_ctx {
 	u16 head;
 	u16 cpuid; /* bigger than needed, see above for reason */
+#define ICE_RLAN_BASE_S 7
 	u64 base;
 	u16 qlen;
 #define ICE_RLAN_CTX_DBUF_S 7
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 5299caf55a7f..78756853ade1 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3986,7 +3986,7 @@ static int ice_setup_rx_ctx(struct ice_ring *ring)
 	/* clear the context structure first */
 	memset(&rlan_ctx, 0, sizeof(rlan_ctx));
 
-	rlan_ctx.base = ring->dma >> 7;
+	rlan_ctx.base = ring->dma >> ICE_RLAN_BASE_S;
 
 	rlan_ctx.qlen = ring->count;
 
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 03/13] ice: Fix missing shift
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

From: Bruce Allan <bruce.w.allan@intel.com>

The ITR index of the interrupt cause for OICR and FW interrupts is not
being shifted correctly so when masked with PFINT_*_CTL_ITR_INDX_M it
will always be zero.  Luckily, ICE_RX_ITR is zero anyway so the end result
is the same, but the shift should be fixed in case the value of ICE_RX_ITR
ever changes or it is substituted with another value that needs to be
shifted properly.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
[Anirudh Venkataramanan <anirudh.venkataramanan@intel.com> cleaned up commit message]
---
 drivers/net/ethernet/intel/ice/ice_main.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 78756853ade1..450911345144 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -2058,15 +2058,17 @@ static int ice_req_irq_msix_misc(struct ice_pf *pf)
 skip_req_irq:
 	ice_ena_misc_vector(pf);
 
-	val = (pf->oicr_idx & PFINT_OICR_CTL_MSIX_INDX_M) |
-	      (ICE_RX_ITR & PFINT_OICR_CTL_ITR_INDX_M) |
-	      PFINT_OICR_CTL_CAUSE_ENA_M;
+	val = ((pf->oicr_idx & PFINT_OICR_CTL_MSIX_INDX_M) |
+	       ((ICE_RX_ITR << PFINT_OICR_CTL_ITR_INDX_S) &
+		PFINT_OICR_CTL_ITR_INDX_M) |
+	       PFINT_OICR_CTL_CAUSE_ENA_M);
 	wr32(hw, PFINT_OICR_CTL, val);
 
 	/* This enables Admin queue Interrupt causes */
-	val = (pf->oicr_idx & PFINT_FW_CTL_MSIX_INDX_M) |
-	      (ICE_RX_ITR & PFINT_FW_CTL_ITR_INDX_M) |
-	      PFINT_FW_CTL_CAUSE_ENA_M;
+	val = ((pf->oicr_idx & PFINT_FW_CTL_MSIX_INDX_M) |
+	       ((ICE_RX_ITR << PFINT_FW_CTL_ITR_INDX_S) &
+		PFINT_FW_CTL_ITR_INDX_M) |
+	       PFINT_FW_CTL_CAUSE_ENA_M);
 	wr32(hw, PFINT_FW_CTL, val);
 
 	itr_gran = hw->itr_gran_200;
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 04/13] ice: Report stats for allocated queues via ethtool stats
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

It is not safe to have the string table for statistics change order or
size over the lifetime of a given netdevice. This is because of the
nature of the 3-step process for obtaining stats. First, userspace
performs a request for the size of the strings table. Second it performs
a separate request for the strings themselves, after allocating space
for the table. Third, it requests the stats themselves, also allocating
space for the table.

If the size decreased, there is potential to see garbage data or stats
values. In the worst case, we could potentially see stats values become
mis-aligned with their strings, so that it looks like a statistic is
being reported differently than it actually is.

Even worse, if the size increased, there is potential that the strings
table or stats table was not allocated large enough and the stats code
could access and write to memory it should not, potentially resulting in
undefined behavior and system crashes.

It isn't even safe if the size always changes under the RTNL lock. This
is because the calls take place over multiple userspace commands, so it
is not possible to hold the RTNL lock for the entire duration of
obtaining strings and stats. Further, not all consumers of the ethtool
API are the userspace ethtool program, and it is possible that one
assumes the strings will not change (valid under the current contract),
and thus only requests the stats values when requesting stats in a loop.

Finally, it's not possible in the general case to detect when the size
changes, because it is quite possible that one value which could impact
the stat size increased, while another decreased. This would result in
the same total number of stats, but reordering them so that stats no
longer line up with the strings they belong to. Since only size changes
aren't enough, we would need some sort of hash or token to determine
when the strings no longer match. This would require extending the
ethtool stats commands, but there is no more space in the relevant
structures.

The real solution to resolve this would be to add a completely new API
for stats, probably over netlink.

In the ice driver, the only thing impacting the stats that is not
constant is the number of queues. Instead of reporting stats for each
used queue, report stats for each allocated queue. We do not change the
number of queues allocated for a given netdevice, as we pass this into
the alloc_etherdev_mq() function to set the num_tx_queues and
num_rx_queues.

This resolves the potential bugs at the slight cost of displaying many
queue statistics which will not be activated.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
[Anirudh Venkataramanan <anirudh.venkataramanan@intel.com> cleaned up commit message]
---
 drivers/net/ethernet/intel/ice/ice.h         |  7 ++++
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 52 +++++++++++++++++++++-------
 2 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index d8b5fff581e7..ed071ea75f20 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -89,6 +89,13 @@ extern const char ice_drv_ver[];
 #define ice_for_each_rxq(vsi, i) \
 	for ((i) = 0; (i) < (vsi)->num_rxq; (i)++)
 
+/* Macros for each allocated tx/rx ring whether used or not in a VSI */
+#define ice_for_each_alloc_txq(vsi, i) \
+	for ((i) = 0; (i) < (vsi)->alloc_txq; (i)++)
+
+#define ice_for_each_alloc_rxq(vsi, i) \
+	for ((i) = 0; (i) < (vsi)->alloc_rxq; (i)++)
+
 struct ice_tc_info {
 	u16 qoffset;
 	u16 qcount;
diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
index 1db304c01d10..807b683a5707 100644
--- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
+++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
@@ -26,7 +26,7 @@ static int ice_q_stats_len(struct net_device *netdev)
 {
 	struct ice_netdev_priv *np = netdev_priv(netdev);
 
-	return ((np->vsi->num_txq + np->vsi->num_rxq) *
+	return ((np->vsi->alloc_txq + np->vsi->alloc_rxq) *
 		(sizeof(struct ice_q_stats) / sizeof(u64)));
 }
 
@@ -218,7 +218,7 @@ static void ice_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 			p += ETH_GSTRING_LEN;
 		}
 
-		ice_for_each_txq(vsi, i) {
+		ice_for_each_alloc_txq(vsi, i) {
 			snprintf(p, ETH_GSTRING_LEN,
 				 "tx-queue-%u.tx_packets", i);
 			p += ETH_GSTRING_LEN;
@@ -226,7 +226,7 @@ static void ice_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
 			p += ETH_GSTRING_LEN;
 		}
 
-		ice_for_each_rxq(vsi, i) {
+		ice_for_each_alloc_rxq(vsi, i) {
 			snprintf(p, ETH_GSTRING_LEN,
 				 "rx-queue-%u.rx_packets", i);
 			p += ETH_GSTRING_LEN;
@@ -253,6 +253,24 @@ static int ice_get_sset_count(struct net_device *netdev, int sset)
 {
 	switch (sset) {
 	case ETH_SS_STATS:
+		/* The number (and order) of strings reported *must* remain
+		 * constant for a given netdevice. This function must not
+		 * report a different number based on run time parameters
+		 * (such as the number of queues in use, or the setting of
+		 * a private ethtool flag). This is due to the nature of the
+		 * ethtool stats API.
+		 *
+		 * Userspace programs such as ethtool must make 3 separate
+		 * ioctl requests, one for size, one for the strings, and
+		 * finally one for the stats. Since these cross into
+		 * userspace, changes to the number or size could result in
+		 * undefined memory access or incorrect string<->value
+		 * correlations for statistics.
+		 *
+		 * Even if it appears to be safe, changes to the size or
+		 * order of strings will suffer from race conditions and are
+		 * not safe.
+		 */
 		return ICE_ALL_STATS_LEN(netdev);
 	default:
 		return -EOPNOTSUPP;
@@ -280,18 +298,26 @@ ice_get_ethtool_stats(struct net_device *netdev,
 	/* populate per queue stats */
 	rcu_read_lock();
 
-	ice_for_each_txq(vsi, j) {
+	ice_for_each_alloc_txq(vsi, j) {
 		ring = READ_ONCE(vsi->tx_rings[j]);
-		if (!ring)
-			continue;
-		data[i++] = ring->stats.pkts;
-		data[i++] = ring->stats.bytes;
+		if (ring) {
+			data[i++] = ring->stats.pkts;
+			data[i++] = ring->stats.bytes;
+		} else {
+			data[i++] = 0;
+			data[i++] = 0;
+		}
 	}
 
-	ice_for_each_rxq(vsi, j) {
+	ice_for_each_alloc_rxq(vsi, j) {
 		ring = READ_ONCE(vsi->rx_rings[j]);
-		data[i++] = ring->stats.pkts;
-		data[i++] = ring->stats.bytes;
+		if (ring) {
+			data[i++] = ring->stats.pkts;
+			data[i++] = ring->stats.bytes;
+		} else {
+			data[i++] = 0;
+			data[i++] = 0;
+		}
 	}
 
 	rcu_read_unlock();
@@ -519,7 +545,7 @@ ice_set_ringparam(struct net_device *netdev, struct ethtool_ringparam *ring)
 		goto done;
 	}
 
-	for (i = 0; i < vsi->num_txq; i++) {
+	for (i = 0; i < vsi->alloc_txq; i++) {
 		/* clone ring and setup updated count */
 		tx_rings[i] = *vsi->tx_rings[i];
 		tx_rings[i].count = new_tx_cnt;
@@ -551,7 +577,7 @@ ice_set_ringparam(struct net_device *netdev, struct ethtool_ringparam *ring)
 		goto done;
 	}
 
-	for (i = 0; i < vsi->num_rxq; i++) {
+	for (i = 0; i < vsi->alloc_rxq; i++) {
 		/* clone ring and setup updated count */
 		rx_rings[i] = *vsi->rx_rings[i];
 		rx_rings[i].count = new_rx_cnt;
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 05/13] ice: Clean control queues only when they are initialized
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

From: Preethi Banala <preethi.banala@intel.com>

Clean control queues only when they are initialized. One of the ways to
validate if the basic initialization is done is by checking value of
cq->sq.head and cq->rq.head variables that specify the register address.
This patch adds a check to avoid NULL pointer dereference crash when tried
to shutdown uninitialized control queue.

Signed-off-by: Preethi Banala <preethi.banala@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_controlq.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_controlq.c b/drivers/net/ethernet/intel/ice/ice_controlq.c
index 7c511f144ed6..c064416080e7 100644
--- a/drivers/net/ethernet/intel/ice/ice_controlq.c
+++ b/drivers/net/ethernet/intel/ice/ice_controlq.c
@@ -597,10 +597,14 @@ static enum ice_status ice_init_check_adminq(struct ice_hw *hw)
 	return 0;
 
 init_ctrlq_free_rq:
-	ice_shutdown_rq(hw, cq);
-	ice_shutdown_sq(hw, cq);
-	mutex_destroy(&cq->sq_lock);
-	mutex_destroy(&cq->rq_lock);
+	if (cq->rq.head) {
+		ice_shutdown_rq(hw, cq);
+		mutex_destroy(&cq->rq_lock);
+	}
+	if (cq->sq.head) {
+		ice_shutdown_sq(hw, cq);
+		mutex_destroy(&cq->sq_lock);
+	}
 	return status;
 }
 
@@ -706,10 +710,14 @@ static void ice_shutdown_ctrlq(struct ice_hw *hw, enum ice_ctl_q q_type)
 		return;
 	}
 
-	ice_shutdown_sq(hw, cq);
-	ice_shutdown_rq(hw, cq);
-	mutex_destroy(&cq->sq_lock);
-	mutex_destroy(&cq->rq_lock);
+	if (cq->sq.head) {
+		ice_shutdown_sq(hw, cq);
+		mutex_destroy(&cq->sq_lock);
+	}
+	if (cq->rq.head) {
+		ice_shutdown_rq(hw, cq);
+		mutex_destroy(&cq->rq_lock);
+	}
 }
 
 /**
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 06/13] ice: Fix bugs in control queue processing
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

This patch is a consolidation of multiple bug fixes for control queue
processing.

1)  In ice_clean_adminq_subtask() remove unnecessary reads/writes to
    registers. The bits PFINT_FW_CTL, PFINT_MBX_CTL and PFINT_SB_CTL
    are not set when an interrupt arrives, which means that clearing them
    again can be omitted.

2)  Get an accurate value in "pending" by re-reading the control queue
    head register from the hardware.

3)  Fix a corner case involving lost control queue messages by checking
    for new control messages (using ice_ctrlq_pending) before exiting the
    cleanup routine.

Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_controlq.c |  5 ++++-
 drivers/net/ethernet/intel/ice/ice_main.c     | 26 ++++++++++++++++++++++----
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_controlq.c b/drivers/net/ethernet/intel/ice/ice_controlq.c
index c064416080e7..62be72fdc8f3 100644
--- a/drivers/net/ethernet/intel/ice/ice_controlq.c
+++ b/drivers/net/ethernet/intel/ice/ice_controlq.c
@@ -1065,8 +1065,11 @@ ice_clean_rq_elem(struct ice_hw *hw, struct ice_ctl_q_info *cq,
 
 clean_rq_elem_out:
 	/* Set pending if needed, unlock and return */
-	if (pending)
+	if (pending) {
+		/* re-read HW head to calculate actual pending messages */
+		ntu = (u16)(rd32(hw, cq->rq.head) & cq->rq.head_mask);
 		*pending = (u16)((ntc > ntu ? cq->rq.count : 0) + (ntu - ntc));
+	}
 clean_rq_elem_err:
 	mutex_unlock(&cq->rq_lock);
 
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 450911345144..81b04e1d7ea2 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -916,6 +916,21 @@ static int __ice_clean_ctrlq(struct ice_pf *pf, enum ice_ctl_q q_type)
 	return pending && (i == ICE_DFLT_IRQ_WORK);
 }
 
+/**
+ * ice_ctrlq_pending - check if there is a difference between ntc and ntu
+ * @hw: pointer to hardware info
+ * @cq: control queue information
+ *
+ * returns true if there are pending messages in a queue, false if there aren't
+ */
+static bool ice_ctrlq_pending(struct ice_hw *hw, struct ice_ctl_q_info *cq)
+{
+	u16 ntu;
+
+	ntu = (u16)(rd32(hw, cq->rq.head) & cq->rq.head_mask);
+	return cq->rq.next_to_clean != ntu;
+}
+
 /**
  * ice_clean_adminq_subtask - clean the AdminQ rings
  * @pf: board private structure
@@ -923,7 +938,6 @@ static int __ice_clean_ctrlq(struct ice_pf *pf, enum ice_ctl_q q_type)
 static void ice_clean_adminq_subtask(struct ice_pf *pf)
 {
 	struct ice_hw *hw = &pf->hw;
-	u32 val;
 
 	if (!test_bit(__ICE_ADMINQ_EVENT_PENDING, pf->state))
 		return;
@@ -933,9 +947,13 @@ static void ice_clean_adminq_subtask(struct ice_pf *pf)
 
 	clear_bit(__ICE_ADMINQ_EVENT_PENDING, pf->state);
 
-	/* re-enable Admin queue interrupt causes */
-	val = rd32(hw, PFINT_FW_CTL);
-	wr32(hw, PFINT_FW_CTL, (val | PFINT_FW_CTL_CAUSE_ENA_M));
+	/* There might be a situation where new messages arrive to a control
+	 * queue between processing the last message and clearing the
+	 * EVENT_PENDING bit. So before exiting, check queue head again (using
+	 * ice_ctrlq_pending) and process new messages if any.
+	 */
+	if (ice_ctrlq_pending(hw, &hw->adminq))
+		__ice_clean_ctrlq(pf, ICE_CTL_Q_ADMIN);
 
 	ice_flush(hw);
 }
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 07/13] ice: Use order_base_2 to calculate higher power of 2
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

Currently, we use a combination of ilog2 and is_power_of_2() to
calculate the next power of 2 for the qcount. This appears to be causing
a warning on some combinations of GCC and the Linux kernel:

MODPOST 1 modules
WARNING: "____ilog2_NaN" [ice.ko] undefined!

This appears to because because GCC realizes that qcount could be zero
in some circumstances and thus attempts to link against the
intentionally undefined ___ilog2_NaN function.

The order_base_2 function is intentionally defined to return 0 when
passed 0 as an argument, and thus will be safe to use here.

This not only fixes the warning but makes the resulting code slightly
cleaner, and is really what we should have used originally.

Also update the comment to make it more clear that we are rounding up,
not just incrementing the ilog2 of qcount unconditionally.

Without this patch, we get warnings when building against the upstream
kernel.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
[Anirudh Venkataramanan <anirudh.venkataramanan@intel.com> minor cleanup for upstream submission]
---
 drivers/net/ethernet/intel/ice/ice_main.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 81b04e1d7ea2..feeca75912ec 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1313,11 +1313,8 @@ static void ice_vsi_setup_q_map(struct ice_vsi *vsi, struct ice_vsi_ctx *ctxt)
 		qcount = numq_tc;
 	}
 
-	/* find higher power-of-2 of qcount */
-	pow = ilog2(qcount);
-
-	if (!is_power_of_2(qcount))
-		pow++;
+	/* find the (rounded up) power-of-2 of qcount */
+	pow = order_base_2(qcount);
 
 	for (i = 0; i < ICE_MAX_TRAFFIC_CLASS; i++) {
 		if (!(vsi->tc_cfg.ena_tc & BIT(i))) {
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 08/13] ice: Don't explicitly set port_vlan_bits to 0
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

From: Brett Creeley <brett.creeley@intel.com>

This patch fixes the following smatch warning originally reported by
Dan Carpenter:
ice_set_dflt_vsi_ctx() warn: odd binop '0x0 & 0x18'

In ice_set_dflt_vsi_ctx() we clear bits 3 and 4 for legacy behavior
(i.e. show VLAN, DEI and UP in the receive descriptors). However
the logic to do this generated the above mentioned smatch warning.

Bits 3 and 4 in port_vlan_flags are anyway set to zero by the memset on
ctxt->info field, so remove the logic to set these explicitly to 0. This
fixes the smatch warning as well.

CC: Shannon Nelson <shannon.nelson@oracle.com>
Signed-off-by: Brett Creeley <brett.creeley@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
[Anirudh Venkataramanan <anirudh.venkataramanan@intel.com> rewrote commit message]
---
 drivers/net/ethernet/intel/ice/ice_main.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index feeca75912ec..586c6e615a98 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1367,14 +1367,13 @@ static void ice_set_dflt_vsi_ctx(struct ice_vsi_ctx *ctxt)
 	ctxt->info.sw_flags = ICE_AQ_VSI_SW_FLAG_SRC_PRUNE;
 	/* Traffic from VSI can be sent to LAN */
 	ctxt->info.sw_flags2 = ICE_AQ_VSI_SW_FLAG_LAN_ENA;
-	/* Allow all packets untagged/tagged */
+	/* By default bits 3 and 4 in port_vlan_flags are 0's which results in
+	 * legacy behavior (show VLAN, DEI, and UP) in descriptor. Also, allow
+	 * all packets untagged/tagged.
+	 */
 	ctxt->info.port_vlan_flags = ((ICE_AQ_VSI_PVLAN_MODE_ALL &
 				       ICE_AQ_VSI_PVLAN_MODE_M) >>
 				      ICE_AQ_VSI_PVLAN_MODE_S);
-	/* Show VLAN/UP from packets in Rx descriptors */
-	ctxt->info.port_vlan_flags |= ((ICE_AQ_VSI_PVLAN_EMOD_STR_BOTH &
-					ICE_AQ_VSI_PVLAN_EMOD_M) >>
-				       ICE_AQ_VSI_PVLAN_EMOD_S);
 	/* Have 1:1 UP mapping for both ingress/egress tables */
 	table |= ICE_UP_TABLE_TRANSLATE(0, 0);
 	table |= ICE_UP_TABLE_TRANSLATE(1, 1);
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 09/13] ice: Set VLAN flags correctly
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

From: Brett Creeley <brett.creeley@intel.com>

Set the ICE_AQ_VSI_PVLAN_MODE_ALL bit to allow the driver to add a VLAN
tag to all packets it sends.

Signed-off-by: Brett Creeley <brett.creeley@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
[Anirudh Venkataramanan <anirudh.venkataramanan@intel.com> cleaned up commit message]
---
 drivers/net/ethernet/intel/ice/ice_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 586c6e615a98..71cc5b6e6237 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3779,6 +3779,9 @@ static int ice_vsi_manage_vlan_stripping(struct ice_vsi *vsi, bool ena)
 		ctxt.info.port_vlan_flags = ICE_AQ_VSI_PVLAN_EMOD_NOTHING;
 	}
 
+	/* Allow all packets untagged/tagged */
+	ctxt.info.port_vlan_flags |= ICE_AQ_VSI_PVLAN_MODE_ALL;
+
 	ctxt.info.valid_sections = cpu_to_le16(ICE_AQ_VSI_PROP_VLAN_VALID);
 	ctxt.vsi_num = vsi->vsi_num;
 
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 10/13] ice: Update to interrupts enabled in OICR
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

From: Bruce Allan <bruce.w.allan@intel.com>

Remove the following interrupt causes that are not applicable or not
handled:
- PFINT_OICR_HLP_RDY_M
- PFINT_OICR_CPM_RDY_M
- PFINT_OICR_GPIO_M
- PFINT_OICR_STORM_DETECT_M

Add the following interrupt cause that's actually handled in ice_misc_intr:
- PFINT_OICR_PE_CRITERR_M

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
[Anirudh Venkataramanan <anirudh.venkataramanan@intel.com> Removed defines from ice_hw_autogen.h]
---
 drivers/net/ethernet/intel/ice/ice_hw_autogen.h | 8 --------
 drivers/net/ethernet/intel/ice/ice_main.c       | 9 +++------
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
index 499904874b3f..6076fc87df9d 100644
--- a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
+++ b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
@@ -121,10 +121,6 @@
 #define PFINT_FW_CTL_CAUSE_ENA_S	30
 #define PFINT_FW_CTL_CAUSE_ENA_M	BIT(PFINT_FW_CTL_CAUSE_ENA_S)
 #define PFINT_OICR			0x0016CA00
-#define PFINT_OICR_HLP_RDY_S		14
-#define PFINT_OICR_HLP_RDY_M		BIT(PFINT_OICR_HLP_RDY_S)
-#define PFINT_OICR_CPM_RDY_S		15
-#define PFINT_OICR_CPM_RDY_M		BIT(PFINT_OICR_CPM_RDY_S)
 #define PFINT_OICR_ECC_ERR_S		16
 #define PFINT_OICR_ECC_ERR_M		BIT(PFINT_OICR_ECC_ERR_S)
 #define PFINT_OICR_MAL_DETECT_S		19
@@ -133,10 +129,6 @@
 #define PFINT_OICR_GRST_M		BIT(PFINT_OICR_GRST_S)
 #define PFINT_OICR_PCI_EXCEPTION_S	21
 #define PFINT_OICR_PCI_EXCEPTION_M	BIT(PFINT_OICR_PCI_EXCEPTION_S)
-#define PFINT_OICR_GPIO_S		22
-#define PFINT_OICR_GPIO_M		BIT(PFINT_OICR_GPIO_S)
-#define PFINT_OICR_STORM_DETECT_S	24
-#define PFINT_OICR_STORM_DETECT_M	BIT(PFINT_OICR_STORM_DETECT_S)
 #define PFINT_OICR_HMC_ERR_S		26
 #define PFINT_OICR_HMC_ERR_M		BIT(PFINT_OICR_HMC_ERR_S)
 #define PFINT_OICR_PE_CRITERR_S		28
diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index 71cc5b6e6237..bcd39654ab4b 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -1702,15 +1702,12 @@ static void ice_ena_misc_vector(struct ice_pf *pf)
 	wr32(hw, PFINT_OICR_ENA, 0);	/* disable all */
 	rd32(hw, PFINT_OICR);		/* read to clear */
 
-	val = (PFINT_OICR_HLP_RDY_M |
-	       PFINT_OICR_CPM_RDY_M |
-	       PFINT_OICR_ECC_ERR_M |
+	val = (PFINT_OICR_ECC_ERR_M |
 	       PFINT_OICR_MAL_DETECT_M |
 	       PFINT_OICR_GRST_M |
 	       PFINT_OICR_PCI_EXCEPTION_M |
-	       PFINT_OICR_GPIO_M |
-	       PFINT_OICR_STORM_DETECT_M |
-	       PFINT_OICR_HMC_ERR_M);
+	       PFINT_OICR_HMC_ERR_M |
+	       PFINT_OICR_PE_CRITERR_M);
 
 	wr32(hw, PFINT_OICR_ENA, val);
 
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 11/13] ice: Fix couple of null pointer dereference issues
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

When ice_ena_msix_range() fails to reserve vectors, a devm_kfree() warning
was seen in the error flow path. So check pf->irq_tracker before use in
ice_clear_interrupt_scheme().

In ice_vsi_cfg(), check vsi->netdev before use.

Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_main.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index bcd39654ab4b..e5b8eb0c4581 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -3259,8 +3259,10 @@ static void ice_clear_interrupt_scheme(struct ice_pf *pf)
 	if (test_bit(ICE_FLAG_MSIX_ENA, pf->flags))
 		ice_dis_msix(pf);
 
-	devm_kfree(&pf->pdev->dev, pf->irq_tracker);
-	pf->irq_tracker = NULL;
+	if (pf->irq_tracker) {
+		devm_kfree(&pf->pdev->dev, pf->irq_tracker);
+		pf->irq_tracker = NULL;
+	}
 }
 
 /**
@@ -4114,11 +4116,12 @@ static int ice_vsi_cfg(struct ice_vsi *vsi)
 {
 	int err;
 
-	ice_set_rx_mode(vsi->netdev);
-
-	err = ice_restore_vlan(vsi);
-	if (err)
-		return err;
+	if (vsi->netdev) {
+		ice_set_rx_mode(vsi->netdev);
+		err = ice_restore_vlan(vsi);
+		if (err)
+			return err;
+	}
 
 	err = ice_vsi_cfg_txqs(vsi);
 	if (!err)
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 12/13] ice: Fix potential return of uninitialized value
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

In ice_vsi_setup_[tx|rx]_rings, err is uninitialized which can result in
a garbage value return to the caller. Fix that.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
[Anirudh Venkataramanan <anirudh.venkataramanan@intel.com> cleaned up commit message]
---
 drivers/net/ethernet/intel/ice/ice_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
index e5b8eb0c4581..3782569b450a 100644
--- a/drivers/net/ethernet/intel/ice/ice_main.c
+++ b/drivers/net/ethernet/intel/ice/ice_main.c
@@ -4887,7 +4887,7 @@ int ice_down(struct ice_vsi *vsi)
  */
 static int ice_vsi_setup_tx_rings(struct ice_vsi *vsi)
 {
-	int i, err;
+	int i, err = 0;
 
 	if (!vsi->num_txq) {
 		dev_err(&vsi->back->pdev->dev, "VSI %d has 0 Tx queues\n",
@@ -4912,7 +4912,7 @@ static int ice_vsi_setup_tx_rings(struct ice_vsi *vsi)
  */
 static int ice_vsi_setup_rx_rings(struct ice_vsi *vsi)
 {
-	int i, err;
+	int i, err = 0;
 
 	if (!vsi->num_rxq) {
 		dev_err(&vsi->back->pdev->dev, "VSI %d has 0 Rx queues\n",
-- 
2.14.3


^ permalink raw reply related

* [Intel-wired-lan] [PATCH v2 13/13] ice: Change struct members from bool to u8
From: Anirudh Venkataramanan @ 2018-07-23 22:05 UTC (permalink / raw)
  To: intel-wired-lan
In-Reply-To: <20180723220520.16678-1-anirudh.venkataramanan@intel.com>

From: Bruce Allan <bruce.w.allan@intel.com>

Recent versions of checkpatch have a new warning based on a documented
preference of Linus to not use bool in structures due to wasted space and
the size of bool is implementation dependent.  For more information, see
the email thread at https://lkml.org/lkml/2017/11/21/384.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
 drivers/net/ethernet/intel/ice/ice.h        |  8 ++++----
 drivers/net/ethernet/intel/ice/ice_switch.h |  6 +++---
 drivers/net/ethernet/intel/ice/ice_txrx.h   |  2 +-
 drivers/net/ethernet/intel/ice/ice_type.h   | 16 ++++++++--------
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index ed071ea75f20..868f4a1d0f72 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -196,9 +196,9 @@ struct ice_vsi {
 	struct list_head tmp_sync_list;		/* MAC filters to be synced */
 	struct list_head tmp_unsync_list;	/* MAC filters to be unsynced */
 
-	bool irqs_ready;
-	bool current_isup;		 /* Sync 'link up' logging */
-	bool stat_offsets_loaded;
+	u8 irqs_ready;
+	u8 current_isup;		 /* Sync 'link up' logging */
+	u8 stat_offsets_loaded;
 
 	/* queue information */
 	u8 tx_mapping_mode;		 /* ICE_MAP_MODE_[CONTIG|SCATTER] */
@@ -269,7 +269,7 @@ struct ice_pf {
 	struct ice_hw_port_stats stats;
 	struct ice_hw_port_stats stats_prev;
 	struct ice_hw hw;
-	bool stat_prev_loaded;	/* has previous stats been loaded */
+	u8 stat_prev_loaded;	/* has previous stats been loaded */
 	char int_name[ICE_INT_NAME_STR_LEN];
 };
 
diff --git a/drivers/net/ethernet/intel/ice/ice_switch.h b/drivers/net/ethernet/intel/ice/ice_switch.h
index 6f4a0d159dbf..9b8ec128ee31 100644
--- a/drivers/net/ethernet/intel/ice/ice_switch.h
+++ b/drivers/net/ethernet/intel/ice/ice_switch.h
@@ -17,7 +17,7 @@ struct ice_vsi_ctx {
 	u16 vsis_unallocated;
 	u16 flags;
 	struct ice_aqc_vsi_props info;
-	bool alloc_from_pool;
+	u8 alloc_from_pool;
 };
 
 enum ice_sw_fwd_act_type {
@@ -94,8 +94,8 @@ struct ice_fltr_info {
 	u8 qgrp_size;
 
 	/* Rule creations populate these indicators basing on the switch type */
-	bool lb_en;	/* Indicate if packet can be looped back */
-	bool lan_en;	/* Indicate if packet can be forwarded to the uplink */
+	u8 lb_en;	/* Indicate if packet can be looped back */
+	u8 lan_en;	/* Indicate if packet can be forwarded to the uplink */
 };
 
 /* Bookkeeping structure to hold bitmap of VSIs corresponding to VSI list id */
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h
index 567067b650c4..31bc998fe200 100644
--- a/drivers/net/ethernet/intel/ice/ice_txrx.h
+++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
@@ -143,7 +143,7 @@ struct ice_ring {
 	u16 next_to_use;
 	u16 next_to_clean;
 
-	bool ring_active;		/* is ring online or not */
+	u8 ring_active;			/* is ring online or not */
 
 	/* stats structs */
 	struct ice_q_stats	stats;
diff --git a/drivers/net/ethernet/intel/ice/ice_type.h b/drivers/net/ethernet/intel/ice/ice_type.h
index 99c8a9a71b5e..97c366e0ca59 100644
--- a/drivers/net/ethernet/intel/ice/ice_type.h
+++ b/drivers/net/ethernet/intel/ice/ice_type.h
@@ -83,7 +83,7 @@ struct ice_link_status {
 	u64 phy_type_low;
 	u16 max_frame_size;
 	u16 link_speed;
-	bool lse_ena;	/* Link Status Event notification */
+	u8 lse_ena;	/* Link Status Event notification */
 	u8 link_info;
 	u8 an_info;
 	u8 ext_info;
@@ -101,7 +101,7 @@ struct ice_phy_info {
 	struct ice_link_status link_info_old;
 	u64 phy_type_low;
 	enum ice_media_type media_type;
-	bool get_link_info;
+	u8 get_link_info;
 };
 
 /* Common HW capabilities for SW use */
@@ -167,7 +167,7 @@ struct ice_nvm_info {
 	u32 oem_ver;              /* OEM version info */
 	u16 sr_words;             /* Shadow RAM size in words */
 	u16 ver;                  /* NVM package version */
-	bool blank_nvm_mode;      /* is NVM empty (no FW present) */
+	u8 blank_nvm_mode;        /* is NVM empty (no FW present) */
 };
 
 /* Max number of port to queue branches w.r.t topology */
@@ -181,7 +181,7 @@ struct ice_sched_node {
 	struct ice_aqc_txsched_elem_data info;
 	u32 agg_id;			/* aggregator group id */
 	u16 vsi_id;
-	bool in_use;			/* suspended or in use */
+	u8 in_use;			/* suspended or in use */
 	u8 tx_sched_layer;		/* Logical Layer (1-9) */
 	u8 num_children;
 	u8 tc_num;
@@ -218,7 +218,7 @@ struct ice_sched_vsi_info {
 struct ice_sched_tx_policy {
 	u16 max_num_vsis;
 	u8 max_num_lan_qs_per_tc[ICE_MAX_TRAFFIC_CLASS];
-	bool rdma_ena;
+	u8 rdma_ena;
 };
 
 struct ice_port_info {
@@ -243,7 +243,7 @@ struct ice_port_info {
 	struct list_head agg_list;	/* lists all aggregator */
 	u8 lport;
 #define ICE_LPORT_MASK		0xff
-	bool is_vf;
+	u8 is_vf;
 };
 
 struct ice_switch_info {
@@ -287,7 +287,7 @@ struct ice_hw {
 	u8 max_cgds;
 	u8 sw_entry_point_layer;
 
-	bool evb_veb;		/* true for VEB, false for VEPA */
+	u8 evb_veb;		/* true for VEB, false for VEPA */
 	struct ice_bus_info bus;
 	struct ice_nvm_info nvm;
 	struct ice_hw_dev_caps dev_caps;	/* device capabilities */
@@ -318,7 +318,7 @@ struct ice_hw {
 	u8 itr_gran_100;
 	u8 itr_gran_50;
 	u8 itr_gran_25;
-	bool ucast_shared;	/* true if VSIs can share unicast addr */
+	u8 ucast_shared;	/* true if VSIs can share unicast addr */
 
 };
 
-- 
2.14.3


^ permalink raw reply related

* Re: [NOT YET PULL] Trial of labeling lines in code snippets
From: Paul E. McKenney @ 2018-07-23 21:03 UTC (permalink / raw)
  To: Akira Yokosawa; +Cc: perfbook
In-Reply-To: <3a77a91b-6747-0d23-df5e-382c073f48aa@gmail.com>

On Tue, Jul 24, 2018 at 12:12:17AM +0900, Akira Yokosawa wrote:
> On 2018/07/24 00:09:37 +0900, Akira Yokosawa wrote:
> > On 2018/07/22 15:49:06 -0700, Paul E. McKenney wrote:
> >> On Mon, Jul 23, 2018 at 12:43:05AM +0900, Akira Yokosawa wrote:
> >>> On 2018/07/21 09:51:03 -0700, Paul E. McKenney wrote:
> >>>> On Sat, Jul 21, 2018 at 09:19:53AM +0900, Akira Yokosawa wrote:
> >>>>> Hi Paul,
> >>>>>
> >>>>> So, an experimental branch to label particular lines in code
> >>>>> snippets is ready for review.
> >>>>> The branch is based on current master of yours.
> >>>>>
> >>>>> I've converted 4 code snippets in Section 4.2. and updated
> >>>>> corresponding code samples (sans forkjoin.c because it is
> >>>>> quite simplified in Listing 4.1).
> >>>>>
> >>>>> Does this approach look reasonable to you?
> >>>>
> >>>> This does look promising, thank you!  Please give me some time to think
> >>>> this over a bit.  My normal approach would end up with very long labels
> >>>> in the code, which might be OK.  The argument against is that tying the
> >>>> listing caption to the actual code might not be a great thing.
> >>>
> >>> One idea is to embed meta labels in code samples.
> >>> When converting them to a proper Verbatim sources, short labels such
> >>> as " //\lnlbl{foo}" can be converted to long ones such as
> >>> "%lnlbl[ln:<chapter>:<basename of snippet file>:foo]" in the script
> >>> to remove " //".
> >>>
> >>> To do this, we need to embed extra meta data as comments in sample
> >>> code.
> >>>
> >>> For example, the source of waitall() function (in api-pthread.h) can be
> >>> written as the following (in the form of \lnlbl{}):
> >>>
> >>> ----
> >>> /*
> >>>  * Wait on all child processes.
> >>>  */
> >>> static __inline__ void waitall(void)
> >>> {
> >>> // \begin{snippet}[chapter=toolsoftrade,name=waitall,commandchars=[\%\[\]]
> >>> 	int pid;
> >>> 	int status;
> >>>
> >>> 	for (;;) {				//\lnlbl{loopa}
> >>> 		pid = wait(&status);		//\lnlb{wait}
> >>> 		if (pid == -1) {
> >>> 			if (errno == ECHILD)	//\lnlb{ECHILD}
> >>> 				break;		//\lnlbl{break}
> >>> 			perror("wait");		//\lnlbl{perror}
> >>> 			exit(EXIT_FAILURE);	//\lnlbl{exit}
> >>> 		}
> >>> 		poll(NULL, 0, 1);
> >>> 	}					//\lnlbl{loopb}
> >>> }
> >>> // \end{snippet}
> >>> ...
> >>> ----
> >>>
> >>> , and it can be extracted and converted into CodeSamples/api-pthread/waitall.tex
> >>> in the following way (including substitution of escape charactors):
> >>>
> >>> \begin{Verbatim}[commandchars=[\%\[\]]
> >>> 	int pid;
> >>> 	int status;
> >>>
> >>> 	for (;;) {%lnlbl[ln:toolsoftrade:waitall:loopa]
> >>> 		pid = wait(&status);%lnlbl[ln:toolsoftrade:waitall:wait]
> >>> 		if (pid == -1) {
> >>> 			if (errno == ECHILD)%lnlbl[ln:toolsoftrade:waitall:ECHILD]
> >>> 				break;%lnlbl[ln:toolsoftrade:break]
> >>> 			perror("wait");%lnlbl[ln:toolsoftrade:perror]
> >>> 			exit(EXIT_FAILURE);%lnlbl[ln:toolsoftrade:exit]
> >>> 		}
> >>> 		poll(NULL, 0, 1);
> >>> 	}%lnlbl[ln:toolsoftrade:loopb]
> >>> }
> >>> \end{Verbatim}
> >>>
> >>> This file can be read in toolsoftrade.tex in the following way:
> >>>
> >>> ---
> >>> \begin{listing}[tbp]
> >>> \input{CodeSamples/api-pthread/waitall}
> >>> \caption{Using the \tco{wait()} Primitive}
> >>> \label{lst:toolsoftrade:Using the wait() Primitive}
> >>> \end{listing}
> >>> ---
> >>>
> >>> The conversion script should not be so hard to implement.
> >>> Update of build scripts should also be possible to automate these
> >>> conversions.
> >>>
> >>> If you'd like to use the caption of the listing in the final labels,
> >>> the caption should also be given in the meta \begin{snippet} command.
> >>> But for lines in code snippets, using file names as labels sounds
> >>> reasonable to me.
> >>
> >> I believe that your scheme is pretty close.  My thought is directory,
> >> file, function, label.  You have all but file already.  That allows
> >> captions to change without changing the line label, which seems like a
> >> good thing.
> > 
> > In the end, "chapter=foo,name=bar,func=baz" can be expressed as
> > "labelprefix=foo:bar:baz in the option to meta \begin{snippet} command.
> 
> I meant:
> 
>   "labelprefix=foo:bar:baz"

Sounds very good, thank you!!!

							Thanx, Paul

> Akira
> 
> > So you can put any string you like there.
> > 
> > I'll do v2 of the trial branch along this scheme, hopefully
> > including build script/Makefile updates.  Maybe in a week or so.
> > 
> >        Thanks, Akira
> >  
> >>
> >> Seem reasonable?
> >>
> >> 							Thanx, Paul
> >>
> >>> Thoughts?
> >>>
> >>>         Thanks, Akira
> >>>
> >>>>
> >>>> Again, looks promising, thank you!
> >>>>
> >>>> 							Thanx, Paul
> >>>>
> >>>>>         Thanks, Akira
> >>>>>
> >>>>> ---
> >>>>> The following changes since commit e3191997557e0d33b862e267fb4d5971e879ca07:
> >>>>>
> >>>>>   utilities: Provide scripts instead of broken symlinks (2018-07-18 11:40:57 -0700)
> >>>>>
> >>>>> are available in the git repository at:
> >>>>>
> >>>>>   https://github.com/akiyks/perfbook trial-fancyvrb
> >>>>>
> >>>>> for you to fetch changes up to 2154a175e469e6238c89d4c494d204a9ddbaa20a:
> >>>>>
> >>>>>   [EXP] Update hyphen2endash (2018-07-21 09:17:34 +0900)
> >>>>>
> >>>>> ----------------------------------------------------------------
> >>>>> Akira Yokosawa (3):
> >>>>>       [EXP] toolsoftrade: Add labels in code samples as comments
> >>>>>       [EXP] toolsoftrade: Reference line in code snippets by label
> >>>>>       [EXP] Update hyphen2endash
> >>>>>
> >>>>>  CodeSamples/api-pthreads/api-pthreads.h |  14 +-
> >>>>>  CodeSamples/toolsoftrade/forkjoinvar.c  |  10 +-
> >>>>>  CodeSamples/toolsoftrade/pcreate.c      |   5 +-
> >>>>>  perfbook.tex                            |   6 +
> >>>>>  toolsoftrade/toolsoftrade.tex           | 221 +++++++++++++++++---------------
> >>>>>  utilities/hyphen2endash.sh              |   3 +
> >>>>>  6 files changed, 141 insertions(+), 118 deletions(-)
> >>>>>
> >>>>
> >>>
> >>
> > 
> 


^ permalink raw reply

* Re: [RESEND] Spectre-v2 (IBPB/IBRS) and SSBD fixes for 4.4.y
From: Jiri Kosina @ 2018-07-23 22:06 UTC (permalink / raw)
  To: Srivatsa S. Bhat
  Cc: Dave Hansen, Wanpeng Li, Andi Kleen, linux-tip-commits, Piotr Luc,
	Mel Gorman, arjan.van.de.ven, xen-devel, Alexander Sergeyev,
	Brian Gerst, Andy Lutomirski, MickaëlSalaün,
	Thomas Gleixner, Joe Konno, Laura Abbott, Will Drewry, gregkh,
	linux-kernel, Jia Zhang, Andrew Morton, Linus Torvalds,
	David Woodhouse, KarimAllah Ahmed, Dave Hansen
In-Reply-To: <153156030832.10043.13438231886571087086.stgit@srivatsa-ubuntu>

On Sat, 14 Jul 2018, Srivatsa S. Bhat wrote:

> This patch series is a backport of the Spectre-v2 fixes (IBPB/IBRS)
> and patches for the Speculative Store Bypass vulnerability to 4.4.y
> (they apply cleanly on top of 4.4.140).

FWIW -- not sure how much inspiration you took from our SLE 4.4-based 
tree, but most of the stuff is already there for quite some time 
(including the non-upstream IBRS on kernel boundary on SKL+, trampoline 
stack for PTI (which the original port didn't have), etc).

The IBRS SKL+ stuff has not been picked up by Greg, as it's non-upstream, 
and the trampoline stack I believe was pointed out to stable@, but noone 
really sat down and did the port (our codebase is different than 4.4.x 
stable base), but it definitely should be done if someone has to put 100% 
trust into the PTI port (either that, or at least zeroing out the kernel 
thread thread stack ... we used to have temporarily that before we 
switched over to proper entry trampoline in this version as well).

Thanks,

-- 
Jiri Kosina
SUSE Labs


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

^ permalink raw reply

* Re: [PATCHv3 2/2] mtd: m25p80: restore the status of SPI flash when exiting
From: Brian Norris @ 2018-07-23 22:06 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Zhiqiang Hou, linux-mtd, Linux Kernel, David Woodhouse,
	Boris BREZILLON, Marek Vasut, Richard Weinberger, Cyrille Pitchen
In-Reply-To: <20180723221002.736e0830@bbrezillon>

Hi Boris,

On Mon, Jul 23, 2018 at 1:10 PM, Boris Brezillon
<boris.brezillon@bootlin.com> wrote:
> On Mon, 23 Jul 2018 11:13:50 -0700
> Brian Norris <computersforpeace@gmail.com> wrote:
>> I noticed this got merged, but I wanted to put my 2 cents in here:
>
> I wish you had replied to this thread when it was posted (more than
> 6 months ago). Reverting the patch now implies making some people
> unhappy because they'll have to resort to their old out-of-tree
> hacks :-(.

I'd say I'm sorry for not following things closely these days, but I'm
not really that sorry. There are plenty of other capable hands. And if
y'all shoot yourselves in the foot, so be it. This patch isn't going
to blow things up, but now that I did finally notice it (because it
happened to show up in a list of backports I was looking at), I
thought better late than never to remind you.

For way of notification: Marek already noticed that we've started down
a slippery slope months ago:

https://lkml.org/lkml/2018/4/8/141
Re: [PATCH] mtd: spi-nor: clear Extended Address Reg on switch to
3-byte addressing.

I'm not quite sure why that wasn't taken to its logical conclusion --
that the hack should be reverted.

This problem has been noted many times already, and we've always
stayed on the side of *avoiding* this hack. A few references from a
search of my email:

http://lists.infradead.org/pipermail/linux-mtd/2013-March/046343.html
[PATCH 1/3] mtd: m25p80: utilize dedicated 4-byte addressing commands

http://lists.infradead.org/pipermail/barebox/2014-September/020682.html
[RFC] MTD m25p80 3-byte addressing and boot problem

http://lists.infradead.org/pipermail/linux-mtd/2015-February/057683.html
[PATCH 2/2] m25p80: if supported put chip to deep power down if not used

>> On Wed, Dec 06, 2017 at 10:53:42AM +0800, Zhiqiang Hou wrote:
>> > From: Hou Zhiqiang <Zhiqiang.Hou@nxp.com>
>> >
>> > Restore the status to be compatible with legacy devices.
>> > Take Freescale eSPI boot for example, it copies (in 3 Byte
>> > addressing mode) the RCW and bootloader images from SPI flash
>> > without firing a reset signal previously, so the reboot command
>> > will fail without reseting the addressing mode of SPI flash.
>> > This patch implement .shutdown function to restore the status
>> > in reboot process, and add the same operation to the .remove
>> > function.
>>
>> We have previously rejected this patch multiple times, because the above
>> comment demonstrates a broken product.
>
> If we were to only support working HW parts, I fear Linux would not
> support a lot of HW (that's even more true when it comes to flashes :P).

You stopped allowing UBI to attach to MLC NAND recently, no? That
sounds like almost the same boat -- you've probably killed quite a few
shitty products, if they were to use mainline directly.

Anyway, that's derailing the issue. Supporting broken hardware isn't
something you try to do by applying the same hack to all systems. You
normally try to apply your hack as narrowly as possible. You seem to
imply that below. So maybe that's a solution to move forward with. But
I'd personally be just as happy to see the patch reverted.

>> You cannot guarantee that all
>> reboots will invoke the .shutdown() method -- what about crashes? What
>> about watchdog resets? IIUC, those will hit the same broken behavior,
>> and have unexepcted behavior in your bootloader.
>
> Yes, there are corner cases that are not addressed with this approach,

Is a system crash really a corner case? :D

> but it still seems to improve things. Of course, that means the
> user should try to re-route all HW reset sources to SW ones (RESET input
> pin muxed to the GPIO controller, watchdog generating an interrupt
> instead of directly asserting the RESET output pin), which is not always
> possible, but even when it's not, isn't it better to have a setup that
> works fine 99% of the time instead of 50% of the time?

Perhaps, but not at the expense of future development. And
realistically, no one is doing that if they have this hack. Most
people won't even know that this hack is protecting them at all (so
again, they won't try to mitigate the problem any further).

>> I suppose one could argue for doing this in remove(), but AIUI you're
>> just papering over system bugs by introducing the shutdown() function
>> here. Thus, I'd prefer we drop the shutdown() method to avoid misleading
>> other users of this driver.
>
> I understand your point. But if the problem is about making sure people
> designing new boards get that right, why not complaining at probe time
> when things are wrong?
>
> I mean, spi_nor_restore() seems to only do something on very specific
> NORs (those on which a SW RESET does not resets the addressing
> mode).

The point isn't that SW RESET doesn't reset the addressing mode -- it
does on any flash I've seen. The point is that most systems are built
around a stateless assumption in these flash. IIRC, there wasn't even
a SW RESET command at all until these "huge" flash came around and
stateful addressing modes came about. So boot ROMs and bootloaders
would have to be updated to start figuring out when/how to do this SW
RESET. And once two vendors start doing it differently (I'm not sure:
have they done this already? I think so) it's no longer something a
boot ROM will get right.

The only way to get this stuff right is to have a hardware reset, or
else to avoid all of the stateful modes in software.

> So, how about adding a flag that says "my board has the NOR HW
> RESET pin wired" (there would be a DT props to set that flag). Then you
> add a WARN_ON() when this flag is not set and a NOR chip impacted by
> this bug is detected.

I'd kinda prefer the reverse. There really isn't a need to document
anything for a working system (software usually can't control this
RESET pin). The burden should be on the b0rked system to document
where it needs unsound hacks to survive.

> This way you make sure people are informed that
> they're doing something wrong, and for those who can't change their HW
> (because it's already widely deployed), you have a fix that improve
> things.

Or even better: put this hack behind a DT flag, so that one has to
admit that their board design is broken before it will even do
anything. Proposal: "linux,badly-designed-flash-reset".

But, I'd prefer just (partially?) reverting this, and let the authors
submit something that works. We're not obligated to keep bad hacks in
the kernel.

Brian

^ permalink raw reply

* Re: [PATCH nf] netfilter: nf_tables: move dumper state alloation into ->start
From: Pablo Neira Ayuso @ 2018-07-23 22:07 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, netdev, cscnull
In-Reply-To: <20180723104714.24008-1-fw@strlen.de>

On Mon, Jul 23, 2018 at 12:47:14PM +0200, Florian Westphal wrote:
> Shaochun Chen points out we leak dumper filter state allocations
> stored in dump_control->data in case there is an error before netlink sets
> cb_running (after which ->done will be called at some point).
> 
> In order to fix this, add .start functions and do the allocations
> there.
> 
> ->done is going to clean up, and in case error occurs before
> ->start invocation no cleanups need to be done anymore.

Applied, thanks Florian.

^ permalink raw reply

* Re: [PATCH 0/3] PTI for x86-32 Fixes and Updates
From: Dave Hansen @ 2018-07-23 22:07 UTC (permalink / raw)
  To: Josh Poimboeuf, Pavel Machek
  Cc: Linus Torvalds, Joerg Roedel, Thomas Gleixner, Ingo Molnar,
	Peter Anvin, the arch/x86 maintainers, Linux Kernel Mailing List,
	linux-mm, Andrew Lutomirski, Jürgen Groß,
	Peter Zijlstra, Borislav Petkov, Jiri Kosina, Boris Ostrovsky,
	Brian Gerst, David Laight, Denys Vlasenko, Eduardo Valentin,
	Greg Kroah-Hartman, Will Deacon, Liguori, Anthony, Daniel Gruss,
	Hugh Dickins, Kees Cook, Andrea Arcangeli, Waiman Long,
	David H . Gutteridge, Joerg Roedel, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim
In-Reply-To: <20180723215958.jozblzq4sv7gp7uj@treble>

On 07/23/2018 02:59 PM, Josh Poimboeuf wrote:
> On Mon, Jul 23, 2018 at 11:38:30PM +0200, Pavel Machek wrote:
>> But for now I'd like at least "global" option of turning pti on/off
>> during runtime for benchmarking. Let me see...
>>
>> Something like this, or is it going to be way more complex? Does
>> anyone have patch by chance?
> RHEL/CentOS has a global PTI enable/disable, which uses stop_machine().

Let's not forget PTI's NX-for-userspace in the kernel page tables.  That
provides Spectre V2 mitigation as well as Meltdown.

^ permalink raw reply

* [PATCH] tpm: add support for partial reads
From: Jason Gunthorpe @ 2018-07-23 22:08 UTC (permalink / raw)
  To: linux-security-module
In-Reply-To: <dabf6cf5-e62c-721f-9f19-18da34d52ec1@intel.com>

On Mon, Jul 23, 2018 at 03:00:20PM -0700, Tadeusz Struk wrote:
> On 07/23/2018 02:56 PM, Jason Gunthorpe wrote:
> > The proposed patch doesn't clear the data_pending if the entire buffer
> > is not consumed, so of course it is ABI breaking, that really isn't OK.
> 
> The data_pending will be cleared by the timeout handler if the user doesn't
> read the response fully before the timeout expires. The is the same situation
> if the user would not read the response at all.

That causes write() to fail with EBUSY

NAK from me on breaking the ABI like this

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [Fuego] Problem of cant find “useradd” in Functional.glibc
From: Tim.Bird @ 2018-07-23 22:12 UTC (permalink / raw)
  To: lixm.fnst, fuego
In-Reply-To: <62079D2F712F7747B0BDCC6821B8DF0D013D2794@G08CNEXMBPEKD02.g08.fujitsu.local>



> -----Original Message-----
> From: Behalf Of Li, Xiaoming
> Hi all:
>
> I have a question of cant find command “useradd” when working on
> /engine/tests/Functional.glibc/fuego_test.sh.
> 
> Over a preliminary investigation, I found that the default path(or maybe bin
> path)loaded by“ssh non-interactive non-login” does not include “useradd”.

Are you saying that this line is failing:
is_on_target_path useradd PROGRAM_USERADD
?

>  
> Methods below could solve this problem, anyone knows a better one ?
> 
> 1)source /etc/profile;
> 
> 2)    use “useradd” with absolute path, like user/sbin/useradd,        without
> checked  by is_on_target_path;

According to the Linux File System Hierarchy Standard, useradd is supposed
to be located in /usr/sbin.  See https://refspecs.linuxfoundation.org/FHS_3.0/fhs-3.0.pdf
So I think it would be acceptable to specify the full path: /usr/sbin/useradd.

A bit more robust is to use the value obtained from is_on_target (or is_on_target_path).

If /usr/sbin is not in the ssh interactive login PATH, then try this:
is_on_target useradd PROGRAM_USERADD /bin:/sbin:/usr/bin:/usr/sbin

In the echo statement that creates run-test.sh, you should be able to
use
    $PROGRAM_USERADD fuego_test_special;
instead of
    useradd fuego_test_special

As an aside, do we want the $(realpath /lib/libBrlkenLocale.so.1) 
variable substitution to happen on the host at run-test.sh creation
time, or on the target at run-test.sh execution time.  It looks to me
like it's getting substituted on host, but should rather be substituted
on target.  To do this, I think the $ would need to be escaped in this
set of echo lines.

I hope this is useful.
 -- Tim

^ permalink raw reply

* Re: [Qemu-devel] [PATCH for-3.0?] target/arm: Add sve-max-vq cpu property to -cpu max
From: Peter Maydell @ 2018-07-23 22:11 UTC (permalink / raw)
  To: Richard Henderson
  Cc: QEMU Developers, Markus Armbruster, Alex Bennée,
	Michael Roth
In-Reply-To: <20180723220047.31594-1-richard.henderson@linaro.org>

On 23 July 2018 at 23:00, Richard Henderson
<richard.henderson@linaro.org> wrote:
> This allows the default (and maximum) vector length to be set
> from the command-line.  Which is extraordinarily helpful in
> debuging problems depending on vector length without having to
> bake knowledge of PR_SET_SVE_VL into every guest binary.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>
> The argument for inclding this in 3.0 is that there appear to be
> several groups experimenting with SVE and using fixed vector lengths.
> While this is oddly against the design of SVE, it's probably going
> to keep happening.  Plus, the wildly useful debugging aspect.

...on the other hand, it's a new feature and we're about to
cut rc2 tomorrow. I'd prefer to leave this til after 3.0 releases.

thanks
-- PMM

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.