* Re: [PATCH 1/2] devicetree/bindings: display: Add bindings for LVDS panels
From: Laurent Pinchart @ 2016-11-14 23:39 UTC (permalink / raw)
To: dri-devel
Cc: Rob Herring, devicetree@vger.kernel.org, Laurent Pinchart,
open list:MEDIA DRIVERS FOR RENESAS - FCP, Tomi Valkeinen,
linux-media@vger.kernel.org
In-Reply-To: <1488637.i0jADhlNmg@avalon>
Hi Rob,
Ping ?
On Monday 17 Oct 2016 15:42:56 Laurent Pinchart wrote:
> On Friday 14 Oct 2016 07:40:14 Rob Herring wrote:
> > On Sun, Oct 9, 2016 at 11:33 AM, Laurent Pinchart wrote:
> >> On Saturday 08 Oct 2016 20:29:39 Rob Herring wrote:
> >>> On Tue, Oct 04, 2016 at 07:23:29PM +0300, Laurent Pinchart wrote:
> >>>> LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>> Multiple incompatible data link layers have been used over time to
> >>>> transmit image data to LVDS panels. This binding supports display
> >>>> panels compatible with the JEIDA-59-1999, Open-LDI and VESA SWPG
> >>>> specifications.
> >>>>
> >>>> Signed-off-by: Laurent Pinchart
> >>>> <laurent.pinchart+renesas@ideasonboard.com>
> >>>> ---
> >>>>
> >>>> .../bindings/display/panel/panel-lvds.txt | 119 ++++++++++++
> >>>> 1 file changed, 119 insertions(+)
> >>>> create mode 100644
> >>>> Documentation/devicetree/bindings/display/panel/panel-lvds.txt>
> >>>>
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> new file mode 100644
> >>>> index 000000000000..250861f2673e
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/display/panel/panel-lvds.txt
> >>>> @@ -0,0 +1,119 @@
> >>>> +Generic LVDS Panel
> >>>> +==================
> >>>> +
> >>>> +LVDS is a physical layer specification defined in ANSI/TIA/EIA-644-A.
> >>>> Multiple
> >>>> +incompatible data link layers have been used over time to transmit
> >>>> image data
> >>>> +to LVDS panels. This bindings supports display panels compatible with
> >>>> the
> >>>> +following specifications.
> >>>> +
> >>>> +[JEIDA] "Digital Interface Standards for Monitor", JEIDA-59-1999,
> >>>> February
> >>>> +1999 (Version 1.0), Japan Electronic Industry Development Association
> >>>> (JEIDA)
> >>>> +[LDI] "Open LVDS Display Interface", May 1999 (Version 0.95),
> >>>> National
> >>>> +Semiconductor
> >>>> +[VESA] "VESA Notebook Panel Standard", October 2007 (Version 1.0),
> >>>> Video
> >>>> +Electronics Standards Association (VESA)
> >>>> +
> >>>> +Device compatible with those specifications have been marketed under
> >>>> the
> >>>> +FPD-Link and FlatLink brands.
> >>>> +
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible: shall contain "panel-lvds"
> >>>
> >>> Maybe as a fallback, but on its own, no way.
> >>
> >> Which brings an interesting question: when designing generic DT
> >> bindings, what's the rule regarding
>
> Looks like I forgot part of the question. I meant to ask what is the rule
> regarding usage of more precise compatible strings ?
>
> For instance (but perhaps not the best example), the
> input/rotary-encoder.txt bindings define a "rotary-encoder" compatible
> string, with no other bindings defining more precise compatible strings for
> the exact rotary encoder model. When it comes to panels I believe it makes
> sense to define model-specific compatible strings even if they're unused by
> drivers. I'm however wondering what the rule is there, and where those
> device-specific compatible strings should be defined. I'd like to avoid
> using one file per panel model as done today for the simple-panel bindings.
>
> > Call it "simple" so I can easily NAK it. :)
> >
> > Define a generic structure, not a single binding trying to serve all.
> >
> >>> > +- width-mm: panel display width in millimeters
> >>> > +- height-mm: panel display height in millimeters
> >>>
> >>> This is already documented for all panels IIRC.
> >>
> >> Note that this DT binding has nothing to do with the simple-panel
> >> binding. It is instead similar to the panel-dpi and panel-dsi-cm bindings
> >> (which currently don't but should specify the panel size in DT). The LVDS
> >> panel driver will *not* include any panel-specific information such as
> >> size or timings, these are specified in DT.
> >
> > The panel bindings aren't really different. The biggest difference was
> > location in the tree, but we now generally allow panels to be either a
> > child of the LCD controller or connected with OF graph. We probably
> > need to work on restructuring the panel bindings a bit. We should have
> > an inheritance with a base panel binding of things like size, label,
> > graph, backlight, etc, then perhaps an interface specific bindings for
> > LVDS, DSI, and parallel, then a panel specific binding. With this the
> > panel specific binding is typically just a compatible string and which
> > inherited properties apply to it.
>
> That sounds good to me, but we have multiple models for panel bindings.
>
> As you mentioned panels can be referenced through an LCD controller node
> property containing a phandle to the panel node, or through OF graph. That's
> a situation we have today, and we need to keep supporting both (at least
> for existing panels, perhaps not for the new ones).
>
> Another difference is how to express panel data such as size and timings.
> The simple-panel DT bindings don't contain such data and expects the
> drivers to contain a table of panel data for all models supported, while
> the DPI, DSI and now the proposed LVDS panel bindings contain properties
> for panel data.
>
> How would you like to reconcile all that ?
>
> >>>> +- data-mapping: the color signals mapping order, "jeida-18",
> >>>> "jeida-24"
> >>>> + or "vesa-24"
> >>>
> >>> Maybe this should be part of the compatible.
> >>
> >> I've thought about it, but given that some panels support selecting
> >> between multiple modes (through a mode pin that is usually hardwired), I
> >> believe a separate DT property makes sense.
> >
> > Okay.
> >
> >> Furthermore, LVDS data organization is controlled by the combination of
> >> both data-mapping and data-mirror. It makes little sense from my point
> >> of view to handle one as part of the compatible string and the other one
> >> as a separate property.
> >>
> >>> > +Optional properties:
> >>> > +- label: a symbolic name for the panel
> >>>
> >>> Could be for any panel or display connector.
> >>
> >> Yes, but I'm not sure to understand how that's relevant :-)
> >
> > Meaning it should be a common property.
>
> Sure. So you expect me to reorganize all the panels and connectors DT
> bindings in order to get this one merged ? :-)
>
> >>>> +- avdd-supply: reference to the regulator that powers the panel
> >>>> analog supply
> >>>> +- dvdd-supply: reference to the regulator that powers the panel
> >>>> digital supply
> >>>
> >>> Which one has to be powered on first, what voltage, and with what time
> >>> in between? This is why "generic" or "simple" bindings don't work.
> >>
> >> The above-mentioned specifications also define connectors, pinouts and
> >> power supplies, but many LVDS panels compatible with the LVDS physical
> >> and data layers use a different connector with small differences in
> >> power
> >> supplies.
> >>
> >> I believe the voltage is irrelevant here, it doesn't need to be
> >> controlled by the operating system. Power supplies order and timing is
> >> relevant, I'll investigate the level of differences between panels. I'm
> >> also fine with dropping those properties for now.
> >
> > Whether you have control of the supplies is dependent on the board.
> > Dropping them is just puts us in the simple binding trap. The simple
> > bindings start out that way and then people keep adding to them.
>
> Damn, you can't be fooled easily ;-)
>
> On a more serious note, I'd like to design the bindings in a way that
> wouldn't require adding device-specific code in the driver for each panel
> model, given that in most cases power supply handling will be generic.
> What's your opinion about a generic power supply model that would be used
> in the default case, with the option to override it with device-specific
> code when needed ?
>
> >>>> +- data-mirror: if set, reverse the bit order on all data lanes (6 to
> >>>> 0 instead
> >>>> + of 0 to 6)
> >
> > On this one, make the name describe the order. "mirror" requires that
> > I know what is normal ordering. Perhaps "data-msb-first".
>
> Sounds good to me, I'll use that.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] qla2xxx: do not abort all commands in the adapter during EEH recovery
From: Martin K. Petersen @ 2016-11-14 23:39 UTC (permalink / raw)
To: Mauricio Faria de Oliveira
Cc: Himanshu.Madhani, qla2xxx-upstream, martin.petersen, jejb,
linux-scsi, linux-kernel
In-Reply-To: <1479158782-4544-1-git-send-email-mauricfo@linux.vnet.ibm.com>
>>>>> "Mauricio" == Mauricio Faria de Oliveira <mauricfo@linux.vnet.ibm.com> writes:
Mauricio> The previous commit ("qla2xxx: fix invalid DMA access after
Mauricio> command aborts in PCI device remove") introduced a regression
Mauricio> during an EEH recovery, since the change to the
Mauricio> qla2x00_abort_all_cmds() function calls qla2xxx_eh_abort(),
Mauricio> which verifies the EEH recovery condition but handles it
Mauricio> heavy-handed. (commit a465537ad1a4 "qla2xxx: Disable the
Mauricio> adapter and skip error recovery in case of register
Mauricio> disconnect.")
Applied to 4.9/scsi-fixes.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH v9 1/5] mfd: mxs-lradc: Add support for mxs-lradc MFD
From: Fabio Estevam @ 2016-11-14 23:39 UTC (permalink / raw)
To: Marek Vasut
Cc: Ksenija Stanojevic, linux-kernel, Lee Jones, Dmitry Torokhov,
linux-input, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald, linux-iio, Harald Geyer, Stefan Wahren,
Fabio Estevam
In-Reply-To: <9560d7a2-98b2-732d-c91a-944ae381107c@denx.de>
On Mon, Nov 14, 2016 at 4:43 PM, Marek Vasut <marex@denx.de> wrote:
>> +#define MXS_LRADC_BASE 0x80050000
>
> Just for my understanding, why is this hardware address hard-coded here?
> Shouldn't that be fetched from DT ?
Yes, this base address should be retrieved from device tree.
^ permalink raw reply
* Re: [PATCH] scsi: hpsa: free irq on q indexed by h->intr_mode and not i
From: Martin K. Petersen @ 2016-11-14 23:36 UTC (permalink / raw)
To: Colin King
Cc: Christoph Hellwig, Don Brace, James E . J . Bottomley,
Martin K . Petersen, esc.storagedev, linux-scsi, linux-kernel
In-Reply-To: <20161114125935.12158-1-colin.king@canonical.com>
>>>>> "Colin" == Colin King <colin.king@canonical.com> writes:
Colin> From: Colin Ian King <colin.king@canonical.com> Use correct index
Colin> on q, use h->intr_mode instead of i. Issue detected using static
Colin> analysis with cppcheck
Applied to 4.10/scsi-queue.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH 1/3] Documentation: DT: Add entry for FSL LS1012A RDB, FRDM, QDS boards
From: Rob Herring @ 2016-11-14 23:35 UTC (permalink / raw)
To: Harninder Rai
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
oss-fOR+EgIDQEHk1uMJSBkQmQ, Bhaskar Upadhaya,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1478714651-12679-1-git-send-email-harninder.rai-3arQi8VN3Tc@public.gmane.org>
On Wed, Nov 09, 2016 at 11:34:11PM +0530, Harninder Rai wrote:
> Signed-off-by: Harninder Rai <harninder.rai-3arQi8VN3Tc@public.gmane.org>
> Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya-3arQi8VN3Tc@public.gmane.org>
> ---
> Documentation/devicetree/bindings/arm/fsl.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 1/3] Documentation: DT: Add entry for FSL LS1012A RDB, FRDM, QDS boards
From: Rob Herring @ 2016-11-14 23:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1478714651-12679-1-git-send-email-harninder.rai@nxp.com>
On Wed, Nov 09, 2016 at 11:34:11PM +0530, Harninder Rai wrote:
> Signed-off-by: Harninder Rai <harninder.rai@nxp.com>
> Signed-off-by: Bhaskar Upadhaya <Bhaskar.Upadhaya@nxp.com>
> ---
> Documentation/devicetree/bindings/arm/fsl.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: linux-next: BUG: unable to handle kernel NULL pointer dereference in __sk_mem_raise_allocated()
From: Eric Dumazet @ 2016-11-14 23:35 UTC (permalink / raw)
To: Andrei Vagin; +Cc: Linux Kernel Network Developers, Paolo Abeni
In-Reply-To: <CANaxB-yCk8hhP68L4Q2nFOJht8sqgXGGQO2AftpHs0u1xyGG5A@mail.gmail.com>
On Mon, 2016-11-14 at 15:24 -0800, Andrei Vagin wrote:
> Hi Paolo,
>
> Our test system detected a kernel oops. Looks like a problem in the
> "udp: refactor memory accounting" series.
>
> # good: [f970bd9e3a06f06df8d8ecf1f8ad2c8615cc17eb] udp: implement
> memory accounting helpers
> git bisect good f970bd9e3a06f06df8d8ecf1f8ad2c8615cc17eb
> # bad: [2d0e30c30f84d08dc16f0f2af41f1b8a85f0755e] bpf: add helper for
> retrieving current numa node id
> git bisect bad 2d0e30c30f84d08dc16f0f2af41f1b8a85f0755e
> # bad: [a10b91b8b81c29b87ff5a6d58c1402898337b956] Merge branch 'udpmem'
> git bisect bad a10b91b8b81c29b87ff5a6d58c1402898337b956
> # good: [850cbaddb52dfd4e0c7cabe2c168dd34b44ae0b9] udp: use it's own
> memory accounting schema
> git bisect good 850cbaddb52dfd4e0c7cabe2c168dd34b44ae0b9
> # first bad commit: [a10b91b8b81c29b87ff5a6d58c1402898337b956] Merge
> branch 'udpmem'
>
>
> [ 112.472363] BUG: unable to handle kernel NULL pointer dereference
> at (null)
> [ 112.473360] IP: [<ffffffffb76f8031>] __sk_mem_raise_allocated+0x31/0x3f0
> [ 112.474156] PGD 62a08067 [ 112.474455] PUD 2b8bf067
> PMD 0 [ 112.474856]
> [ 112.475054] Oops: 0002 [#1] SMP
> [ 112.475431] Modules linked in: nf_conntrack_netlink udp_diag
> tcp_diag inet_diag netlink_diag af_packet_diag unix_diag binfmt_misc
> nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4
> xt_conntrack nf_conntrack nfnetlink ip6table_filter ip6_tables ppdev
> sunrpc crc32c_intel joydev virtio_balloon virtio_net i2c_piix4
> parport_pc parport acpi_cpufreq tpm_tis tpm_tis_core tpm virtio_blk
> serio_raw virtio_pci virtio_ring virtio ata_generic pata_acpi
> [ 112.480594] CPU: 1 PID: 7405 Comm: socket_udplite Not tainted 4.8.0+ #84
> [ 112.481377] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.9.1-1.fc24 04/01/2014
> [ 112.482375] task: ffff928a5b5fa540 task.stack: ffffb3b484a0c000
> [ 112.483059] RIP: 0010:[<ffffffffb76f8031>] [<ffffffffb76f8031>]
> __sk_mem_raise_allocated+0x31/0x3f0
> [ 112.484135] RSP: 0018:ffff928abfd03b18 EFLAGS: 00010296
> [ 112.484758] RAX: 0000000000000001 RBX: ffff928aa293cfc0 RCX: 0000000000000001
> [ 112.485585] RDX: 0000000000000000 RSI: 0000000000001000 RDI: ffff928aa293cfc0
> [ 112.486414] RBP: ffff928abfd03b48 R08: 0de4c53600000000 R09: 0000000000000000
> [ 112.487241] R10: 000000006226b971 R11: 0000000000000000 R12: ffff928aa293cfc0
> [ 112.488064] R13: 0000000000000001 R14: ffffffffb7f0d5a0 R15: 0000000000001000
> [ 112.488893] FS: 00007f058067a700(0000) GS:ffff928abfd00000(0000)
> knlGS:0000000000000000
> [ 112.489807] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 112.490447] CR2: 0000000000000000 CR3: 000000002b8f5000 CR4: 00000000000006e0
> [ 112.491248] DR0: 00000000000100a0 DR1: 0000000000000000 DR2: 0000000000000000
> [ 112.492025] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
> [ 112.492808] Stack:
> [ 112.493038] 0000000100000300 ffff928aa293cfc0 ffff928a651b9c00
> 0000000000000300
> [ 112.493912] ffff928aa293d108 0000000000001000 ffff928abfd03b88
> ffffffffb779e094
> [ 112.494782] ffff928abfd03b70 ffff928a651b9c00 ffff928aa293cfc0
> 0000000000000000
Thanks for the report.
I guess following patch would be needed ?
diff --git a/net/ipv4/udplite.c b/net/ipv4/udplite.c
index af817158d830c0da080935ba29e012dffbb89112..12604c0371c451efcc9aad278bb86be9ac4bb813 100644
--- a/net/ipv4/udplite.c
+++ b/net/ipv4/udplite.c
@@ -54,6 +54,7 @@ struct proto udplite_prot = {
.hash = udp_lib_hash,
.unhash = udp_lib_unhash,
.get_port = udp_v4_get_port,
+ .memory_allocated = &udp_memory_allocated,
.obj_size = sizeof(struct udp_sock),
.h.udp_table = &udplite_table,
#ifdef CONFIG_COMPAT
diff --git a/net/ipv6/udplite.c b/net/ipv6/udplite.c
index 47d0d2b87106558fece3496479198005c55b99e7..946025c888cc9519fb3523edbbe8afbb18273326 100644
--- a/net/ipv6/udplite.c
+++ b/net/ipv6/udplite.c
@@ -49,6 +49,7 @@ struct proto udplitev6_prot = {
.hash = udp_lib_hash,
.unhash = udp_lib_unhash,
.get_port = udp_v6_get_port,
+ .memory_allocated = &udp_memory_allocated,
.obj_size = sizeof(struct udp6_sock),
.h.udp_table = &udplite_table,
#ifdef CONFIG_COMPAT
^ permalink raw reply related
* [oe-layersetup][PATCH 1/2] arago-krogoth-config: un-pin meta-qt5 as QWaylandIntegration got fixed
From: Denys Dmytriyenko @ 2016-11-14 23:34 UTC (permalink / raw)
To: meta-arago
From: Denys Dmytriyenko <denys@ti.com>
Signed-off-by: Denys Dmytriyenko <denys@ti.com>
---
configs/arago-krogoth-config.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configs/arago-krogoth-config.txt b/configs/arago-krogoth-config.txt
index cccc0f9..731a69d 100644
--- a/configs/arago-krogoth-config.txt
+++ b/configs/arago-krogoth-config.txt
@@ -3,7 +3,7 @@
bitbake,git://git.openembedded.org/bitbake,1.30,HEAD,layers=
meta-arago,git://arago-project.org/git/meta-arago.git,krogoth,HEAD,layers=meta-arago-distro:meta-arago-extras
-meta-qt5,git://github.com/meta-qt5/meta-qt5.git,krogoth,48b4084675a15fe3fd10b58f54728dfaa2bd6c72,layers=
+meta-qt5,git://github.com/meta-qt5/meta-qt5.git,krogoth,HEAD,layers=
meta-openembedded,git://git.openembedded.org/meta-openembedded,krogoth,HEAD,layers=meta-networking:meta-ruby:meta-python:meta-oe
meta-ti,git://git.yoctoproject.org/meta-ti,krogoth,HEAD,layers=
meta-linaro,git://git.linaro.org/openembedded/meta-linaro.git,krogoth,HEAD,layers=meta-linaro-toolchain:meta-optee
--
2.7.4
^ permalink raw reply related
* [oe-layersetup][PATCH 2/2] arago-morty-config: add config to track morty branches
From: Denys Dmytriyenko @ 2016-11-14 23:34 UTC (permalink / raw)
To: meta-arago
In-Reply-To: <1479166471-30960-1-git-send-email-denis@denix.org>
From: Denys Dmytriyenko <denys@ti.com>
meta-ti and meta-arago point to master for now, until remaining issues fixed
Signed-off-by: Denys Dmytriyenko <denys@ti.com>
---
configs/arago-morty-config.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 configs/arago-morty-config.txt
diff --git a/configs/arago-morty-config.txt b/configs/arago-morty-config.txt
new file mode 100644
index 0000000..ce7dbac
--- /dev/null
+++ b/configs/arago-morty-config.txt
@@ -0,0 +1,12 @@
+# This file takes repo entries in the format
+# repo name,repo uri,repo branch,repo commit[,layers=layer1:layer2...:layern]
+
+bitbake,git://git.openembedded.org/bitbake,1.32,HEAD,layers=
+meta-arago,git://arago-project.org/git/meta-arago.git,master,HEAD,layers=meta-arago-distro:meta-arago-extras
+meta-qt5,git://github.com/meta-qt5/meta-qt5.git,morty,HEAD,layers=
+meta-openembedded,git://git.openembedded.org/meta-openembedded,morty,HEAD,layers=meta-networking:meta-ruby:meta-python:meta-oe
+meta-ti,git://git.yoctoproject.org/meta-ti,master,HEAD,layers=
+meta-linaro,git://git.linaro.org/openembedded/meta-linaro.git,morty,HEAD,layers=meta-linaro-toolchain:meta-optee
+oe-core,git://git.openembedded.org/openembedded-core,morty,HEAD,layers=meta
+OECORELAYERCONF=./sample-files/bblayers.conf.sample
+OECORELOCALCONF=./sample-files/local-arago64.conf.sample
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] isci: fix typo in deg_dbg message
From: Martin K. Petersen @ 2016-11-14 23:34 UTC (permalink / raw)
To: Colin King
Cc: Intel SCU Linux support, Artur Paszkiewicz,
James E . J . Bottomley, Martin K . Petersen, linux-scsi,
linux-kernel
In-Reply-To: <20161112183026.9626-1-colin.king@canonical.com>
>>>>> "Colin" == Colin King <colin.king@canonical.com> writes:
Colin> From: Colin Ian King <colin.king@canonical.com> Trivial fix to
Colin> typo "repsonse" to "response" in dev_dbg message.
Applied to 4.10/scsi-queue.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH] iscsi: fix spelling mistakes in dev_warn messages
From: Martin K. Petersen @ 2016-11-14 23:33 UTC (permalink / raw)
To: Colin King
Cc: Intel SCU Linux support, Artur Paszkiewicz,
James E . J . Bottomley, Martin K . Petersen, linux-scsi,
linux-kernel
In-Reply-To: <20161112164950.5605-1-colin.king@canonical.com>
>>>>> "Colin" == Colin King <colin.king@canonical.com> writes:
Colin> From: Colin Ian King <colin.king@canonical.com> Trivial fix to
Colin> spelling mistake "suspeneded" to "suspended" in dev_warn messages
Applied to 4.10/scsi-queue with s/iscsi/isci/.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* [ovmf baseline-only test] 68037: all pass
From: Platform Team regression test user @ 2016-11-14 23:33 UTC (permalink / raw)
To: xen-devel, osstest-admin
This run is configured for baseline tests only.
flight 68037 ovmf real [real]
http://osstest.xs.citrite.net/~osstest/testlogs/logs/68037/
Perfect :-)
All tests in this flight passed as required
version targeted for testing:
ovmf 653bde546200af46573cdfac88257f03b3420d88
baseline version:
ovmf bab82372a9e6ce066fa725a792e71d07191046ca
Last test of basis 68035 2016-11-14 14:46:46 Z 0 days
Testing same since 68037 2016-11-14 19:20:10 Z 0 days 1 attempts
------------------------------------------------------------
People who touched revisions under test:
Haojian Zhuang <haojian.zhuang@linaro.org>
Ryan Harkin <ryan.harkin@linaro.org>
jobs:
build-amd64-xsm pass
build-i386-xsm pass
build-amd64 pass
build-i386 pass
build-amd64-libvirt pass
build-i386-libvirt pass
build-amd64-pvops pass
build-i386-pvops pass
test-amd64-amd64-xl-qemuu-ovmf-amd64 pass
test-amd64-i386-xl-qemuu-ovmf-amd64 pass
------------------------------------------------------------
sg-report-flight on osstest.xs.citrite.net
logs: /home/osstest/logs
images: /home/osstest/images
Logs, config files, etc. are available at
http://osstest.xs.citrite.net/~osstest/testlogs/logs
Test harness code can be found at
http://xenbits.xensource.com/gitweb?p=osstest.git;a=summary
Push not applicable.
------------------------------------------------------------
commit 653bde546200af46573cdfac88257f03b3420d88
Author: Haojian Zhuang <haojian.zhuang@linaro.org>
Date: Sun Nov 13 14:47:53 2016 +0800
EmbeddedPkg: MmcDxe: add SPEC_VERS field in CSD structure
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
commit e88fcb47c12ac3e21eb6efd9a39f9834c1ba28d3
Author: Haojian Zhuang <haojian.zhuang@linaro.org>
Date: Sun Nov 13 14:47:52 2016 +0800
EmbeddedPkg: MmcDxe: declare ECSD structure
Declare fields in ECSD structure. And drop the original 128 words
arrary.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
commit 36bec33a999388c460e32413e5e9ddf554d2ec78
Author: Haojian Zhuang <haojian.zhuang@linaro.org>
Date: Sun Nov 13 14:47:51 2016 +0800
EmbeddedPkg: MmcDxe: move ECSD into CardInfo structure
Since ECSD also describes the information of card, move it into
structure CardInfo.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
commit 3201075377f80af632465361155f7498c177bad1
Author: Haojian Zhuang <haojian.zhuang@linaro.org>
Date: Sun Nov 13 14:47:50 2016 +0800
EmbeddedPkg: MmcDxe: wait OCR busy bit free
According to eMMC spec, OCR.PowerUp bit is also busy bit. If the busy
bit is '0', CMD1 should be sent and OCR should be fetched again. And add
a timeout counter on the repeated steps.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply
* Re: [PATCH V4 2/2] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers
From: Bjorn Helgaas @ 2016-11-14 23:33 UTC (permalink / raw)
To: Dongdong Liu
Cc: arnd, rafael, Lorenzo.Pieralisi, tn, wangzhou1, pratyush.anand,
linux-pci, linux-acpi, linux-kernel, jcm, gabriele.paoloni,
charles.chenxin, hanjun.guo, linuxarm
In-Reply-To: <1478682897-119874-3-git-send-email-liudongdong3@huawei.com>
On Wed, Nov 09, 2016 at 05:14:57PM +0800, Dongdong Liu wrote:
> PCIe controller in Hip05/HIP06/HIP07 SoCs is not ECAM compliant.
> It is non ECAM only for the RC bus config space;for any other bus
> underneath the root bus we support ECAM access.
> Add specific quirks for PCI config space accessors.This involves:
> 1. New initialization call hisi_pcie_init() to obtain rc base
> addresses from PNP0C02 as subdevice of PNP0A03.
> 2. New entry in common quirk array.
>
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> ---
> MAINTAINERS | 1 +
> drivers/acpi/pci_mcfg.c | 13 ++++
> drivers/pci/host/Kconfig | 8 ++
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pcie-hisi-acpi.c | 157 ++++++++++++++++++++++++++++++++++++++
> include/linux/pci-ecam.h | 5 ++
> 6 files changed, 185 insertions(+)
> create mode 100644 drivers/pci/host/pcie-hisi-acpi.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1cd38a7..b224caa 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9358,6 +9358,7 @@ L: linux-pci@vger.kernel.org
> S: Maintained
> F: Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> F: drivers/pci/host/pcie-hisi.c
> +F: drivers/pci/host/pcie-hisi-acpi.c
>
> PCIE DRIVER FOR ROCKCHIP
> M: Shawn Lin <shawn.lin@rock-chips.com>
> diff --git a/drivers/acpi/pci_mcfg.c b/drivers/acpi/pci_mcfg.c
> index ac21db3..b1b6fc7 100644
> --- a/drivers/acpi/pci_mcfg.c
> +++ b/drivers/acpi/pci_mcfg.c
> @@ -57,6 +57,19 @@ struct mcfg_fixup {
> { "QCOM ", "QDF2432 ", 1, 5, MCFG_BUS_ANY, &pci_32b_ops },
> { "QCOM ", "QDF2432 ", 1, 6, MCFG_BUS_ANY, &pci_32b_ops },
> { "QCOM ", "QDF2432 ", 1, 7, MCFG_BUS_ANY, &pci_32b_ops },
> +#ifdef CONFIG_PCI_HISI_ACPI
> + #define PCI_ACPI_QUIRK_QUAD_DOM(table_id, seg, ops) \
> + { "HISI ", table_id, 0, seg + 0, MCFG_BUS_ANY, ops }, \
> + { "HISI ", table_id, 0, seg + 1, MCFG_BUS_ANY, ops }, \
> + { "HISI ", table_id, 0, seg + 2, MCFG_BUS_ANY, ops }, \
> + { "HISI ", table_id, 0, seg + 3, MCFG_BUS_ANY, ops }
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP05 ", 0, &hisi_pcie_ops),
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP06 ", 0, &hisi_pcie_ops),
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 0, &hisi_pcie_ops),
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 4, &hisi_pcie_ops),
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 8, &hisi_pcie_ops),
> + PCI_ACPI_QUIRK_QUAD_DOM("HIP07 ", 12, &hisi_pcie_ops),
> +#endif
> };
>
> static char mcfg_oem_id[ACPI_OEM_ID_SIZE];
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index ae98644..9ff2bcd 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -227,6 +227,14 @@ config PCI_HISI
> Say Y here if you want PCIe controller support on HiSilicon
> Hip05 and Hip06 and Hip07 SoCs
>
> +config PCI_HISI_ACPI
> + depends on ACPI && ARM64
> + bool "HiSilicon Hip05 and Hip06 and Hip07 SoCs ACPI PCIe controllers"
> + select PNP
> + help
> + Say Y here if you want ACPI PCIe controller support on HiSilicon
> + Hip05 and Hip06 and Hip07 SoCs
> +
> config PCIE_QCOM
> bool "Qualcomm PCIe controller"
> depends on ARCH_QCOM && OF
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 084cb49..9402858 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
> obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
> obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
> obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
> +obj-$(CONFIG_PCI_HISI_ACPI) += pcie-hisi-acpi.o
> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
> obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
> diff --git a/drivers/pci/host/pcie-hisi-acpi.c b/drivers/pci/host/pcie-hisi-acpi.c
> new file mode 100644
> index 0000000..aade4b5
> --- /dev/null
> +++ b/drivers/pci/host/pcie-hisi-acpi.c
> @@ -0,0 +1,157 @@
> +/*
> + * PCIe host controller driver for HiSilicon HipXX SoCs
> + *
> + * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
> + *
> + * Author: Dongdong Liu <liudongdong3@huawei.com>
> + * Gabriele Paoloni <gabriele.paoloni@huawei.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/pci.h>
> +#include <linux/pci-acpi.h>
> +#include <linux/pci-ecam.h>
> +
> +#define DEBUG0 0x728
> +#define PCIE_LTSSM_LINKUP_STATE 0x11
> +#define PCIE_LTSSM_STATE_MASK 0x3F
These are now unused.
> +static const struct acpi_device_id hisi_pcie_rc_res_ids[] = {
> + {"HISI0081", 0},
> + {"", 0},
> +};
> +
> +static int hisi_pcie_acpi_rd_conf(struct pci_bus *bus, u32 devfn, int where,
> + int size, u32 *val)
> +{
> + struct pci_config_window *cfg = bus->sysdata;
> + int dev = PCI_SLOT(devfn);
> +
> + if (bus->number == cfg->busr.start) {
> + /* access only one slot on each root port */
> + if (dev > 0)
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + else
> + return pci_generic_config_read32(bus, devfn, where,
> + size, val);
> + }
> +
> + return pci_generic_config_read(bus, devfn, where, size, val);
> +}
> +
> +static int hisi_pcie_acpi_wr_conf(struct pci_bus *bus, u32 devfn,
> + int where, int size, u32 val)
> +{
> + struct pci_config_window *cfg = bus->sysdata;
> + int dev = PCI_SLOT(devfn);
> +
> + if (bus->number == cfg->busr.start) {
> + /* access only one slot on each root port */
> + if (dev > 0)
> + return PCIBIOS_DEVICE_NOT_FOUND;
> + else
> + return pci_generic_config_write32(bus, devfn, where,
> + size, val);
> + }
> +
> + return pci_generic_config_write(bus, devfn, where, size, val);
> +}
> +
> +static void __iomem *hisi_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
> + int where)
> +{
> + struct pci_config_window *cfg = bus->sysdata;
> + void __iomem *reg_base = cfg->priv;
> +
> + if (bus->number == cfg->busr.start)
> + return reg_base + where;
> + else
> + return pci_ecam_map_bus(bus, devfn, where);
> +}
> +
> +/*
> + * Retrieve RC base and size from sub-device under the RC
> + * Device (RES1)
> + * {
> + * Name (_HID, "HISI0081")
> + * Name (_CID, "PNP0C02")
> + * Name (_CRS, ResourceTemplate (){
> + * Memory32Fixed (ReadWrite, 0xb0080000, 0x10000)
> + * })
> + * }
> + */
> +static int hisi_pcie_rc_addr_get(struct acpi_device *adev,
> + void __iomem **addr)
> +{
> + struct acpi_device *child_adev;
> + struct list_head list;
> + struct resource *res;
> + struct resource_entry *entry;
> + unsigned long flags;
> + int ret;
> +
> + list_for_each_entry(child_adev, &adev->children, node) {
> + ret = acpi_match_device_ids(child_adev, hisi_pcie_rc_res_ids);
> + if (ret)
> + continue;
> +
> + INIT_LIST_HEAD(&list);
> + flags = IORESOURCE_MEM;
> + ret = acpi_dev_get_resources(child_adev, &list,
> + acpi_dev_filter_resource_type_cb,
> + (void *)flags);
> + if (ret < 0) {
> + dev_err(&child_adev->dev,
> + "failed to parse _CRS method, error code %d\n",
> + ret);
> + return ret;
> + } else if (ret == 0) {
> + dev_err(&child_adev->dev,
> + "no IO and memory resources present in _CRS\n");
> + return -EINVAL;
> + }
> +
> + entry = list_first_entry(&list, struct resource_entry, node);
> + res = entry->res;
> + *addr = devm_ioremap(&child_adev->dev,
> + res->start, resource_size(res));
> + acpi_dev_free_resource_list(&list);
> + if (IS_ERR(*addr)) {
> + dev_err(&child_adev->dev, "error with ioremap\n");
> + return -ENOMEM;
> + }
> +
> + return 0;
> + }
> +
> + return -EINVAL;
> +}
OK, so MCFG (with the possible help of quirks) tells us where *most*
of this RC's ECAM space is, but it doesn't tell us where the root bus
ECAM space is, which is why we need this block of ugly code. Right?
Is there any way we can compute the root bus reg_base from the address
we got from MCFG? Is it a constant that realistically is not going to
be modified?
I would rather have the PNP0C02 device at the root of the ACPI
namespace (under \_SB) if possible, the way x86 systems do it.
I don't know whether that's actually a requirement, but the PCI
Firmware Spec does suggest it, and it's better to follow that existing
practice whenever possible.
I'm still looking for the specifics of how these resources are
described, i.e., a dmesg log, /proc/iomem, and maybe an ACPI dump.
These could go in a bugzilla entry, with a URL in this changelog.
> +
> +static int hisi_pcie_init(struct pci_config_window *cfg)
> +{
> + int ret;
> + struct acpi_device *adev = to_acpi_device(cfg->parent);
> + void __iomem *reg_base;
> +
> + ret = hisi_pcie_rc_addr_get(adev, ®_base);
> + if (ret) {
> + dev_err(&adev->dev, "can't get rc base address");
> + return ret;
> + }
> +
> + cfg->priv = reg_base;
> +
> + return 0;
> +}
> +
> +struct pci_ecam_ops hisi_pcie_ops = {
> + .bus_shift = 20,
> + .init = hisi_pcie_init,
> + .pci_ops = {
> + .map_bus = hisi_pcie_map_bus,
> + .read = hisi_pcie_acpi_rd_conf,
> + .write = hisi_pcie_acpi_wr_conf,
> + }
> +};
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index f5740b7..1b24d97 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -67,4 +67,9 @@ void __iomem *pci_ecam_map_bus(struct pci_bus *bus, unsigned int devfn,
> int pci_host_common_probe(struct platform_device *pdev,
> struct pci_ecam_ops *ops);
> #endif
> +
> +#ifdef CONFIG_PCI_HISI_ACPI
> +extern struct pci_ecam_ops hisi_pcie_ops;
> +#endif
> +
> #endif
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v2] f2fs: don't wait writeback for datas during checkpoint
From: Jaegeuk Kim @ 2016-11-14 23:32 UTC (permalink / raw)
To: Chao Yu; +Cc: chao, linux-kernel, linux-f2fs-devel
In-Reply-To: <20161114110412.110237-1-yuchao0@huawei.com>
Hi Chao,
On Mon, Nov 14, 2016 at 07:04:12PM +0800, Chao Yu wrote:
> Normally, while committing checkpoint, we will wait on all pages to be
> writebacked no matter the page is data or metadata, so in scenario where
> there are lots of data IO being submitted with metadata, we may suffer
> long latency for waiting writeback during checkpoint.
>
> Indeed, we only care about persistence for pages with metadata, but not
> pages with data, as file system consistent are only related to metadate,
> so in order to avoid encountering long latency in above scenario, let's
> recognize and reference metadata in submitted IOs, wait writeback only
> for metadatas.
Hmm, another concern comes, which is related to GCed data like below scenario.
1. Write data X
2. Sync
3. Move data X by GC
4. Checkpoint
5. Power-cut
In this case, we should guarantee data X which was migrated by GC during #3.
If we don't care about end_io in #4 Checkpoint, we can lose the data after
#5 Power-cut.
Any idea?
Thanks,
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/checkpoint.c | 2 +-
> fs/f2fs/data.c | 36 ++++++++++++++++++++++++++++++++----
> fs/f2fs/debug.c | 7 ++++---
> fs/f2fs/f2fs.h | 8 +++++---
> 4 files changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 7bece59..bdf8a50 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1003,7 +1003,7 @@ static void wait_on_all_pages_writeback(struct f2fs_sb_info *sbi)
> for (;;) {
> prepare_to_wait(&sbi->cp_wait, &wait, TASK_UNINTERRUPTIBLE);
>
> - if (!atomic_read(&sbi->nr_wb_bios))
> + if (!get_pages(sbi, F2FS_WB_META))
> break;
>
> io_schedule_timeout(5*HZ);
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 66d2aee..f52cec3 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -29,6 +29,26 @@
> #include "trace.h"
> #include <trace/events/f2fs.h>
>
> +static bool f2fs_is_meta_data(struct page *page)
> +{
> + struct address_space *mapping = page->mapping;
> + struct f2fs_sb_info *sbi;
> + struct inode *inode;
> +
> + /* it is bounce page of encrypted regular inode */
> + if (!mapping)
> + return false;
> +
> + inode = mapping->host;
> + sbi = F2FS_I_SB(inode);
> +
> + if ((inode->i_ino == F2FS_META_INO(sbi) &&
> + page->index < MAIN_BLKADDR(sbi)) ||
> + inode->i_ino == F2FS_NODE_INO(sbi) ||
> + S_ISDIR(inode->i_mode))
> + return true;
> + return false;
> +}
> static void f2fs_read_end_io(struct bio *bio)
> {
> struct bio_vec *bvec;
> @@ -73,6 +93,7 @@ static void f2fs_write_end_io(struct bio *bio)
>
> bio_for_each_segment_all(bvec, bio, i) {
> struct page *page = bvec->bv_page;
> + bool is_meta = f2fs_is_meta_data(page);
>
> fscrypt_pullback_bio_page(&page, true);
>
> @@ -80,9 +101,10 @@ static void f2fs_write_end_io(struct bio *bio)
> mapping_set_error(page->mapping, -EIO);
> f2fs_stop_checkpoint(sbi, true);
> }
> + dec_page_count(sbi, is_meta ? F2FS_WB_META : F2FS_WB_DATA);
> end_page_writeback(page);
> }
> - if (atomic_dec_and_test(&sbi->nr_wb_bios) &&
> + if (!get_pages(sbi, F2FS_WB_META) &&
> wq_has_sleeper(&sbi->cp_wait))
> wake_up(&sbi->cp_wait);
>
> @@ -111,7 +133,6 @@ static inline void __submit_bio(struct f2fs_sb_info *sbi,
> struct bio *bio, enum page_type type)
> {
> if (!is_read_io(bio_op(bio))) {
> - atomic_inc(&sbi->nr_wb_bios);
> if (f2fs_sb_mounted_blkzoned(sbi->sb) &&
> current->plug && (type == DATA || type == NODE))
> blk_finish_plug(current->plug);
> @@ -272,6 +293,15 @@ void f2fs_submit_page_mbio(struct f2fs_io_info *fio)
> verify_block_addr(sbi, fio->old_blkaddr);
> verify_block_addr(sbi, fio->new_blkaddr);
>
> + bio_page = fio->encrypted_page ? fio->encrypted_page : fio->page;
> +
> + if (!is_read) {
> + bool is_meta;
> +
> + is_meta = f2fs_is_meta_data(bio_page);
> + inc_page_count(sbi, is_meta ? F2FS_WB_META : F2FS_WB_DATA);
> + }
> +
> down_write(&io->io_rwsem);
>
> if (io->bio && (io->last_block_in_bio != fio->new_blkaddr - 1 ||
> @@ -284,8 +314,6 @@ void f2fs_submit_page_mbio(struct f2fs_io_info *fio)
> io->fio = *fio;
> }
>
> - bio_page = fio->encrypted_page ? fio->encrypted_page : fio->page;
> -
> if (bio_add_page(io->bio, bio_page, PAGE_SIZE, 0) <
> PAGE_SIZE) {
> __submit_merged_bio(io);
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index 2fdf233..f2d87de 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -50,7 +50,8 @@ static void update_general_status(struct f2fs_sb_info *sbi)
> si->ndirty_files = sbi->ndirty_inode[FILE_INODE];
> si->ndirty_all = sbi->ndirty_inode[DIRTY_META];
> si->inmem_pages = get_pages(sbi, F2FS_INMEM_PAGES);
> - si->wb_bios = atomic_read(&sbi->nr_wb_bios);
> + si->nr_wb_meta = get_pages(sbi, F2FS_WB_META);
> + si->nr_wb_data = get_pages(sbi, F2FS_WB_DATA);
> si->total_count = (int)sbi->user_block_count / sbi->blocks_per_seg;
> si->rsvd_segs = reserved_segments(sbi);
> si->overp_segs = overprovision_segments(sbi);
> @@ -313,8 +314,8 @@ static int stat_show(struct seq_file *s, void *v)
> seq_printf(s, " - Inner Struct Count: tree: %d(%d), node: %d\n",
> si->ext_tree, si->zombie_tree, si->ext_node);
> seq_puts(s, "\nBalancing F2FS Async:\n");
> - seq_printf(s, " - inmem: %4d, wb_bios: %4d\n",
> - si->inmem_pages, si->wb_bios);
> + seq_printf(s, " - inmem: %4d, wb_meta: %4d, wb_data: %4d\n",
> + si->inmem_pages, si->nr_wb_meta, si->nr_wb_data);
> seq_printf(s, " - nodes: %4d in %4d\n",
> si->ndirty_node, si->node_pages);
> seq_printf(s, " - dents: %4d in dirs:%4d (%4d)\n",
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 23a937f..4df301c 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -682,6 +682,8 @@ enum count_type {
> F2FS_DIRTY_META,
> F2FS_INMEM_PAGES,
> F2FS_DIRTY_IMETA,
> + F2FS_WB_META,
> + F2FS_WB_DATA,
> NR_COUNT_TYPE,
> };
>
> @@ -849,7 +851,6 @@ struct f2fs_sb_info {
> block_t discard_blks; /* discard command candidats */
> block_t last_valid_block_count; /* for recovery */
> u32 s_next_generation; /* for NFS support */
> - atomic_t nr_wb_bios; /* # of writeback bios */
>
> /* # of pages, see count_type */
> atomic_t nr_pages[NR_COUNT_TYPE];
> @@ -1263,7 +1264,8 @@ static inline void inc_page_count(struct f2fs_sb_info *sbi, int count_type)
> {
> atomic_inc(&sbi->nr_pages[count_type]);
>
> - if (count_type == F2FS_DIRTY_DATA || count_type == F2FS_INMEM_PAGES)
> + if (count_type == F2FS_DIRTY_DATA || count_type == F2FS_INMEM_PAGES ||
> + count_type == F2FS_WB_META || count_type == F2FS_WB_DATA)
> return;
>
> set_sbi_flag(sbi, SBI_IS_DIRTY);
> @@ -2219,7 +2221,7 @@ struct f2fs_stat_info {
> unsigned int ndirty_dirs, ndirty_files, ndirty_all;
> int nats, dirty_nats, sits, dirty_sits, free_nids, alloc_nids;
> int total_count, utilization;
> - int bg_gc, wb_bios;
> + int bg_gc, nr_wb_meta, nr_wb_data;
> int inline_xattr, inline_inode, inline_dir, orphans;
> unsigned int valid_count, valid_node_count, valid_inode_count, discard_blks;
> unsigned int bimodal, avg_vblocks;
> --
> 2.8.2.311.gee88674
------------------------------------------------------------------------------
^ permalink raw reply
* Re: [PATCH v2] f2fs: don't wait writeback for datas during checkpoint
From: Jaegeuk Kim @ 2016-11-14 23:32 UTC (permalink / raw)
To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao
In-Reply-To: <20161114110412.110237-1-yuchao0@huawei.com>
Hi Chao,
On Mon, Nov 14, 2016 at 07:04:12PM +0800, Chao Yu wrote:
> Normally, while committing checkpoint, we will wait on all pages to be
> writebacked no matter the page is data or metadata, so in scenario where
> there are lots of data IO being submitted with metadata, we may suffer
> long latency for waiting writeback during checkpoint.
>
> Indeed, we only care about persistence for pages with metadata, but not
> pages with data, as file system consistent are only related to metadate,
> so in order to avoid encountering long latency in above scenario, let's
> recognize and reference metadata in submitted IOs, wait writeback only
> for metadatas.
Hmm, another concern comes, which is related to GCed data like below scenario.
1. Write data X
2. Sync
3. Move data X by GC
4. Checkpoint
5. Power-cut
In this case, we should guarantee data X which was migrated by GC during #3.
If we don't care about end_io in #4 Checkpoint, we can lose the data after
#5 Power-cut.
Any idea?
Thanks,
>
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
> fs/f2fs/checkpoint.c | 2 +-
> fs/f2fs/data.c | 36 ++++++++++++++++++++++++++++++++----
> fs/f2fs/debug.c | 7 ++++---
> fs/f2fs/f2fs.h | 8 +++++---
> 4 files changed, 42 insertions(+), 11 deletions(-)
>
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 7bece59..bdf8a50 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -1003,7 +1003,7 @@ static void wait_on_all_pages_writeback(struct f2fs_sb_info *sbi)
> for (;;) {
> prepare_to_wait(&sbi->cp_wait, &wait, TASK_UNINTERRUPTIBLE);
>
> - if (!atomic_read(&sbi->nr_wb_bios))
> + if (!get_pages(sbi, F2FS_WB_META))
> break;
>
> io_schedule_timeout(5*HZ);
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 66d2aee..f52cec3 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -29,6 +29,26 @@
> #include "trace.h"
> #include <trace/events/f2fs.h>
>
> +static bool f2fs_is_meta_data(struct page *page)
> +{
> + struct address_space *mapping = page->mapping;
> + struct f2fs_sb_info *sbi;
> + struct inode *inode;
> +
> + /* it is bounce page of encrypted regular inode */
> + if (!mapping)
> + return false;
> +
> + inode = mapping->host;
> + sbi = F2FS_I_SB(inode);
> +
> + if ((inode->i_ino == F2FS_META_INO(sbi) &&
> + page->index < MAIN_BLKADDR(sbi)) ||
> + inode->i_ino == F2FS_NODE_INO(sbi) ||
> + S_ISDIR(inode->i_mode))
> + return true;
> + return false;
> +}
> static void f2fs_read_end_io(struct bio *bio)
> {
> struct bio_vec *bvec;
> @@ -73,6 +93,7 @@ static void f2fs_write_end_io(struct bio *bio)
>
> bio_for_each_segment_all(bvec, bio, i) {
> struct page *page = bvec->bv_page;
> + bool is_meta = f2fs_is_meta_data(page);
>
> fscrypt_pullback_bio_page(&page, true);
>
> @@ -80,9 +101,10 @@ static void f2fs_write_end_io(struct bio *bio)
> mapping_set_error(page->mapping, -EIO);
> f2fs_stop_checkpoint(sbi, true);
> }
> + dec_page_count(sbi, is_meta ? F2FS_WB_META : F2FS_WB_DATA);
> end_page_writeback(page);
> }
> - if (atomic_dec_and_test(&sbi->nr_wb_bios) &&
> + if (!get_pages(sbi, F2FS_WB_META) &&
> wq_has_sleeper(&sbi->cp_wait))
> wake_up(&sbi->cp_wait);
>
> @@ -111,7 +133,6 @@ static inline void __submit_bio(struct f2fs_sb_info *sbi,
> struct bio *bio, enum page_type type)
> {
> if (!is_read_io(bio_op(bio))) {
> - atomic_inc(&sbi->nr_wb_bios);
> if (f2fs_sb_mounted_blkzoned(sbi->sb) &&
> current->plug && (type == DATA || type == NODE))
> blk_finish_plug(current->plug);
> @@ -272,6 +293,15 @@ void f2fs_submit_page_mbio(struct f2fs_io_info *fio)
> verify_block_addr(sbi, fio->old_blkaddr);
> verify_block_addr(sbi, fio->new_blkaddr);
>
> + bio_page = fio->encrypted_page ? fio->encrypted_page : fio->page;
> +
> + if (!is_read) {
> + bool is_meta;
> +
> + is_meta = f2fs_is_meta_data(bio_page);
> + inc_page_count(sbi, is_meta ? F2FS_WB_META : F2FS_WB_DATA);
> + }
> +
> down_write(&io->io_rwsem);
>
> if (io->bio && (io->last_block_in_bio != fio->new_blkaddr - 1 ||
> @@ -284,8 +314,6 @@ void f2fs_submit_page_mbio(struct f2fs_io_info *fio)
> io->fio = *fio;
> }
>
> - bio_page = fio->encrypted_page ? fio->encrypted_page : fio->page;
> -
> if (bio_add_page(io->bio, bio_page, PAGE_SIZE, 0) <
> PAGE_SIZE) {
> __submit_merged_bio(io);
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index 2fdf233..f2d87de 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -50,7 +50,8 @@ static void update_general_status(struct f2fs_sb_info *sbi)
> si->ndirty_files = sbi->ndirty_inode[FILE_INODE];
> si->ndirty_all = sbi->ndirty_inode[DIRTY_META];
> si->inmem_pages = get_pages(sbi, F2FS_INMEM_PAGES);
> - si->wb_bios = atomic_read(&sbi->nr_wb_bios);
> + si->nr_wb_meta = get_pages(sbi, F2FS_WB_META);
> + si->nr_wb_data = get_pages(sbi, F2FS_WB_DATA);
> si->total_count = (int)sbi->user_block_count / sbi->blocks_per_seg;
> si->rsvd_segs = reserved_segments(sbi);
> si->overp_segs = overprovision_segments(sbi);
> @@ -313,8 +314,8 @@ static int stat_show(struct seq_file *s, void *v)
> seq_printf(s, " - Inner Struct Count: tree: %d(%d), node: %d\n",
> si->ext_tree, si->zombie_tree, si->ext_node);
> seq_puts(s, "\nBalancing F2FS Async:\n");
> - seq_printf(s, " - inmem: %4d, wb_bios: %4d\n",
> - si->inmem_pages, si->wb_bios);
> + seq_printf(s, " - inmem: %4d, wb_meta: %4d, wb_data: %4d\n",
> + si->inmem_pages, si->nr_wb_meta, si->nr_wb_data);
> seq_printf(s, " - nodes: %4d in %4d\n",
> si->ndirty_node, si->node_pages);
> seq_printf(s, " - dents: %4d in dirs:%4d (%4d)\n",
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 23a937f..4df301c 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -682,6 +682,8 @@ enum count_type {
> F2FS_DIRTY_META,
> F2FS_INMEM_PAGES,
> F2FS_DIRTY_IMETA,
> + F2FS_WB_META,
> + F2FS_WB_DATA,
> NR_COUNT_TYPE,
> };
>
> @@ -849,7 +851,6 @@ struct f2fs_sb_info {
> block_t discard_blks; /* discard command candidats */
> block_t last_valid_block_count; /* for recovery */
> u32 s_next_generation; /* for NFS support */
> - atomic_t nr_wb_bios; /* # of writeback bios */
>
> /* # of pages, see count_type */
> atomic_t nr_pages[NR_COUNT_TYPE];
> @@ -1263,7 +1264,8 @@ static inline void inc_page_count(struct f2fs_sb_info *sbi, int count_type)
> {
> atomic_inc(&sbi->nr_pages[count_type]);
>
> - if (count_type == F2FS_DIRTY_DATA || count_type == F2FS_INMEM_PAGES)
> + if (count_type == F2FS_DIRTY_DATA || count_type == F2FS_INMEM_PAGES ||
> + count_type == F2FS_WB_META || count_type == F2FS_WB_DATA)
> return;
>
> set_sbi_flag(sbi, SBI_IS_DIRTY);
> @@ -2219,7 +2221,7 @@ struct f2fs_stat_info {
> unsigned int ndirty_dirs, ndirty_files, ndirty_all;
> int nats, dirty_nats, sits, dirty_sits, free_nids, alloc_nids;
> int total_count, utilization;
> - int bg_gc, wb_bios;
> + int bg_gc, nr_wb_meta, nr_wb_data;
> int inline_xattr, inline_inode, inline_dir, orphans;
> unsigned int valid_count, valid_node_count, valid_inode_count, discard_blks;
> unsigned int bimodal, avg_vblocks;
> --
> 2.8.2.311.gee88674
^ permalink raw reply
* RE: [PATCH 1/3] PCI: hv: use the correct buffer size in new_pcichild_device()
From: KY Srinivasan @ 2016-11-14 22:58 UTC (permalink / raw)
To: Dexuan Cui, Bjorn Helgaas, linux-pci@vger.kernel.org,
devel@linuxdriverproject.org
Cc: gregkh@linuxfoundation.org, Haiyang Zhang, Stephen Hemminger,
Jake Oshins, Hadden Hoppert, Vitaly Kuznetsov,
jasowang@redhat.com, apw@canonical.com, olaf@aepfle.de,
linux-kernel@vger.kernel.org
In-Reply-To: <MWHPR03MB26695762E11D248A4181F43EBFB80@MWHPR03MB2669.namprd03.prod.outlook.com>
> -----Original Message-----
> From: Dexuan Cui
> Sent: Wednesday, November 9, 2016 11:18 PM
> To: Bjorn Helgaas <bhelgaas@google.com>; linux-pci@vger.kernel.org;
> devel@linuxdriverproject.org
> Cc: gregkh@linuxfoundation.org; KY Srinivasan <kys@microsoft.com>;
> Haiyang Zhang <haiyangz@microsoft.com>; Stephen Hemminger
> <sthemmin@microsoft.com>; Jake Oshins <jakeo@microsoft.com>; Hadden
> Hoppert <haddenh@microsoft.com>; Vitaly Kuznetsov
> <vkuznets@redhat.com>; jasowang@redhat.com; apw@canonical.com;
> olaf@aepfle.de; linux-kernel@vger.kernel.org
> Subject: [PATCH 1/3] PCI: hv: use the correct buffer size in
> new_pcichild_device()
>
> We don't really need such a big on-stack buffer.
> vmbus_sendpacket() here only uses sizeof(struct pci_child_message).
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> CC: Jake Oshins <jakeo@microsoft.com>
> Cc: KY Srinivasan <kys@microsoft.com>
> CC: Haiyang Zhang <haiyangz@microsoft.com>
> CC: Vitaly Kuznetsov <vkuznets@redhat.com>
Thanks Dexuan.
Acked-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/pci/host/pci-hyperv.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 763ff87..93ed64a 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1271,9 +1271,9 @@ static struct hv_pci_dev
> *new_pcichild_device(struct hv_pcibus_device *hbus,
> struct hv_pci_dev *hpdev;
> struct pci_child_message *res_req;
> struct q_res_req_compl comp_pkt;
> - union {
> - struct pci_packet init_packet;
> - u8 buffer[0x100];
> + struct {
> + struct pci_packet init_packet;
> + u8 buffer[sizeof(struct pci_child_message)];
> } pkt;
> unsigned long flags;
> int ret;
> --
> 2.7.4
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v2 0/4] ppc/pnv: XSCOM fixes and unit tests
From: David Gibson @ 2016-11-14 23:11 UTC (permalink / raw)
To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, Alexander Graf
In-Reply-To: <1479114778-3881-1-git-send-email-clg@kaod.org>
[-- Attachment #1: Type: text/plain, Size: 1394 bytes --]
On Mon, Nov 14, 2016 at 10:12:54AM +0100, Cédric Le Goater wrote:
> Hello,
>
> Here is a little serie adding some fixes for the XSCOM registers of
> the POWER9 cores and a unit test.
>
> Changes since v1 :
>
> - fixed pnv_xscom_addr() for 32bit host systems
> - replace hweight_long() by ctpop64()
>
> Tested on a 32-bit LE system and on 64-bit LE and BE systems.
Applied to ppc-for-2.8.
I fixed a trivial nit in 4/4 - you had a 5 space indent, instead of a
4 space indent in one place.
>
> Thanks,
>
> C.
>
> Cédric Le Goater (3):
> ppc/pnv: add a 'xscom_core_base' field to PnvChipClass
> ppc/pnv: fix xscom address translation for POWER9
> tests: add XSCOM tests for the PowerNV machine
>
> David Gibson (1):
> ppc/pnv: Fix fatal bug on 32-bit hosts
>
> hw/ppc/pnv.c | 10 +++-
> hw/ppc/pnv_xscom.c | 8 +--
> include/hw/ppc/pnv.h | 1 +
> include/hw/ppc/pnv_xscom.h | 5 +-
> tests/Makefile.include | 1 +
> tests/pnv-xscom-test.c | 140 +++++++++++++++++++++++++++++++++++++++++++++
> 6 files changed, 156 insertions(+), 9 deletions(-)
> create mode 100644 tests/pnv-xscom-test.c
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] [SCSI] mpt3sas: Fix secure erase premature termination (v4)
From: Martin K. Petersen @ 2016-11-14 23:30 UTC (permalink / raw)
To: Andrey Grodzovsky
Cc: Martin K. Petersen, PDL-MPT-FUSIONLINUX, Igor Rybak, Ezra Kohavi,
linux-scsi, Sathya Prakash, Chaitra P B, Suganath Prabu Subramani,
Sreekanth Reddy, Hannes Reinecke, stable
In-Reply-To: <CAJphD_rAW+fmJq3dK8-a-fpDOdMcLk2t0dUGVUrowXFQ=jA08A@mail.gmail.com>
>>>>> "Andrey" == Andrey Grodzovsky <andrey2805@gmail.com> writes:
Andrey,
Andrey> Regarding older code where there is still a separate mpt2sas
Andrey> driver, should a separate patch to be done or this fix will be
Andrey> ported there ?
Feel free to submit a mpt2sas patch to the pre-4.4 stable trees.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH V4] xen/arm: domain_build: allocate lowmem for dom0 as much as possible
From: Stefano Stabellini @ 2016-11-14 23:28 UTC (permalink / raw)
To: Andrii Anisov
Cc: Peng Fan, Stefano Stabellini, wei.liu2, xen-devel, Julien Grall,
Peng Fan
In-Reply-To: <CAC1Wxdh-ZZi+CzO6C=wOMxu2jFFH7dOky47JpoZx7L8z3COTbw@mail.gmail.com>
On Mon, 14 Nov 2016, Andrii Anisov wrote:
> > Could you define unacceptable performance drop? Have you tried to measure
> > what would be the impact?
>
> > I know it can be bad, depending on the class of protocols. I think that
> > if numbers were provided to demonstrate that bounce buffers (the swiotlb
> > in Linux) are too slow for a given use case
>
> Unfortunately I could not come up with exact requirements numbers.
> Introducing another memcpy (what bouncing buffer approach does) for
> block or network IO would not only reduce the operation performance
> but also increase the overall system load.
> All what we does for any of our PV driver solutions is avoiding data
> copying inside FE-BE pair in order to increase performance, reduce
> latency and system load.
I think it might be worth running those numbers: you might be surprised
by how well a simple data copy protocol can perform, even on ARM.
For example, take a look at PVCalls which is entirely based on data
copies:
http://marc.info/?l=xen-devel&m=147639616310487
I have already shown that it performs better than netfront/netback on
x86 in this blog post:
https://blog.xenproject.org/2016/08/30/pv-calls-a-new-paravirtualized-protocol-for-posix-syscalls/
I have just run the numbers on ARM64 (APM m400) and it is still much
faster than netfront/netback. This is what I get by running iperf -c in
a VM and iperf -s in Dom0:
PVCalls Netfront/Netback
-P 1 9.9 gbit/s 4.53 gbit/s
-P 2 17.4 gbit/s 5.57 gbit/s
-P 4 24.36 gbit/s 5.34 gbit/s
PVCalls is still significantly faster than Netfront/Netback.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply
* Re: [PATCH] scsi: megaraid_sas: add in missing white spaces in error messages text
From: Martin K. Petersen @ 2016-11-14 23:27 UTC (permalink / raw)
To: Colin King
Cc: Kashyap Desai, Sumit Saxena, Shivasharan S,
James E . J . Bottomley, Martin K . Petersen, megaraidlinux.pdl,
linux-scsi, linux-kernel
In-Reply-To: <20161112162524.4585-1-colin.king@canonical.com>
>>>>> "Colin" == Colin King <colin.king@canonical.com> writes:
Colin> A couple of dev_printk messages spans two lines and the literal
Colin> string is missing a white space between words. Add the white
Colin> space.
Applied to 4.10/scsi-queue.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH] scsi_transport_fc: Hold queue lock while calling blk_run_queue_async()
From: Martin K. Petersen @ 2016-11-14 23:26 UTC (permalink / raw)
To: Bart Van Assche
Cc: James Bottomley, Martin K. Petersen, Hannes Reinecke, James Smart,
linux-scsi@vger.kernel.org
In-Reply-To: <fb002f66-4648-9653-63b1-9c24a2c02666@sandisk.com>
>>>>> "Bart" == Bart Van Assche <bart.vanassche@sandisk.com> writes:
Bart> It is required to hold the queue lock when calling
Bart> blk_run_queue_async() to avoid that a race between
Bart> blk_run_queue_async() and blk_cleanup_queue() is
Bart> triggered. Additionally, remove the get_device() and put_device()
Bart> calls from fc_bsg_goose_queue. It is namely the responsibility of
Bart> the caller of fc_bsg_goose_queue() to ensure that the bsg queue
Bart> does not disappear while fc_bsg_goose_queue() is in progress.
Applied to 4.10/scsi-queue.
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v3] pcie_aer: Convert pcie_aer_init to Error
From: Michael S. Tsirkin @ 2016-11-14 23:26 UTC (permalink / raw)
To: Cao jin; +Cc: qemu-devel, marcel, dmitry, jasowang
In-Reply-To: <1478177676-2636-1-git-send-email-caoj.fnst@cn.fujitsu.com>
On Thu, Nov 03, 2016 at 08:54:36PM +0800, Cao jin wrote:
> When user specify invalid property aer_log_max, device should fail to
> create, and report appropriate message.
>
> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
I'll review and merge after release. Pls ping me then.
> ---
> v3 changelog:
> 1. get rid of PCIE_AER_LOG_MAX_UNSET
>
> hw/net/e1000e.c | 2 +-
> hw/pci-bridge/ioh3420.c | 3 ++-
> hw/pci-bridge/xio3130_downstream.c | 3 ++-
> hw/pci-bridge/xio3130_upstream.c | 3 ++-
> hw/pci/pcie_aer.c | 17 +++++++----------
> include/hw/pci/pcie_aer.h | 3 ++-
> 6 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index 4994e1c..89f96eb 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -472,7 +472,7 @@ static void e1000e_pci_realize(PCIDevice *pci_dev, Error **errp)
> hw_error("Failed to initialize PM capability");
> }
>
> - if (pcie_aer_init(pci_dev, e1000e_aer_offset, PCI_ERR_SIZEOF) < 0) {
> + if (pcie_aer_init(pci_dev, e1000e_aer_offset, PCI_ERR_SIZEOF, NULL) < 0) {
> hw_error("Failed to initialize AER capability");
> }
>
> diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
> index c8b5ac4..04180af 100644
> --- a/hw/pci-bridge/ioh3420.c
> +++ b/hw/pci-bridge/ioh3420.c
> @@ -135,8 +135,9 @@ static int ioh3420_initfn(PCIDevice *d)
> goto err_pcie_cap;
> }
>
> - rc = pcie_aer_init(d, IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
> + rc = pcie_aer_init(d, IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF, &err);
> if (rc < 0) {
> + error_report_err(err);
> goto err;
> }
> pcie_aer_root_init(d);
> diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
> index cef6e13..5713341 100644
> --- a/hw/pci-bridge/xio3130_downstream.c
> +++ b/hw/pci-bridge/xio3130_downstream.c
> @@ -97,8 +97,9 @@ static int xio3130_downstream_initfn(PCIDevice *d)
> goto err_pcie_cap;
> }
>
> - rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
> + rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF, &err);
> if (rc < 0) {
> + error_report_err(err);
> goto err;
> }
>
> diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
> index 4ad0440..94c1691 100644
> --- a/hw/pci-bridge/xio3130_upstream.c
> +++ b/hw/pci-bridge/xio3130_upstream.c
> @@ -85,8 +85,9 @@ static int xio3130_upstream_initfn(PCIDevice *d)
> pcie_cap_flr_init(d);
> pcie_cap_deverr_init(d);
>
> - rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
> + rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF, &err);
> if (rc < 0) {
> + error_report_err(err);
> goto err;
> }
>
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index 048ce6a..2a4bd5a 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -29,6 +29,7 @@
> #include "hw/pci/msi.h"
> #include "hw/pci/pci_bus.h"
> #include "hw/pci/pcie_regs.h"
> +#include "qapi/error.h"
>
> //#define DEBUG_PCIE
> #ifdef DEBUG_PCIE
> @@ -96,21 +97,17 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log)
> aer_log->log_num = 0;
> }
>
> -int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size)
> +int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size,
> + Error **errp)
> {
> - PCIExpressDevice *exp;
> -
> pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
> offset, size);
> - exp = &dev->exp;
> - exp->aer_cap = offset;
> + dev->exp.aer_cap = offset;
>
> - /* log_max is property */
> - if (dev->exp.aer_log.log_max == PCIE_AER_LOG_MAX_UNSET) {
> - dev->exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
> - }
> - /* clip down the value to avoid unreasobale memory usage */
> + /* clip down the value to avoid unreasonable memory usage */
> if (dev->exp.aer_log.log_max > PCIE_AER_LOG_MAX_LIMIT) {
> + error_setg(errp, "Invalid aer_log_max %d. The max number of aer log "
> + "is %d", dev->exp.aer_log.log_max, PCIE_AER_LOG_MAX_LIMIT);
> return -EINVAL;
> }
> dev->exp.aer_log.log = g_malloc0(sizeof dev->exp.aer_log.log[0] *
> diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
> index c2ee4e2..1cce61a 100644
> --- a/include/hw/pci/pcie_aer.h
> +++ b/include/hw/pci/pcie_aer.h
> @@ -87,7 +87,8 @@ struct PCIEAERErr {
>
> extern const VMStateDescription vmstate_pcie_aer_log;
>
> -int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size);
> +int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size,
> + Error **errp);
> void pcie_aer_exit(PCIDevice *dev);
> void pcie_aer_write_config(PCIDevice *dev,
> uint32_t addr, uint32_t val, int len);
> --
> 2.1.0
>
>
^ permalink raw reply
* Re: [Qemu-devel] [PATCH v12 10/22] vfio iommu type1: Add support for mediated devices
From: Alex Williamson @ 2016-11-14 23:25 UTC (permalink / raw)
To: Kirti Wankhede
Cc: pbonzini, kraxel, cjia, qemu-devel, kvm, kevin.tian, jike.song,
bjsdjshi, linux-kernel
In-Reply-To: <1479138156-28905-11-git-send-email-kwankhede@nvidia.com>
On Mon, 14 Nov 2016 21:12:24 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:
> VFIO IOMMU drivers are designed for the devices which are IOMMU capable.
> Mediated device only uses IOMMU APIs, the underlying hardware can be
> managed by an IOMMU domain.
>
> Aim of this change is:
> - To use most of the code of TYPE1 IOMMU driver for mediated devices
> - To support direct assigned device and mediated device in single module
>
> This change adds pin and unpin support for mediated device to TYPE1 IOMMU
> backend module. More details:
> - Domain for external user is tracked separately in vfio_iommu structure.
> It is allocated when group for first mdev device is attached.
> - Pages pinned for external domain are tracked in each vfio_dma structure
> for that iova range.
> - Page tracking rb-tree in vfio_dma keeps <iova, pfn, ref_count>. Key of
> rb-tree is iova, but it actually aims to track pfns.
> - On external pin request for an iova, page is pinned only once, if iova
> is already pinned and tracked, ref_count is incremented.
This is referring only to the external (ie. pfn_list) tracking only,
correct? In general, a page can be pinned up to twice per iova
referencing it, once for an iommu mapped domain and again in the
pfn_list, right?
> - External unin request unpins pages only when ref_count is 0.
^^^^ unpin
> - Pinned pages list is used to verify unpinning request and to unpin
> remaining pages while detaching the group for that device.
> - Page accounting is updated to account in its address space where the
> pages are pinned/unpinned, i.e dma->task
> - Accouting for mdev device is only done if there is no iommu capable
> domain in the container. When there is a direct device assigned to the
> container and that domain is iommu capable, all pages are already pinned
> during DMA_MAP.
> - Page accouting is updated on hot plug and unplug mdev device and pass
> through device.
>
> Tested by assigning below combinations of devices to a single VM:
> - GPU pass through only
> - vGPU device only
> - One GPU pass through and one vGPU device
> - Linux VM hot plug and unplug vGPU device while GPU pass through device
> exist
> - Linux VM hot plug and unplug GPU pass through device while vGPU device
> exist
>
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: I295d6f0f2e0579b8d9882bfd8fd5a4194b97bd9a
> ---
> drivers/vfio/vfio_iommu_type1.c | 587 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 526 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 50aca95cf61e..2697d874dd35 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -37,6 +37,7 @@
> #include <linux/vfio.h>
> #include <linux/workqueue.h>
> #include <linux/pid_namespace.h>
> +#include <linux/mdev.h>
>
> #define DRIVER_VERSION "0.2"
> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
> @@ -56,6 +57,7 @@ MODULE_PARM_DESC(disable_hugepages,
>
> struct vfio_iommu {
> struct list_head domain_list;
> + struct vfio_domain *external_domain; /* domain for external user */
> struct mutex lock;
> struct rb_root dma_list;
> bool v2;
> @@ -76,7 +78,9 @@ struct vfio_dma {
> unsigned long vaddr; /* Process virtual addr */
> size_t size; /* Map size (bytes) */
> int prot; /* IOMMU_READ/WRITE */
> + bool iommu_mapped;
> struct task_struct *task;
> + struct rb_root pfn_list; /* Ex-user pinned pfn list */
> };
>
> struct vfio_group {
> @@ -85,6 +89,21 @@ struct vfio_group {
> };
>
> /*
> + * Guest RAM pinning working set or DMA target
> + */
> +struct vfio_pfn {
> + struct rb_node node;
> + dma_addr_t iova; /* Device address */
> + unsigned long pfn; /* Host pfn */
> + atomic_t ref_count;
> +};
> +
> +#define IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu) \
> + (!list_empty(&iommu->domain_list))
> +
> +static int put_pfn(unsigned long pfn, int prot);
> +
> +/*
> * This code handles mapping and unmapping of user data buffers
> * into DMA'ble space using the IOMMU
> */
> @@ -132,6 +151,97 @@ static void vfio_unlink_dma(struct vfio_iommu *iommu, struct vfio_dma *old)
> rb_erase(&old->node, &iommu->dma_list);
> }
>
> +/*
> + * Helper Functions for host iova-pfn list
> + */
> +static struct vfio_pfn *vfio_find_vpfn(struct vfio_dma *dma, dma_addr_t iova)
> +{
> + struct vfio_pfn *vpfn;
> + struct rb_node *node = dma->pfn_list.rb_node;
> +
> + while (node) {
> + vpfn = rb_entry(node, struct vfio_pfn, node);
> +
> + if (iova < vpfn->iova)
> + node = node->rb_left;
> + else if (iova > vpfn->iova)
> + node = node->rb_right;
> + else
> + return vpfn;
> + }
> + return NULL;
> +}
> +
> +static void vfio_link_pfn(struct vfio_dma *dma,
> + struct vfio_pfn *new)
> +{
> + struct rb_node **link, *parent = NULL;
> + struct vfio_pfn *vpfn;
> +
> + link = &dma->pfn_list.rb_node;
> + while (*link) {
> + parent = *link;
> + vpfn = rb_entry(parent, struct vfio_pfn, node);
> +
> + if (new->iova < vpfn->iova)
> + link = &(*link)->rb_left;
> + else
> + link = &(*link)->rb_right;
> + }
> +
> + rb_link_node(&new->node, parent, link);
> + rb_insert_color(&new->node, &dma->pfn_list);
> +}
> +
> +static void vfio_unlink_pfn(struct vfio_dma *dma, struct vfio_pfn *old)
> +{
> + rb_erase(&old->node, &dma->pfn_list);
> +}
> +
> +static int vfio_add_to_pfn_list(struct vfio_dma *dma, dma_addr_t iova,
> + unsigned long pfn)
> +{
> + struct vfio_pfn *vpfn;
> +
> + vpfn = kzalloc(sizeof(*vpfn), GFP_KERNEL);
> + if (!vpfn)
> + return -ENOMEM;
> +
> + vpfn->iova = iova;
> + vpfn->pfn = pfn;
> + atomic_set(&vpfn->ref_count, 1);
> + vfio_link_pfn(dma, vpfn);
> + return 0;
> +}
> +
> +static void vfio_remove_from_pfn_list(struct vfio_dma *dma,
> + struct vfio_pfn *vpfn)
> +{
> + vfio_unlink_pfn(dma, vpfn);
> + kfree(vpfn);
> +}
> +
> +static struct vfio_pfn *vfio_iova_get_vfio_pfn(struct vfio_dma *dma,
> + unsigned long iova)
> +{
> + struct vfio_pfn *vpfn = vfio_find_vpfn(dma, iova);
> +
> + if (vpfn)
> + atomic_inc(&vpfn->ref_count);
> + return vpfn;
> +}
> +
> +static int vfio_iova_put_vfio_pfn(struct vfio_dma *dma, struct vfio_pfn *vpfn)
> +{
> + int ret = 0;
> +
> + if (atomic_dec_and_test(&vpfn->ref_count)) {
> + ret = put_pfn(vpfn->pfn, dma->prot);
> + vfio_remove_from_pfn_list(dma, vpfn);
> + }
> + return ret;
> +}
> +
> struct vwork {
> struct mm_struct *mm;
> long npage;
> @@ -270,7 +380,6 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
> }
>
> up_read(&mm->mmap_sem);
> -
> return ret;
> }
>
> @@ -280,28 +389,35 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
> * first page and all consecutive pages with the same locking.
> */
> static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
> - long npage, int prot, unsigned long *pfn_base)
> + long npage, unsigned long *pfn_base)
> {
> unsigned long limit;
> bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
> CAP_IPC_LOCK);
> struct mm_struct *mm;
> - long ret, i;
> + long ret, i, lock_acct = 0;
> bool rsvd;
> + struct vfio_pfn *vpfn;
> + dma_addr_t iova;
>
> mm = get_task_mm(dma->task);
> if (!mm)
> return -ENODEV;
>
> - ret = vaddr_get_pfn(mm, vaddr, prot, pfn_base);
> + ret = vaddr_get_pfn(mm, vaddr, dma->prot, pfn_base);
> if (ret)
> goto pin_pg_remote_exit;
>
> + iova = vaddr - dma->vaddr + dma->iova;
> + vpfn = vfio_find_vpfn(dma, iova);
> + if (!vpfn)
> + lock_acct = 1;
> +
> rsvd = is_invalid_reserved_pfn(*pfn_base);
> limit = task_rlimit(dma->task, RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>
> - if (!rsvd && !lock_cap && mm->locked_vm + 1 > limit) {
> - put_pfn(*pfn_base, prot);
> + if (!rsvd && !lock_cap && mm->locked_vm + lock_acct > limit) {
> + put_pfn(*pfn_base, dma->prot);
> pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n", __func__,
> limit << PAGE_SHIFT);
> ret = -ENOMEM;
> @@ -310,7 +426,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
>
> if (unlikely(disable_hugepages)) {
> if (!rsvd)
> - vfio_lock_acct(dma->task, 1);
> + vfio_lock_acct(dma->task, lock_acct);
> ret = 1;
> goto pin_pg_remote_exit;
> }
> @@ -319,18 +435,24 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
> for (i = 1, vaddr += PAGE_SIZE; i < npage; i++, vaddr += PAGE_SIZE) {
> unsigned long pfn = 0;
>
> - ret = vaddr_get_pfn(mm, vaddr, prot, &pfn);
> + ret = vaddr_get_pfn(mm, vaddr, dma->prot, &pfn);
> if (ret)
> break;
>
> if (pfn != *pfn_base + i ||
> rsvd != is_invalid_reserved_pfn(pfn)) {
> - put_pfn(pfn, prot);
> + put_pfn(pfn, dma->prot);
> break;
> }
>
> - if (!rsvd && !lock_cap && mm->locked_vm + i + 1 > limit) {
> - put_pfn(pfn, prot);
> + iova = vaddr - dma->vaddr + dma->iova;
nit, this could be incremented in the for loop just like vaddr, we
don't need to create it from scratch (iova += PAGE_SIZE).
> + vpfn = vfio_find_vpfn(dma, iova);
> + if (!vpfn)
> + lock_acct++;
> +
> + if (!rsvd && !lock_cap &&
> + mm->locked_vm + lock_acct + 1 > limit) {
lock_acct was incremented just above, why is this lock_acct + 1? I
think we're off by one here (ie. remove the +1)?
> + put_pfn(pfn, dma->prot);
> pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
> __func__, limit << PAGE_SHIFT);
> break;
> @@ -338,7 +460,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
> }
>
> if (!rsvd)
> - vfio_lock_acct(dma->task, i);
> + vfio_lock_acct(dma->task, lock_acct);
> ret = i;
>
> pin_pg_remote_exit:
> @@ -346,14 +468,78 @@ pin_pg_remote_exit:
> return ret;
> }
>
> -static long vfio_unpin_pages_remote(struct vfio_dma *dma, unsigned long pfn,
> - long npage, int prot, bool do_accounting)
> +static long vfio_unpin_pages_remote(struct vfio_dma *dma, dma_addr_t iova,
> + unsigned long pfn, long npage,
> + bool do_accounting)
> {
> - unsigned long unlocked = 0;
> + long unlocked = 0, locked = 0;
> long i;
>
> - for (i = 0; i < npage; i++)
> - unlocked += put_pfn(pfn++, prot);
> + for (i = 0; i < npage; i++) {
> + struct vfio_pfn *vpfn;
> +
> + unlocked += put_pfn(pfn++, dma->prot);
> +
> + vpfn = vfio_find_vpfn(dma, iova + (i << PAGE_SHIFT));
> + if (vpfn)
> + locked++;
This isn't taking into account reserved pages (ex. device mmaps). In
the pinning path above we skip accounting of reserved pages, put_pfn
also only increments for non-reserved pages, but vfio_find_vpfn()
doesn't care, so it's possible that (locked - unlocked) could result in
a positive value. Maybe something like:
if (put_pfn(...)) {
unlocked++;
if (vfio_find_vpfn(...))
locked++;
}
> + }
> +
> + if (do_accounting)
> + vfio_lock_acct(dma->task, locked - unlocked);
> +
> + return unlocked;
> +}
> +
> +static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
> + unsigned long *pfn_base, bool do_accounting)
> +{
> + unsigned long limit;
> + bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
> + CAP_IPC_LOCK);
> + struct mm_struct *mm;
> + int ret;
> + bool rsvd;
> +
> + mm = get_task_mm(dma->task);
> + if (!mm)
> + return -ENODEV;
> +
> + ret = vaddr_get_pfn(mm, vaddr, dma->prot, pfn_base);
> + if (ret)
> + goto pin_page_exit;
> +
> + rsvd = is_invalid_reserved_pfn(*pfn_base);
> + limit = task_rlimit(dma->task, RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> +
> + if (!rsvd && !lock_cap && mm->locked_vm + 1 > limit) {
> + put_pfn(*pfn_base, dma->prot);
> + pr_warn("%s: Task %s (%d) RLIMIT_MEMLOCK (%ld) exceeded\n",
> + __func__, dma->task->comm, task_pid_nr(dma->task),
> + limit << PAGE_SHIFT);
> + ret = -ENOMEM;
> + goto pin_page_exit;
> + }
> +
> + if (!rsvd && do_accounting)
> + vfio_lock_acct(dma->task, 1);
> + ret = 1;
> +
> +pin_page_exit:
> + mmput(mm);
> + return ret;
> +}
> +
> +static int vfio_unpin_page_external(struct vfio_dma *dma, dma_addr_t iova,
> + bool do_accounting)
> +{
> + int unlocked;
> + struct vfio_pfn *vpfn = vfio_find_vpfn(dma, iova);
> +
> + if (!vpfn)
> + return 0;
> +
> + unlocked = vfio_iova_put_vfio_pfn(dma, vpfn);
>
> if (do_accounting)
> vfio_lock_acct(dma->task, -unlocked);
> @@ -361,14 +547,148 @@ static long vfio_unpin_pages_remote(struct vfio_dma *dma, unsigned long pfn,
> return unlocked;
> }
>
> -static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
> +static int vfio_iommu_type1_pin_pages(void *iommu_data,
> + unsigned long *user_pfn,
> + int npage, int prot,
> + unsigned long *phys_pfn)
> +{
> + struct vfio_iommu *iommu = iommu_data;
> + int i, j, ret;
> + unsigned long remote_vaddr;
> + unsigned long *pfn = phys_pfn;
nit, why do we have this variable rather than using phys_pfn directly?
Maybe before we had unwind support we incremented this rather than
indexing it?
> + struct vfio_dma *dma;
> + bool do_accounting;
> +
> + if (!iommu || !user_pfn || !phys_pfn)
> + return -EINVAL;
> +
> + /* Supported for v2 version only */
> + if (!iommu->v2)
> + return -EACCES;
> +
> + mutex_lock(&iommu->lock);
> +
> + if (!iommu->external_domain) {
> + ret = -EINVAL;
> + goto pin_done;
> + }
> +
> + /*
> + * If iommu capable domain exist in the container then all pages are
> + * already pinned and accounted. Accouting should be done if there is no
> + * iommu capable domain in the container.
> + */
> + do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu);
> +
> + for (i = 0; i < npage; i++) {
> + dma_addr_t iova;
> + struct vfio_pfn *vpfn;
> +
> + iova = user_pfn[i] << PAGE_SHIFT;
> + dma = vfio_find_dma(iommu, iova, 0);
> + if (!dma) {
> + ret = -EINVAL;
> + goto pin_unwind;
> + }
> +
> + if ((dma->prot & prot) != prot) {
> + pr_info("%s: dma->prot: 0x%x prot: 0x%x\n",
> + __func__, dma->prot, prot);
> + ret = -EPERM;
> + goto pin_unwind;
> + }
> +
> + vpfn = vfio_iova_get_vfio_pfn(dma, iova);
> + if (vpfn) {
> + pfn[i] = vpfn->pfn;
> + continue;
> + }
> +
> + remote_vaddr = dma->vaddr + iova - dma->iova;
> + ret = vfio_pin_page_external(dma, remote_vaddr, &pfn[i],
> + do_accounting);
> + if (ret <= 0) {
> + WARN_ON(!ret);
> + goto pin_unwind;
> + }
> +
> + ret = vfio_add_to_pfn_list(dma, iova, pfn[i]);
> + if (ret) {
> + vfio_unpin_page_external(dma, iova, do_accounting);
> + goto pin_unwind;
> + }
> + }
> +
> + ret = i;
> + goto pin_done;
> +
> +pin_unwind:
> + pfn[i] = 0;
> + for (j = 0; j < i; j++) {
> + dma_addr_t iova;
> +
> + iova = user_pfn[j] << PAGE_SHIFT;
> + dma = vfio_find_dma(iommu, iova, 0);
> + vfio_unpin_page_external(dma, iova, do_accounting);
> + pfn[j] = 0;
> + }
> +pin_done:
> + mutex_unlock(&iommu->lock);
> + return ret;
> +}
> +
> +static int vfio_iommu_type1_unpin_pages(void *iommu_data,
> + unsigned long *user_pfn,
> + int npage)
> +{
> + struct vfio_iommu *iommu = iommu_data;
> + bool do_accounting;
> + int unlocked = 0, i;
> +
> + if (!iommu || !user_pfn)
> + return -EINVAL;
> +
> + /* Supported for v2 version only */
> + if (!iommu->v2)
> + return -EACCES;
> +
> + mutex_lock(&iommu->lock);
> +
> + if (!iommu->external_domain) {
> + mutex_unlock(&iommu->lock);
> + return -EINVAL;
> + }
> +
> + do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu);
> + for (i = 0; i < npage; i++) {
> + struct vfio_dma *dma;
> + dma_addr_t iova;
> +
> + iova = user_pfn[i] << PAGE_SHIFT;
> + dma = vfio_find_dma(iommu, iova, 0);
> + if (!dma)
> + goto unpin_exit;
> + unlocked += vfio_unpin_page_external(dma, iova, do_accounting);
> + }
> +
> +unpin_exit:
> + mutex_unlock(&iommu->lock);
> + return unlocked;
This is not returning what it's supposed to. unlocked is going to be
less than or equal to the number of pages unpinned. We don't need to
track unlocked, I think we're just tracking where we are in the unpin
array, whether it was partial or complete. I think we want:
return i > npage ? npage : i;
Or maybe we can make it more obvious if there's an error on the first
unpin entry:
return i > npage ? npage : (i > 0 ? i : -EINVAL);
> +}
> +
> +static long vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma,
> + bool do_accounting)
> {
> dma_addr_t iova = dma->iova, end = dma->iova + dma->size;
> struct vfio_domain *domain, *d;
> long unlocked = 0;
>
> if (!dma->size)
> - return;
> + return 0;
> +
> + if (!IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu))
> + return 0;
> +
> /*
> * We use the IOMMU to track the physical addresses, otherwise we'd
> * need a much more complicated tracking system. Unfortunately that
> @@ -410,20 +730,26 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
> if (WARN_ON(!unmapped))
> break;
>
> - unlocked += vfio_unpin_pages_remote(dma, phys >> PAGE_SHIFT,
> + unlocked += vfio_unpin_pages_remote(dma, iova,
> + phys >> PAGE_SHIFT,
> unmapped >> PAGE_SHIFT,
> - dma->prot, false);
> + false);
> iova += unmapped;
>
> cond_resched();
> }
>
> - vfio_lock_acct(dma->task, -unlocked);
> + dma->iommu_mapped = false;
> + if (do_accounting) {
> + vfio_lock_acct(dma->task, -unlocked);
> + return 0;
> + }
> + return unlocked;
> }
>
> static void vfio_remove_dma(struct vfio_iommu *iommu, struct vfio_dma *dma)
> {
> - vfio_unmap_unpin(iommu, dma);
> + vfio_unmap_unpin(iommu, dma, true);
> vfio_unlink_dma(iommu, dma);
> put_task_struct(dma->task);
> kfree(dma);
> @@ -606,8 +932,7 @@ static int vfio_pin_map_dma(struct vfio_iommu *iommu, struct vfio_dma *dma,
> while (size) {
> /* Pin a contiguous chunk of memory */
> npage = vfio_pin_pages_remote(dma, vaddr + dma->size,
> - size >> PAGE_SHIFT, dma->prot,
> - &pfn);
> + size >> PAGE_SHIFT, &pfn);
> if (npage <= 0) {
> WARN_ON(!npage);
> ret = (int)npage;
> @@ -618,8 +943,8 @@ static int vfio_pin_map_dma(struct vfio_iommu *iommu, struct vfio_dma *dma,
> ret = vfio_iommu_map(iommu, iova + dma->size, pfn, npage,
> dma->prot);
> if (ret) {
> - vfio_unpin_pages_remote(dma, pfn, npage,
> - dma->prot, true);
> + vfio_unpin_pages_remote(dma, iova + dma->size, pfn,
> + npage, true);
> break;
> }
>
> @@ -627,6 +952,8 @@ static int vfio_pin_map_dma(struct vfio_iommu *iommu, struct vfio_dma *dma,
> dma->size += npage << PAGE_SHIFT;
> }
>
> + dma->iommu_mapped = true;
> +
> if (ret)
> vfio_remove_dma(iommu, dma);
>
> @@ -682,11 +1009,16 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
> dma->prot = prot;
> get_task_struct(current);
> dma->task = current;
> + dma->pfn_list = RB_ROOT;
>
> /* Insert zero-sized and grow as we map chunks of it */
> vfio_link_dma(iommu, dma);
>
> - ret = vfio_pin_map_dma(iommu, dma, size);
> + /* Don't pin and map if container doesn't contain IOMMU capable domain*/
> + if (!IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu))
> + dma->size = size;
> + else
> + ret = vfio_pin_map_dma(iommu, dma, size);
> do_map_err:
> mutex_unlock(&iommu->lock);
> return ret;
> @@ -715,10 +1047,6 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
> d = list_first_entry(&iommu->domain_list, struct vfio_domain, next);
> n = rb_first(&iommu->dma_list);
>
> - /* If there's not a domain, there better not be any mappings */
> - if (WARN_ON(n && !d))
> - return -EINVAL;
> -
> for (; n; n = rb_next(n)) {
> struct vfio_dma *dma;
> dma_addr_t iova;
> @@ -727,21 +1055,49 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
> iova = dma->iova;
>
> while (iova < dma->iova + dma->size) {
> - phys_addr_t phys = iommu_iova_to_phys(d->domain, iova);
> + phys_addr_t phys;
> size_t size;
>
> - if (WARN_ON(!phys)) {
> - iova += PAGE_SIZE;
> - continue;
> + if (dma->iommu_mapped) {
> + phys_addr_t p;
> + dma_addr_t i;
> +
> + phys = iommu_iova_to_phys(d->domain, iova);
> +
> + if (WARN_ON(!phys)) {
> + iova += PAGE_SIZE;
> + continue;
> + }
> +
> + size = PAGE_SIZE;
> + p = phys + size;
> + i = iova + size;
> + while (i < dma->iova + dma->size &&
> + p == iommu_iova_to_phys(d->domain, i)) {
> + size += PAGE_SIZE;
> + p += PAGE_SIZE;
> + i += PAGE_SIZE;
> + }
> + } else {
> + unsigned long pfn;
> + unsigned long vaddr = dma->vaddr +
> + (iova - dma->iova);
> + size_t n = dma->iova + dma->size - iova;
> + long npage;
> +
> + npage = vfio_pin_pages_remote(dma, vaddr,
> + n >> PAGE_SHIFT,
> + &pfn);
> + if (npage <= 0) {
> + WARN_ON(!npage);
> + ret = (int)npage;
> + return ret;
> + }
> +
> + phys = pfn << PAGE_SHIFT;
> + size = npage << PAGE_SHIFT;
> }
>
> - size = PAGE_SIZE;
> -
> - while (iova + size < dma->iova + dma->size &&
> - phys + size == iommu_iova_to_phys(d->domain,
> - iova + size))
> - size += PAGE_SIZE;
> -
> ret = iommu_map(domain->domain, iova, phys,
> size, dma->prot | domain->prot);
> if (ret)
> @@ -749,8 +1105,8 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
>
> iova += size;
> }
> + dma->iommu_mapped = true;
> }
> -
> return 0;
> }
>
> @@ -806,7 +1162,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> struct vfio_iommu *iommu = iommu_data;
> struct vfio_group *group;
> struct vfio_domain *domain, *d;
> - struct bus_type *bus = NULL;
> + struct bus_type *bus = NULL, *mdev_bus;
> int ret;
>
> mutex_lock(&iommu->lock);
> @@ -818,6 +1174,13 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> }
> }
>
> + if (iommu->external_domain) {
> + if (find_iommu_group(iommu->external_domain, iommu_group)) {
> + mutex_unlock(&iommu->lock);
> + return -EINVAL;
> + }
> + }
> +
> group = kzalloc(sizeof(*group), GFP_KERNEL);
> domain = kzalloc(sizeof(*domain), GFP_KERNEL);
> if (!group || !domain) {
> @@ -832,6 +1195,25 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> if (ret)
> goto out_free;
>
> + mdev_bus = symbol_get(mdev_bus_type);
> +
> + if (mdev_bus) {
> + if ((bus == mdev_bus) && !iommu_present(bus)) {
> + symbol_put(mdev_bus_type);
> + if (!iommu->external_domain) {
> + INIT_LIST_HEAD(&domain->group_list);
> + iommu->external_domain = domain;
> + } else
> + kfree(domain);
> +
> + list_add(&group->next,
> + &iommu->external_domain->group_list);
> + mutex_unlock(&iommu->lock);
> + return 0;
> + }
> + symbol_put(mdev_bus_type);
> + }
> +
> domain->domain = iommu_domain_alloc(bus);
> if (!domain->domain) {
> ret = -EIO;
> @@ -922,6 +1304,46 @@ static void vfio_iommu_unmap_unpin_all(struct vfio_iommu *iommu)
> vfio_remove_dma(iommu, rb_entry(node, struct vfio_dma, node));
> }
>
> +static void vfio_iommu_unmap_unpin_reaccount(struct vfio_iommu *iommu)
> +{
> + struct rb_node *n, *p;
> +
> + n = rb_first(&iommu->dma_list);
> + for (; n; n = rb_next(n)) {
> + struct vfio_dma *dma;
> + long locked = 0, unlocked = 0;
> +
> + dma = rb_entry(n, struct vfio_dma, node);
> + unlocked += vfio_unmap_unpin(iommu, dma, false);
> + p = rb_first(&dma->pfn_list);
> + for (; p; p = rb_next(p))
> + locked++;
We don't know that these weren't reserved pages. If the vendor driver
was pinning peer-to-peer ranges vfio_unmap_unpin() might have returned
0 yet we're assuming each is locked RAM, so our accounting can go
upside down.
> + vfio_lock_acct(dma->task, locked - unlocked);
> + }
> +}
> +
> +static void vfio_external_unpin_all(struct vfio_iommu *iommu,
> + bool do_accounting)
> +{
> + struct rb_node *n, *p;
> +
> + n = rb_first(&iommu->dma_list);
> + for (; n; n = rb_next(n)) {
> + struct vfio_dma *dma;
> +
> + dma = rb_entry(n, struct vfio_dma, node);
> + while ((p = rb_first(&dma->pfn_list))) {
> + int unlocked;
> + struct vfio_pfn *vpfn = rb_entry(p, struct vfio_pfn,
> + node);
> +
> + unlocked = vfio_iova_put_vfio_pfn(dma, vpfn);
> + if (do_accounting)
> + vfio_lock_acct(dma->task, -unlocked);
nit, we could batch these further, only updating accounting once per
vfio_dma, or once entirely.
> + }
> + }
> +}
> +
> static void vfio_iommu_type1_detach_group(void *iommu_data,
> struct iommu_group *iommu_group)
> {
> @@ -931,6 +1353,26 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
>
> mutex_lock(&iommu->lock);
>
> + if (iommu->external_domain) {
> + domain = iommu->external_domain;
> + group = find_iommu_group(domain, iommu_group);
> + if (group) {
> + list_del(&group->next);
> + kfree(group);
> +
> + if (list_empty(&domain->group_list)) {
> + if (!IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu)) {
> + vfio_external_unpin_all(iommu, true);
> + vfio_iommu_unmap_unpin_all(iommu);
> + } else
> + vfio_external_unpin_all(iommu, false);
> + kfree(domain);
> + iommu->external_domain = NULL;
> + }
> + goto detach_group_done;
> + }
> + }
> +
> list_for_each_entry(domain, &iommu->domain_list, next) {
> group = find_iommu_group(domain, iommu_group);
> if (!group)
> @@ -940,21 +1382,27 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
> list_del(&group->next);
> kfree(group);
> /*
> - * Group ownership provides privilege, if the group
> - * list is empty, the domain goes away. If it's the
> - * last domain, then all the mappings go away too.
> + * Group ownership provides privilege, if the group list is
> + * empty, the domain goes away. If it's the last domain with
> + * iommu and external domain doesn't exist, then all the
> + * mappings go away too. If it's the last domain with iommu and
> + * external domain exist, update accounting
> */
> if (list_empty(&domain->group_list)) {
> - if (list_is_singular(&iommu->domain_list))
> - vfio_iommu_unmap_unpin_all(iommu);
> + if (list_is_singular(&iommu->domain_list)) {
> + if (!iommu->external_domain)
> + vfio_iommu_unmap_unpin_all(iommu);
> + else
> + vfio_iommu_unmap_unpin_reaccount(iommu);
> + }
> iommu_domain_free(domain->domain);
> list_del(&domain->next);
> kfree(domain);
> }
> - goto done;
> + break;
> }
>
> -done:
> +detach_group_done:
> mutex_unlock(&iommu->lock);
> }
>
> @@ -986,27 +1434,42 @@ static void *vfio_iommu_type1_open(unsigned long arg)
> return iommu;
> }
>
> +static void vfio_release_domain(struct vfio_domain *domain, bool external)
> +{
> + struct vfio_group *group, *group_tmp;
> +
> + list_for_each_entry_safe(group, group_tmp,
> + &domain->group_list, next) {
> + if (!external)
> + iommu_detach_group(domain->domain, group->iommu_group);
> + list_del(&group->next);
> + kfree(group);
> + }
> +
> + if (!external)
> + iommu_domain_free(domain->domain);
> +}
> +
> static void vfio_iommu_type1_release(void *iommu_data)
> {
> struct vfio_iommu *iommu = iommu_data;
> struct vfio_domain *domain, *domain_tmp;
> - struct vfio_group *group, *group_tmp;
> +
> + if (iommu->external_domain) {
> + vfio_release_domain(iommu->external_domain, true);
> + vfio_external_unpin_all(iommu, false);
> + kfree(iommu->external_domain);
> + iommu->external_domain = NULL;
> + }
>
> vfio_iommu_unmap_unpin_all(iommu);
>
> list_for_each_entry_safe(domain, domain_tmp,
> &iommu->domain_list, next) {
> - list_for_each_entry_safe(group, group_tmp,
> - &domain->group_list, next) {
> - iommu_detach_group(domain->domain, group->iommu_group);
> - list_del(&group->next);
> - kfree(group);
> - }
> - iommu_domain_free(domain->domain);
> + vfio_release_domain(domain, false);
> list_del(&domain->next);
> kfree(domain);
> }
> -
> kfree(iommu);
> }
>
> @@ -1110,6 +1573,8 @@ static const struct vfio_iommu_driver_ops vfio_iommu_driver_ops_type1 = {
> .ioctl = vfio_iommu_type1_ioctl,
> .attach_group = vfio_iommu_type1_attach_group,
> .detach_group = vfio_iommu_type1_detach_group,
> + .pin_pages = vfio_iommu_type1_pin_pages,
> + .unpin_pages = vfio_iommu_type1_unpin_pages,
> };
>
> static int __init vfio_iommu_type1_init(void)
^ permalink raw reply
* Re: [PATCH v12 10/22] vfio iommu type1: Add support for mediated devices
From: Alex Williamson @ 2016-11-14 23:25 UTC (permalink / raw)
To: Kirti Wankhede
Cc: pbonzini, kraxel, cjia, qemu-devel, kvm, kevin.tian, jike.song,
bjsdjshi, linux-kernel
In-Reply-To: <1479138156-28905-11-git-send-email-kwankhede@nvidia.com>
On Mon, 14 Nov 2016 21:12:24 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:
> VFIO IOMMU drivers are designed for the devices which are IOMMU capable.
> Mediated device only uses IOMMU APIs, the underlying hardware can be
> managed by an IOMMU domain.
>
> Aim of this change is:
> - To use most of the code of TYPE1 IOMMU driver for mediated devices
> - To support direct assigned device and mediated device in single module
>
> This change adds pin and unpin support for mediated device to TYPE1 IOMMU
> backend module. More details:
> - Domain for external user is tracked separately in vfio_iommu structure.
> It is allocated when group for first mdev device is attached.
> - Pages pinned for external domain are tracked in each vfio_dma structure
> for that iova range.
> - Page tracking rb-tree in vfio_dma keeps <iova, pfn, ref_count>. Key of
> rb-tree is iova, but it actually aims to track pfns.
> - On external pin request for an iova, page is pinned only once, if iova
> is already pinned and tracked, ref_count is incremented.
This is referring only to the external (ie. pfn_list) tracking only,
correct? In general, a page can be pinned up to twice per iova
referencing it, once for an iommu mapped domain and again in the
pfn_list, right?
> - External unin request unpins pages only when ref_count is 0.
^^^^ unpin
> - Pinned pages list is used to verify unpinning request and to unpin
> remaining pages while detaching the group for that device.
> - Page accounting is updated to account in its address space where the
> pages are pinned/unpinned, i.e dma->task
> - Accouting for mdev device is only done if there is no iommu capable
> domain in the container. When there is a direct device assigned to the
> container and that domain is iommu capable, all pages are already pinned
> during DMA_MAP.
> - Page accouting is updated on hot plug and unplug mdev device and pass
> through device.
>
> Tested by assigning below combinations of devices to a single VM:
> - GPU pass through only
> - vGPU device only
> - One GPU pass through and one vGPU device
> - Linux VM hot plug and unplug vGPU device while GPU pass through device
> exist
> - Linux VM hot plug and unplug GPU pass through device while vGPU device
> exist
>
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> Change-Id: I295d6f0f2e0579b8d9882bfd8fd5a4194b97bd9a
> ---
> drivers/vfio/vfio_iommu_type1.c | 587 +++++++++++++++++++++++++++++++++++-----
> 1 file changed, 526 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 50aca95cf61e..2697d874dd35 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -37,6 +37,7 @@
> #include <linux/vfio.h>
> #include <linux/workqueue.h>
> #include <linux/pid_namespace.h>
> +#include <linux/mdev.h>
>
> #define DRIVER_VERSION "0.2"
> #define DRIVER_AUTHOR "Alex Williamson <alex.williamson@redhat.com>"
> @@ -56,6 +57,7 @@ MODULE_PARM_DESC(disable_hugepages,
>
> struct vfio_iommu {
> struct list_head domain_list;
> + struct vfio_domain *external_domain; /* domain for external user */
> struct mutex lock;
> struct rb_root dma_list;
> bool v2;
> @@ -76,7 +78,9 @@ struct vfio_dma {
> unsigned long vaddr; /* Process virtual addr */
> size_t size; /* Map size (bytes) */
> int prot; /* IOMMU_READ/WRITE */
> + bool iommu_mapped;
> struct task_struct *task;
> + struct rb_root pfn_list; /* Ex-user pinned pfn list */
> };
>
> struct vfio_group {
> @@ -85,6 +89,21 @@ struct vfio_group {
> };
>
> /*
> + * Guest RAM pinning working set or DMA target
> + */
> +struct vfio_pfn {
> + struct rb_node node;
> + dma_addr_t iova; /* Device address */
> + unsigned long pfn; /* Host pfn */
> + atomic_t ref_count;
> +};
> +
> +#define IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu) \
> + (!list_empty(&iommu->domain_list))
> +
> +static int put_pfn(unsigned long pfn, int prot);
> +
> +/*
> * This code handles mapping and unmapping of user data buffers
> * into DMA'ble space using the IOMMU
> */
> @@ -132,6 +151,97 @@ static void vfio_unlink_dma(struct vfio_iommu *iommu, struct vfio_dma *old)
> rb_erase(&old->node, &iommu->dma_list);
> }
>
> +/*
> + * Helper Functions for host iova-pfn list
> + */
> +static struct vfio_pfn *vfio_find_vpfn(struct vfio_dma *dma, dma_addr_t iova)
> +{
> + struct vfio_pfn *vpfn;
> + struct rb_node *node = dma->pfn_list.rb_node;
> +
> + while (node) {
> + vpfn = rb_entry(node, struct vfio_pfn, node);
> +
> + if (iova < vpfn->iova)
> + node = node->rb_left;
> + else if (iova > vpfn->iova)
> + node = node->rb_right;
> + else
> + return vpfn;
> + }
> + return NULL;
> +}
> +
> +static void vfio_link_pfn(struct vfio_dma *dma,
> + struct vfio_pfn *new)
> +{
> + struct rb_node **link, *parent = NULL;
> + struct vfio_pfn *vpfn;
> +
> + link = &dma->pfn_list.rb_node;
> + while (*link) {
> + parent = *link;
> + vpfn = rb_entry(parent, struct vfio_pfn, node);
> +
> + if (new->iova < vpfn->iova)
> + link = &(*link)->rb_left;
> + else
> + link = &(*link)->rb_right;
> + }
> +
> + rb_link_node(&new->node, parent, link);
> + rb_insert_color(&new->node, &dma->pfn_list);
> +}
> +
> +static void vfio_unlink_pfn(struct vfio_dma *dma, struct vfio_pfn *old)
> +{
> + rb_erase(&old->node, &dma->pfn_list);
> +}
> +
> +static int vfio_add_to_pfn_list(struct vfio_dma *dma, dma_addr_t iova,
> + unsigned long pfn)
> +{
> + struct vfio_pfn *vpfn;
> +
> + vpfn = kzalloc(sizeof(*vpfn), GFP_KERNEL);
> + if (!vpfn)
> + return -ENOMEM;
> +
> + vpfn->iova = iova;
> + vpfn->pfn = pfn;
> + atomic_set(&vpfn->ref_count, 1);
> + vfio_link_pfn(dma, vpfn);
> + return 0;
> +}
> +
> +static void vfio_remove_from_pfn_list(struct vfio_dma *dma,
> + struct vfio_pfn *vpfn)
> +{
> + vfio_unlink_pfn(dma, vpfn);
> + kfree(vpfn);
> +}
> +
> +static struct vfio_pfn *vfio_iova_get_vfio_pfn(struct vfio_dma *dma,
> + unsigned long iova)
> +{
> + struct vfio_pfn *vpfn = vfio_find_vpfn(dma, iova);
> +
> + if (vpfn)
> + atomic_inc(&vpfn->ref_count);
> + return vpfn;
> +}
> +
> +static int vfio_iova_put_vfio_pfn(struct vfio_dma *dma, struct vfio_pfn *vpfn)
> +{
> + int ret = 0;
> +
> + if (atomic_dec_and_test(&vpfn->ref_count)) {
> + ret = put_pfn(vpfn->pfn, dma->prot);
> + vfio_remove_from_pfn_list(dma, vpfn);
> + }
> + return ret;
> +}
> +
> struct vwork {
> struct mm_struct *mm;
> long npage;
> @@ -270,7 +380,6 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
> }
>
> up_read(&mm->mmap_sem);
> -
> return ret;
> }
>
> @@ -280,28 +389,35 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
> * first page and all consecutive pages with the same locking.
> */
> static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
> - long npage, int prot, unsigned long *pfn_base)
> + long npage, unsigned long *pfn_base)
> {
> unsigned long limit;
> bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
> CAP_IPC_LOCK);
> struct mm_struct *mm;
> - long ret, i;
> + long ret, i, lock_acct = 0;
> bool rsvd;
> + struct vfio_pfn *vpfn;
> + dma_addr_t iova;
>
> mm = get_task_mm(dma->task);
> if (!mm)
> return -ENODEV;
>
> - ret = vaddr_get_pfn(mm, vaddr, prot, pfn_base);
> + ret = vaddr_get_pfn(mm, vaddr, dma->prot, pfn_base);
> if (ret)
> goto pin_pg_remote_exit;
>
> + iova = vaddr - dma->vaddr + dma->iova;
> + vpfn = vfio_find_vpfn(dma, iova);
> + if (!vpfn)
> + lock_acct = 1;
> +
> rsvd = is_invalid_reserved_pfn(*pfn_base);
> limit = task_rlimit(dma->task, RLIMIT_MEMLOCK) >> PAGE_SHIFT;
>
> - if (!rsvd && !lock_cap && mm->locked_vm + 1 > limit) {
> - put_pfn(*pfn_base, prot);
> + if (!rsvd && !lock_cap && mm->locked_vm + lock_acct > limit) {
> + put_pfn(*pfn_base, dma->prot);
> pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n", __func__,
> limit << PAGE_SHIFT);
> ret = -ENOMEM;
> @@ -310,7 +426,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
>
> if (unlikely(disable_hugepages)) {
> if (!rsvd)
> - vfio_lock_acct(dma->task, 1);
> + vfio_lock_acct(dma->task, lock_acct);
> ret = 1;
> goto pin_pg_remote_exit;
> }
> @@ -319,18 +435,24 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
> for (i = 1, vaddr += PAGE_SIZE; i < npage; i++, vaddr += PAGE_SIZE) {
> unsigned long pfn = 0;
>
> - ret = vaddr_get_pfn(mm, vaddr, prot, &pfn);
> + ret = vaddr_get_pfn(mm, vaddr, dma->prot, &pfn);
> if (ret)
> break;
>
> if (pfn != *pfn_base + i ||
> rsvd != is_invalid_reserved_pfn(pfn)) {
> - put_pfn(pfn, prot);
> + put_pfn(pfn, dma->prot);
> break;
> }
>
> - if (!rsvd && !lock_cap && mm->locked_vm + i + 1 > limit) {
> - put_pfn(pfn, prot);
> + iova = vaddr - dma->vaddr + dma->iova;
nit, this could be incremented in the for loop just like vaddr, we
don't need to create it from scratch (iova += PAGE_SIZE).
> + vpfn = vfio_find_vpfn(dma, iova);
> + if (!vpfn)
> + lock_acct++;
> +
> + if (!rsvd && !lock_cap &&
> + mm->locked_vm + lock_acct + 1 > limit) {
lock_acct was incremented just above, why is this lock_acct + 1? I
think we're off by one here (ie. remove the +1)?
> + put_pfn(pfn, dma->prot);
> pr_warn("%s: RLIMIT_MEMLOCK (%ld) exceeded\n",
> __func__, limit << PAGE_SHIFT);
> break;
> @@ -338,7 +460,7 @@ static long vfio_pin_pages_remote(struct vfio_dma *dma, unsigned long vaddr,
> }
>
> if (!rsvd)
> - vfio_lock_acct(dma->task, i);
> + vfio_lock_acct(dma->task, lock_acct);
> ret = i;
>
> pin_pg_remote_exit:
> @@ -346,14 +468,78 @@ pin_pg_remote_exit:
> return ret;
> }
>
> -static long vfio_unpin_pages_remote(struct vfio_dma *dma, unsigned long pfn,
> - long npage, int prot, bool do_accounting)
> +static long vfio_unpin_pages_remote(struct vfio_dma *dma, dma_addr_t iova,
> + unsigned long pfn, long npage,
> + bool do_accounting)
> {
> - unsigned long unlocked = 0;
> + long unlocked = 0, locked = 0;
> long i;
>
> - for (i = 0; i < npage; i++)
> - unlocked += put_pfn(pfn++, prot);
> + for (i = 0; i < npage; i++) {
> + struct vfio_pfn *vpfn;
> +
> + unlocked += put_pfn(pfn++, dma->prot);
> +
> + vpfn = vfio_find_vpfn(dma, iova + (i << PAGE_SHIFT));
> + if (vpfn)
> + locked++;
This isn't taking into account reserved pages (ex. device mmaps). In
the pinning path above we skip accounting of reserved pages, put_pfn
also only increments for non-reserved pages, but vfio_find_vpfn()
doesn't care, so it's possible that (locked - unlocked) could result in
a positive value. Maybe something like:
if (put_pfn(...)) {
unlocked++;
if (vfio_find_vpfn(...))
locked++;
}
> + }
> +
> + if (do_accounting)
> + vfio_lock_acct(dma->task, locked - unlocked);
> +
> + return unlocked;
> +}
> +
> +static int vfio_pin_page_external(struct vfio_dma *dma, unsigned long vaddr,
> + unsigned long *pfn_base, bool do_accounting)
> +{
> + unsigned long limit;
> + bool lock_cap = ns_capable(task_active_pid_ns(dma->task)->user_ns,
> + CAP_IPC_LOCK);
> + struct mm_struct *mm;
> + int ret;
> + bool rsvd;
> +
> + mm = get_task_mm(dma->task);
> + if (!mm)
> + return -ENODEV;
> +
> + ret = vaddr_get_pfn(mm, vaddr, dma->prot, pfn_base);
> + if (ret)
> + goto pin_page_exit;
> +
> + rsvd = is_invalid_reserved_pfn(*pfn_base);
> + limit = task_rlimit(dma->task, RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> +
> + if (!rsvd && !lock_cap && mm->locked_vm + 1 > limit) {
> + put_pfn(*pfn_base, dma->prot);
> + pr_warn("%s: Task %s (%d) RLIMIT_MEMLOCK (%ld) exceeded\n",
> + __func__, dma->task->comm, task_pid_nr(dma->task),
> + limit << PAGE_SHIFT);
> + ret = -ENOMEM;
> + goto pin_page_exit;
> + }
> +
> + if (!rsvd && do_accounting)
> + vfio_lock_acct(dma->task, 1);
> + ret = 1;
> +
> +pin_page_exit:
> + mmput(mm);
> + return ret;
> +}
> +
> +static int vfio_unpin_page_external(struct vfio_dma *dma, dma_addr_t iova,
> + bool do_accounting)
> +{
> + int unlocked;
> + struct vfio_pfn *vpfn = vfio_find_vpfn(dma, iova);
> +
> + if (!vpfn)
> + return 0;
> +
> + unlocked = vfio_iova_put_vfio_pfn(dma, vpfn);
>
> if (do_accounting)
> vfio_lock_acct(dma->task, -unlocked);
> @@ -361,14 +547,148 @@ static long vfio_unpin_pages_remote(struct vfio_dma *dma, unsigned long pfn,
> return unlocked;
> }
>
> -static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
> +static int vfio_iommu_type1_pin_pages(void *iommu_data,
> + unsigned long *user_pfn,
> + int npage, int prot,
> + unsigned long *phys_pfn)
> +{
> + struct vfio_iommu *iommu = iommu_data;
> + int i, j, ret;
> + unsigned long remote_vaddr;
> + unsigned long *pfn = phys_pfn;
nit, why do we have this variable rather than using phys_pfn directly?
Maybe before we had unwind support we incremented this rather than
indexing it?
> + struct vfio_dma *dma;
> + bool do_accounting;
> +
> + if (!iommu || !user_pfn || !phys_pfn)
> + return -EINVAL;
> +
> + /* Supported for v2 version only */
> + if (!iommu->v2)
> + return -EACCES;
> +
> + mutex_lock(&iommu->lock);
> +
> + if (!iommu->external_domain) {
> + ret = -EINVAL;
> + goto pin_done;
> + }
> +
> + /*
> + * If iommu capable domain exist in the container then all pages are
> + * already pinned and accounted. Accouting should be done if there is no
> + * iommu capable domain in the container.
> + */
> + do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu);
> +
> + for (i = 0; i < npage; i++) {
> + dma_addr_t iova;
> + struct vfio_pfn *vpfn;
> +
> + iova = user_pfn[i] << PAGE_SHIFT;
> + dma = vfio_find_dma(iommu, iova, 0);
> + if (!dma) {
> + ret = -EINVAL;
> + goto pin_unwind;
> + }
> +
> + if ((dma->prot & prot) != prot) {
> + pr_info("%s: dma->prot: 0x%x prot: 0x%x\n",
> + __func__, dma->prot, prot);
> + ret = -EPERM;
> + goto pin_unwind;
> + }
> +
> + vpfn = vfio_iova_get_vfio_pfn(dma, iova);
> + if (vpfn) {
> + pfn[i] = vpfn->pfn;
> + continue;
> + }
> +
> + remote_vaddr = dma->vaddr + iova - dma->iova;
> + ret = vfio_pin_page_external(dma, remote_vaddr, &pfn[i],
> + do_accounting);
> + if (ret <= 0) {
> + WARN_ON(!ret);
> + goto pin_unwind;
> + }
> +
> + ret = vfio_add_to_pfn_list(dma, iova, pfn[i]);
> + if (ret) {
> + vfio_unpin_page_external(dma, iova, do_accounting);
> + goto pin_unwind;
> + }
> + }
> +
> + ret = i;
> + goto pin_done;
> +
> +pin_unwind:
> + pfn[i] = 0;
> + for (j = 0; j < i; j++) {
> + dma_addr_t iova;
> +
> + iova = user_pfn[j] << PAGE_SHIFT;
> + dma = vfio_find_dma(iommu, iova, 0);
> + vfio_unpin_page_external(dma, iova, do_accounting);
> + pfn[j] = 0;
> + }
> +pin_done:
> + mutex_unlock(&iommu->lock);
> + return ret;
> +}
> +
> +static int vfio_iommu_type1_unpin_pages(void *iommu_data,
> + unsigned long *user_pfn,
> + int npage)
> +{
> + struct vfio_iommu *iommu = iommu_data;
> + bool do_accounting;
> + int unlocked = 0, i;
> +
> + if (!iommu || !user_pfn)
> + return -EINVAL;
> +
> + /* Supported for v2 version only */
> + if (!iommu->v2)
> + return -EACCES;
> +
> + mutex_lock(&iommu->lock);
> +
> + if (!iommu->external_domain) {
> + mutex_unlock(&iommu->lock);
> + return -EINVAL;
> + }
> +
> + do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu);
> + for (i = 0; i < npage; i++) {
> + struct vfio_dma *dma;
> + dma_addr_t iova;
> +
> + iova = user_pfn[i] << PAGE_SHIFT;
> + dma = vfio_find_dma(iommu, iova, 0);
> + if (!dma)
> + goto unpin_exit;
> + unlocked += vfio_unpin_page_external(dma, iova, do_accounting);
> + }
> +
> +unpin_exit:
> + mutex_unlock(&iommu->lock);
> + return unlocked;
This is not returning what it's supposed to. unlocked is going to be
less than or equal to the number of pages unpinned. We don't need to
track unlocked, I think we're just tracking where we are in the unpin
array, whether it was partial or complete. I think we want:
return i > npage ? npage : i;
Or maybe we can make it more obvious if there's an error on the first
unpin entry:
return i > npage ? npage : (i > 0 ? i : -EINVAL);
> +}
> +
> +static long vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma,
> + bool do_accounting)
> {
> dma_addr_t iova = dma->iova, end = dma->iova + dma->size;
> struct vfio_domain *domain, *d;
> long unlocked = 0;
>
> if (!dma->size)
> - return;
> + return 0;
> +
> + if (!IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu))
> + return 0;
> +
> /*
> * We use the IOMMU to track the physical addresses, otherwise we'd
> * need a much more complicated tracking system. Unfortunately that
> @@ -410,20 +730,26 @@ static void vfio_unmap_unpin(struct vfio_iommu *iommu, struct vfio_dma *dma)
> if (WARN_ON(!unmapped))
> break;
>
> - unlocked += vfio_unpin_pages_remote(dma, phys >> PAGE_SHIFT,
> + unlocked += vfio_unpin_pages_remote(dma, iova,
> + phys >> PAGE_SHIFT,
> unmapped >> PAGE_SHIFT,
> - dma->prot, false);
> + false);
> iova += unmapped;
>
> cond_resched();
> }
>
> - vfio_lock_acct(dma->task, -unlocked);
> + dma->iommu_mapped = false;
> + if (do_accounting) {
> + vfio_lock_acct(dma->task, -unlocked);
> + return 0;
> + }
> + return unlocked;
> }
>
> static void vfio_remove_dma(struct vfio_iommu *iommu, struct vfio_dma *dma)
> {
> - vfio_unmap_unpin(iommu, dma);
> + vfio_unmap_unpin(iommu, dma, true);
> vfio_unlink_dma(iommu, dma);
> put_task_struct(dma->task);
> kfree(dma);
> @@ -606,8 +932,7 @@ static int vfio_pin_map_dma(struct vfio_iommu *iommu, struct vfio_dma *dma,
> while (size) {
> /* Pin a contiguous chunk of memory */
> npage = vfio_pin_pages_remote(dma, vaddr + dma->size,
> - size >> PAGE_SHIFT, dma->prot,
> - &pfn);
> + size >> PAGE_SHIFT, &pfn);
> if (npage <= 0) {
> WARN_ON(!npage);
> ret = (int)npage;
> @@ -618,8 +943,8 @@ static int vfio_pin_map_dma(struct vfio_iommu *iommu, struct vfio_dma *dma,
> ret = vfio_iommu_map(iommu, iova + dma->size, pfn, npage,
> dma->prot);
> if (ret) {
> - vfio_unpin_pages_remote(dma, pfn, npage,
> - dma->prot, true);
> + vfio_unpin_pages_remote(dma, iova + dma->size, pfn,
> + npage, true);
> break;
> }
>
> @@ -627,6 +952,8 @@ static int vfio_pin_map_dma(struct vfio_iommu *iommu, struct vfio_dma *dma,
> dma->size += npage << PAGE_SHIFT;
> }
>
> + dma->iommu_mapped = true;
> +
> if (ret)
> vfio_remove_dma(iommu, dma);
>
> @@ -682,11 +1009,16 @@ static int vfio_dma_do_map(struct vfio_iommu *iommu,
> dma->prot = prot;
> get_task_struct(current);
> dma->task = current;
> + dma->pfn_list = RB_ROOT;
>
> /* Insert zero-sized and grow as we map chunks of it */
> vfio_link_dma(iommu, dma);
>
> - ret = vfio_pin_map_dma(iommu, dma, size);
> + /* Don't pin and map if container doesn't contain IOMMU capable domain*/
> + if (!IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu))
> + dma->size = size;
> + else
> + ret = vfio_pin_map_dma(iommu, dma, size);
> do_map_err:
> mutex_unlock(&iommu->lock);
> return ret;
> @@ -715,10 +1047,6 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
> d = list_first_entry(&iommu->domain_list, struct vfio_domain, next);
> n = rb_first(&iommu->dma_list);
>
> - /* If there's not a domain, there better not be any mappings */
> - if (WARN_ON(n && !d))
> - return -EINVAL;
> -
> for (; n; n = rb_next(n)) {
> struct vfio_dma *dma;
> dma_addr_t iova;
> @@ -727,21 +1055,49 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
> iova = dma->iova;
>
> while (iova < dma->iova + dma->size) {
> - phys_addr_t phys = iommu_iova_to_phys(d->domain, iova);
> + phys_addr_t phys;
> size_t size;
>
> - if (WARN_ON(!phys)) {
> - iova += PAGE_SIZE;
> - continue;
> + if (dma->iommu_mapped) {
> + phys_addr_t p;
> + dma_addr_t i;
> +
> + phys = iommu_iova_to_phys(d->domain, iova);
> +
> + if (WARN_ON(!phys)) {
> + iova += PAGE_SIZE;
> + continue;
> + }
> +
> + size = PAGE_SIZE;
> + p = phys + size;
> + i = iova + size;
> + while (i < dma->iova + dma->size &&
> + p == iommu_iova_to_phys(d->domain, i)) {
> + size += PAGE_SIZE;
> + p += PAGE_SIZE;
> + i += PAGE_SIZE;
> + }
> + } else {
> + unsigned long pfn;
> + unsigned long vaddr = dma->vaddr +
> + (iova - dma->iova);
> + size_t n = dma->iova + dma->size - iova;
> + long npage;
> +
> + npage = vfio_pin_pages_remote(dma, vaddr,
> + n >> PAGE_SHIFT,
> + &pfn);
> + if (npage <= 0) {
> + WARN_ON(!npage);
> + ret = (int)npage;
> + return ret;
> + }
> +
> + phys = pfn << PAGE_SHIFT;
> + size = npage << PAGE_SHIFT;
> }
>
> - size = PAGE_SIZE;
> -
> - while (iova + size < dma->iova + dma->size &&
> - phys + size == iommu_iova_to_phys(d->domain,
> - iova + size))
> - size += PAGE_SIZE;
> -
> ret = iommu_map(domain->domain, iova, phys,
> size, dma->prot | domain->prot);
> if (ret)
> @@ -749,8 +1105,8 @@ static int vfio_iommu_replay(struct vfio_iommu *iommu,
>
> iova += size;
> }
> + dma->iommu_mapped = true;
> }
> -
> return 0;
> }
>
> @@ -806,7 +1162,7 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> struct vfio_iommu *iommu = iommu_data;
> struct vfio_group *group;
> struct vfio_domain *domain, *d;
> - struct bus_type *bus = NULL;
> + struct bus_type *bus = NULL, *mdev_bus;
> int ret;
>
> mutex_lock(&iommu->lock);
> @@ -818,6 +1174,13 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> }
> }
>
> + if (iommu->external_domain) {
> + if (find_iommu_group(iommu->external_domain, iommu_group)) {
> + mutex_unlock(&iommu->lock);
> + return -EINVAL;
> + }
> + }
> +
> group = kzalloc(sizeof(*group), GFP_KERNEL);
> domain = kzalloc(sizeof(*domain), GFP_KERNEL);
> if (!group || !domain) {
> @@ -832,6 +1195,25 @@ static int vfio_iommu_type1_attach_group(void *iommu_data,
> if (ret)
> goto out_free;
>
> + mdev_bus = symbol_get(mdev_bus_type);
> +
> + if (mdev_bus) {
> + if ((bus == mdev_bus) && !iommu_present(bus)) {
> + symbol_put(mdev_bus_type);
> + if (!iommu->external_domain) {
> + INIT_LIST_HEAD(&domain->group_list);
> + iommu->external_domain = domain;
> + } else
> + kfree(domain);
> +
> + list_add(&group->next,
> + &iommu->external_domain->group_list);
> + mutex_unlock(&iommu->lock);
> + return 0;
> + }
> + symbol_put(mdev_bus_type);
> + }
> +
> domain->domain = iommu_domain_alloc(bus);
> if (!domain->domain) {
> ret = -EIO;
> @@ -922,6 +1304,46 @@ static void vfio_iommu_unmap_unpin_all(struct vfio_iommu *iommu)
> vfio_remove_dma(iommu, rb_entry(node, struct vfio_dma, node));
> }
>
> +static void vfio_iommu_unmap_unpin_reaccount(struct vfio_iommu *iommu)
> +{
> + struct rb_node *n, *p;
> +
> + n = rb_first(&iommu->dma_list);
> + for (; n; n = rb_next(n)) {
> + struct vfio_dma *dma;
> + long locked = 0, unlocked = 0;
> +
> + dma = rb_entry(n, struct vfio_dma, node);
> + unlocked += vfio_unmap_unpin(iommu, dma, false);
> + p = rb_first(&dma->pfn_list);
> + for (; p; p = rb_next(p))
> + locked++;
We don't know that these weren't reserved pages. If the vendor driver
was pinning peer-to-peer ranges vfio_unmap_unpin() might have returned
0 yet we're assuming each is locked RAM, so our accounting can go
upside down.
> + vfio_lock_acct(dma->task, locked - unlocked);
> + }
> +}
> +
> +static void vfio_external_unpin_all(struct vfio_iommu *iommu,
> + bool do_accounting)
> +{
> + struct rb_node *n, *p;
> +
> + n = rb_first(&iommu->dma_list);
> + for (; n; n = rb_next(n)) {
> + struct vfio_dma *dma;
> +
> + dma = rb_entry(n, struct vfio_dma, node);
> + while ((p = rb_first(&dma->pfn_list))) {
> + int unlocked;
> + struct vfio_pfn *vpfn = rb_entry(p, struct vfio_pfn,
> + node);
> +
> + unlocked = vfio_iova_put_vfio_pfn(dma, vpfn);
> + if (do_accounting)
> + vfio_lock_acct(dma->task, -unlocked);
nit, we could batch these further, only updating accounting once per
vfio_dma, or once entirely.
> + }
> + }
> +}
> +
> static void vfio_iommu_type1_detach_group(void *iommu_data,
> struct iommu_group *iommu_group)
> {
> @@ -931,6 +1353,26 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
>
> mutex_lock(&iommu->lock);
>
> + if (iommu->external_domain) {
> + domain = iommu->external_domain;
> + group = find_iommu_group(domain, iommu_group);
> + if (group) {
> + list_del(&group->next);
> + kfree(group);
> +
> + if (list_empty(&domain->group_list)) {
> + if (!IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu)) {
> + vfio_external_unpin_all(iommu, true);
> + vfio_iommu_unmap_unpin_all(iommu);
> + } else
> + vfio_external_unpin_all(iommu, false);
> + kfree(domain);
> + iommu->external_domain = NULL;
> + }
> + goto detach_group_done;
> + }
> + }
> +
> list_for_each_entry(domain, &iommu->domain_list, next) {
> group = find_iommu_group(domain, iommu_group);
> if (!group)
> @@ -940,21 +1382,27 @@ static void vfio_iommu_type1_detach_group(void *iommu_data,
> list_del(&group->next);
> kfree(group);
> /*
> - * Group ownership provides privilege, if the group
> - * list is empty, the domain goes away. If it's the
> - * last domain, then all the mappings go away too.
> + * Group ownership provides privilege, if the group list is
> + * empty, the domain goes away. If it's the last domain with
> + * iommu and external domain doesn't exist, then all the
> + * mappings go away too. If it's the last domain with iommu and
> + * external domain exist, update accounting
> */
> if (list_empty(&domain->group_list)) {
> - if (list_is_singular(&iommu->domain_list))
> - vfio_iommu_unmap_unpin_all(iommu);
> + if (list_is_singular(&iommu->domain_list)) {
> + if (!iommu->external_domain)
> + vfio_iommu_unmap_unpin_all(iommu);
> + else
> + vfio_iommu_unmap_unpin_reaccount(iommu);
> + }
> iommu_domain_free(domain->domain);
> list_del(&domain->next);
> kfree(domain);
> }
> - goto done;
> + break;
> }
>
> -done:
> +detach_group_done:
> mutex_unlock(&iommu->lock);
> }
>
> @@ -986,27 +1434,42 @@ static void *vfio_iommu_type1_open(unsigned long arg)
> return iommu;
> }
>
> +static void vfio_release_domain(struct vfio_domain *domain, bool external)
> +{
> + struct vfio_group *group, *group_tmp;
> +
> + list_for_each_entry_safe(group, group_tmp,
> + &domain->group_list, next) {
> + if (!external)
> + iommu_detach_group(domain->domain, group->iommu_group);
> + list_del(&group->next);
> + kfree(group);
> + }
> +
> + if (!external)
> + iommu_domain_free(domain->domain);
> +}
> +
> static void vfio_iommu_type1_release(void *iommu_data)
> {
> struct vfio_iommu *iommu = iommu_data;
> struct vfio_domain *domain, *domain_tmp;
> - struct vfio_group *group, *group_tmp;
> +
> + if (iommu->external_domain) {
> + vfio_release_domain(iommu->external_domain, true);
> + vfio_external_unpin_all(iommu, false);
> + kfree(iommu->external_domain);
> + iommu->external_domain = NULL;
> + }
>
> vfio_iommu_unmap_unpin_all(iommu);
>
> list_for_each_entry_safe(domain, domain_tmp,
> &iommu->domain_list, next) {
> - list_for_each_entry_safe(group, group_tmp,
> - &domain->group_list, next) {
> - iommu_detach_group(domain->domain, group->iommu_group);
> - list_del(&group->next);
> - kfree(group);
> - }
> - iommu_domain_free(domain->domain);
> + vfio_release_domain(domain, false);
> list_del(&domain->next);
> kfree(domain);
> }
> -
> kfree(iommu);
> }
>
> @@ -1110,6 +1573,8 @@ static const struct vfio_iommu_driver_ops vfio_iommu_driver_ops_type1 = {
> .ioctl = vfio_iommu_type1_ioctl,
> .attach_group = vfio_iommu_type1_attach_group,
> .detach_group = vfio_iommu_type1_detach_group,
> + .pin_pages = vfio_iommu_type1_pin_pages,
> + .unpin_pages = vfio_iommu_type1_unpin_pages,
> };
>
> static int __init vfio_iommu_type1_init(void)
^ permalink raw reply
* RE: [PATCH] remote-curl: don't hang when a server dies before any output
From: David Turner @ 2016-11-14 23:25 UTC (permalink / raw)
To: 'Jeff King'; +Cc: git@vger.kernel.org, spearce@spearce.org
In-Reply-To: <20161114194049.mktpsvgdhex2f4zv@sigill.intra.peff.net>
> -----Original Message-----
> From: Jeff King [mailto:peff@peff.net]
> Sent: Monday, November 14, 2016 2:41 PM
> To: David Turner
> Cc: git@vger.kernel.org; spearce@spearce.org
> Subject: Re: [PATCH] remote-curl: don't hang when a server dies before any
> output
>
> On Mon, Nov 14, 2016 at 01:24:31PM -0500, Jeff King wrote:
>
> > 2. Have remote-curl understand enough of the protocol that it can
> > abort rather than hang.
> >
> > I think that's effectively the approach of your patch, but for one
> > specific case. But could we, for example, make sure that everything
> > we proxy is a complete set of pktlines and ends with a flush? And
> > if not, then we hang up on fetch-pack.
> >
> > I _think_ that would work, because even the pack is always encased
> > in pktlines for smart-http.
>
> So something like this. It turned out to be a lot uglier than I had hoped
> because we get fed the data from curl in odd-sized chunks, so we need a
> state machine.
>
> But it does seem to work. At least it doesn't seem to break anything in
> the test suite, and it fixes the new tests you added. I'd worry that
> there's some obscure case where the response isn't packetized in the same
> way.
Overall, this looks good to me. The state machine is pretty clean. I think I would have used a tiny buffer for the length field, and then I would have regretted it. Your way looks nicer than my unwritten patch would have looked.
> +#define READ_ONE_HEX(shift) do { \
> + int val = hexval(buf[0]); \
> + if (val < 0) { \
> + warning("error on %d", *buf); \
> + rpc->pktline_state = RPC_PKTLINE_ERROR; \
> + return; \
> + } \
> + rpc->pktline_len |= val << shift; \
Nit: parenthesize shift here, since it is a parameter to a macro.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.