Re: [PATCH V4 0/8] virtio: Solution to restrict memory access under Xen using xen-grant DMA-mapping layer
On 02.06.22 21:23, Oleksandr Tyshchenko wrote: From: Oleksandr Tyshchenko Hello all. The purpose of this patch series is to add support for restricting memory access under Xen using specific grant table [1] based DMA-mapping layer. Patch series is based on Juergen Gross’ initial work [2] which implies using grant references instead of raw guest physical addresses (GPA) for the virtio communications (some kind of the software IOMMU). You can find RFC-V3 patch series (and previous discussions) at [3]. !!! Please note, the only diff between V3 and V4 is in commit #5, also I have collected the acks (commits ##4-7). The high level idea is to create new Xen’s grant table based DMA-mapping layer for the guest Linux whose main purpose is to provide a special 64-bit DMA address which is formed by using the grant reference (for a page to be shared with the backend) with offset and setting the highest address bit (this is for the backend to be able to distinguish grant ref based DMA address from normal GPA). For this to work we need the ability to allocate contiguous (consecutive) grant references for multi-page allocations. And the backend then needs to offer VIRTIO_F_ACCESS_PLATFORM and VIRTIO_F_VERSION_1 feature bits (it must support virtio-mmio modern transport for 64-bit addresses in the virtqueue). Xen's grant mapping mechanism is the secure and safe solution to share pages between domains which proven to work and works for years (in the context of traditional Xen PV drivers for example). So far, the foreign mapping is used for the virtio backend to map and access guest memory. With the foreign mapping, the backend is able to map arbitrary pages from the guest memory (or even from Dom0 memory). And as the result, the malicious backend which runs in a non-trusted domain can take advantage of this. Instead, with the grant mapping the backend is only allowed to map pages which were explicitly granted by the guest before and nothing else. According to the discussions in various mainline threads this solution would likely be welcome because it perfectly fits in the security model Xen provides. What is more, the grant table based solution requires zero changes to the Xen hypervisor itself at least with virtio-mmio and DT (in comparison, for example, with "foreign mapping + virtio-iommu" solution which would require the whole new complex emulator in hypervisor in addition to new functionality/hypercall to pass IOVA from the virtio backend running elsewhere to the hypervisor and translate it to the GPA before mapping into P2M or denying the foreign mapping request if no corresponding IOVA-GPA mapping present in the IOMMU page table for that particular device). We only need to update toolstack to insert "xen,grant-dma" IOMMU node (to be referred by the virtio-mmio device using "iommus" property) when creating a guest device-tree (this is an indicator for the guest to use Xen grant mappings scheme for that device with the endpoint ID being used as ID of Xen domain where the corresponding backend is running, the backend domid is used as an argument to the grant mapping APIs). It worth mentioning that toolstack patch is based on non upstreamed yet “Virtio support for toolstack on Arm” series which is on review now [4]. Please note the following: - Patch series only covers Arm and virtio-mmio (device-tree) for now. To enable the restricted memory access feature on Arm the following option should be set: CONFIG_XEN_VIRTIO=y - Patch series is based on "kernel: add new infrastructure for platform_has() support" patch series which is on review now [5] - Xen should be built with the following options: CONFIG_IOREQ_SERVER=y CONFIG_EXPERT=y Patch series is rebased on "for-linus-5.19" branch [1] with "platform_has()" series applied and tested on Renesas Salvator-X board + H3 ES3.0 SoC (Arm64) with standalone userspace (non-Qemu) virtio-mmio based virtio-disk backend running in Driver domain and Linux guest running on existing virtio-blk driver (frontend). No issues were observed. Guest domain 'reboot/destroy' use-cases work properly. I have also tested other use-cases such as assigning several virtio block devices or a mix of virtio and Xen PV block devices to the guest. Patch series was build-tested on Arm32 and x86. 1. Xen changes located at (last patch): https://github.com/otyshchenko1/xen/commits/libxl_virtio_next2_1 2. Linux changes located at (last 8 patches): https://github.com/otyshchenko1/linux/commits/virtio_grant9 3. virtio-disk changes located at: https://github.com/otyshchenko1/virtio-disk/commits/virtio_grant Any feedback/help would be highly appreciated. [1] https://xenbits.xenproject.org/docs/4.16-testing/misc/grant-tables.txt [2] https://www.youtube.com/watch?v=IrlEdaIUDPk [3] https://lore.kernel.org/xen-devel/1649963973-22879-1-git-send-email-olekst...@gmail.com/
Re: [PATCH 2/2] vdpa/mlx5: clean up indenting in handle_ctrl_vlan()
On 6/6/2022 11:50 PM, Dan Carpenter wrote: These lines were supposed to be indented. Signed-off-by: Dan Carpenter Acked-by: Si-Wei Liu --- drivers/vdpa/mlx5/net/mlx5_vnet.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c index c964f4161d7f..83607b7488f1 100644 --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c @@ -1817,10 +1817,10 @@ static virtio_net_ctrl_ack handle_ctrl_vlan(struct mlx5_vdpa_dev *mvdev, u8 cmd) status = VIRTIO_NET_OK; break; default: - break; -} + break; + } -return status; + return status; } static void mlx5_cvq_kick_handler(struct work_struct *work) ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/2] vdpa/mlx5: fix error code for deleting vlan
On 6/6/2022 11:49 PM, Dan Carpenter wrote:
Return success if we were able to delete a vlan. The current code
always returns failure.
Fixes: baf2ad3f6a98 ("vdpa/mlx5: Add RX MAC VLAN filter support")
Signed-off-by: Dan Carpenter
Acked-by: Si-Wei Liu
---
From review. (Not tested).
drivers/vdpa/mlx5/net/mlx5_vnet.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index b7a955479156..c964f4161d7f 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1814,6 +1814,7 @@ static virtio_net_ctrl_ack handle_ctrl_vlan(struct
mlx5_vdpa_dev *mvdev, u8 cmd)
id = mlx5vdpa16_to_cpu(mvdev, vlan);
mac_vlan_del(ndev, ndev->config.mac, id, true);
+ status = VIRTIO_NET_OK;
break;
default:
break;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] fuse: allow skipping abort interface for virtiofs
On Tue, Jun 07, 2022 at 07:05:04PM +0800, Xie Yongji wrote:
> The commit 15c8e72e88e0 ("fuse: allow skipping control
> interface and forced unmount") tries to remove the control
> interface for virtio-fs since it does not support aborting
> requests which are being processed. But it doesn't work now.
Aha.., so "no_control" basically has no effect? I was looking at
the code and did not find anybody using "no_control" and I was
wondering who is making use of "no_control" variable.
I mounted virtiofs and noticed a directory named "40" showed up
under /sys/fs/fuse/connections/. That must be belonging to
virtiofs instance, I am assuming.
BTW, if there are multiple fuse connections, how will one figure
out which directory belongs to which instance. Because without knowing
that, one will be shooting in dark while trying to read/write any
of the control files.
So I think a separate patch should be sent which just gets rid of
"no_control" saying nobody uses. it.
>
> This commit fixes the bug, but only remove the abort interface
> instead since other interfaces should be useful.
Hmm.., so writing to "abort" file is bad as it ultimately does.
fc->connected = 0;
So getting rid of this file till we support aborting the pending
requests properly, makes sense.
I think this probably should be a separate patch which explains
why adding "no_abort_control" is a good idea.
Thanks
Vivek
>
> Fixes: 15c8e72e88e0 ("fuse: allow skipping control interface and forced
> unmount")
> Signed-off-by: Xie Yongji
> ---
> fs/fuse/control.c | 4 ++--
> fs/fuse/fuse_i.h| 6 +++---
> fs/fuse/inode.c | 2 +-
> fs/fuse/virtio_fs.c | 2 +-
> 4 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/fs/fuse/control.c b/fs/fuse/control.c
> index 7cede9a3bc96..d93d8ea3a090 100644
> --- a/fs/fuse/control.c
> +++ b/fs/fuse/control.c
> @@ -272,8 +272,8 @@ int fuse_ctl_add_conn(struct fuse_conn *fc)
>
> if (!fuse_ctl_add_dentry(parent, fc, "waiting", S_IFREG | 0400, 1,
>NULL, _ctl_waiting_ops) ||
> - !fuse_ctl_add_dentry(parent, fc, "abort", S_IFREG | 0200, 1,
> - NULL, _ctl_abort_ops) ||
> + (!fc->no_abort_control && !fuse_ctl_add_dentry(parent, fc, "abort",
> + S_IFREG | 0200, 1, NULL, _ctl_abort_ops)) ||
> !fuse_ctl_add_dentry(parent, fc, "max_background", S_IFREG | 0600,
>1, NULL, _conn_max_background_ops) ||
> !fuse_ctl_add_dentry(parent, fc, "congestion_threshold",
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 488b460e046f..e29a4e2f2b35 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -507,7 +507,7 @@ struct fuse_fs_context {
> bool default_permissions:1;
> bool allow_other:1;
> bool destroy:1;
> - bool no_control:1;
> + bool no_abort_control:1;
> bool no_force_umount:1;
> bool legacy_opts_show:1;
> enum fuse_dax_mode dax_mode;
> @@ -766,8 +766,8 @@ struct fuse_conn {
> /* Delete dentries that have gone stale */
> unsigned int delete_stale:1;
>
> - /** Do not create entry in fusectl fs */
> - unsigned int no_control:1;
> + /** Do not create abort entry in fusectl fs */
> + unsigned int no_abort_control:1;
>
> /** Do not allow MNT_FORCE umount */
> unsigned int no_force_umount:1;
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 8c0665c5dff8..02a16cd35f42 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1564,7 +1564,7 @@ int fuse_fill_super_common(struct super_block *sb,
> struct fuse_fs_context *ctx)
> fc->legacy_opts_show = ctx->legacy_opts_show;
> fc->max_read = max_t(unsigned int, 4096, ctx->max_read);
> fc->destroy = ctx->destroy;
> - fc->no_control = ctx->no_control;
> + fc->no_abort_control = ctx->no_abort_control;
> fc->no_force_umount = ctx->no_force_umount;
>
> err = -ENOMEM;
> diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c
> index 8db53fa67359..af369bea6dbb 100644
> --- a/fs/fuse/virtio_fs.c
> +++ b/fs/fuse/virtio_fs.c
> @@ -1287,7 +1287,7 @@ static inline void virtio_fs_ctx_set_defaults(struct
> fuse_fs_context *ctx)
> ctx->max_read = UINT_MAX;
> ctx->blksize = 512;
> ctx->destroy = true;
> - ctx->no_control = true;
> + ctx->no_abort_control = true;
> ctx->no_force_umount = true;
> }
>
> --
> 2.20.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 15/20] balloon: Convert to migrate_folio
On Tue, Jun 07, 2022 at 03:24:15PM +0100, Matthew Wilcox wrote:
> On Tue, Jun 07, 2022 at 09:36:21AM +0200, David Hildenbrand wrote:
> > On 06.06.22 22:40, Matthew Wilcox (Oracle) wrote:
> > > const struct address_space_operations balloon_aops = {
> > > - .migratepage = balloon_page_migrate,
> > > + .migrate_folio = balloon_migrate_folio,
> > > .isolate_page = balloon_page_isolate,
> > > .putback_page = balloon_page_putback,
> > > };
> >
> > I assume you're working on conversion of the other callbacks as well,
> > because otherwise, this ends up looking a bit inconsistent and confusing :)
>
> My intention was to finish converting aops for the next merge window.
>
> However, it seems to me that we goofed back in 2016 by merging
> commit bda807d44454. isolate_page() and putback_page() should
> never have been part of address_space_operations.
>
> I'm about to embark on creating a new migrate_operations struct
> for drivers to use that contains only isolate/putback/migrate.
> No filesystem uses isolate/putback, so those can just be deleted.
> Both migrate_operations & address_space_operations will contain a
> migrate callback.
Well, that went more smoothly than I thought it would.
I can't see a nice way to split this patch up (other than making secretmem
its own patch). We just don't have enough bits in struct page to support
both ways of handling PageMovable at the same time, so we can't convert
one driver at a time. The diffstat is pretty compelling.
The patch is on top of this patch series; I think it probably makes
sense to shuffle it to be first, to avoid changing these drivers to
folios, then changing them back.
Questions:
Is what I've done with zsmalloc acceptable? The locking in that
file is rather complex.
Can we now eliminate balloon_mnt / balloon_fs from cmm.c? I haven't even
compiled thatfile , but it seems like the filesystem serves no use now.
Similar question for vmw_balloon.c, although I have compiled that.
---
I just spotted a bug with zs_unregister_migration(); it won't compile
without CONFIG_MIGRATION. I'll fix that up if the general approach is
acceptable.
arch/powerpc/platforms/pseries/cmm.c | 13
drivers/misc/vmw_balloon.c | 10 --
include/linux/balloon_compaction.h |6 +---
include/linux/fs.h |2 -
include/linux/migrate.h | 27 ++
include/linux/page-flags.h |2 -
mm/balloon_compaction.c | 18 ++--
mm/compaction.c | 29 ---
mm/migrate.c | 23 ---
mm/secretmem.c |6
mm/util.c|4 +-
mm/z3fold.c | 45 ++
mm/zsmalloc.c| 52 +--
13 files changed, 83 insertions(+), 154 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/cmm.c
b/arch/powerpc/platforms/pseries/cmm.c
index 15ed8206c463..2ecbab3db723 100644
--- a/arch/powerpc/platforms/pseries/cmm.c
+++ b/arch/powerpc/platforms/pseries/cmm.c
@@ -578,23 +578,10 @@ static int cmm_balloon_compaction_init(void)
return rc;
}
- b_dev_info.inode = alloc_anon_inode(balloon_mnt->mnt_sb);
- if (IS_ERR(b_dev_info.inode)) {
- rc = PTR_ERR(b_dev_info.inode);
- b_dev_info.inode = NULL;
- kern_unmount(balloon_mnt);
- balloon_mnt = NULL;
- return rc;
- }
-
- b_dev_info.inode->i_mapping->a_ops = _aops;
return 0;
}
static void cmm_balloon_compaction_deinit(void)
{
- if (b_dev_info.inode)
- iput(b_dev_info.inode);
- b_dev_info.inode = NULL;
kern_unmount(balloon_mnt);
balloon_mnt = NULL;
}
diff --git a/drivers/misc/vmw_balloon.c b/drivers/misc/vmw_balloon.c
index 086ce77d9074..4a6755934bb5 100644
--- a/drivers/misc/vmw_balloon.c
+++ b/drivers/misc/vmw_balloon.c
@@ -1869,10 +1869,6 @@ static int vmballoon_migratepage(struct balloon_dev_info
*b_dev_info,
*/
static void vmballoon_compaction_deinit(struct vmballoon *b)
{
- if (!IS_ERR(b->b_dev_info.inode))
- iput(b->b_dev_info.inode);
-
- b->b_dev_info.inode = NULL;
kern_unmount(vmballoon_mnt);
vmballoon_mnt = NULL;
}
@@ -1895,12 +1891,6 @@ static __init int vmballoon_compaction_init(struct
vmballoon *b)
return PTR_ERR(vmballoon_mnt);
b->b_dev_info.migratepage = vmballoon_migratepage;
- b->b_dev_info.inode = alloc_anon_inode(vmballoon_mnt->mnt_sb);
-
- if (IS_ERR(b->b_dev_info.inode))
- return PTR_ERR(b->b_dev_info.inode);
-
- b->b_dev_info.inode->i_mapping->a_ops = _aops;
return 0;
}
diff --git a/include/linux/balloon_compaction.h
b/include/linux/balloon_compaction.h
index edb7f6d41faa..5ca2d5699620 100644
---
Re: [PATCH 14/20] hugetlb: Convert to migrate_folio
On Tue, Jun 07, 2022 at 02:13:26PM +0800, kernel test robot wrote: >fs/hugetlbfs/inode.c: In function 'hugetlbfs_migrate_folio': > >> fs/hugetlbfs/inode.c:990:17: error: implicit declaration of function > >> 'folio_migrate_copy' [-Werror=implicit-function-declaration] > 990 | folio_migrate_copy(dst, src); > | ^~ > >> fs/hugetlbfs/inode.c:992:17: error: implicit declaration of function > >> 'folio_migrate_flags'; did you mean 'folio_mapping_flags'? > >> [-Werror=implicit-function-declaration] > 992 | folio_migrate_flags(dst, src); > | ^~~ > | folio_mapping_flags >cc1: some warnings being treated as errors Thanks, fixed. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 04/20] mm/migrate: Convert buffer_migrate_page() to buffer_migrate_folio()
On Tue, Jun 07, 2022 at 11:37:45AM +0800, kernel test robot wrote: > All warnings (new ones prefixed by >>): > > >> mm/migrate.c:775: warning: expecting prototype for > >> buffer_migrate_folio_noref(). Prototype was for > >> buffer_migrate_folio_norefs() instead No good deed (turning documentation into kerneldoc) goes unpunished ... thanks, fixed. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 15/20] balloon: Convert to migrate_folio
On Tue, Jun 07, 2022 at 09:36:21AM +0200, David Hildenbrand wrote:
> On 06.06.22 22:40, Matthew Wilcox (Oracle) wrote:
> > const struct address_space_operations balloon_aops = {
> > - .migratepage = balloon_page_migrate,
> > + .migrate_folio = balloon_migrate_folio,
> > .isolate_page = balloon_page_isolate,
> > .putback_page = balloon_page_putback,
> > };
>
> I assume you're working on conversion of the other callbacks as well,
> because otherwise, this ends up looking a bit inconsistent and confusing :)
My intention was to finish converting aops for the next merge window.
However, it seems to me that we goofed back in 2016 by merging
commit bda807d44454. isolate_page() and putback_page() should
never have been part of address_space_operations.
I'm about to embark on creating a new migrate_operations struct
for drivers to use that contains only isolate/putback/migrate.
No filesystem uses isolate/putback, so those can just be deleted.
Both migrate_operations & address_space_operations will contain a
migrate callback.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 05/20] mm/migrate: Convert expected_page_refs() to folio_expected_refs()
On Tue, Jun 07, 2022 at 09:41:57AM -0400, Brian Foster wrote:
> On Mon, Jun 06, 2022 at 09:40:35PM +0100, Matthew Wilcox (Oracle) wrote:
> > -static int expected_page_refs(struct address_space *mapping, struct page
> > *page)
> > +static int folio_expected_refs(struct address_space *mapping,
> > + struct folio *folio)
> > {
> > - int expected_count = 1;
> > + int refs = 1;
> > + if (!mapping)
> > + return refs;
> >
> > - if (mapping)
> > - expected_count += compound_nr(page) + page_has_private(page);
> > - return expected_count;
> > + refs += folio_nr_pages(folio);
> > + if (folio_get_private(folio))
> > + refs++;
>
> Why not folio_has_private() (as seems to be used for later
> page_has_private() conversions) here?
We have a horrid confusion that I'm trying to clean up stealthily
without anyone noticing. I would have gotten away with it too if it
weren't for you pesky kids.
#define PAGE_FLAGS_PRIVATE \
(1UL << PG_private | 1UL << PG_private_2)
static inline int page_has_private(struct page *page)
{
return !!(page->flags & PAGE_FLAGS_PRIVATE);
}
So what this function is saying is that there is one extra refcount
expected on the struct page if PG_private _or_ PG_private_2 is set.
How are filesystems expected to manage their page's refcount with this
rule? Increment the refcount when setting PG_private unless
PG_private_2 is already set? Decrement the refcount when clearing
PG_private_2 unless PG_private is set?
This is garbage. IMO, PG_private_2 should have no bearing on the page's
refcount. Only btrfs and the netfs's use private_2 and neither of them
do anything to the refcount when setting/clearing it. So that's what
I'm implementing here.
> > +
> > + return refs;;
>
> Nit: extra ;
Oh, that's where it went ;-) I had a compile error due to a missing
semicolon at some point, and thought it was just a typo ...
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 05/20] mm/migrate: Convert expected_page_refs() to folio_expected_refs()
On Mon, Jun 06, 2022 at 09:40:35PM +0100, Matthew Wilcox (Oracle) wrote:
> Now that both callers have a folio, convert this function to
> take a folio & rename it.
>
> Signed-off-by: Matthew Wilcox (Oracle)
> ---
> mm/migrate.c | 19 ---
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 77b8c662c9ca..e0a593e5b5f9 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -337,13 +337,18 @@ void pmd_migration_entry_wait(struct mm_struct *mm,
> pmd_t *pmd)
> }
> #endif
>
> -static int expected_page_refs(struct address_space *mapping, struct page
> *page)
> +static int folio_expected_refs(struct address_space *mapping,
> + struct folio *folio)
> {
> - int expected_count = 1;
> + int refs = 1;
> + if (!mapping)
> + return refs;
>
> - if (mapping)
> - expected_count += compound_nr(page) + page_has_private(page);
> - return expected_count;
> + refs += folio_nr_pages(folio);
> + if (folio_get_private(folio))
> + refs++;
Why not folio_has_private() (as seems to be used for later
page_has_private() conversions) here?
> +
> + return refs;;
Nit: extra ;
Brian
> }
>
> /*
> @@ -360,7 +365,7 @@ int folio_migrate_mapping(struct address_space *mapping,
> XA_STATE(xas, >i_pages, folio_index(folio));
> struct zone *oldzone, *newzone;
> int dirty;
> - int expected_count = expected_page_refs(mapping, >page) +
> extra_count;
> + int expected_count = folio_expected_refs(mapping, folio) + extra_count;
> long nr = folio_nr_pages(folio);
>
> if (!mapping) {
> @@ -670,7 +675,7 @@ static int __buffer_migrate_folio(struct address_space
> *mapping,
> return migrate_page(mapping, >page, >page, mode);
>
> /* Check whether page does not have extra refs before we do more work */
> - expected_count = expected_page_refs(mapping, >page);
> + expected_count = folio_expected_refs(mapping, src);
> if (folio_ref_count(src) != expected_count)
> return -EAGAIN;
>
> --
> 2.35.1
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio_ring: remove unnecessary to_vvq call
On Tue, Jun 07, 2022 at 08:08:58AM +, Bo Liu (刘波)-浪潮信息 wrote: > This patch removes unnecessary code and generates smaller binary files. > Thanks Can you post info on which files are smaller an by how much please? -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 04/20] mm/migrate: Convert buffer_migrate_page() to buffer_migrate_folio()
Hi "Matthew,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v5.19-rc1 next-20220607]
[cannot apply to jaegeuk-f2fs/dev-test trondmy-nfs/linux-next kdave/for-next
xfs-linux/for-next]
[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]
url:
https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/Convert-aops-migratepage-to-aops-migrate_folio/20220607-044509
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
f2906aa863381afb0015a9eb7fefad885d4e5a56
config: s390-randconfig-c005-20220606
(https://download.01.org/0day-ci/archive/20220607/202206071552.6lodsclw-...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project
b92436efcb7813fc481b30f2593a4907568d917a)
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
#
https://github.com/intel-lab-lkp/linux/commit/96e64ba8b1be545885d89f44b1d8b968b22bdb4d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Matthew-Wilcox-Oracle/Convert-aops-migratepage-to-aops-migrate_folio/20220607-044509
git checkout 96e64ba8b1be545885d89f44b1d8b968b22bdb4d
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1
O=build_dir ARCH=s390 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> mm/migrate.c:775: warning: expecting prototype for
>> buffer_migrate_folio_noref(). Prototype was for
>> buffer_migrate_folio_norefs() instead
vim +775 mm/migrate.c
89cb0888ca1483 Jan Kara2018-12-28 758
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 759) /**
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 760) *
buffer_migrate_folio_noref() - Migration function for folios with buffers.
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 761) * @mapping: The
address space containing @src.
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 762) * @dst: The folio to
migrate to.
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 763) * @src: The folio to
migrate from.
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 764) * @mode: How to
migrate the folio.
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 765) *
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 766) * Like
buffer_migrate_folio() except that this variant is more careful
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 767) * and checks that
there are also no buffer head references. This function
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 768) * is the right one for
mappings where buffer heads are directly looked
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 769) * up and referenced
(such as block device mappings).
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 770) *
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 771) * Return: 0 on success
or a negative errno on failure.
89cb0888ca1483 Jan Kara2018-12-28 772 */
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 773) int
buffer_migrate_folio_norefs(struct address_space *mapping,
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 774) struct
folio *dst, struct folio *src, enum migrate_mode mode)
89cb0888ca1483 Jan Kara2018-12-28 @775 {
96e64ba8b1be54 Matthew Wilcox (Oracle 2022-06-06 776) return
__buffer_migrate_folio(mapping, dst, src, mode, true);
89cb0888ca1483 Jan Kara2018-12-28 777 }
9361401eb7619c David Howells 2006-09-30 778 #endif
1d8b85ccf1ed53 Christoph Lameter 2006-06-23 779
--
0-DAY CI Kernel Test Service
https://01.org/lkp
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 15/20] balloon: Convert to migrate_folio
On 06.06.22 22:40, Matthew Wilcox (Oracle) wrote:
> This is little more than changing the types over; there's no real work
> being done in this function.
>
> Signed-off-by: Matthew Wilcox (Oracle)
> ---
> mm/balloon_compaction.c | 15 +++
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/mm/balloon_compaction.c b/mm/balloon_compaction.c
> index 4b8eab4b3f45..3f75b876ad76 100644
> --- a/mm/balloon_compaction.c
> +++ b/mm/balloon_compaction.c
> @@ -230,11 +230,10 @@ static void balloon_page_putback(struct page *page)
>
>
> /* move_to_new_page() counterpart for a ballooned page */
> -static int balloon_page_migrate(struct address_space *mapping,
> - struct page *newpage, struct page *page,
> - enum migrate_mode mode)
> +static int balloon_migrate_folio(struct address_space *mapping,
> + struct folio *dst, struct folio *src, enum migrate_mode mode)
> {
> - struct balloon_dev_info *balloon = balloon_page_device(page);
> + struct balloon_dev_info *balloon = balloon_page_device(>page);
>
> /*
>* We can not easily support the no copy case here so ignore it as it
> @@ -244,14 +243,14 @@ static int balloon_page_migrate(struct address_space
> *mapping,
> if (mode == MIGRATE_SYNC_NO_COPY)
> return -EINVAL;
>
> - VM_BUG_ON_PAGE(!PageLocked(page), page);
> - VM_BUG_ON_PAGE(!PageLocked(newpage), newpage);
> + VM_BUG_ON_FOLIO(!folio_test_locked(src), src);
> + VM_BUG_ON_FOLIO(!folio_test_locked(dst), dst);
>
> - return balloon->migratepage(balloon, newpage, page, mode);
> + return balloon->migratepage(balloon, >page, >page, mode);
> }
>
> const struct address_space_operations balloon_aops = {
> - .migratepage = balloon_page_migrate,
> + .migrate_folio = balloon_migrate_folio,
> .isolate_page = balloon_page_isolate,
> .putback_page = balloon_page_putback,
> };
I assume you're working on conversion of the other callbacks as well,
because otherwise, this ends up looking a bit inconsistent and confusing :)
Change LGTM.
--
Thanks,
David / dhildenb
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 2/2] vdpa/mlx5: clean up indenting in handle_ctrl_vlan()
These lines were supposed to be indented. Signed-off-by: Dan Carpenter --- drivers/vdpa/mlx5/net/mlx5_vnet.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c b/drivers/vdpa/mlx5/net/mlx5_vnet.c index c964f4161d7f..83607b7488f1 100644 --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c @@ -1817,10 +1817,10 @@ static virtio_net_ctrl_ack handle_ctrl_vlan(struct mlx5_vdpa_dev *mvdev, u8 cmd) status = VIRTIO_NET_OK; break; default: - break; -} + break; + } -return status; + return status; } static void mlx5_cvq_kick_handler(struct work_struct *work) -- 2.35.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 1/2] vdpa/mlx5: fix error code for deleting vlan
Return success if we were able to delete a vlan. The current code
always returns failure.
Fixes: baf2ad3f6a98 ("vdpa/mlx5: Add RX MAC VLAN filter support")
Signed-off-by: Dan Carpenter
---
>From review. (Not tested).
drivers/vdpa/mlx5/net/mlx5_vnet.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index b7a955479156..c964f4161d7f 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1814,6 +1814,7 @@ static virtio_net_ctrl_ack handle_ctrl_vlan(struct
mlx5_vdpa_dev *mvdev, u8 cmd)
id = mlx5vdpa16_to_cpu(mvdev, vlan);
mac_vlan_del(ndev, ndev->config.mac, id, true);
+ status = VIRTIO_NET_OK;
break;
default:
break;
--
2.35.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] virtio_ring: remove unnecessary to_vvq call
On Mon, Jun 06, 2022 at 08:59:51PM -0400, Bo Liu wrote:
> In many functions, the parameter passed in is "_vq", which still call
> to_vvq() to get 'vq'. It can avoid unnecessary call of to_vvq() by directly
> passing in the parameter "vq".
>
> Signed-off-by: Bo Liu
What does the patch accomplish? Is the generated binary faster? smaller?
> ---
> drivers/virtio/virtio_ring.c | 100 ++-
> 1 file changed, 41 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 13a7348cedff..f82db59fdbdc 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -648,9 +648,8 @@ static inline int virtqueue_add_split(struct virtqueue
> *_vq,
> return -ENOMEM;
> }
>
> -static bool virtqueue_kick_prepare_split(struct virtqueue *_vq)
> +static bool virtqueue_kick_prepare_split(struct vring_virtqueue *vq)
> {
> - struct vring_virtqueue *vq = to_vvq(_vq);
> u16 new, old;
> bool needs_kick;
>
> @@ -667,12 +666,12 @@ static bool virtqueue_kick_prepare_split(struct
> virtqueue *_vq)
> LAST_ADD_TIME_INVALID(vq);
>
> if (vq->event) {
> - needs_kick = vring_need_event(virtio16_to_cpu(_vq->vdev,
> + needs_kick = vring_need_event(virtio16_to_cpu(vq->vq.vdev,
> vring_avail_event(>split.vring)),
> new, old);
> } else {
> needs_kick = !(vq->split.vring.used->flags &
> - cpu_to_virtio16(_vq->vdev,
> + cpu_to_virtio16(vq->vq.vdev,
> VRING_USED_F_NO_NOTIFY));
> }
> END_USE(vq);
> @@ -735,11 +734,10 @@ static inline bool more_used_split(const struct
> vring_virtqueue *vq)
> vq->split.vring.used->idx);
> }
>
> -static void *virtqueue_get_buf_ctx_split(struct virtqueue *_vq,
> +static void *virtqueue_get_buf_ctx_split(struct vring_virtqueue *vq,
>unsigned int *len,
>void **ctx)
> {
> - struct vring_virtqueue *vq = to_vvq(_vq);
> void *ret;
> unsigned int i;
> u16 last_used;
> @@ -761,9 +759,9 @@ static void *virtqueue_get_buf_ctx_split(struct virtqueue
> *_vq,
> virtio_rmb(vq->weak_barriers);
>
> last_used = (vq->last_used_idx & (vq->split.vring.num - 1));
> - i = virtio32_to_cpu(_vq->vdev,
> + i = virtio32_to_cpu(vq->vq.vdev,
> vq->split.vring.used->ring[last_used].id);
> - *len = virtio32_to_cpu(_vq->vdev,
> + *len = virtio32_to_cpu(vq->vq.vdev,
> vq->split.vring.used->ring[last_used].len);
>
> if (unlikely(i >= vq->split.vring.num)) {
> @@ -785,7 +783,7 @@ static void *virtqueue_get_buf_ctx_split(struct virtqueue
> *_vq,
> if (!(vq->split.avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT))
> virtio_store_mb(vq->weak_barriers,
> _used_event(>split.vring),
> - cpu_to_virtio16(_vq->vdev, vq->last_used_idx));
> + cpu_to_virtio16(vq->vq.vdev,
> vq->last_used_idx));
>
> LAST_ADD_TIME_INVALID(vq);
>
> @@ -793,10 +791,8 @@ static void *virtqueue_get_buf_ctx_split(struct
> virtqueue *_vq,
> return ret;
> }
>
> -static void virtqueue_disable_cb_split(struct virtqueue *_vq)
> +static void virtqueue_disable_cb_split(struct vring_virtqueue *vq)
> {
> - struct vring_virtqueue *vq = to_vvq(_vq);
> -
> if (!(vq->split.avail_flags_shadow & VRING_AVAIL_F_NO_INTERRUPT)) {
> vq->split.avail_flags_shadow |= VRING_AVAIL_F_NO_INTERRUPT;
> if (vq->event)
> @@ -804,14 +800,13 @@ static void virtqueue_disable_cb_split(struct virtqueue
> *_vq)
> vring_used_event(>split.vring) = 0x0;
> else
> vq->split.vring.avail->flags =
> - cpu_to_virtio16(_vq->vdev,
> + cpu_to_virtio16(vq->vq.vdev,
> vq->split.avail_flags_shadow);
> }
> }
>
> -static unsigned int virtqueue_enable_cb_prepare_split(struct virtqueue *_vq)
> +static unsigned int virtqueue_enable_cb_prepare_split(struct vring_virtqueue
> *vq)
> {
> - struct vring_virtqueue *vq = to_vvq(_vq);
> u16 last_used_idx;
>
> START_USE(vq);
> @@ -825,26 +820,23 @@ static unsigned int
> virtqueue_enable_cb_prepare_split(struct virtqueue *_vq)
> vq->split.avail_flags_shadow &= ~VRING_AVAIL_F_NO_INTERRUPT;
> if (!vq->event)
> vq->split.vring.avail->flags =
> - cpu_to_virtio16(_vq->vdev,
> + cpu_to_virtio16(vq->vq.vdev,
>
Re: [PATCH 6/6] vDPA: fix 'cast to restricted le16' warnings in vdpa_dev_net_config_fill()
On Mon, Jun 6, 2022 at 4:22 PM Zhu, Lingshan wrote: > > > > On 6/2/2022 3:40 PM, Jason Wang wrote: > > On Thu, Jun 2, 2022 at 10:48 AM Zhu Lingshan wrote: > >> This commit fixes spars warnings: cast to restricted __le16 > >> in function vdpa_dev_net_config_fill() > >> > >> Signed-off-by: Zhu Lingshan > >> --- > >> drivers/vdpa/vdpa.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > >> index 50a11ece603e..2719ce9962fc 100644 > >> --- a/drivers/vdpa/vdpa.c > >> +++ b/drivers/vdpa/vdpa.c > >> @@ -837,11 +837,11 @@ static int vdpa_dev_net_config_fill(struct > >> vdpa_device *vdev, struct sk_buff *ms > >> config.mac)) > >> return -EMSGSIZE; > >> > >> - val_u16 = le16_to_cpu(config.status); > >> + val_u16 = le16_to_cpu((__force __le16)config.status); > > Can we use virtio accessors like virtio16_to_cpu()? > I will work out a vdpa16_to_cpu() I meant __virtio16_to_cpu(true, xxx) actually here. Thanks > > Thanks, > Zhu Lingshan > > > > Thanks > > > >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16)) > >> return -EMSGSIZE; > >> > >> - val_u16 = le16_to_cpu(config.mtu); > >> + val_u16 = le16_to_cpu((__force __le16)config.mtu); > >> if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16)) > >> return -EMSGSIZE; > >> > >> -- > >> 2.31.1 > >> > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 5/6] vDPA: answer num of queue pairs = 1 to userspace when VIRTIO_NET_F_MQ == 0
On Mon, Jun 6, 2022 at 4:22 PM Zhu, Lingshan wrote: > > > > On 6/2/2022 3:38 PM, Jason Wang wrote: > > On Thu, Jun 2, 2022 at 10:48 AM Zhu Lingshan wrote: > >> If VIRTIO_NET_F_MQ == 0, the virtio device should have one queue pair, > >> so when userspace querying queue pair numbers, it should return mq=1 > >> than zero > > Spec said: > > > > "max_virtqueue_pairs only exists if VIRTIO_NET_F_MQ is set" > > > > So we are probably fine. > I thinks it is asking how many queue > pairs(VDPA_ATTR_DEV_NET_CFG_MAX_VQP), so answering 0 may not be correct. > > Thanks, > Zhu Lingshan Please add the result of the userspace vdpa tool before and after this patch in the changlog in next version. Thanks > > > > Thanks > > > >> Signed-off-by: Zhu Lingshan > >> --- > >> drivers/vdpa/vdpa.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c > >> index 030d96bdeed2..50a11ece603e 100644 > >> --- a/drivers/vdpa/vdpa.c > >> +++ b/drivers/vdpa/vdpa.c > >> @@ -818,9 +818,10 @@ static int vdpa_dev_net_mq_config_fill(struct > >> vdpa_device *vdev, > >> u16 val_u16; > >> > >> if ((features & BIT_ULL(VIRTIO_NET_F_MQ)) == 0) > >> - return 0; > >> + val_u16 = 1; > >> + else > >> + val_u16 = le16_to_cpu((__force > >> __le16)config->max_virtqueue_pairs); > >> > >> - val_u16 = le16_to_cpu(config->max_virtqueue_pairs); > >> return nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, val_u16); > >> } > >> > >> -- > >> 2.31.1 > >> > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 14/20] hugetlb: Convert to migrate_folio
Hi "Matthew,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.19-rc1 next-20220607]
[cannot apply to jaegeuk-f2fs/dev-test trondmy-nfs/linux-next kdave/for-next
xfs-linux/for-next]
[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]
url:
https://github.com/intel-lab-lkp/linux/commits/Matthew-Wilcox-Oracle/Convert-aops-migratepage-to-aops-migrate_folio/20220607-044509
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
f2906aa863381afb0015a9eb7fefad885d4e5a56
config: ia64-randconfig-r015-20220605
(https://download.01.org/0day-ci/archive/20220607/202206071414.41wgg8fp-...@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/intel-lab-lkp/linux/commit/b038962c9c8c2ab77c71dfba24356ce24bd7a242
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review
Matthew-Wilcox-Oracle/Convert-aops-migratepage-to-aops-migrate_folio/20220607-044509
git checkout b038962c9c8c2ab77c71dfba24356ce24bd7a242
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1
O=build_dir ARCH=ia64 SHELL=/bin/bash fs/hugetlbfs/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
fs/hugetlbfs/inode.c: In function 'hugetlbfs_migrate_folio':
>> fs/hugetlbfs/inode.c:990:17: error: implicit declaration of function
>> 'folio_migrate_copy' [-Werror=implicit-function-declaration]
990 | folio_migrate_copy(dst, src);
| ^~
>> fs/hugetlbfs/inode.c:992:17: error: implicit declaration of function
>> 'folio_migrate_flags'; did you mean 'folio_mapping_flags'?
>> [-Werror=implicit-function-declaration]
992 | folio_migrate_flags(dst, src);
| ^~~
| folio_mapping_flags
cc1: some warnings being treated as errors
vim +/folio_migrate_copy +990 fs/hugetlbfs/inode.c
972
973 static int hugetlbfs_migrate_folio(struct address_space *mapping,
974 struct folio *dst, struct folio *src,
975 enum migrate_mode mode)
976 {
977 int rc;
978
979 rc = migrate_huge_page_move_mapping(mapping, dst, src);
980 if (rc != MIGRATEPAGE_SUCCESS)
981 return rc;
982
983 if (hugetlb_page_subpool(>page)) {
984 hugetlb_set_page_subpool(>page,
985
hugetlb_page_subpool(>page));
986 hugetlb_set_page_subpool(>page, NULL);
987 }
988
989 if (mode != MIGRATE_SYNC_NO_COPY)
> 990 folio_migrate_copy(dst, src);
991 else
> 992 folio_migrate_flags(dst, src);
993
994 return MIGRATEPAGE_SUCCESS;
995 }
996
--
0-DAY CI Kernel Test Service
https://01.org/lkp
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
