Re: [PATCH] fw_cfg: don't use DMA mapping for fw_cfg device

[Peter Xu]

On Mon, Jan 15, 2018 at 12:22:48PM +0100, Marc-Andre Lureau wrote:
> Hi
> 
> On Mon, Jan 15, 2018 at 9:55 AM, Peter Xu <peterx@xxxxxxxxxx> wrote:
> > fw_cfg device does not need IOMMU protection, so use physical addresses
> > always.  That's how QEMU implements fw_cfg.  Otherwise we'll see call
> > traces during boot when vIOMMU is enabled in guest:
> >
> > [    1.018306] ------------[ cut here ]------------
> > [    1.018314] WARNING: CPU: 1 PID: 1 at drivers/firmware/qemu_fw_cfg.c:152 fw_cfg_dma_transfer+0x399/0x500
> > [    1.018315] fw_cfg_dma_transfer: failed to map fw_cfg_dma
> > [    1.018316] Modules linked in:
> > [    1.018320] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 3.10.0-827.el7.x86_64 #1
> > [    1.018321] Hardware name: Red Hat KVM, BIOS 1.11.0-1.el7 04/01/2014
> > [    1.018322] Call Trace:
> > [    1.018330]  [<ffffffffafcf6b3a>] dump_stack+0x19/0x1b
> > [    1.018334]  [<ffffffffaf68fff8>] __warn+0xd8/0x100
> > [    1.018336]  [<ffffffffaf69007f>] warn_slowpath_fmt+0x5f/0x80
> > [    1.018338]  [<ffffffffafb67329>] fw_cfg_dma_transfer+0x399/0x500
> > [    1.018340]  [<ffffffffafb6753c>] fw_cfg_read_blob+0xac/0x1c0
> > [    1.018342]  [<ffffffffafb67720>] fw_cfg_register_dir_entries+0x80/0x450
> > [    1.018344]  [<ffffffffafb67d02>] fw_cfg_sysfs_probe+0x212/0x3f0
> > [    1.018347]  [<ffffffffafa70352>] platform_drv_probe+0x42/0x110
> > [    1.018350]  [<ffffffffafa6e002>] driver_probe_device+0xc2/0x3e0
> > [    1.018352]  [<ffffffffafa6e3f3>] __driver_attach+0x93/0xa0
> > [    1.018354]  [<ffffffffafa6e360>] ? __device_attach+0x40/0x40
> > [    1.018359]  [<ffffffffafa6bbd3>] bus_for_each_dev+0x73/0xc0
> > [    1.018362]  [<ffffffffafa6d97e>] driver_attach+0x1e/0x20
> > [    1.018364]  [<ffffffffafa6d420>] bus_add_driver+0x200/0x2d0
> > [    1.018366]  [<ffffffffb03c2041>] ? firmware_map_add_early+0x58/0x58
> > [    1.018368]  [<ffffffffafa6ea84>] driver_register+0x64/0xf0
> > [    1.018370]  [<ffffffffafa7013a>] __platform_driver_register+0x4a/0x50
> > [    1.018372]  [<ffffffffb03c2075>] fw_cfg_sysfs_init+0x34/0x61
> > [    1.018376]  [<ffffffffaf602108>] do_one_initcall+0xb8/0x230
> > [    1.018379]  [<ffffffffb036b34a>] kernel_init_freeable+0x17a/0x219
> > [    1.018381]  [<ffffffffb036ab19>] ? initcall_blacklist+0xb0/0xb0
> > [    1.018383]  [<ffffffffafce5b30>] ? rest_init+0x80/0x80
> > [    1.018385]  [<ffffffffafce5b3e>] kernel_init+0xe/0xf0
> > [    1.018388]  [<ffffffffafd08898>] ret_from_fork+0x58/0x90
> > [    1.018390]  [<ffffffffafce5b30>] ? rest_init+0x80/0x80
> > [    1.018392] ---[ end trace d00a5b71608a8f59 ]---
> >
> > Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1533367
> > Fixes: e90cb816599b ("fw_cfg: do DMA read operation", 2017-11-28)
> > CC: Marc-Andrà Lureau <marcandre.lureau@xxxxxxxxxx>
> > CC: Michael S. Tsirkin <mst@xxxxxxxxxx>
> > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> > --
> >
> > This is based on tree:
> >   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git/log/?h=vhost
> >
> > Please review, thanks.
> >
> > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> 
> The DMA business is confusing, sadly I didn't get much clue what I was
> supposed to do. What I can say:
> 
> Tested-by: Marc-Andrà Lureau <marcandre.lureau@xxxxxxxxxx>

Thanks for confirming this.

> 
> Should the series be removed from Michael tree and I squash your fix &
> send a v10?
> 
> Fwiw, "fw_cfg: write vmcoreinfo details" should also be fixed to
> allocate memory (unless your approach fixes that?)

Yes, IMHO this patch should also work for writes (though not tested).

Thanks,

-- 
Peter Xu