From: Jason Gunthorpe <jgg@nvidia.com> To: Will Deacon <will@kernel.org> Cc: Catalin Marinas <catalin.marinas@arm.com>, Lorenzo Pieralisi <lpieralisi@kernel.org>, ankita@nvidia.com, maz@kernel.org, oliver.upton@linux.dev, aniketa@nvidia.com, cjia@nvidia.com, kwankhede@nvidia.com, targupta@nvidia.com, vsethi@nvidia.com, acurrid@nvidia.com, apopple@nvidia.com, jhubbard@nvidia.com, danw@nvidia.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 2/2] KVM: arm64: allow the VM to select DEVICE_* and NORMAL_NC for IO memory Date: Thu, 12 Oct 2023 15:36:24 -0300 [thread overview] Message-ID: <20231012183624.GN3952@nvidia.com> (raw) In-Reply-To: <20231012163931.GA12592@willie-the-truck> On Thu, Oct 12, 2023 at 05:39:31PM +0100, Will Deacon wrote: > All I'm asking for is justification as to why Normal-NC is the right > memory type rather than any other normal memory type. If it's not possible > to explain that architecturally, then I'm not sure this change belongs in > architecture code. Well, I think Catalin summarized it nicely, I second his ask at the end: We are basically at your scenario below - can you justify why DEVICE_GRE is correct today, architecturally? We could not. Earlier someone said uncontained failure prevention, but a deep dive on that found it is not so. > Ultimately, we need to be able to maintain this stuff, so we can't just > blindly implement changes based on a combination of off-list discussions > and individual product needs. For example, if somebody else rocks up > tomorrow and asks for this to be Normal-writethrough, what grounds do > we have to say no if we've taken this change already? Hmm. Something got lost here. This patch is talking about the S2 MemAttr[3:0]. There are only 4 relevant values (when FEAT_S2FWB) (see D5.5.5 in ARM DDI 0487F.c) 0b001 - Today: force VM to be Device nGnRE 0b101 - Proposed: prevent the VM from selecting cachable, allow it to choose Device-* or NormalNC 0b110 - Force write back. Would totally break MMIO, ruled out 0b111 - Allow the VM to select anything, including cachable. This is nice, but summarizing Catalin's remarks: a) The kernel can't do cache maintenance (defeats FWB) b) Catalin's concerns about MTE and Atomics triggering uncontained failures c) It is unclear about uncontained failures for cachable in general So the only argument is 001 v 110 v 111 Catalin has explained why 111 is not good as a default. Most likely with some future ACPI/BSA/etc work, and some cache syncing in the kernel, someone could define a way to allow 111 as a choice. So, I think we can rule out 111 as being the default choice without more the kernel getting more detailed system level knowledge. Further, patch 1 is about making 110 a driver-opt-in choice for VFIO memory which reduces the need for 111. For 001 v 110: 001 is less functional in the VM. 001 offers no advantage. !FEAT_S2FWB has similar choices and similar argument. So, IMHO, if someone comes to ask for something it would be to ask for 111 and we do have a set of pretty clear reasons why it should not be 111. (indeed we wanted to ask for that instead of patch 1 but there are too many things required to get there), Jason
WARNING: multiple messages have this Message-ID (diff)
From: Jason Gunthorpe <jgg@nvidia.com> To: Will Deacon <will@kernel.org> Cc: Catalin Marinas <catalin.marinas@arm.com>, Lorenzo Pieralisi <lpieralisi@kernel.org>, ankita@nvidia.com, maz@kernel.org, oliver.upton@linux.dev, aniketa@nvidia.com, cjia@nvidia.com, kwankhede@nvidia.com, targupta@nvidia.com, vsethi@nvidia.com, acurrid@nvidia.com, apopple@nvidia.com, jhubbard@nvidia.com, danw@nvidia.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 2/2] KVM: arm64: allow the VM to select DEVICE_* and NORMAL_NC for IO memory Date: Thu, 12 Oct 2023 15:36:24 -0300 [thread overview] Message-ID: <20231012183624.GN3952@nvidia.com> (raw) In-Reply-To: <20231012163931.GA12592@willie-the-truck> On Thu, Oct 12, 2023 at 05:39:31PM +0100, Will Deacon wrote: > All I'm asking for is justification as to why Normal-NC is the right > memory type rather than any other normal memory type. If it's not possible > to explain that architecturally, then I'm not sure this change belongs in > architecture code. Well, I think Catalin summarized it nicely, I second his ask at the end: We are basically at your scenario below - can you justify why DEVICE_GRE is correct today, architecturally? We could not. Earlier someone said uncontained failure prevention, but a deep dive on that found it is not so. > Ultimately, we need to be able to maintain this stuff, so we can't just > blindly implement changes based on a combination of off-list discussions > and individual product needs. For example, if somebody else rocks up > tomorrow and asks for this to be Normal-writethrough, what grounds do > we have to say no if we've taken this change already? Hmm. Something got lost here. This patch is talking about the S2 MemAttr[3:0]. There are only 4 relevant values (when FEAT_S2FWB) (see D5.5.5 in ARM DDI 0487F.c) 0b001 - Today: force VM to be Device nGnRE 0b101 - Proposed: prevent the VM from selecting cachable, allow it to choose Device-* or NormalNC 0b110 - Force write back. Would totally break MMIO, ruled out 0b111 - Allow the VM to select anything, including cachable. This is nice, but summarizing Catalin's remarks: a) The kernel can't do cache maintenance (defeats FWB) b) Catalin's concerns about MTE and Atomics triggering uncontained failures c) It is unclear about uncontained failures for cachable in general So the only argument is 001 v 110 v 111 Catalin has explained why 111 is not good as a default. Most likely with some future ACPI/BSA/etc work, and some cache syncing in the kernel, someone could define a way to allow 111 as a choice. So, I think we can rule out 111 as being the default choice without more the kernel getting more detailed system level knowledge. Further, patch 1 is about making 110 a driver-opt-in choice for VFIO memory which reduces the need for 111. For 001 v 110: 001 is less functional in the VM. 001 offers no advantage. !FEAT_S2FWB has similar choices and similar argument. So, IMHO, if someone comes to ask for something it would be to ask for 111 and we do have a set of pretty clear reasons why it should not be 111. (indeed we wanted to ask for that instead of patch 1 but there are too many things required to get there), Jason _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-10-12 18:36 UTC|newest] Thread overview: 110+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-09-07 18:14 [PATCH v1 0/2] KVM: arm64: support write combining and cachable IO memory in VMs ankita 2023-09-07 18:14 ` ankita 2023-09-07 18:14 ` [PATCH v1 1/2] KVM: arm64: determine memory type from VMA ankita 2023-09-07 18:14 ` ankita 2023-09-07 19:12 ` Jason Gunthorpe 2023-09-07 19:12 ` Jason Gunthorpe 2023-10-05 16:15 ` Catalin Marinas 2023-10-05 16:15 ` Catalin Marinas 2023-10-05 16:54 ` Jason Gunthorpe 2023-10-05 16:54 ` Jason Gunthorpe 2023-10-10 14:25 ` Catalin Marinas 2023-10-10 14:25 ` Catalin Marinas 2023-10-10 15:05 ` Jason Gunthorpe 2023-10-10 15:05 ` Jason Gunthorpe 2023-10-10 17:19 ` Catalin Marinas 2023-10-10 17:19 ` Catalin Marinas 2023-10-10 18:23 ` Jason Gunthorpe 2023-10-10 18:23 ` Jason Gunthorpe 2023-10-11 17:45 ` Catalin Marinas 2023-10-11 17:45 ` Catalin Marinas 2023-10-11 18:38 ` Jason Gunthorpe 2023-10-11 18:38 ` Jason Gunthorpe 2023-10-12 16:16 ` Catalin Marinas 2023-10-12 16:16 ` Catalin Marinas 2024-03-10 3:49 ` Ankit Agrawal 2024-03-10 3:49 ` Ankit Agrawal 2024-03-19 13:38 ` Jason Gunthorpe 2024-03-19 13:38 ` Jason Gunthorpe 2023-10-23 13:20 ` Shameerali Kolothum Thodi 2023-10-23 13:20 ` Shameerali Kolothum Thodi 2023-09-07 18:14 ` [PATCH v1 2/2] KVM: arm64: allow the VM to select DEVICE_* and NORMAL_NC for IO memory ankita 2023-09-07 18:14 ` ankita 2023-09-08 16:40 ` Catalin Marinas 2023-09-08 16:40 ` Catalin Marinas 2023-09-11 14:57 ` Lorenzo Pieralisi 2023-09-11 14:57 ` Lorenzo Pieralisi 2023-09-11 17:20 ` Jason Gunthorpe 2023-09-11 17:20 ` Jason Gunthorpe 2023-09-13 15:26 ` Lorenzo Pieralisi 2023-09-13 15:26 ` Lorenzo Pieralisi 2023-09-13 18:54 ` Jason Gunthorpe 2023-09-13 18:54 ` Jason Gunthorpe 2023-09-26 8:31 ` Lorenzo Pieralisi 2023-09-26 8:31 ` Lorenzo Pieralisi 2023-09-26 12:25 ` Jason Gunthorpe 2023-09-26 12:25 ` Jason Gunthorpe 2023-09-26 13:52 ` Catalin Marinas 2023-09-26 13:52 ` Catalin Marinas 2023-09-26 16:12 ` Lorenzo Pieralisi 2023-09-26 16:12 ` Lorenzo Pieralisi 2023-10-05 9:56 ` Lorenzo Pieralisi 2023-10-05 9:56 ` Lorenzo Pieralisi 2023-10-05 11:56 ` Jason Gunthorpe 2023-10-05 11:56 ` Jason Gunthorpe 2023-10-05 14:08 ` Lorenzo Pieralisi 2023-10-05 14:08 ` Lorenzo Pieralisi 2023-10-12 12:35 ` Will Deacon 2023-10-12 12:35 ` Will Deacon 2023-10-12 13:20 ` Jason Gunthorpe 2023-10-12 13:20 ` Jason Gunthorpe 2023-10-12 14:29 ` Lorenzo Pieralisi 2023-10-12 14:29 ` Lorenzo Pieralisi 2023-10-12 13:53 ` Catalin Marinas 2023-10-12 13:53 ` Catalin Marinas 2023-10-12 14:48 ` Will Deacon 2023-10-12 14:48 ` Will Deacon 2023-10-12 15:44 ` Jason Gunthorpe 2023-10-12 15:44 ` Jason Gunthorpe 2023-10-12 16:39 ` Will Deacon 2023-10-12 16:39 ` Will Deacon 2023-10-12 18:36 ` Jason Gunthorpe [this message] 2023-10-12 18:36 ` Jason Gunthorpe 2023-10-13 9:29 ` Will Deacon 2023-10-13 9:29 ` Will Deacon 2023-10-12 17:26 ` Catalin Marinas 2023-10-12 17:26 ` Catalin Marinas 2023-10-13 9:29 ` Will Deacon 2023-10-13 9:29 ` Will Deacon 2023-10-13 13:08 ` Catalin Marinas 2023-10-13 13:08 ` Catalin Marinas 2023-10-13 13:45 ` Jason Gunthorpe 2023-10-13 13:45 ` Jason Gunthorpe 2023-10-19 11:07 ` Catalin Marinas 2023-10-19 11:07 ` Catalin Marinas 2023-10-19 11:51 ` Jason Gunthorpe 2023-10-19 11:51 ` Jason Gunthorpe 2023-10-20 11:21 ` Catalin Marinas 2023-10-20 11:21 ` Catalin Marinas 2023-10-20 11:47 ` Jason Gunthorpe 2023-10-20 11:47 ` Jason Gunthorpe 2023-10-20 14:03 ` Lorenzo Pieralisi 2023-10-20 14:03 ` Lorenzo Pieralisi 2023-10-20 14:28 ` Jason Gunthorpe 2023-10-20 14:28 ` Jason Gunthorpe 2023-10-19 13:35 ` Lorenzo Pieralisi 2023-10-19 13:35 ` Lorenzo Pieralisi 2023-10-13 15:28 ` Lorenzo Pieralisi 2023-10-13 15:28 ` Lorenzo Pieralisi 2023-10-19 11:12 ` Catalin Marinas 2023-10-19 11:12 ` Catalin Marinas 2023-11-09 15:34 ` Lorenzo Pieralisi 2023-11-09 15:34 ` Lorenzo Pieralisi 2023-11-10 14:26 ` Jason Gunthorpe 2023-11-10 14:26 ` Jason Gunthorpe 2023-11-13 0:42 ` Lorenzo Pieralisi 2023-11-13 0:42 ` Lorenzo Pieralisi 2023-11-13 17:41 ` Catalin Marinas 2023-11-13 17:41 ` Catalin Marinas 2023-10-12 12:27 ` Will Deacon 2023-10-12 12:27 ` Will Deacon
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20231012183624.GN3952@nvidia.com \ --to=jgg@nvidia.com \ --cc=acurrid@nvidia.com \ --cc=aniketa@nvidia.com \ --cc=ankita@nvidia.com \ --cc=apopple@nvidia.com \ --cc=catalin.marinas@arm.com \ --cc=cjia@nvidia.com \ --cc=danw@nvidia.com \ --cc=jhubbard@nvidia.com \ --cc=kvmarm@lists.linux.dev \ --cc=kwankhede@nvidia.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lpieralisi@kernel.org \ --cc=maz@kernel.org \ --cc=oliver.upton@linux.dev \ --cc=targupta@nvidia.com \ --cc=vsethi@nvidia.com \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.