* [PATCH v3] net: ethernet: fix uninitialized pointers with free attribute
From: Ally Heev @ 2025-11-06 11:55 UTC (permalink / raw)
To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: Aleksandr Loktionov, intel-wired-lan, netdev, linux-kernel,
linux-hyperv, Dan Carpenter, Ally Heev
Uninitialized pointers with `__free` attribute can cause undefined
behavior as the memory assigned randomly to the pointer is freed
automatically when the pointer goes out of scope.
It is better to initialize and assign pointers with `__free`
attribute in one statement to ensure proper scope-based cleanup.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
Signed-off-by: Ally Heev <allyheev@gmail.com>
---
Changes in v3:
- fixed style issues
- Link to v2: https://lore.kernel.org/r/20251106-aheev-uninitialized-free-attr-net-ethernet-v2-1-048da0c5d6b6@gmail.com
Changes in v2:
- fixed non-pointer initialization to NULL
- NOTE: drop v1
- Link to v1: https://lore.kernel.org/r/20251105-aheev-uninitialized-free-attr-net-ethernet-v1-1-f6ea84bbd750@gmail.com
---
drivers/net/ethernet/intel/ice/ice_flow.c | 5 +++--
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c b/drivers/net/ethernet/intel/ice/ice_flow.c
index 6d5c939dc8a515c252cd2b77d155b69fa264ee92..3590dacf3ee57879b3809d715e40bb290e40c4aa 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.c
+++ b/drivers/net/ethernet/intel/ice/ice_flow.c
@@ -1573,12 +1573,13 @@ ice_flow_set_parser_prof(struct ice_hw *hw, u16 dest_vsi, u16 fdir_vsi,
struct ice_parser_profile *prof, enum ice_block blk)
{
u64 id = find_first_bit(prof->ptypes, ICE_FLOW_PTYPE_MAX);
- struct ice_flow_prof_params *params __free(kfree);
u8 fv_words = hw->blk[blk].es.fvw;
int status;
int i, idx;
- params = kzalloc(sizeof(*params), GFP_KERNEL);
+ struct ice_flow_prof_params *params __free(kfree) =
+ kzalloc(sizeof(*params), GFP_KERNEL);
+
if (!params)
return -ENOMEM;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index cbb5fa30f5a0ec778c1ee30470da3ca21cc1af24..368138715cd55cd1dadc686931cdda51c7a5130d 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -1012,7 +1012,6 @@ static int idpf_send_get_caps_msg(struct idpf_adapter *adapter)
*/
static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
{
- struct virtchnl2_get_lan_memory_regions *rcvd_regions __free(kfree);
struct idpf_vc_xn_params xn_params = {
.vc_op = VIRTCHNL2_OP_GET_LAN_MEMORY_REGIONS,
.recv_buf.iov_len = IDPF_CTLQ_MAX_BUF_LEN,
@@ -1023,7 +1022,9 @@ static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
ssize_t reply_sz;
int err = 0;
- rcvd_regions = kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL);
+ struct virtchnl2_get_lan_memory_regions *rcvd_regions __free(kfree) =
+ kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL);
+
if (!rcvd_regions)
return -ENOMEM;
---
base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
change-id: 20251105-aheev-uninitialized-free-attr-net-ethernet-7d106e4ab3f7
Best regards,
--
Ally Heev <allyheev@gmail.com>
^ permalink raw reply related
* Re: [Intel-wired-lan] [PATCH v2] net: ethernet: fix uninitialized pointers with free attr
From: ally heev @ 2025-11-06 11:37 UTC (permalink / raw)
To: Loktionov, Aleksandr, Nguyen, Anthony L, Kitszel, Przemyslaw,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, K. Y. Srinivasan, Haiyang Zhang, Wei Liu,
Cui, Dexuan
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
Dan Carpenter
In-Reply-To: <DS4PPF7551E6552E6053CC02144D0987191E5C2A@DS4PPF7551E6552.namprd11.prod.outlook.com>
On Thu, 2025-11-06 at 10:18 +0000, Loktionov, Aleksandr wrote:
[..]
> Code style:
> The new declaration + initializer is good, but please ensure both hunks stay within ~80 columns in drivers/net/*.
> Wrapping like this is fine:
>
> struct ice_flow_prof_params *params __free(kfree) =
> kzalloc(sizeof(*params), GFP_KERNEL);
I ran checkpatch with `$max_line_length` set to 80. It didn't throw any errors/warnings
Regards,
Ally
^ permalink raw reply
* RE: [Intel-wired-lan] [PATCH v2] net: ethernet: fix uninitialized pointers with free attr
From: Loktionov, Aleksandr @ 2025-11-06 10:18 UTC (permalink / raw)
To: Ally Heev, Nguyen, Anthony L, Kitszel, Przemyslaw, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Cui, Dexuan
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org,
Dan Carpenter
In-Reply-To: <20251106-aheev-uninitialized-free-attr-net-ethernet-v2-1-048da0c5d6b6@gmail.com>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Ally Heev
> Sent: Thursday, November 6, 2025 11:09 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; K. Y. Srinivasan <kys@microsoft.com>;
> Haiyang Zhang <haiyangz@microsoft.com>; Wei Liu <wei.liu@kernel.org>;
> Cui, Dexuan <decui@microsoft.com>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-hyperv@vger.kernel.org; Dan Carpenter
> <dan.carpenter@linaro.org>; Ally Heev <allyheev@gmail.com>
> Subject: [Intel-wired-lan] [PATCH v2] net: ethernet: fix uninitialized
> pointers with free attr
>
> Uninitialized pointers with `__free` attribute can cause undefined
> behaviour as the memory assigned(randomly) to the pointer is freed
> automatically when the pointer goes out of scope
>
> net/ethernet doesn't have any bugs related to this as of now, but it
> is better to initialize and assign pointers with `__free` attr in one
> statement to ensure proper scope-based cleanup
>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
> Signed-off-by: Ally Heev <allyheev@gmail.com>
> ---
> Changes in v2:
> - fixed non pointer initialization to NULL
> - NOTE: drop v1
> - Link to v1: https://lore.kernel.org/r/20251105-aheev-uninitialized-
> free-attr-net-ethernet-v1-1-f6ea84bbd750@gmail.com
> ---
...
> Best regards,
> --
> Ally Heev <allyheev@gmail.com>
Good day, Ally
Thanks for addressing the cleanup attribute initialization.
A few upstream‑style nits before this can go in:
Commit message:
Use US spelling: behavior instead of behaviour.
Expand abbreviations: replace attr with attribute.
Drop speculative sentence: “net/ethernet doesn't have any bugs …” – not needed for a fix commit.
Minor grammar: add a period before “net/ethernet”; remove parentheses in assigned(randomly).
Changelog:
Hyphenate “non‑pointer” in “fixed non pointer initialization to NULL”.
Subject line:
net: ethernet: fix uninitialized pointers with __free attribute (spell out “attribute”).
Code style:
The new declaration + initializer is good, but please ensure both hunks stay within ~80 columns in drivers/net/*.
Wrapping like this is fine:
struct ice_flow_prof_params *params __free(kfree) =
kzalloc(sizeof(*params), GFP_KERNEL);
No functional issues found; just these style and commit message cleanups.
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH] net: ethernet: fix uninitialized pointers with free attr
From: ally heev @ 2025-11-06 10:13 UTC (permalink / raw)
To: kernel test robot, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: oe-kbuild-all, netdev, intel-wired-lan, linux-kernel,
linux-hyperv, Dan Carpenter
In-Reply-To: <202511061627.TYBaNPrX-lkp@intel.com>
On Thu, 2025-11-06 at 17:06 +0800, kernel test robot wrote:
> Hi Ally,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on c9cfc122f03711a5124b4aafab3211cf4d35a2ac]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Ally-Heev/net-ethernet-fix-uninitialized-pointers-with-free-attr/20251105-192022
> base: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
> patch link: https://lore.kernel.org/r/20251105-aheev-uninitialized-free-attr-net-ethernet-v1-1-f6ea84bbd750%40gmail.com
> patch subject: [PATCH] net: ethernet: fix uninitialized pointers with free attr
> config: x86_64-randconfig-015-20251106 (https://download.01.org/0day-ci/archive/20251106/202511061627.TYBaNPrX-lkp@intel.com/config)
> compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251106/202511061627.TYBaNPrX-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202511061627.TYBaNPrX-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> In file included from include/uapi/linux/posix_types.h:5,
> from include/uapi/linux/types.h:14,
> from include/linux/types.h:6,
> from include/linux/objtool_types.h:7,
> from include/linux/objtool.h:5,
> from arch/x86/include/asm/bug.h:7,
> from include/linux/bug.h:5,
> from include/linux/vfsdebug.h:5,
> from include/linux/fs.h:5,
> from include/linux/debugfs.h:15,
> from drivers/net/ethernet/microsoft/mana/gdma_main.c:4:
> drivers/net/ethernet/microsoft/mana/gdma_main.c: In function 'irq_setup':
> > > include/linux/stddef.h:8:14: error: invalid initializer
> 8 | #define NULL ((void *)0)
> | ^
> drivers/net/ethernet/microsoft/mana/gdma_main.c:1508:55: note: in expansion of macro 'NULL'
> 1508 | cpumask_var_t cpus __free(free_cpumask_var) = NULL;
> | ^~~~
>
>
> vim +8 include/linux/stddef.h
>
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 6
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 7 #undef NULL
> ^1da177e4c3f41 Linus Torvalds 2005-04-16 @8 #define NULL ((void *)0)
> 6e218287432472 Richard Knutsson 2006-09-30 9
Sorry. I think I messed up config somehow during build. Hence, couldn't
catch the error in local. Fixed in v2
^ permalink raw reply
* [PATCH v2] net: ethernet: fix uninitialized pointers with free attr
From: Ally Heev @ 2025-11-06 10:08 UTC (permalink / raw)
To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: intel-wired-lan, netdev, linux-kernel, linux-hyperv,
Dan Carpenter, Ally Heev
Uninitialized pointers with `__free` attribute can cause undefined
behaviour as the memory assigned(randomly) to the pointer is freed
automatically when the pointer goes out of scope
net/ethernet doesn't have any bugs related to this as of now,
but it is better to initialize and assign pointers with `__free` attr
in one statement to ensure proper scope-based cleanup
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
Signed-off-by: Ally Heev <allyheev@gmail.com>
---
Changes in v2:
- fixed non pointer initialization to NULL
- NOTE: drop v1
- Link to v1: https://lore.kernel.org/r/20251105-aheev-uninitialized-free-attr-net-ethernet-v1-1-f6ea84bbd750@gmail.com
---
drivers/net/ethernet/intel/ice/ice_flow.c | 5 +++--
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c b/drivers/net/ethernet/intel/ice/ice_flow.c
index 6d5c939dc8a515c252cd2b77d155b69fa264ee92..3590dacf3ee57879b3809d715e40bb290e40c4aa 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.c
+++ b/drivers/net/ethernet/intel/ice/ice_flow.c
@@ -1573,12 +1573,13 @@ ice_flow_set_parser_prof(struct ice_hw *hw, u16 dest_vsi, u16 fdir_vsi,
struct ice_parser_profile *prof, enum ice_block blk)
{
u64 id = find_first_bit(prof->ptypes, ICE_FLOW_PTYPE_MAX);
- struct ice_flow_prof_params *params __free(kfree);
u8 fv_words = hw->blk[blk].es.fvw;
int status;
int i, idx;
- params = kzalloc(sizeof(*params), GFP_KERNEL);
+ struct ice_flow_prof_params *params __free(kfree) =
+ kzalloc(sizeof(*params), GFP_KERNEL);
+
if (!params)
return -ENOMEM;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index cbb5fa30f5a0ec778c1ee30470da3ca21cc1af24..368138715cd55cd1dadc686931cdda51c7a5130d 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -1012,7 +1012,6 @@ static int idpf_send_get_caps_msg(struct idpf_adapter *adapter)
*/
static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
{
- struct virtchnl2_get_lan_memory_regions *rcvd_regions __free(kfree);
struct idpf_vc_xn_params xn_params = {
.vc_op = VIRTCHNL2_OP_GET_LAN_MEMORY_REGIONS,
.recv_buf.iov_len = IDPF_CTLQ_MAX_BUF_LEN,
@@ -1023,7 +1022,9 @@ static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
ssize_t reply_sz;
int err = 0;
- rcvd_regions = kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL);
+ struct virtchnl2_get_lan_memory_regions *rcvd_regions __free(kfree) =
+ kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL);
+
if (!rcvd_regions)
return -ENOMEM;
---
base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
change-id: 20251105-aheev-uninitialized-free-attr-net-ethernet-7d106e4ab3f7
Best regards,
--
Ally Heev <allyheev@gmail.com>
^ permalink raw reply related
* Re: [PATCH] net: ethernet: fix uninitialized pointers with free attr
From: kernel test robot @ 2025-11-06 9:06 UTC (permalink / raw)
To: Ally Heev, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: oe-kbuild-all, netdev, intel-wired-lan, linux-kernel,
linux-hyperv, Dan Carpenter, Ally Heev
In-Reply-To: <20251105-aheev-uninitialized-free-attr-net-ethernet-v1-1-f6ea84bbd750@gmail.com>
Hi Ally,
kernel test robot noticed the following build errors:
[auto build test ERROR on c9cfc122f03711a5124b4aafab3211cf4d35a2ac]
url: https://github.com/intel-lab-lkp/linux/commits/Ally-Heev/net-ethernet-fix-uninitialized-pointers-with-free-attr/20251105-192022
base: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
patch link: https://lore.kernel.org/r/20251105-aheev-uninitialized-free-attr-net-ethernet-v1-1-f6ea84bbd750%40gmail.com
patch subject: [PATCH] net: ethernet: fix uninitialized pointers with free attr
config: x86_64-randconfig-015-20251106 (https://download.01.org/0day-ci/archive/20251106/202511061627.TYBaNPrX-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251106/202511061627.TYBaNPrX-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511061627.TYBaNPrX-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/uapi/linux/posix_types.h:5,
from include/uapi/linux/types.h:14,
from include/linux/types.h:6,
from include/linux/objtool_types.h:7,
from include/linux/objtool.h:5,
from arch/x86/include/asm/bug.h:7,
from include/linux/bug.h:5,
from include/linux/vfsdebug.h:5,
from include/linux/fs.h:5,
from include/linux/debugfs.h:15,
from drivers/net/ethernet/microsoft/mana/gdma_main.c:4:
drivers/net/ethernet/microsoft/mana/gdma_main.c: In function 'irq_setup':
>> include/linux/stddef.h:8:14: error: invalid initializer
8 | #define NULL ((void *)0)
| ^
drivers/net/ethernet/microsoft/mana/gdma_main.c:1508:55: note: in expansion of macro 'NULL'
1508 | cpumask_var_t cpus __free(free_cpumask_var) = NULL;
| ^~~~
vim +8 include/linux/stddef.h
^1da177e4c3f41 Linus Torvalds 2005-04-16 6
^1da177e4c3f41 Linus Torvalds 2005-04-16 7 #undef NULL
^1da177e4c3f41 Linus Torvalds 2005-04-16 @8 #define NULL ((void *)0)
6e218287432472 Richard Knutsson 2006-09-30 9
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* RE: [PATCH v3] mshv: Extend create partition ioctl to support cpu features
From: Michael Kelley @ 2025-11-06 4:56 UTC (permalink / raw)
To: Nuno Das Neves, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, wei.liu@kernel.org,
muislam@microsoft.com, easwar.hariharan@linux.microsoft.com
Cc: kys@microsoft.com, haiyangz@microsoft.com, decui@microsoft.com,
longli@microsoft.com, skinsburskii@linux.microsoft.com,
romank@linux.microsoft.com, Jinank Jain
In-Reply-To: <31e9a5be-05d6-49f8-893e-2d8986dc8ce4@linux.microsoft.com>
From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Wednesday, November 5, 2025 11:10 AM
>
> On 11/5/2025 9:41 AM, Michael Kelley wrote:
> > From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Tuesday, November 4, 2025 1:47 PM
> >>
> >> From: Muminul Islam <muislam@microsoft.com>
> >>
> >> The existing mshv create partition ioctl does not provide a way to
> >> specify which cpu features are enabled in the guest. Instead, it
> >> attempts to enable all features and those that are not supported are
> >> silently disabled by the hypervisor.
> >>
> >> This was done to reduce unnecessary complexity and is sufficient for
> >> many cases. However, new scenarios require fine-grained control over
> >> these features.
> >>
> >> Define a new mshv_create_partition_v2 structure which supports
> >> passing the disabled processor and xsave feature bits through to the
> >> create partition hypercall directly.
> >>
> >> The kernel does not introspect the bits in these new fields as they
> >> are part of the hypervisor ABI. Require the caller to provide the
> >> number of cpu feature banks passed, to support extending the number
> >> of banks in future. Disable all banks that are not specified to ensure
> >> the behavior is predictable with newer hypervisors.
> >>
> >> Introduce a new flag MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES which enables
> >> the new structure. If unset, the original mshv_create_partition struct
> >> is used, with the old behavior of enabling all features.
> >>
> >> Co-developed-by: Jinank Jain <jinankjain@microsoft.com>
> >> Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
> >> Signed-off-by: Muminul Islam <muislam@microsoft.com>
> >> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> >> ---
> >> Changes in v3:
> >> - Remove the new cpu features definitions in hvhdk.h, and retain the
> >> old behavior of enabling all features for the old struct. For the v2
> >> struct, still disable unspecified feature banks, since that makes it
> >> robust to future extensions.
> >> - Amend comments and commit message to reflect the above
> >> - Fix unused variable on arm64 [kernel test robot]
> >>
> >> Changes in v2:
> >> - Fix exposure of CONFIG_X86_64 to uapi [kernel test robot]
> >> - Fix compilation issue on arm64 [kernel test robot]
> >>
> >> ---
> >> drivers/hv/mshv_root_main.c | 94 ++++++++++++++++++++++++++++++-------
> >> include/uapi/linux/mshv.h | 34 ++++++++++++++
> >> 2 files changed, 110 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> >> index d542a0143bb8..814465a0912d 100644
> >> --- a/drivers/hv/mshv_root_main.c
> >> +++ b/drivers/hv/mshv_root_main.c
> >> @@ -1900,43 +1900,101 @@ add_partition(struct mshv_partition *partition)
> >> return 0;
> >> }
> >>
> >> -static long
> >> -mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
> >> +static_assert(MSHV_NUM_CPU_FEATURES_BANKS <=
> >> + HV_PARTITION_PROCESSOR_FEATURES_BANKS);
> >> +
> >> +static long mshv_ioctl_process_pt_flags(void __user *user_arg, u64 *pt_flags,
> >> + struct hv_partition_creation_properties *cr_props,
> >> + union hv_partition_isolation_properties *isol_props)
> >> {
> >> - struct mshv_create_partition args;
> >> - u64 creation_flags;
> >> - struct hv_partition_creation_properties creation_properties = {};
> >> - union hv_partition_isolation_properties isolation_properties = {};
> >> - struct mshv_partition *partition;
> >> - struct file *file;
> >> - int fd;
> >> - long ret;
> >> + int i;
> >> + struct mshv_create_partition_v2 args;
> >> + union hv_partition_processor_features *disabled_procs;
> >> + union hv_partition_processor_xsave_features *disabled_xsave;
> >>
> >> - if (copy_from_user(&args, user_arg, sizeof(args)))
> >> + /* First, copy orig struct in case user is on previous versions */
> >> + if (copy_from_user(&args, user_arg,
> >> + sizeof(struct mshv_create_partition)))
> >> return -EFAULT;
> >>
> >> if ((args.pt_flags & ~MSHV_PT_FLAGS_MASK) ||
> >> args.pt_isolation >= MSHV_PT_ISOLATION_COUNT)
> >> return -EINVAL;
> >>
> >> + disabled_procs = &cr_props->disabled_processor_features;
> >> + disabled_xsave = &cr_props->disabled_processor_xsave_features;
> >> +
> >> + /* Check if user provided newer struct with feature fields */
> >> + if (args.pt_flags & BIT(MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES)) {
> >
> > Should probably be "BIT_ULL" instead of "BIT" since args.pt_flags is a long long field,
> > though it really doesn't matter as long as the number of flags is <= 32.
> >
> Noted, thanks
>
> >> + if (copy_from_user(&args, user_arg, sizeof(args)))
> >> + return -EFAULT;
> >> +
> >> + if (args.pt_num_cpu_fbanks > MSHV_NUM_CPU_FEATURES_BANKS ||
> >> + mshv_field_nonzero(args, pt_rsvd) ||
> >> + mshv_field_nonzero(args, pt_rsvd1))
> >> + return -EINVAL;
> >> +
> >> +
> >> + for (i = 0; i < args.pt_num_cpu_fbanks; i++)
> >> + disabled_procs->as_uint64[i] = args.pt_cpu_fbanks[i];
> >> +
> >> + /* Disable any features left unspecified */
> >> + for (; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
> >> + disabled_procs->as_uint64[i] = -1;
> >
> > I'm trying to convince myself that disabling unspecified features is the right
> > thing to do. In the current hypervisor scenario with 2 banks, if the VMM caller
> > specifies only one bank of disable flags, then all the features in the 2nd bank
> > are disabled. That's certainly the reverse from the current code which
> > always enables all features, and from the hypervisor itself which in the
> > hypercall ABI defines the flags as "disable" flags rather than "enable" flags.
> >
> > Then in a scenario where a new version of the hypervisor shows up with
> > support for 3 banks, the old VMM code that only knows about 2 banks
> > will cause all features in that 3rd bank to be disabled. Again, that's the
> > reverse of the current code.
> >
> > I guess it depends on how the hypervisor defines any such new features.
> > Are they typically defined to be benign if they are enabled by default? Or
> > is the polarity the opposite, where the VMM must know about new
> > features before they are enabled? The hypercall interface seems to imply
> > the former but maybe I'm reading too much into it.
> >
> The intent is to provide an interface which allows the VMM to control exactly
> which features are enabled/disabled. E.g. for live migration of a VM, if the
> target machine has more features available and they are enabled inadvertently,
> the state may not be restored properly (particularly an issue for xsave).
>
> So to me it makes sense to disable anything unspecified. In general, enabling
> features by default doesn't cause problems, it's only for specific scenarios
> like the above. I suppose that's why it's a "disable" mask, though I can't
> say I fully understand the reasoning...
>
> > A code comment about the thinking here would be useful for future readers.
> >
> Noted. I can repeat the reasoning from the commit message if that is
> sufficient:
> "
> Disable all banks that are not specified to ensure
> the behavior is predictable with newer hypervisors.
> "
Before deciding on the wording of the comment, one more thought
occurred to me. union hv_partition_processor_features is currently
two 64-bit banks. Bank 0 currently has 63 features plus 1 reserved bit.
Bank 1 currently has 22 features, and 42 reserved bits. A new version
of the hypervisor could use one or more of those 42 reserved bits for
new features.
If a VMM is running on a newer hypervisor version that implements
features the VMM is unaware of, the intent is that those features
should be disabled by default. So is the expectation that when a
VMM provides Bank 0 and Bank 1 values, it should set all the
reserved bits to 1? (currently the single bit in Bank 0 and the 42 bits
in Bank 1) My point is that the default disabling of new features can't
be handled entirely by the kernel implementation of the ioctl based
on the bank count passed in the argument to the ioctl. The VMM
must cooperate as well. And such splitting of the responsibility
seems rather messy.
I see three cleaner alternatives:
1) Have the argument to the ioctl pass the "max known feature number"
instead of the bank count. Then the kernel implementation could set to
1 all feature bits after that max. This alternative makes the kernel
fully responsible for doing the default disabling, based on what the
VMM tells the kernel it knows about.
2) Define the expected future Bank 2 and Bank 3 fields in
struct mshv_create_partition_v2, and require the VMM to set them to
all 1's as well as the reserved fields in Bank 0 and Bank 1. This alternative
makes the VMM fully responsible for doing the default disabling.
3) As a variant of my #2, invert the polarity of the bits in the pt_cpu_fbanks
field of the ioctl argument, so that from the VMM's standpoint they are
feature *enable* bits, not feature *disable* bits. The VMM sets bits for
the features it knows about and wants to have enabled, which is the
much more common pattern. Kernel code would then invert each bank
before passing to the hypercall. The Bank 2 and Bank 3 fields would be
set to zero by the VMM, and get the same kernel treatment when future
hypervisor versions accept the additional banks.
Thoughts?
Michael
^ permalink raw reply
* Re: [PATCH net-next v2] net: mana: Implement ndo_tx_timeout and serialize queue resets per port.
From: Dipayaan Roy @ 2025-11-06 4:37 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, longli, kotaranov, horms, shradhagupta, ssengar, ernis,
shirazsaleem, linux-hyperv, netdev, linux-kernel, linux-rdma,
dipayanroy
In-Reply-To: <20251029182233.59aea2d3@kernel.org>
On Wed, Oct 29, 2025 at 06:22:33PM -0700, Jakub Kicinski wrote:
> On Mon, 27 Oct 2025 13:13:51 -0700 Dipayaan Roy wrote:
> > Implement .ndo_tx_timeout for MANA so any stalled TX queue can be detected
> > and a device-controlled port reset for all queues can be scheduled to a
> > ordered workqueue. The reset for all queues on stall detection is
> > recomended by hardware team.
> >
> > The change introduces a single ordered workqueue
> > ("mana_per_port_queue_reset_wq") with WQ_UNBOUND | WQ_MEM_RECLAIM and
> > queues exactly one work_struct per port onto it. This achieves:
> >
> > * Global FIFO across all port reset requests (alloc_ordered_workqueue).
>
> Why does this matter?
>
As per HW team,it should be processing only one port reset at any point
of time, so we need the serialization.
> > * Natural per-port de-duplication: the same work_struct cannot be
> > queued twice while pending/running.
>
> That's true for all work_structs even without a separate wq.
>
Right, (verbose, will remove).
> > * Avoids hogging a per-CPU kworker for long, may-sleep reset paths
> > (WQ_UNBOUND).
>
> ?
>
> > * Guarantees forward progress during memory pressure
> > (WQ_MEM_RECLAIM rescuer).
>
> ? meaning? What scenario are you preventing?
>
I was trying to explain the usage/benefit of these flags, but seems verbose here.
I will rephrase the better in v3.
> > Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> > ---
> > Changes in v2:
> > - Fixed cosmetic changes.
> > ---
> > drivers/net/ethernet/microsoft/mana/mana_en.c | 82 +++++++++++++++++++
> > include/net/mana/gdma.h | 6 +-
> > include/net/mana/mana.h | 15 ++++
> > 3 files changed, 102 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > index f4fc86f20213..05b7046ae3b5 100644
> > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> > @@ -258,6 +258,45 @@ static int mana_get_gso_hs(struct sk_buff *skb)
> > return gso_hs;
> > }
> >
> > +static void mana_per_port_queue_reset_work_handler(struct work_struct *work)
> > +{
> > + struct mana_queue_reset_work *reset_queue_work =
> > + container_of(work, struct mana_queue_reset_work, work);
> > + struct mana_port_context *apc = reset_queue_work->apc;
> > + struct net_device *ndev = apc->ndev;
> > + struct mana_context *ac = apc->ac;
> > + int err;
> > +
> > + if (!rtnl_trylock()) {
>
> Use disable_work_sync() in the remove path and you won't have to do
> the retry dance, right?
>
Looks like we might end up the same issue with both cancel_work_sync and
disable_work_sync(), unless the call to them is made out of rtnl_lock/unlock
in mana_remove? If thats the right approach we can drop rtnl_trylock here and
have only rtnl_lock().
> > + /* Someone else holds RTNL, requeue and exit. */
> > + queue_work(ac->per_port_queue_reset_wq,
> > + &apc->queue_reset_work.work);
> > + return;
> > + }
>
> > +static void mana_tx_timeout(struct net_device *netdev, unsigned int txqueue)
> > +{
> > + struct mana_port_context *apc = netdev_priv(netdev);
> > + struct mana_context *ac = apc->ac;
> > + struct gdma_context *gc = ac->gdma_dev->gdma_context;
> > +
> > + netdev_warn(netdev, "%s(): called on txq: %u\n", __func__, txqueue);
>
> I believe that core already prints this sort of thing, please don't
> duplicate.
>
Ok, will address it in v3.
> > + /* Already in service, hence tx queue reset is not required.*/
> > + if (gc->in_service)
> > + return;
> > +
> > + /* Note: If there are pending queue reset work for this port(apc),
> > + * subsequent request queued up from here are ignored. This is because
> > + * we are using the same work instance per port(apc).
> > + */
> > + queue_work(ac->per_port_queue_reset_wq, &apc->queue_reset_work.work);
> > +}
> > +
> > static int mana_shaper_set(struct net_shaper_binding *binding,
> > const struct net_shaper *shaper,
> > struct netlink_ext_ack *extack)
> > @@ -844,7 +902,9 @@ static const struct net_device_ops mana_devops = {
> > .ndo_bpf = mana_bpf,
> > .ndo_xdp_xmit = mana_xdp_xmit,
> > .ndo_change_mtu = mana_change_mtu,
> > + .ndo_tx_timeout = mana_tx_timeout,
>
> other ndos were aligned with tabs you're using spaces
>
Ok, will address it in v3.
> > .net_shaper_ops = &mana_shaper_ops,
> > +
> > };
> >
> > static void mana_cleanup_port_context(struct mana_port_context *apc)
> > @@ -3218,6 +3278,7 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
> > ndev->min_mtu = ETH_MIN_MTU;
> > ndev->needed_headroom = MANA_HEADROOM;
> > ndev->dev_port = port_idx;
> > + ndev->watchdog_timeo = MANA_TXQ_TIMEOUT;
>
> why you need a define for this is unclear. You can use 15 * HZ here
> directly
>
Ok, will address it in v3.
> > SET_NETDEV_DEV(ndev, gc->dev);
> >
> > netif_set_tso_max_size(ndev, GSO_MAX_SIZE);
> > @@ -3255,6 +3316,11 @@ static int mana_probe_port(struct mana_context *ac, int port_idx,
> >
> > debugfs_create_u32("current_speed", 0400, apc->mana_port_debugfs, &apc->speed);
> >
> > + /* Initialize the per port queue reset work.*/
> > + apc->queue_reset_work.apc = apc;
> > + INIT_WORK(&apc->queue_reset_work.work,
> > + mana_per_port_queue_reset_work_handler);
> > +
> > return 0;
> >
> > free_indir:
> > @@ -3456,6 +3522,15 @@ int mana_probe(struct gdma_dev *gd, bool resuming)
> > if (ac->num_ports > MAX_PORTS_IN_MANA_DEV)
> > ac->num_ports = MAX_PORTS_IN_MANA_DEV;
> >
> > + ac->per_port_queue_reset_wq =
> > + alloc_ordered_workqueue("mana_per_port_queue_reset_wq",
> > + WQ_UNBOUND | WQ_MEM_RECLAIM);
> > + if (!ac->per_port_queue_reset_wq) {
> > + dev_err(dev, "Failed to allocate per port queue reset workqueue\n");
> > + err = -ENOMEM;
> > + goto out;
> > + }
> > +
> > if (!resuming) {
> > for (i = 0; i < ac->num_ports; i++) {
> > err = mana_probe_port(ac, i, &ac->ports[i]);
> > @@ -3528,6 +3603,8 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
> > */
> > rtnl_lock();
> >
> > + cancel_work_sync(&apc->queue_reset_work.work);
> > +
> > err = mana_detach(ndev, false);
> > if (err)
> > netdev_err(ndev, "Failed to detach vPort %d: %d\n",
> > @@ -3547,6 +3624,11 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
> > free_netdev(ndev);
> > }
> >
> > + if (ac->per_port_queue_reset_wq) {
> > + destroy_workqueue(ac->per_port_queue_reset_wq);
> > + ac->per_port_queue_reset_wq = NULL;
> > + }
> > +
> > mana_destroy_eq(ac);
> > out:
> > mana_gd_deregister_device(gd);
> > diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
> > index 57df78cfbf82..1f8c536ba3be 100644
> > --- a/include/net/mana/gdma.h
> > +++ b/include/net/mana/gdma.h
> > @@ -591,6 +591,9 @@ enum {
> > /* Driver can self reset on FPGA Reconfig EQE notification */
> > #define GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE BIT(17)
> >
> > +/* Driver detects stalled send queues and recovers them */
> > +#define GDMA_DRV_CAP_FLAG_1_HANDLE_STALL_SQ_RECOVERY BIT(18)
> > +
> > #define GDMA_DRV_CAP_FLAGS1 \
> > (GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
> > GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
> > @@ -599,7 +602,8 @@ enum {
> > GDMA_DRV_CAP_FLAG_1_DEV_LIST_HOLES_SUP | \
> > GDMA_DRV_CAP_FLAG_1_DYNAMIC_IRQ_ALLOC_SUPPORT | \
> > GDMA_DRV_CAP_FLAG_1_SELF_RESET_ON_EQE | \
> > - GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE)
> > + GDMA_DRV_CAP_FLAG_1_HANDLE_RECONFIG_EQE | \
> > + GDMA_DRV_CAP_FLAG_1_HANDLE_STALL_SQ_RECOVERY)
> >
> > #define GDMA_DRV_CAP_FLAGS2 0
> >
> > diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
> > index 0921485565c0..e0b44ae2226a 100644
> > --- a/include/net/mana/mana.h
> > +++ b/include/net/mana/mana.h
> > @@ -67,6 +67,11 @@ enum TRI_STATE {
> >
> > #define MANA_RX_FRAG_ALIGNMENT 64
> >
> > +/* Timeout value for Txq stall detection & recovery used by ndo_tx_timeout.
> > + * The value is chosen after considering fpga re-config scenarios.
> > + */
> > +#define MANA_TXQ_TIMEOUT (15 * HZ)
> > +
> > struct mana_stats_rx {
> > u64 packets;
> > u64 bytes;
> > @@ -475,13 +480,23 @@ struct mana_context {
> >
> > struct mana_eq *eqs;
> > struct dentry *mana_eqs_debugfs;
> > + struct workqueue_struct *per_port_queue_reset_wq;
> >
> > struct net_device *ports[MAX_PORTS_IN_MANA_DEV];
> > };
> >
> > +struct mana_queue_reset_work {
> > +
>
> nit spurious empty line
>
Ok, will address it in v3.
> > + /* Work structure */
> > + struct work_struct work;
> > + /* Pointer to the port context */
> > + struct mana_port_context *apc;
>
> This struct is placed in apc, unclear why you need a backpointer
> instead of using container_of()
>
Right, I will address this in v3.
> > +};
> > +
> > struct mana_port_context {
> > struct mana_context *ac;
> > struct net_device *ndev;
> > + struct mana_queue_reset_work queue_reset_work;
> >
> > u8 mac_addr[ETH_ALEN];
> >
>
> --
> pw-bot: cr
Thank you for the review comments.
I will work on addressing them in the v3.
Regards
Dipayaan Roy
^ permalink raw reply
* Re: [PATCH net-next v2] net: mana: Handle SKB if TX SGEs exceed hardware limit
From: Jakub Kicinski @ 2025-11-06 0:17 UTC (permalink / raw)
To: Aditya Garg
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, longli, kotaranov, horms, shradhagupta, ssengar, ernis,
dipayanroy, shirazsaleem, linux-hyperv, netdev, linux-kernel,
linux-rdma, gargaditya
In-Reply-To: <82bcd959-571e-42ce-b341-cbfa19f9f86d@linux.microsoft.com>
On Wed, 5 Nov 2025 22:10:23 +0530 Aditya Garg wrote:
> >> if (err) {
> >> (void)skb_dequeue_tail(&txq->pending_skbs);
> >> + mana_unmap_skb(skb, apc);
> >> netdev_warn(ndev, "Failed to post TX OOB: %d\n", err);
> >
> > You have a print right here and in the callee. This condition must
> > (almost) never happen in practice. It's likely fine to just drop
> > the packet.
>
> The logs placed in callee doesn't covers all the failure scenarios,
> hence I feel to have this log here with proper status. Maybe I can
> remove the log in the callee?
I think my point was that since there are logs (per packet!) when the
condition is hit -- if it did in fact hit with any noticeable frequency
your users would have complained. So handling the condition gracefully
and returning BUSY is likely just unnecessary complexity in practice.
The logs themselves I don't care all that much about. Sure, having two
lines for one error is a bit unclean.
> > Either way -- this should be a separate patch.
> >
> Are you suggesting a separate patch altogether or two patch in the same
> series?
The changes feel related enough to make them a series, but either way
is fine.
^ permalink raw reply
* Re: [PATCH net-next v3] net: mana: Fix incorrect speed reported by debugfs
From: Jacob Keller @ 2025-11-05 22:15 UTC (permalink / raw)
To: Erni Sri Satya Vennela, kys, haiyangz, wei.liu, decui,
andrew+netdev, davem, edumazet, kuba, pabeni, shradhagupta,
ssengar, dipayanroy, shirazsaleem, kotaranov, longli,
linux-hyperv, netdev, linux-kernel
In-Reply-To: <1762369468-32570-1-git-send-email-ernis@linux.microsoft.com>
[-- Attachment #1.1: Type: text/plain, Size: 1701 bytes --]
On 11/5/2025 11:04 AM, Erni Sri Satya Vennela wrote:
> Once the netshaper is created for MANA, the current bandwidth
> is reported in debugfs like this:
>
> $ sudo ./tools/net/ynl/pyynl/cli.py \
> --spec Documentation/netlink/specs/net_shaper.yaml \
> --do set \
> --json '{"ifindex":'3',
> "handle":{ "scope": "netdev", "id":'1' },
> "bw-max": 200000000 }'
> None
>
> $ sudo cat /sys/kernel/debug/mana/1/vport0/current_speed
> 200
>
> After the shaper is deleted, it is expected to report
> the maximum speed supported by the SKU. But currently it is
> reporting 0, which is incorrect.
>
> Fix this inconsistency, by resetting apc->speed to apc->max_speed
> during deletion of the shaper object. This will improve
> readability and debuggability.
>
> Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> ---
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> Changes in v3:
> * Remove Fixes tag.> Changes in v2:
> * Add Fixes tag.
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 0142fd98392c..9d56bfefd755 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -814,7 +814,7 @@ static int mana_shaper_del(struct net_shaper_binding *binding,
> /* Reset mana port context parameters */
> apc->handle.id = 0;
> apc->handle.scope = NET_SHAPER_SCOPE_UNSPEC;
> - apc->speed = 0;
> + apc->speed = apc->max_speed;
> }
>
> return err;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]
^ permalink raw reply
* Re: [PATCH v3] mshv: Extend create partition ioctl to support cpu features
From: Nuno Das Neves @ 2025-11-05 19:10 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, wei.liu@kernel.org,
muislam@microsoft.com, easwar.hariharan@linux.microsoft.com
Cc: kys@microsoft.com, haiyangz@microsoft.com, decui@microsoft.com,
longli@microsoft.com, skinsburskii@linux.microsoft.com,
romank@linux.microsoft.com, Jinank Jain
In-Reply-To: <SN6PR02MB41571FC666D406397858A377D4C5A@SN6PR02MB4157.namprd02.prod.outlook.com>
On 11/5/2025 9:41 AM, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Tuesday, November 4, 2025 1:47 PM
>>
>> From: Muminul Islam <muislam@microsoft.com>
>>
>> The existing mshv create partition ioctl does not provide a way to
>> specify which cpu features are enabled in the guest. Instead, it
>> attempts to enable all features and those that are not supported are
>> silently disabled by the hypervisor.
>>
>> This was done to reduce unnecessary complexity and is sufficient for
>> many cases. However, new scenarios require fine-grained control over
>> these features.
>>
>> Define a new mshv_create_partition_v2 structure which supports
>> passing the disabled processor and xsave feature bits through to the
>> create partition hypercall directly.
>>
>> The kernel does not introspect the bits in these new fields as they
>> are part of the hypervisor ABI. Require the caller to provide the
>> number of cpu feature banks passed, to support extending the number
>> of banks in future. Disable all banks that are not specified to ensure
>> the behavior is predictable with newer hypervisors.
>>
>> Introduce a new flag MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES which enables
>> the new structure. If unset, the original mshv_create_partition struct
>> is used, with the old behavior of enabling all features.
>>
>> Co-developed-by: Jinank Jain <jinankjain@microsoft.com>
>> Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
>> Signed-off-by: Muminul Islam <muislam@microsoft.com>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> ---
>> Changes in v3:
>> - Remove the new cpu features definitions in hvhdk.h, and retain the
>> old behavior of enabling all features for the old struct. For the v2
>> struct, still disable unspecified feature banks, since that makes it
>> robust to future extensions.
>> - Amend comments and commit message to reflect the above
>> - Fix unused variable on arm64 [kernel test robot]
>>
>> Changes in v2:
>> - Fix exposure of CONFIG_X86_64 to uapi [kernel test robot]
>> - Fix compilation issue on arm64 [kernel test robot]
>>
>> ---
>> drivers/hv/mshv_root_main.c | 94 ++++++++++++++++++++++++++++++-------
>> include/uapi/linux/mshv.h | 34 ++++++++++++++
>> 2 files changed, 110 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
>> index d542a0143bb8..814465a0912d 100644
>> --- a/drivers/hv/mshv_root_main.c
>> +++ b/drivers/hv/mshv_root_main.c
>> @@ -1900,43 +1900,101 @@ add_partition(struct mshv_partition *partition)
>> return 0;
>> }
>>
>> -static long
>> -mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
>> +static_assert(MSHV_NUM_CPU_FEATURES_BANKS <=
>> + HV_PARTITION_PROCESSOR_FEATURES_BANKS);
>> +
>> +static long mshv_ioctl_process_pt_flags(void __user *user_arg, u64 *pt_flags,
>> + struct hv_partition_creation_properties *cr_props,
>> + union hv_partition_isolation_properties *isol_props)
>> {
>> - struct mshv_create_partition args;
>> - u64 creation_flags;
>> - struct hv_partition_creation_properties creation_properties = {};
>> - union hv_partition_isolation_properties isolation_properties = {};
>> - struct mshv_partition *partition;
>> - struct file *file;
>> - int fd;
>> - long ret;
>> + int i;
>> + struct mshv_create_partition_v2 args;
>> + union hv_partition_processor_features *disabled_procs;
>> + union hv_partition_processor_xsave_features *disabled_xsave;
>>
>> - if (copy_from_user(&args, user_arg, sizeof(args)))
>> + /* First, copy orig struct in case user is on previous versions */
>> + if (copy_from_user(&args, user_arg,
>> + sizeof(struct mshv_create_partition)))
>> return -EFAULT;
>>
>> if ((args.pt_flags & ~MSHV_PT_FLAGS_MASK) ||
>> args.pt_isolation >= MSHV_PT_ISOLATION_COUNT)
>> return -EINVAL;
>>
>> + disabled_procs = &cr_props->disabled_processor_features;
>> + disabled_xsave = &cr_props->disabled_processor_xsave_features;
>> +
>> + /* Check if user provided newer struct with feature fields */
>> + if (args.pt_flags & BIT(MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES)) {
>
> Should probably be "BIT_ULL" instead of "BIT" since args.pt_flags is a long long field,
> though it really doesn't matter as long as the number of flags is <= 32.
>
Noted, thanks
>> + if (copy_from_user(&args, user_arg, sizeof(args)))
>> + return -EFAULT;
>> +
>> + if (args.pt_num_cpu_fbanks > MSHV_NUM_CPU_FEATURES_BANKS ||
>> + mshv_field_nonzero(args, pt_rsvd) ||
>> + mshv_field_nonzero(args, pt_rsvd1))
>> + return -EINVAL;
>> +
>> +
>> + for (i = 0; i < args.pt_num_cpu_fbanks; i++)
>> + disabled_procs->as_uint64[i] = args.pt_cpu_fbanks[i];
>> +
>> + /* Disable any features left unspecified */
>> + for (; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
>> + disabled_procs->as_uint64[i] = -1;
>
> I'm trying to convince myself that disabling unspecified features is the right
> thing to do. In the current hypervisor scenario with 2 banks, if the VMM caller
> specifies only one bank of disable flags, then all the features in the 2nd bank
> are disabled. That's certainly the reverse from the current code which
> always enables all features, and from the hypervisor itself which in the
> hypercall ABI defines the flags as "disable" flags rather than "enable" flags.
>
> Then in a scenario where a new version of the hypervisor shows up with
> support for 3 banks, the old VMM code that only knows about 2 banks
> will cause all features in that 3rd bank to be disabled. Again, that's the
> reverse of the current code.
>
> I guess it depends on how the hypervisor defines any such new features.
> Are they typically defined to be benign if they are enabled by default? Or
> is the polarity the opposite, where the VMM must know about new
> features before they are enabled? The hypercall interface seems to imply
> the former but maybe I'm reading too much into it.
>
The intent is to provide an interface which allows the VMM to control exactly
which features are enabled/disabled. E.g. for live migration of a VM, if the
target machine has more features available and they are enabled inadvertently,
the state may not be restored properly (particularly an issue for xsave).
So to me it makes sense to disable anything unspecified. In general, enabling
features by default doesn't cause problems, it's only for specific scenarios
like the above. I suppose that's why it's a "disable" mask, though I can't
say I fully understand the reasoning...
> A code comment about the thinking here would be useful for future readers.
>
Noted. I can repeat the reasoning from the commit message if that is
sufficient:
"
Disable all banks that are not specified to ensure
the behavior is predictable with newer hypervisors.
"
>> +
>> +#if IS_ENABLED(CONFIG_X86_64)
>> + disabled_xsave->as_uint64 = args.pt_disabled_xsave;
>> +#else
>> + if (mshv_field_nonzero(args, pt_rsvd2))
>> + return -EINVAL;
>> +
>> + disabled_xsave->as_uint64 = -1;
>
> Does the arm64 version of the hypercall do anything with this field?
> Or does it just ignore it? I would be more inclined to set ignored
> fields to zero unless there's an identifiable reason otherwise.
> (especially since the VMM is required to pass in zero on arm64).
>
Good point. Checking the code, it seems to be completely ignored on arm64.
I will remove this line.
>> +#endif
>> + } else {
>> + /* v1 behavior: try to enable everything */
>> + for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
>> + disabled_procs->as_uint64[i] = 0;
>> +
>> + disabled_xsave->as_uint64 = 0;
>> + }
>> +
>> /* Only support EXO partitions */
>> - creation_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
>> + *pt_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION |
>> + HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
>>
>> if (args.pt_flags & BIT(MSHV_PT_BIT_LAPIC))
>> - creation_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
>> + *pt_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
>> if (args.pt_flags & BIT(MSHV_PT_BIT_X2APIC))
>> - creation_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
>> + *pt_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
>> if (args.pt_flags & BIT(MSHV_PT_BIT_GPA_SUPER_PAGES))
>> - creation_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
>> + *pt_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
>>
>> switch (args.pt_isolation) {
>> case MSHV_PT_ISOLATION_NONE:
>> - isolation_properties.isolation_type =
>> - HV_PARTITION_ISOLATION_TYPE_NONE;
>> + isol_props->isolation_type = HV_PARTITION_ISOLATION_TYPE_NONE;
>> break;
>> }
>>
>> + return 0;
>> +}
>> +
>> +static long
>> +mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
>> +{
>> + u64 creation_flags;
>> + struct hv_partition_creation_properties creation_properties = {};
>> + union hv_partition_isolation_properties isolation_properties = {};
>> + struct mshv_partition *partition;
>> + struct file *file;
>> + int fd;
>> + long ret;
>> +
>> + ret = mshv_ioctl_process_pt_flags(user_arg, &creation_flags,
>> + &creation_properties,
>> + &isolation_properties);
>> + if (ret)
>> + return ret;
>> +
>> partition = kzalloc(sizeof(*partition), GFP_KERNEL);
>> if (!partition)
>> return -ENOMEM;
>> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
>> index 876bfe4e4227..9091946cba23 100644
>> --- a/include/uapi/linux/mshv.h
>> +++ b/include/uapi/linux/mshv.h
>> @@ -26,6 +26,7 @@ enum {
>> MSHV_PT_BIT_LAPIC,
>> MSHV_PT_BIT_X2APIC,
>> MSHV_PT_BIT_GPA_SUPER_PAGES,
>> + MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES,
>> MSHV_PT_BIT_COUNT,
>> };
>>
>> @@ -41,6 +42,8 @@ enum {
>> * @pt_flags: Bitmask of 1 << MSHV_PT_BIT_*
>> * @pt_isolation: MSHV_PT_ISOLATION_*
>> *
>> + * This is the initial/v0 version for backward compatibility.
>> + *
>> * Returns a file descriptor to act as a handle to a guest partition.
>> * At this point the partition is not yet initialized in the hypervisor.
>> * Some operations must be done with the partition in this state, e.g. setting
>> @@ -52,6 +55,37 @@ struct mshv_create_partition {
>> __u64 pt_isolation;
>> };
>>
>> +#define MSHV_NUM_CPU_FEATURES_BANKS 2
>> +
>> +/**
>> + * struct mshv_create_partition_v2
>> + *
>> + * This is extended version of the above initial MSHV_CREATE_PARTITION
>> + * ioctl and allows for following additional parameters:
>> + *
>> + * @pt_num_cpu_fbanks: number of processor feature banks being provided.
>> + * This must not exceed MSHV_NUM_CPU_FEATURES_BANKS.
>> + * If set to less than the number of available banks,
>> + * additional banks will be set to -1 (disabled).
>> + * @pt_cpu_fbanks: disabled processor feature banks array.
>> + * @pt_disabled_xsave: disabled xsave feature bits.
>> + *
>> + * Returns : same as above original mshv_create_partition
>> + */
>> +struct mshv_create_partition_v2 {
>> + __u64 pt_flags;
>> + __u64 pt_isolation;
>> + __u16 pt_num_cpu_fbanks;
>> + __u8 pt_rsvd[6]; /* MBZ */
>> + __u64 pt_cpu_fbanks[MSHV_NUM_CPU_FEATURES_BANKS];
>> + __u64 pt_rsvd1[2]; /* MBZ */
>
> I presume this is for future expansion of the number of banks?
> And that the choice of 2 additional banks is somewhat arbitrary?
>
A 3rd bank is sure to be added in the near future. At this rate
it seems likely a 4th will eventually be needed, though it's hard
to say for sure.
Nuno
> Michael
>
>> +#if defined(__x86_64__)
>> + __u64 pt_disabled_xsave;
>> +#else
>> + __u64 pt_rsvd2; /* MBZ */
>> +#endif
>> +} __packed;
>> +
>> /* /dev/mshv */
>> #define MSHV_CREATE_PARTITION _IOW(MSHV_IOCTL, 0x00, struct
>> mshv_create_partition)
>>
>> --
>> 2.34.1
^ permalink raw reply
* [PATCH net-next v3] net: mana: Fix incorrect speed reported by debugfs
From: Erni Sri Satya Vennela @ 2025-11-05 19:04 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, shradhagupta, ssengar, ernis, dipayanroy,
shirazsaleem, kotaranov, longli, linux-hyperv, netdev,
linux-kernel
Once the netshaper is created for MANA, the current bandwidth
is reported in debugfs like this:
$ sudo ./tools/net/ynl/pyynl/cli.py \
--spec Documentation/netlink/specs/net_shaper.yaml \
--do set \
--json '{"ifindex":'3',
"handle":{ "scope": "netdev", "id":'1' },
"bw-max": 200000000 }'
None
$ sudo cat /sys/kernel/debug/mana/1/vport0/current_speed
200
After the shaper is deleted, it is expected to report
the maximum speed supported by the SKU. But currently it is
reporting 0, which is incorrect.
Fix this inconsistency, by resetting apc->speed to apc->max_speed
during deletion of the shaper object. This will improve
readability and debuggability.
Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
---
Changes in v3:
* Remove Fixes tag.
Changes in v2:
* Add Fixes tag.
---
drivers/net/ethernet/microsoft/mana/mana_en.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index 0142fd98392c..9d56bfefd755 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -814,7 +814,7 @@ static int mana_shaper_del(struct net_shaper_binding *binding,
/* Reset mana port context parameters */
apc->handle.id = 0;
apc->handle.scope = NET_SHAPER_SCOPE_UNSPEC;
- apc->speed = 0;
+ apc->speed = apc->max_speed;
}
return err;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next v2] net: mana: Fix incorrect speed reported by debugfs
From: Erni Sri Satya Vennela @ 2025-11-05 19:03 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, shradhagupta, ssengar, dipayanroy, shirazsaleem,
kotaranov, longli, linux-hyperv, netdev, linux-kernel
In-Reply-To: <20251029185228.0c2da909@kernel.org>
On Wed, Oct 29, 2025 at 06:52:28PM -0700, Jakub Kicinski wrote:
> On Wed, 29 Oct 2025 03:52:34 -0700 Erni Sri Satya Vennela wrote:
> > Fixes: 75cabb46935b ("net: mana: Add support for net_shaper_ops")
>
> I've preferred this without the fixes tag, TBH.
> It's debugfs, nobody is supposed to be using it in production.
> --
> pw-bot: cr
I'll send v3 without the fixes tag.
Thankyou for the pointer.
^ permalink raw reply
* RE: [PATCH v3] mshv: Extend create partition ioctl to support cpu features
From: Michael Kelley @ 2025-11-05 17:41 UTC (permalink / raw)
To: Nuno Das Neves, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, wei.liu@kernel.org,
muislam@microsoft.com, easwar.hariharan@linux.microsoft.com
Cc: kys@microsoft.com, haiyangz@microsoft.com, decui@microsoft.com,
longli@microsoft.com, skinsburskii@linux.microsoft.com,
romank@linux.microsoft.com, Jinank Jain
In-Reply-To: <1762292846-14253-1-git-send-email-nunodasneves@linux.microsoft.com>
From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Tuesday, November 4, 2025 1:47 PM
>
> From: Muminul Islam <muislam@microsoft.com>
>
> The existing mshv create partition ioctl does not provide a way to
> specify which cpu features are enabled in the guest. Instead, it
> attempts to enable all features and those that are not supported are
> silently disabled by the hypervisor.
>
> This was done to reduce unnecessary complexity and is sufficient for
> many cases. However, new scenarios require fine-grained control over
> these features.
>
> Define a new mshv_create_partition_v2 structure which supports
> passing the disabled processor and xsave feature bits through to the
> create partition hypercall directly.
>
> The kernel does not introspect the bits in these new fields as they
> are part of the hypervisor ABI. Require the caller to provide the
> number of cpu feature banks passed, to support extending the number
> of banks in future. Disable all banks that are not specified to ensure
> the behavior is predictable with newer hypervisors.
>
> Introduce a new flag MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES which enables
> the new structure. If unset, the original mshv_create_partition struct
> is used, with the old behavior of enabling all features.
>
> Co-developed-by: Jinank Jain <jinankjain@microsoft.com>
> Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
> Signed-off-by: Muminul Islam <muislam@microsoft.com>
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
> Changes in v3:
> - Remove the new cpu features definitions in hvhdk.h, and retain the
> old behavior of enabling all features for the old struct. For the v2
> struct, still disable unspecified feature banks, since that makes it
> robust to future extensions.
> - Amend comments and commit message to reflect the above
> - Fix unused variable on arm64 [kernel test robot]
>
> Changes in v2:
> - Fix exposure of CONFIG_X86_64 to uapi [kernel test robot]
> - Fix compilation issue on arm64 [kernel test robot]
>
> ---
> drivers/hv/mshv_root_main.c | 94 ++++++++++++++++++++++++++++++-------
> include/uapi/linux/mshv.h | 34 ++++++++++++++
> 2 files changed, 110 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index d542a0143bb8..814465a0912d 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -1900,43 +1900,101 @@ add_partition(struct mshv_partition *partition)
> return 0;
> }
>
> -static long
> -mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
> +static_assert(MSHV_NUM_CPU_FEATURES_BANKS <=
> + HV_PARTITION_PROCESSOR_FEATURES_BANKS);
> +
> +static long mshv_ioctl_process_pt_flags(void __user *user_arg, u64 *pt_flags,
> + struct hv_partition_creation_properties *cr_props,
> + union hv_partition_isolation_properties *isol_props)
> {
> - struct mshv_create_partition args;
> - u64 creation_flags;
> - struct hv_partition_creation_properties creation_properties = {};
> - union hv_partition_isolation_properties isolation_properties = {};
> - struct mshv_partition *partition;
> - struct file *file;
> - int fd;
> - long ret;
> + int i;
> + struct mshv_create_partition_v2 args;
> + union hv_partition_processor_features *disabled_procs;
> + union hv_partition_processor_xsave_features *disabled_xsave;
>
> - if (copy_from_user(&args, user_arg, sizeof(args)))
> + /* First, copy orig struct in case user is on previous versions */
> + if (copy_from_user(&args, user_arg,
> + sizeof(struct mshv_create_partition)))
> return -EFAULT;
>
> if ((args.pt_flags & ~MSHV_PT_FLAGS_MASK) ||
> args.pt_isolation >= MSHV_PT_ISOLATION_COUNT)
> return -EINVAL;
>
> + disabled_procs = &cr_props->disabled_processor_features;
> + disabled_xsave = &cr_props->disabled_processor_xsave_features;
> +
> + /* Check if user provided newer struct with feature fields */
> + if (args.pt_flags & BIT(MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES)) {
Should probably be "BIT_ULL" instead of "BIT" since args.pt_flags is a long long field,
though it really doesn't matter as long as the number of flags is <= 32.
> + if (copy_from_user(&args, user_arg, sizeof(args)))
> + return -EFAULT;
> +
> + if (args.pt_num_cpu_fbanks > MSHV_NUM_CPU_FEATURES_BANKS ||
> + mshv_field_nonzero(args, pt_rsvd) ||
> + mshv_field_nonzero(args, pt_rsvd1))
> + return -EINVAL;
> +
> +
> + for (i = 0; i < args.pt_num_cpu_fbanks; i++)
> + disabled_procs->as_uint64[i] = args.pt_cpu_fbanks[i];
> +
> + /* Disable any features left unspecified */
> + for (; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
> + disabled_procs->as_uint64[i] = -1;
I'm trying to convince myself that disabling unspecified features is the right
thing to do. In the current hypervisor scenario with 2 banks, if the VMM caller
specifies only one bank of disable flags, then all the features in the 2nd bank
are disabled. That's certainly the reverse from the current code which
always enables all features, and from the hypervisor itself which in the
hypercall ABI defines the flags as "disable" flags rather than "enable" flags.
Then in a scenario where a new version of the hypervisor shows up with
support for 3 banks, the old VMM code that only knows about 2 banks
will cause all features in that 3rd bank to be disabled. Again, that's the
reverse of the current code.
I guess it depends on how the hypervisor defines any such new features.
Are they typically defined to be benign if they are enabled by default? Or
is the polarity the opposite, where the VMM must know about new
features before they are enabled? The hypercall interface seems to imply
the former but maybe I'm reading too much into it.
A code comment about the thinking here would be useful for future readers.
> +
> +#if IS_ENABLED(CONFIG_X86_64)
> + disabled_xsave->as_uint64 = args.pt_disabled_xsave;
> +#else
> + if (mshv_field_nonzero(args, pt_rsvd2))
> + return -EINVAL;
> +
> + disabled_xsave->as_uint64 = -1;
Does the arm64 version of the hypercall do anything with this field?
Or does it just ignore it? I would be more inclined to set ignored
fields to zero unless there's an identifiable reason otherwise.
(especially since the VMM is required to pass in zero on arm64).
> +#endif
> + } else {
> + /* v1 behavior: try to enable everything */
> + for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
> + disabled_procs->as_uint64[i] = 0;
> +
> + disabled_xsave->as_uint64 = 0;
> + }
> +
> /* Only support EXO partitions */
> - creation_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
> + *pt_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION |
> + HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
>
> if (args.pt_flags & BIT(MSHV_PT_BIT_LAPIC))
> - creation_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
> + *pt_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
> if (args.pt_flags & BIT(MSHV_PT_BIT_X2APIC))
> - creation_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
> + *pt_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
> if (args.pt_flags & BIT(MSHV_PT_BIT_GPA_SUPER_PAGES))
> - creation_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
> + *pt_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
>
> switch (args.pt_isolation) {
> case MSHV_PT_ISOLATION_NONE:
> - isolation_properties.isolation_type =
> - HV_PARTITION_ISOLATION_TYPE_NONE;
> + isol_props->isolation_type = HV_PARTITION_ISOLATION_TYPE_NONE;
> break;
> }
>
> + return 0;
> +}
> +
> +static long
> +mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
> +{
> + u64 creation_flags;
> + struct hv_partition_creation_properties creation_properties = {};
> + union hv_partition_isolation_properties isolation_properties = {};
> + struct mshv_partition *partition;
> + struct file *file;
> + int fd;
> + long ret;
> +
> + ret = mshv_ioctl_process_pt_flags(user_arg, &creation_flags,
> + &creation_properties,
> + &isolation_properties);
> + if (ret)
> + return ret;
> +
> partition = kzalloc(sizeof(*partition), GFP_KERNEL);
> if (!partition)
> return -ENOMEM;
> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
> index 876bfe4e4227..9091946cba23 100644
> --- a/include/uapi/linux/mshv.h
> +++ b/include/uapi/linux/mshv.h
> @@ -26,6 +26,7 @@ enum {
> MSHV_PT_BIT_LAPIC,
> MSHV_PT_BIT_X2APIC,
> MSHV_PT_BIT_GPA_SUPER_PAGES,
> + MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES,
> MSHV_PT_BIT_COUNT,
> };
>
> @@ -41,6 +42,8 @@ enum {
> * @pt_flags: Bitmask of 1 << MSHV_PT_BIT_*
> * @pt_isolation: MSHV_PT_ISOLATION_*
> *
> + * This is the initial/v0 version for backward compatibility.
> + *
> * Returns a file descriptor to act as a handle to a guest partition.
> * At this point the partition is not yet initialized in the hypervisor.
> * Some operations must be done with the partition in this state, e.g. setting
> @@ -52,6 +55,37 @@ struct mshv_create_partition {
> __u64 pt_isolation;
> };
>
> +#define MSHV_NUM_CPU_FEATURES_BANKS 2
> +
> +/**
> + * struct mshv_create_partition_v2
> + *
> + * This is extended version of the above initial MSHV_CREATE_PARTITION
> + * ioctl and allows for following additional parameters:
> + *
> + * @pt_num_cpu_fbanks: number of processor feature banks being provided.
> + * This must not exceed MSHV_NUM_CPU_FEATURES_BANKS.
> + * If set to less than the number of available banks,
> + * additional banks will be set to -1 (disabled).
> + * @pt_cpu_fbanks: disabled processor feature banks array.
> + * @pt_disabled_xsave: disabled xsave feature bits.
> + *
> + * Returns : same as above original mshv_create_partition
> + */
> +struct mshv_create_partition_v2 {
> + __u64 pt_flags;
> + __u64 pt_isolation;
> + __u16 pt_num_cpu_fbanks;
> + __u8 pt_rsvd[6]; /* MBZ */
> + __u64 pt_cpu_fbanks[MSHV_NUM_CPU_FEATURES_BANKS];
> + __u64 pt_rsvd1[2]; /* MBZ */
I presume this is for future expansion of the number of banks?
And that the choice of 2 additional banks is somewhat arbitrary?
Michael
> +#if defined(__x86_64__)
> + __u64 pt_disabled_xsave;
> +#else
> + __u64 pt_rsvd2; /* MBZ */
> +#endif
> +} __packed;
> +
> /* /dev/mshv */
> #define MSHV_CREATE_PARTITION _IOW(MSHV_IOCTL, 0x00, struct
> mshv_create_partition)
>
> --
> 2.34.1
^ permalink raw reply
* Re: [PATCH net-next v2] net: mana: Handle SKB if TX SGEs exceed hardware limit
From: Aditya Garg @ 2025-11-05 16:40 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
pabeni, longli, kotaranov, horms, shradhagupta, ssengar, ernis,
dipayanroy, shirazsaleem, linux-hyperv, netdev, linux-kernel,
linux-rdma, gargaditya
In-Reply-To: <20251031162611.2a981fdf@kernel.org>
On 01-11-2025 04:56, Jakub Kicinski wrote:
> On Wed, 29 Oct 2025 06:12:35 -0700 Aditya Garg wrote:
>> @@ -289,6 +290,21 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> cq = &apc->tx_qp[txq_idx].tx_cq;
>> tx_stats = &txq->stats;
>>
>> + if (MAX_SKB_FRAGS + 2 > MAX_TX_WQE_SGL_ENTRIES &&
>> + skb_shinfo(skb)->nr_frags + 2 > MAX_TX_WQE_SGL_ENTRIES) {
>> + /* GSO skb with Hardware SGE limit exceeded is not expected here
>> + * as they are handled in mana_features_check() callback
>> + */
>> + if (skb_is_gso(skb))
>> + netdev_warn_once(ndev, "GSO enabled skb exceeds max SGE limit\n");
>
> This could be the same question Simon asked but why do you think you
> need this line? Sure you need to linearize non-GSO but why do you care
> to warn specifically about GSO?! Looks like defensive programming or
> testing leftover..
>
Hi Jakub,
Agreed, The GSO specific warning is redundant. I'll drop it in next
revision.
>> + if (skb_linearize(skb)) {
>> + netdev_warn_once(ndev, "Failed to linearize skb with nr_frags=%d and is_gso=%d\n",
>> + skb_shinfo(skb)->nr_frags,
>> + skb_is_gso(skb));
>
> .. in practice including is_gso() here as you do is probably enough for
> debug
>
Ok
>> + goto tx_drop_count;
>> + }
>> + }
>> +
>> pkg.tx_oob.s_oob.vcq_num = cq->gdma_id;
>> pkg.tx_oob.s_oob.vsq_frame = txq->vsq_frame;
>>
>> @@ -402,8 +418,6 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> }
>> }
>>
>> - WARN_ON_ONCE(pkg.wqe_req.num_sge > MAX_TX_WQE_SGL_ENTRIES);
>> -
>> if (pkg.wqe_req.num_sge <= ARRAY_SIZE(pkg.sgl_array)) {
>> pkg.wqe_req.sgl = pkg.sgl_array;
>> } else {
>> @@ -438,9 +452,13 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>>
>> if (err) {
>> (void)skb_dequeue_tail(&txq->pending_skbs);
>> + mana_unmap_skb(skb, apc);
>> netdev_warn(ndev, "Failed to post TX OOB: %d\n", err);
>
> You have a print right here and in the callee. This condition must
> (almost) never happen in practice. It's likely fine to just drop
> the packet.
> The logs placed in callee doesn't covers all the failure scenarios,
hence I feel to have this log here with proper status. Maybe I can
remove the log in the callee?
> Either way -- this should be a separate patch.
>
Are you suggesting a separate patch altogether or two patch in the same
series?
Based on your suggestion i can work on v3.
Regards,
Aditya
>> - err = NETDEV_TX_BUSY;
>> - goto tx_busy;
>> + if (err == -ENOSPC) {
>> + err = NETDEV_TX_BUSY;
>> + goto tx_busy;
>> + }
>> + goto free_sgl_ptr;
>> }
>>
>> err = NETDEV_TX_OK;
>> @@ -478,6 +496,25 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> return NETDEV_TX_OK;
>> }
^ permalink raw reply
* [PATCH] net: ethernet: fix uninitialized pointers with free attr
From: Ally Heev @ 2025-11-05 11:17 UTC (permalink / raw)
To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui
Cc: intel-wired-lan, netdev, linux-kernel, linux-hyperv,
Dan Carpenter, Ally Heev
Uninitialized pointers with `__free` attribute can cause undefined
behaviour as the memory assigned(randomly) to the pointer is freed
automatically when the pointer goes out of scope
net/ethernet doesn't have any bugs related to this as of now,
but it is better to initialize and assign pointers with `__free` attr
in one statement to ensure proper scope-based cleanup
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aPiG_F5EBQUjZqsl@stanley.mountain/
Signed-off-by: Ally Heev <allyheev@gmail.com>
---
drivers/net/ethernet/intel/ice/ice_flow.c | 5 +++--
drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 5 +++--
drivers/net/ethernet/microsoft/mana/gdma_main.c | 2 +-
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_flow.c b/drivers/net/ethernet/intel/ice/ice_flow.c
index 6d5c939dc8a515c252cd2b77d155b69fa264ee92..3590dacf3ee57879b3809d715e40bb290e40c4aa 100644
--- a/drivers/net/ethernet/intel/ice/ice_flow.c
+++ b/drivers/net/ethernet/intel/ice/ice_flow.c
@@ -1573,12 +1573,13 @@ ice_flow_set_parser_prof(struct ice_hw *hw, u16 dest_vsi, u16 fdir_vsi,
struct ice_parser_profile *prof, enum ice_block blk)
{
u64 id = find_first_bit(prof->ptypes, ICE_FLOW_PTYPE_MAX);
- struct ice_flow_prof_params *params __free(kfree);
u8 fv_words = hw->blk[blk].es.fvw;
int status;
int i, idx;
- params = kzalloc(sizeof(*params), GFP_KERNEL);
+ struct ice_flow_prof_params *params __free(kfree) =
+ kzalloc(sizeof(*params), GFP_KERNEL);
+
if (!params)
return -ENOMEM;
diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index cbb5fa30f5a0ec778c1ee30470da3ca21cc1af24..368138715cd55cd1dadc686931cdda51c7a5130d 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -1012,7 +1012,6 @@ static int idpf_send_get_caps_msg(struct idpf_adapter *adapter)
*/
static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
{
- struct virtchnl2_get_lan_memory_regions *rcvd_regions __free(kfree);
struct idpf_vc_xn_params xn_params = {
.vc_op = VIRTCHNL2_OP_GET_LAN_MEMORY_REGIONS,
.recv_buf.iov_len = IDPF_CTLQ_MAX_BUF_LEN,
@@ -1023,7 +1022,9 @@ static int idpf_send_get_lan_memory_regions(struct idpf_adapter *adapter)
ssize_t reply_sz;
int err = 0;
- rcvd_regions = kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL);
+ struct virtchnl2_get_lan_memory_regions *rcvd_regions __free(kfree) =
+ kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL);
+
if (!rcvd_regions)
return -ENOMEM;
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 43f034e180c41a595ff570b886569685a56f9fee..8dcba7ee36130d94ba3436c99a5cd5f792a2962e 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -1505,7 +1505,7 @@ static int irq_setup(unsigned int *irqs, unsigned int len, int node,
bool skip_first_cpu)
{
const struct cpumask *next, *prev = cpu_none_mask;
- cpumask_var_t cpus __free(free_cpumask_var);
+ cpumask_var_t cpus __free(free_cpumask_var) = NULL;
int cpu, weight;
if (!alloc_cpumask_var(&cpus, GFP_KERNEL))
---
base-commit: c9cfc122f03711a5124b4aafab3211cf4d35a2ac
change-id: 20251105-aheev-uninitialized-free-attr-net-ethernet-7d106e4ab3f7
Best regards,
--
Ally Heev <allyheev@gmail.com>
^ permalink raw reply related
* Re: [PATCH v4 1/2] dt-bindings: microsoft: Add vmbus message-connection-id property
From: Krzysztof Kozlowski @ 2025-11-05 6:55 UTC (permalink / raw)
To: Hardik Garg
Cc: apais, cho, conor+dt, decui, devicetree, haiyangz, hargar,
krzk+dt, kys, linux-hyperv, linux-kernel, robh, ssengar, wei.liu
In-Reply-To: <f6c01c55-8930-459a-baa5-1465c5047b3e@linux.microsoft.com>
On 05/11/2025 02:10, Hardik Garg wrote:
>
> Each guest has a private hypervisor mailbox and cannot access any other
> guest’s communication path. Using an incorrect connection ID does not
> allow eavesdropping or cause interference — it only results in failed
> VMBus initialization because the host drops messages sent to an
> unexpected port. Thus, exposing the correct connection ID to the guest
> is safe and necessary for correct initialization.
>
>> If different values are important for the host, then all guests should
>> use whatever 0 which will map to different values on host by other means
>> of your protocol.
>>
>
> Using a fixed value such as 0 for all guests would not work, because the
> Hyper-V host differentiates between multiple control-plane contexts (for
> example, VTL0 vs VTL2) using distinct connection IDs. The guest must use
> the value assigned by the host, as there is no implicit mapping or
> negotiation protocol to determine it otherwise.
Sorry, I am not going back to three months old discussion.
Therefore I close this topic for me with: since the actual value does
not matter for the host - it will discard all messages which are not
intended to this guest - you can just use value 0 and your hypervisor
will map to proper port.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: microsoft: Add vmbus message-connection-id property
From: Hardik Garg @ 2025-11-05 1:10 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: apais, cho, conor+dt, decui, devicetree, haiyangz, hargar,
krzk+dt, kys, linux-hyperv, linux-kernel, robh, ssengar, wei.liu
In-Reply-To: <94d3e709-8c8b-40cb-a829-92c2012b4e0a@kernel.org>
On 7/25/2025 12:32 AM, Krzysztof Kozlowski wrote:
> On 25/07/2025 00:12, Hardik Garg wrote:
>>> Then all guests can use the same value, 0, making this property redundant.
>>
>> No, they cannot use the same value. The protocol requires different connection IDs for different communication paths.
>> For example, a guest communicating with a VTL0 control plane uses a different connection ID than one communicating with
>> a VTL2 control plane. The host specifies this value based on the guest's configuration, and there is no other discovery
>> method available to determine the correct connection ID.
>
> You completely removed entire thread and discussion making it difficult
> to connect to one of 100 or more discussions I am doing.
>
Apologies for the very late reply — I realize it’s been a few months
since your last feedback. I wanted to take time to revisit the design
and explore possible alternatives to avoid introducing the connection-id
property in the Device Tree. After evaluating the available mechanisms,
unfortunately there isn’t a viable alternative for passing this
information from the host to the guest in the current Hyper-V architecture.
To address your earlier comments in detail:
The connection-id is not an arbitrary or per-guest runtime configuration
parameter — it is a hardware-level identifier that specifies which
Hyper-V message port (or mailbox slot) the guest should use to
communicate with the VMBus control plane. Historically, VMBus always
used a single, fixed connection ID. However, with the introduction of
Virtual Trust Level 2 (VTL2) support, the control plane can now reside
in a different trust level, requiring the guest to communicate through a
different message port. This connection-id is determined by the
hypervisor based on the status of VMBus relay. From the guest’s
perspective, it is completely static for the lifetime of that VM
instance — it never changes at runtime. Once the kernel boots, it must
read this value to establish communication with the correct VMBus
control plane. There is currently no system API, or discoverable
interface that allows the guest to determine this value dynamically.
>
> The guest should not care about the value. Otherwise what if guests
> decides to ignore your DT property and start using other value? Sniffing
> other guests traffic? Causing conflicts or denial of service?
>
Regarding security and isolation:
Each guest has a private hypervisor mailbox and cannot access any other
guest’s communication path. Using an incorrect connection ID does not
allow eavesdropping or cause interference — it only results in failed
VMBus initialization because the host drops messages sent to an
unexpected port. Thus, exposing the correct connection ID to the guest
is safe and necessary for correct initialization.
> If different values are important for the host, then all guests should
> use whatever 0 which will map to different values on host by other means
> of your protocol.
>
Using a fixed value such as 0 for all guests would not work, because the
Hyper-V host differentiates between multiple control-plane contexts (for
example, VTL0 vs VTL2) using distinct connection IDs. The guest must use
the value assigned by the host, as there is no implicit mapping or
negotiation protocol to determine it otherwise.
In summary:
The connection-id is a static hardware configuration detail provided by
the host before boot, not a runtime variable. It is required for correct
message routing between the guest’s VMBus driver and the appropriate
control plane. There is no existing API or firmware interface that can
expose this information. Each guest has a dedicated, isolated mailbox,
so security is preserved. If you still feel that Device Tree is not the
right mechanism, I’m open to discussing alternative approaches that
could convey this information from the host to the guest early during
boot — though based on the current architecture, I have not found a
suitable one yet.
Thank you again for your time, patience, and for your detailed feedback.
I’ll make sure my next submission keeps full context and follows proper
wrapping and quoting conventions.
Thanks,
Hardik
^ permalink raw reply
* [PATCH] mshv: Allow mappings that overlap in uaddr
From: Nuno Das Neves @ 2025-11-04 22:18 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, magnuskulke
Cc: kys, haiyangz, wei.liu, decui, longli, mhklinux, skinsburskii,
prapal, mrathor, muislam, Nuno Das Neves
From: Magnus Kulke <magnuskulke@linux.microsoft.com>
Currently the MSHV driver rejects mappings that would overlap in
userspace.
Some VMMs require the same memory to be mapped to different parts of
the guest's address space, and so working around this restriction is
difficult.
The hypervisor itself doesn't prohibit mappings that overlap in uaddr,
(really in SPA: system physical addresses), so supporting this in the
driver doesn't require any extra work, only the checks need to be
removed.
Since no userspace code up until has been able to overlap regions in
userspace, relaxing this constraint can't break any existing code.
Signed-off-by: Magnus Kulke <magnuskulke@linux.microsoft.com>
Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
drivers/hv/mshv_root_main.c | 19 +------------------
include/uapi/linux/mshv.h | 2 +-
2 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index 814465a0912d..e5da5f2ab6f7 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -1206,21 +1206,6 @@ mshv_partition_region_by_gfn(struct mshv_partition *partition, u64 gfn)
return NULL;
}
-static struct mshv_mem_region *
-mshv_partition_region_by_uaddr(struct mshv_partition *partition, u64 uaddr)
-{
- struct mshv_mem_region *region;
-
- hlist_for_each_entry(region, &partition->pt_mem_regions, hnode) {
- if (uaddr >= region->start_uaddr &&
- uaddr < region->start_uaddr +
- (region->nr_pages << HV_HYP_PAGE_SHIFT))
- return region;
- }
-
- return NULL;
-}
-
/*
* NB: caller checks and makes sure mem->size is page aligned
* Returns: 0 with regionpp updated on success, or -errno
@@ -1235,9 +1220,7 @@ static int mshv_partition_create_region(struct mshv_partition *partition,
/* Reject overlapping regions */
if (mshv_partition_region_by_gfn(partition, mem->guest_pfn) ||
- mshv_partition_region_by_gfn(partition, mem->guest_pfn + nr_pages - 1) ||
- mshv_partition_region_by_uaddr(partition, mem->userspace_addr) ||
- mshv_partition_region_by_uaddr(partition, mem->userspace_addr + mem->size - 1))
+ mshv_partition_region_by_gfn(partition, mem->guest_pfn + nr_pages - 1))
return -EEXIST;
region = vzalloc(sizeof(*region) + sizeof(struct page *) * nr_pages);
diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
index 9091946cba23..b10c8d1cb2ad 100644
--- a/include/uapi/linux/mshv.h
+++ b/include/uapi/linux/mshv.h
@@ -123,7 +123,7 @@ enum {
* @rsvd: MBZ
*
* Map or unmap a region of userspace memory to Guest Physical Addresses (GPA).
- * Mappings can't overlap in GPA space or userspace.
+ * Mappings can't overlap in GPA space.
* To unmap, these fields must match an existing mapping.
*/
struct mshv_user_mem_region {
--
2.34.1
^ permalink raw reply related
* [PATCH v3] mshv: Extend create partition ioctl to support cpu features
From: Nuno Das Neves @ 2025-11-04 21:47 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, wei.liu, muislam, easwar.hariharan
Cc: kys, haiyangz, decui, longli, mhklinux, skinsburskii, romank,
Jinank Jain, Nuno Das Neves
From: Muminul Islam <muislam@microsoft.com>
The existing mshv create partition ioctl does not provide a way to
specify which cpu features are enabled in the guest. Instead, it
attempts to enable all features and those that are not supported are
silently disabled by the hypervisor.
This was done to reduce unnecessary complexity and is sufficient for
many cases. However, new scenarios require fine-grained control over
these features.
Define a new mshv_create_partition_v2 structure which supports
passing the disabled processor and xsave feature bits through to the
create partition hypercall directly.
The kernel does not introspect the bits in these new fields as they
are part of the hypervisor ABI. Require the caller to provide the
number of cpu feature banks passed, to support extending the number
of banks in future. Disable all banks that are not specified to ensure
the behavior is predictable with newer hypervisors.
Introduce a new flag MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES which enables
the new structure. If unset, the original mshv_create_partition struct
is used, with the old behavior of enabling all features.
Co-developed-by: Jinank Jain <jinankjain@microsoft.com>
Signed-off-by: Jinank Jain <jinankjain@microsoft.com>
Signed-off-by: Muminul Islam <muislam@microsoft.com>
Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
Changes in v3:
- Remove the new cpu features definitions in hvhdk.h, and retain the
old behavior of enabling all features for the old struct. For the v2
struct, still disable unspecified feature banks, since that makes it
robust to future extensions.
- Amend comments and commit message to reflect the above
- Fix unused variable on arm64 [kernel test robot]
Changes in v2:
- Fix exposure of CONFIG_X86_64 to uapi [kernel test robot]
- Fix compilation issue on arm64 [kernel test robot]
---
drivers/hv/mshv_root_main.c | 94 ++++++++++++++++++++++++++++++-------
include/uapi/linux/mshv.h | 34 ++++++++++++++
2 files changed, 110 insertions(+), 18 deletions(-)
diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
index d542a0143bb8..814465a0912d 100644
--- a/drivers/hv/mshv_root_main.c
+++ b/drivers/hv/mshv_root_main.c
@@ -1900,43 +1900,101 @@ add_partition(struct mshv_partition *partition)
return 0;
}
-static long
-mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
+static_assert(MSHV_NUM_CPU_FEATURES_BANKS <=
+ HV_PARTITION_PROCESSOR_FEATURES_BANKS);
+
+static long mshv_ioctl_process_pt_flags(void __user *user_arg, u64 *pt_flags,
+ struct hv_partition_creation_properties *cr_props,
+ union hv_partition_isolation_properties *isol_props)
{
- struct mshv_create_partition args;
- u64 creation_flags;
- struct hv_partition_creation_properties creation_properties = {};
- union hv_partition_isolation_properties isolation_properties = {};
- struct mshv_partition *partition;
- struct file *file;
- int fd;
- long ret;
+ int i;
+ struct mshv_create_partition_v2 args;
+ union hv_partition_processor_features *disabled_procs;
+ union hv_partition_processor_xsave_features *disabled_xsave;
- if (copy_from_user(&args, user_arg, sizeof(args)))
+ /* First, copy orig struct in case user is on previous versions */
+ if (copy_from_user(&args, user_arg,
+ sizeof(struct mshv_create_partition)))
return -EFAULT;
if ((args.pt_flags & ~MSHV_PT_FLAGS_MASK) ||
args.pt_isolation >= MSHV_PT_ISOLATION_COUNT)
return -EINVAL;
+ disabled_procs = &cr_props->disabled_processor_features;
+ disabled_xsave = &cr_props->disabled_processor_xsave_features;
+
+ /* Check if user provided newer struct with feature fields */
+ if (args.pt_flags & BIT(MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES)) {
+ if (copy_from_user(&args, user_arg, sizeof(args)))
+ return -EFAULT;
+
+ if (args.pt_num_cpu_fbanks > MSHV_NUM_CPU_FEATURES_BANKS ||
+ mshv_field_nonzero(args, pt_rsvd) ||
+ mshv_field_nonzero(args, pt_rsvd1))
+ return -EINVAL;
+
+
+ for (i = 0; i < args.pt_num_cpu_fbanks; i++)
+ disabled_procs->as_uint64[i] = args.pt_cpu_fbanks[i];
+
+ /* Disable any features left unspecified */
+ for (; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
+ disabled_procs->as_uint64[i] = -1;
+
+#if IS_ENABLED(CONFIG_X86_64)
+ disabled_xsave->as_uint64 = args.pt_disabled_xsave;
+#else
+ if (mshv_field_nonzero(args, pt_rsvd2))
+ return -EINVAL;
+
+ disabled_xsave->as_uint64 = -1;
+#endif
+ } else {
+ /* v1 behavior: try to enable everything */
+ for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
+ disabled_procs->as_uint64[i] = 0;
+
+ disabled_xsave->as_uint64 = 0;
+ }
+
/* Only support EXO partitions */
- creation_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION |
- HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
+ *pt_flags = HV_PARTITION_CREATION_FLAG_EXO_PARTITION |
+ HV_PARTITION_CREATION_FLAG_INTERCEPT_MESSAGE_PAGE_ENABLED;
if (args.pt_flags & BIT(MSHV_PT_BIT_LAPIC))
- creation_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
+ *pt_flags |= HV_PARTITION_CREATION_FLAG_LAPIC_ENABLED;
if (args.pt_flags & BIT(MSHV_PT_BIT_X2APIC))
- creation_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
+ *pt_flags |= HV_PARTITION_CREATION_FLAG_X2APIC_CAPABLE;
if (args.pt_flags & BIT(MSHV_PT_BIT_GPA_SUPER_PAGES))
- creation_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
+ *pt_flags |= HV_PARTITION_CREATION_FLAG_GPA_SUPER_PAGES_ENABLED;
switch (args.pt_isolation) {
case MSHV_PT_ISOLATION_NONE:
- isolation_properties.isolation_type =
- HV_PARTITION_ISOLATION_TYPE_NONE;
+ isol_props->isolation_type = HV_PARTITION_ISOLATION_TYPE_NONE;
break;
}
+ return 0;
+}
+
+static long
+mshv_ioctl_create_partition(void __user *user_arg, struct device *module_dev)
+{
+ u64 creation_flags;
+ struct hv_partition_creation_properties creation_properties = {};
+ union hv_partition_isolation_properties isolation_properties = {};
+ struct mshv_partition *partition;
+ struct file *file;
+ int fd;
+ long ret;
+
+ ret = mshv_ioctl_process_pt_flags(user_arg, &creation_flags,
+ &creation_properties,
+ &isolation_properties);
+ if (ret)
+ return ret;
+
partition = kzalloc(sizeof(*partition), GFP_KERNEL);
if (!partition)
return -ENOMEM;
diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
index 876bfe4e4227..9091946cba23 100644
--- a/include/uapi/linux/mshv.h
+++ b/include/uapi/linux/mshv.h
@@ -26,6 +26,7 @@ enum {
MSHV_PT_BIT_LAPIC,
MSHV_PT_BIT_X2APIC,
MSHV_PT_BIT_GPA_SUPER_PAGES,
+ MSHV_PT_BIT_CPU_AND_XSAVE_FEATURES,
MSHV_PT_BIT_COUNT,
};
@@ -41,6 +42,8 @@ enum {
* @pt_flags: Bitmask of 1 << MSHV_PT_BIT_*
* @pt_isolation: MSHV_PT_ISOLATION_*
*
+ * This is the initial/v0 version for backward compatibility.
+ *
* Returns a file descriptor to act as a handle to a guest partition.
* At this point the partition is not yet initialized in the hypervisor.
* Some operations must be done with the partition in this state, e.g. setting
@@ -52,6 +55,37 @@ struct mshv_create_partition {
__u64 pt_isolation;
};
+#define MSHV_NUM_CPU_FEATURES_BANKS 2
+
+/**
+ * struct mshv_create_partition_v2
+ *
+ * This is extended version of the above initial MSHV_CREATE_PARTITION
+ * ioctl and allows for following additional parameters:
+ *
+ * @pt_num_cpu_fbanks: number of processor feature banks being provided.
+ * This must not exceed MSHV_NUM_CPU_FEATURES_BANKS.
+ * If set to less than the number of available banks,
+ * additional banks will be set to -1 (disabled).
+ * @pt_cpu_fbanks: disabled processor feature banks array.
+ * @pt_disabled_xsave: disabled xsave feature bits.
+ *
+ * Returns : same as above original mshv_create_partition
+ */
+struct mshv_create_partition_v2 {
+ __u64 pt_flags;
+ __u64 pt_isolation;
+ __u16 pt_num_cpu_fbanks;
+ __u8 pt_rsvd[6]; /* MBZ */
+ __u64 pt_cpu_fbanks[MSHV_NUM_CPU_FEATURES_BANKS];
+ __u64 pt_rsvd1[2]; /* MBZ */
+#if defined(__x86_64__)
+ __u64 pt_disabled_xsave;
+#else
+ __u64 pt_rsvd2; /* MBZ */
+#endif
+} __packed;
+
/* /dev/mshv */
#define MSHV_CREATE_PARTITION _IOW(MSHV_IOCTL, 0x00, struct mshv_create_partition)
--
2.34.1
^ permalink raw reply related
* [PATCH] drivers/hv: Use kmalloc_array() instead of kmalloc()
From: Rahul Kumar @ 2025-11-04 12:16 UTC (permalink / raw)
To: kys, haiyangz, wei.liu, decui
Cc: linux-hyperv, linux-kernel, linux-kernel-mentees, skhan,
rk0006818
Documentation/process/deprecated.rst recommends against the use of
kmalloc with dynamic size calculations due to the risk of overflow and
smaller allocation being made than the caller was expecting.
Replace kmalloc() with kmalloc_array() in hv_common.c to make the
intended allocation size clearer and avoid potential overflow issues.
The number of pages (pgcount) is bounded, so overflow is not a
practical concern here. However, using kmalloc_array() better reflects
the intent to allocate an array and improves consistency with other
allocations.
No functional change intended.
Signed-off-by: Rahul Kumar <rk0006818@gmail.com>
---
drivers/hv/hv_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index e109a620c83f..68689beb383c 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -487,7 +487,7 @@ int hv_common_cpu_init(unsigned int cpu)
* online and then taken offline
*/
if (!*inputarg) {
- mem = kmalloc(pgcount * HV_HYP_PAGE_SIZE, flags);
+ mem = kmalloc_array(pgcount, HV_HYP_PAGE_SIZE, flags);
if (!mem)
return -ENOMEM;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next v2] net: mana: Handle SKB if TX SGEs exceed hardware limit
From: Simon Horman @ 2025-11-03 14:58 UTC (permalink / raw)
To: Aditya Garg
Cc: kys, haiyangz, wei.liu, decui, andrew+netdev, davem, edumazet,
kuba, pabeni, longli, kotaranov, shradhagupta, ssengar, ernis,
dipayanroy, shirazsaleem, linux-hyperv, netdev, linux-kernel,
linux-rdma, gargaditya
In-Reply-To: <347c723b-d47c-49c2-9a3b-b49d967f875b@linux.microsoft.com>
On Fri, Oct 31, 2025 at 06:50:10PM +0530, Aditya Garg wrote:
> On 30-10-2025 14:34, Simon Horman wrote:
> > On Wed, Oct 29, 2025 at 06:12:35AM -0700, Aditya Garg wrote:
> > > The MANA hardware supports a maximum of 30 scatter-gather entries (SGEs)
> > > per TX WQE. Exceeding this limit can cause TX failures.
> > > Add ndo_features_check() callback to validate SKB layout before
> > > transmission. For GSO SKBs that would exceed the hardware SGE limit, clear
> > > NETIF_F_GSO_MASK to enforce software segmentation in the stack.
> > > Add a fallback in mana_start_xmit() to linearize non-GSO SKBs that still
> > > exceed the SGE limit.
> > >
> > > Return NETDEV_TX_BUSY only for -ENOSPC from mana_gd_post_work_request(),
> > > send other errors to free_sgl_ptr to free resources and record the tx
> > > drop.
> > >
> > > Co-developed-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> > > Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
> > > Signed-off-by: Aditya Garg <gargaditya@linux.microsoft.com>
> >
> > ...
> >
> > > @@ -289,6 +290,21 @@ netdev_tx_t mana_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> > > cq = &apc->tx_qp[txq_idx].tx_cq;
> > > tx_stats = &txq->stats;
> > > + if (MAX_SKB_FRAGS + 2 > MAX_TX_WQE_SGL_ENTRIES &&
> > > + skb_shinfo(skb)->nr_frags + 2 > MAX_TX_WQE_SGL_ENTRIES) {
> > > + /* GSO skb with Hardware SGE limit exceeded is not expected here
> > > + * as they are handled in mana_features_check() callback
> > > + */
> >
> > Hi,
> >
> > I'm curious to know if we actually need this code.
> > Are there cases where the mana_features_check() doesn't
> > handle things and the kernel will reach this line?
> >
> > > + if (skb_is_gso(skb))
> > > + netdev_warn_once(ndev, "GSO enabled skb exceeds max SGE limit\n");
> > > + if (skb_linearize(skb)) {
> > > + netdev_warn_once(ndev, "Failed to linearize skb with nr_frags=%d and is_gso=%d\n",
> > > + skb_shinfo(skb)->nr_frags,
> > > + skb_is_gso(skb));
> > > + goto tx_drop_count;
> > > + }
> > > + }
> > > +
> > > pkg.tx_oob.s_oob.vcq_num = cq->gdma_id;
> > > pkg.tx_oob.s_oob.vsq_frame = txq->vsq_frame;
> >
> > ...
>
> Hi Simon,
> As it was previously discussed and agreed on with Eric, this is for Non-GSO
> skbs which could have possibly nr_frags greater than hardware limit.
>
> Quoting Eric's comment from v1 thread: https://lore.kernel.org/netdev/CANn89iKwHWdUaeAsdSuZUXG-W8XwyM2oppQL9spKkex0p9-Azw@mail.gmail.com/
> "I think that for non GSO, the linearization attempt is fine.
>
> Note that this is extremely unlikely for non malicious users,
> and MTU being usually small (9K or less),
> the allocation will be much smaller than a GSO packet."
Thanks for the clarification, that makes sense to me.
FTR, Jakub's question (elsewhere) is different to mine.
^ permalink raw reply
* Re: [PATCH v6 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
From: Borislav Petkov @ 2025-11-03 13:40 UTC (permalink / raw)
To: Ricardo Neri
Cc: x86, Krzysztof Kozlowski, Conor Dooley, Rob Herring,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
Michael Kelley, Rafael J. Wysocki, Saurabh Sengar, Chris Oo,
Kirill A. Shutemov, linux-hyperv, devicetree, linux-acpi,
linux-kernel, Ricardo Neri, Rafael J. Wysocki, Yunhong Jiang
In-Reply-To: <20251030054350.GA17477@ranerica-svr.sc.intel.com>
On Wed, Oct 29, 2025 at 10:43:50PM -0700, Ricardo Neri wrote:
> I did not want to enable the whole of ACPI code as I need a tiny portion of it.
> Then yes, saving memory and having a smaller binary were considerations.
>
> The only dependency that ACPI_MADT_WAKEUP has on ACPI is the code to read and
> parse the ACPI table that enumerates the mailbox. (There are a couple of
> declarations for CPU offlining that need tweaking if I want ACPI_MADT_WAKEUP to
> not depend on ACPI at all).
>
> The DeviceTree firmware only needs the code to wake CPUs up. That is the code
> I am carving out.
>
> Having said that, vmlinux and bzImage increase by 4% if I enable ACPI.
So, is it a concern or not? I cannot understand from the above whether you
care about 4% or not.
If you do, I guess you can make a piece of ACPI code available through another
Kconfig option but keep it in the ACPI hierarchy.
Because no matter how you look at it, it is ACPI code which is trying to be
generic and failing at that.
Unless you scrub it completely and make it a generic thing which is used by
ACPI too.
Which would be a separate patchset.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2] mshv: Extend create partition ioctl to support cpu features
From: kernel test robot @ 2025-11-02 14:13 UTC (permalink / raw)
To: Nuno Das Neves, linux-hyperv, linux-kernel, muislam
Cc: llvm, oe-kbuild-all, kys, haiyangz, wei.liu, decui, longli,
mhklinux, skinsburskii, romank, Jinank Jain, Nuno Das Neves
In-Reply-To: <1761860431-11208-1-git-send-email-nunodasneves@linux.microsoft.com>
Hi Nuno,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linus/master]
[also build test WARNING on v6.18-rc3 next-20251031]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nuno-Das-Neves/mshv-Extend-create-partition-ioctl-to-support-cpu-features/20251031-054134
base: linus/master
patch link: https://lore.kernel.org/r/1761860431-11208-1-git-send-email-nunodasneves%40linux.microsoft.com
patch subject: [PATCH v2] mshv: Extend create partition ioctl to support cpu features
config: arm64-randconfig-001-20251102 (https://download.01.org/0day-ci/archive/20251102/202511022246.Tn2DmYLd-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project d2625a438020ad35330cda29c3def102c1687b1b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251102/202511022246.Tn2DmYLd-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511022246.Tn2DmYLd-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/hv/mshv_root_main.c:1875:47: warning: unused variable 'disabled_xsave' [-Wunused-variable]
1875 | union hv_partition_processor_xsave_features *disabled_xsave;
| ^~~~~~~~~~~~~~
1 warning generated.
vim +/disabled_xsave +1875 drivers/hv/mshv_root_main.c
1864
1865 static_assert(MSHV_NUM_CPU_FEATURES_BANKS <=
1866 HV_PARTITION_PROCESSOR_FEATURES_BANKS);
1867
1868 static long mshv_ioctl_process_pt_flags(void __user *user_arg, u64 *pt_flags,
1869 struct hv_partition_creation_properties *cr_props,
1870 union hv_partition_isolation_properties *isol_props)
1871 {
1872 int i;
1873 struct mshv_create_partition_v2 args;
1874 union hv_partition_processor_features *disabled_procs;
> 1875 union hv_partition_processor_xsave_features *disabled_xsave;
1876
1877 /* First, copy orig struct in case user is on previous versions */
1878 if (copy_from_user(&args, user_arg,
1879 sizeof(struct mshv_create_partition)))
1880 return -EFAULT;
1881
1882 if ((args.pt_flags & ~MSHV_PT_FLAGS_MASK) ||
1883 args.pt_isolation >= MSHV_PT_ISOLATION_COUNT)
1884 return -EINVAL;
1885
1886 disabled_procs = &cr_props->disabled_processor_features;
1887
1888 /* Disable all processor features first */
1889 for (i = 0; i < HV_PARTITION_PROCESSOR_FEATURES_BANKS; i++)
1890 disabled_procs->as_uint64[i] = -1;
1891
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [net-next,v4] net: mana: Support HW link state events
From: patchwork-bot+netdevbpf @ 2025-10-31 23:30 UTC (permalink / raw)
To: Haiyang Zhang
Cc: linux-hyperv, netdev, kys, haiyangz, wei.liu, decui,
andrew+netdev, davem, edumazet, kuba, pabeni, longli, kotaranov,
horms, leon, shradhagupta, mlevitsk, ernis, yury.norov,
dipayanroy, shirazsaleem, ssengar, linux-kernel, linux-rdma,
paulros
In-Reply-To: <1761770601-16920-1-git-send-email-haiyangz@linux.microsoft.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 29 Oct 2025 13:43:10 -0700 you wrote:
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> Handle the NIC hardware link state events received from the HW
> channel, then set the proper link state accordingly.
>
> And, add a feature bit, GDMA_DRV_CAP_FLAG_1_HW_VPORT_LINK_AWARE,
> to inform the NIC hardware this handler exists.
>
> [...]
Here is the summary with links:
- [net-next,v4] net: mana: Support HW link state events
https://git.kernel.org/netdev/net-next/c/54133f9b4b53
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox