Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [LINUX PATCH] dma-mapping: Control memset operation using gfp flags
From: Dylan Yip @ 2019-09-20  2:49 UTC (permalink / raw)
  To: satishna, linux-arm-kernel, linux-kernel, iommu; +Cc: Dylan Yip

In case of 4k video buffer, the allocation from a reserved memory is
taking a long time, ~500ms. This is root caused to the memset()
operations on the allocated memory which is consuming more cpu cycles.
Due to this delay, we see that initial frames are being dropped.

To fix this, we have wrapped the default memset, done when allocating
coherent memory, under the __GFP_ZERO flag. So, we only clear
allocated memory if __GFP_ZERO flag is enabled. We believe this
should be safe as the video decoder always writes before reading.
This optimizes decoder initialization as we do not set the __GFP_ZERO
flag when allocating memory for decoder. With this optimization, we
don't see initial frame drops and decoder initialization time is
~100ms.

This patch adds plumbing through dma_alloc functions to pass gfp flag
set by user to __dma_alloc_from_coherent(). Here gfp flag is checked
for __GFP_ZERO. If present, we memset the buffer to 0 otherwise we
skip memset.

Signed-off-by: Dylan Yip <dylan.yip@xilinx.com>
---
 arch/arm/mm/dma-mapping-nommu.c |  2 +-
 include/linux/dma-mapping.h     | 11 +++++++----
 kernel/dma/coherent.c           | 15 +++++++++------
 kernel/dma/mapping.c            |  2 +-
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
index 52b8255..242b2c3 100644
--- a/arch/arm/mm/dma-mapping-nommu.c
+++ b/arch/arm/mm/dma-mapping-nommu.c
@@ -35,7 +35,7 @@ static void *arm_nommu_dma_alloc(struct device *dev, size_t size,
 				 unsigned long attrs)
 
 {
-	void *ret = dma_alloc_from_global_coherent(size, dma_handle);
+	void *ret = dma_alloc_from_global_coherent(size, dma_handle, gfp);
 
 	/*
 	 * dma_alloc_from_global_coherent() may fail because:
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index f7d1eea..b715c9f 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -160,24 +160,27 @@ static inline int is_device_dma_capable(struct device *dev)
  * Don't use them in device drivers.
  */
 int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
-				       dma_addr_t *dma_handle, void **ret);
+				       dma_addr_t *dma_handle, void **ret,
+				       gfp_t flag);
 int dma_release_from_dev_coherent(struct device *dev, int order, void *vaddr);
 
 int dma_mmap_from_dev_coherent(struct device *dev, struct vm_area_struct *vma,
 			    void *cpu_addr, size_t size, int *ret);
 
-void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle);
+void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle,
+				     gfp_t flag);
 int dma_release_from_global_coherent(int order, void *vaddr);
 int dma_mmap_from_global_coherent(struct vm_area_struct *vma, void *cpu_addr,
 				  size_t size, int *ret);
 
 #else
-#define dma_alloc_from_dev_coherent(dev, size, handle, ret) (0)
+#define dma_alloc_from_dev_coherent(dev, size, handle, ret, flag) (0)
 #define dma_release_from_dev_coherent(dev, order, vaddr) (0)
 #define dma_mmap_from_dev_coherent(dev, vma, vaddr, order, ret) (0)
 
 static inline void *dma_alloc_from_global_coherent(ssize_t size,
-						   dma_addr_t *dma_handle)
+						   dma_addr_t *dma_handle,
+						   gfp_t flag)
 {
 	return NULL;
 }
diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
index 29fd659..d85fab5 100644
--- a/kernel/dma/coherent.c
+++ b/kernel/dma/coherent.c
@@ -136,7 +136,7 @@ void dma_release_declared_memory(struct device *dev)
 EXPORT_SYMBOL(dma_release_declared_memory);
 
 static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
-		ssize_t size, dma_addr_t *dma_handle)
+		ssize_t size, dma_addr_t *dma_handle, gfp_t gfp_flag)
 {
 	int order = get_order(size);
 	unsigned long flags;
@@ -158,7 +158,8 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
 	*dma_handle = mem->device_base + (pageno << PAGE_SHIFT);
 	ret = mem->virt_base + (pageno << PAGE_SHIFT);
 	spin_unlock_irqrestore(&mem->spinlock, flags);
-	memset(ret, 0, size);
+	if (gfp_flag & __GFP_ZERO)
+		memset(ret, 0, size);
 	return ret;
 err:
 	spin_unlock_irqrestore(&mem->spinlock, flags);
@@ -172,6 +173,7 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
  * @dma_handle:	This will be filled with the correct dma handle
  * @ret:	This pointer will be filled with the virtual address
  *		to allocated area.
+ * @flag:      gfp flag set by user
  *
  * This function should be only called from per-arch dma_alloc_coherent()
  * to support allocation from per-device coherent memory pools.
@@ -180,24 +182,25 @@ static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
  * generic memory areas, or !0 if dma_alloc_coherent should return @ret.
  */
 int dma_alloc_from_dev_coherent(struct device *dev, ssize_t size,
-		dma_addr_t *dma_handle, void **ret)
+		dma_addr_t *dma_handle, void **ret, gfp_t flag)
 {
 	struct dma_coherent_mem *mem = dev_get_coherent_memory(dev);
 
 	if (!mem)
 		return 0;
 
-	*ret = __dma_alloc_from_coherent(mem, size, dma_handle);
+	*ret = __dma_alloc_from_coherent(mem, size, dma_handle, flag);
 	return 1;
 }
 
-void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle)
+void *dma_alloc_from_global_coherent(ssize_t size, dma_addr_t *dma_handle,
+				     gfp_t flag)
 {
 	if (!dma_coherent_default_memory)
 		return NULL;
 
 	return __dma_alloc_from_coherent(dma_coherent_default_memory, size,
-			dma_handle);
+			dma_handle, flag);
 }
 
 static int __dma_release_from_coherent(struct dma_coherent_mem *mem,
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index b0038ca..bfea1d2 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -272,7 +272,7 @@ void *dma_alloc_attrs(struct device *dev, size_t size, dma_addr_t *dma_handle,
 
 	WARN_ON_ONCE(!dev->coherent_dma_mask);
 
-	if (dma_alloc_from_dev_coherent(dev, size, dma_handle, &cpu_addr))
+	if (dma_alloc_from_dev_coherent(dev, size, dma_handle, &cpu_addr, flag))
 		return cpu_addr;
 
 	/* let the implementation decide on the zone to allocate from: */
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* RE: [EXT] [PATCH v4 0/3] Fix UART DMA freezes for i.MX SOCs
From: Andy Duan @ 2019-09-20  2:44 UTC (permalink / raw)
  To: Philipp Puschmann, linux-kernel@vger.kernel.org
  Cc: festevam@gmail.com, s.hauer@pengutronix.de, vkoul@kernel.org,
	dl-linux-imx, kernel@pengutronix.de, dan.j.williams@intel.com,
	Robin Gong, shawnguo@kernel.org, dmaengine@vger.kernel.or,
	linux-arm-kernel@lists.infradead.org, l.stach@pengutronix.de
In-Reply-To: <20190919142942.12469-1-philipp.puschmann@emlix.com>

From: Philipp Puschmann <philipp.puschmann@emlix.com> Sent: Thursday, September 19, 2019 10:30 PM
> For some years and since many kernel versions there are reports that RX
> UART DMA channel stops working at one point. So far the usual workaround
> was to disable RX DMA. This patches fix the underlying problem.
> 
> When a running sdma script does not find any usable destination buffer to put
> its data into it just leads to stopping the channel being scheduled again. As
> solution we manually retrigger the sdma script for this channel and by this
> dissolve the freeze.
> 
> While this seems to work fine so far, it may come to buffer overruns when the
> channel - even temporary - is stopped. This case has to be addressed by
> device drivers by increasing the number of DMA periods.
> 
> This patch series was tested with the current kernel and backported to kernel
> 4.15 with a special use case using a WL1837MOD via UART and provoking the
> hanging of UART RX DMA within seconds after starting a test application. It
> resulted in well known
>   "Bluetooth: hci0: command 0x0408 tx timeout"
> errors and complete stop of UART data reception. Our Bluetooth traffic
> consists of many independent small packets, mostly only a few bytes, causing
> high usage of periods.
> 
> Changelog v4:
>  - fixed the fixes tags
> 
> Changelog v3:
>  - fixes typo in dma_wmb
>  - add fixes tags
> 
> Changelog v2:
>  - adapt title (this patches are not only for i.MX6)
>  - improve some comments and patch descriptions
>  - add a dma_wb() around BD_DONE flag
>  - add Reviewed-by tags
>  - split off  "serial: imx: adapt rx buffer and dma periods"
> 
> Philipp Puschmann (3):
>   dmaengine: imx-sdma: fix buffer ownership
>   dmaengine: imx-sdma: fix dma freezes
>   dmaengine: imx-sdma: drop redundant variable
> 
>  drivers/dma/imx-sdma.c | 32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)
> 
> --
> 2.23.0

The patch set look fine that is really to fix some corner issue from the logical view.

Reviewed-by: Fugang Duan <fugang.duan@nxp.com>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [tech-privileged] [RFC PATCH V1] riscv-privileged: Add broadcast mode to sfence.vma
From: Benjamin Herrenschmidt @ 2019-09-20  2:27 UTC (permalink / raw)
  To: Andrew Waterman, Guo Ren
  Cc: julien.thierry, catalin.marinas, palmer, will.deacon, Atish.Patra,
	julien.grall, gary, linux-riscv, kvmarm, jean-philippe,
	linux-csky, rppt, Guo Ren, jacob.jun.pan, tech-privileged,
	marc.zyngier, linux-arm-kernel, feiteng_li, Anup.Patel,
	linux-kernel, iommu, dwmw2
In-Reply-To: <CA++6G0AzDs7w3gjjT4tDZjWBiMPf4Bvd36Ak0xdzfcJdPriKiw@mail.gmail.com>

On Thu, 2019-09-19 at 09:04 -0700, Andrew Waterman wrote:
> This needs to be discussed and debated at length; proposing edits to
> the spec at this stage is putting the cart before the horse!
> 
> We shouldn’t change the definition of the existing SFENCE.VMA
> instruction to accomplish this. It’s also not abundantly clear to me
> that this should be an instruction: TLB shootdown looks more like
> MMIO.

Quite a few points to make here:

 - TLB shootdown as MMIO is a problem when you start trying to do it
directly from guests (which is very desirable). I can elaborate if you
want, but it generally boils down to having a pile of copies of the
MMIO resources to assign to guests and having to constantly change
mappings which is unrealistic.

 - I generally have very serious doubts as to the value of doing
broadcast TLB shootdowns in HW. I went at lenght about it during Linux
Plumbers Conference, but I'll try to summarise here, from my experience
back at IBM working on POWER. In no special order:

   * It doesn't scale well. You have to drain all the target CPU queues
of already translated load stores, keep ordering, etc... it causes a
giant fabric traffic jam. Experience has shown that on some POWER
systems, it becomes extremely expensive.

   * Some OS such as Linux track which CPU has seen a given context.
That allows to "target" TLB invalidations in a more sensible way.
Broadcast instructions tend to lose that ability (it's hard to do in HW
esp. in a way that can be virtualized properly).

   * Because those instructions can take a very long time (for the
above reasons and some of below ones), or at least whatever context
synchronizing instruction that follow which waits for completion of the
invalidations, you end up with a CPU effectively "stuck" for a long
time, not taking interrupts, including those routed to higher priority
levels (ie. hypervisor etc...). This is problematic. A completion
polling mechanism is preferable so that once can still handle such work
while waiting but is hard to architect/implement properly when done as
"instructions" since they can happen concurrently from multiple
contexts. It's easier with MMIO but that has other issues.

   * It introduces races with anything that does SW walk of page
tables. For example MMIO emulation by a hypervisor cannot be done race-
free if the guest can do its own broadcast invalidations and the
hypervisor has to walk the guest page tables to translate. I can
elaborate if requested.

   * Those invalidations need to also target nest agents that can hold
translations, such as IOMMUs that can operate in usr contexts etc...
Such IOMMUs can take a VERY LONG time to process invalidations,
especially if translations have been checked out by devices such as
PCIe devices using ATS. Some GPUs for example can hit a worst case of
hundreds of *milliseconds* to process a TLB invalidation.

 - Now for the proposed scheme. I really don't like introducing a *new*
way of tagging an address space using the PPN. It's a hack. The right
way is to ensure that the existing context tags are big enough to not
require re-use and thus can be treated as global context tags by the
hypervisor and OS. IE. have big enough VMIDs and ASIDs that each
running VM can have a global stable VMID and each process within a
given VM can have a global (within that VM) ASID instead of playing
reallocation of ASID tricks at context switch (that's very inefficient
anyway, so that should be fixed for anything that claims performance
and scalability).

Now all of these things can probably have solutions but experience
doesn't seem to indicate that it's really worthwhile. We are better off
making sure we have a really fast IPI path to perform those via
interrupts and locally to the targetted CPUs IMHO.

Cheers,
Ben.

> 
> On Thu, Sep 19, 2019 at 5:36 AM Guo Ren <guoren@kernel.org> wrote:
> > From: Guo Ren <ren_guo@c-sky.com>
> > 
> > The patch is for https://github.com/riscv/riscv-isa-manual
> > 
> > The proposal has been talked in LPC-2019 RISC-V MC ref [1]. Here is
> > the
> > formal patch.
> > 
> > Introduction
> > ============
> > 
> > Using the Hardware TLB broadcast invalidation instruction to
> > maintain the
> > system TLB is a good choice and it'll simplify the system software
> > design.
> > The proposal hopes to add a broadcast mode to the sfence.vma in the
> > riscv-privilege specification. To support the sfence.vma broadcast
> > mode,
> > there are two modification introduced below:
> > 
> >  1) Add PGD.PPN (root page table's PPN) as the unique identifier of
> > the
> >     address space in addition to asid/vmid. Compared to the
> > dynamically
> >     changed asid/vmid, PGD.PPN is fixed throughout the address
> > space life
> >     cycle. This feature enables uniform address space
> > identification
> >     between different TLB systems (actually, it's difficult to
> > unify the
> >     asid/vmid between the CPU system and the IOMMU system, because
> > their
> >     mechanisms are different)
> > 
> >  2) Modify the definition of the sfence.vma instruction from
> > synchronous
> >     mode to asynchronous mode, which means that the completion of
> > the TLB
> >     operation is not guaranteed when the sfence.vma instruction
> > retires.
> >     It needs to be completed by checking the flag bit on the hart.
> > The
> >     sfence.vma request finish can notify the software by generating
> > an
> >     interrupt. This function alleviates the large delay of TLB
> > invalidation
> >     in the PCI ATS system.
> > 
> > Add S1/S2.PGD.PPN for ASID/VMID
> > ===============================
> > 
> > PGD is global directory (defined in linux) and PPN is page physical
> > number
> > (defined in riscv-spec). PGD.PNN corresponds to the root page table
> > pointer
> > of the address space, i.e. mm->pgd (linux concept).
> > 
> > In CPU/IOMMU TLB, we use asid/vmid to distinguish the address space
> > of
> > process or virtual machine. Due to the limitation of id encoding,
> > it can
> > only represent a part(window) of the address space. S1/S2.PGD.PPN
> > are the
> > root page table's PPNs of the address spaces and S1/S2.PGD.PPN are
> > the
> > unique identifier of the address spaces.
> > 
> > For the CPU SMP system, you can use context switch to perform the
> > necessary
> > software mechanism to ensure that the asid/vmid on all harts is
> > consistent
> > (please refer to the arm64 asid mechanism). In this way, the TLB
> > broadcast
> > invalidation instruction can determine the address space processed
> > on all
> > harts by asid/vmid.
> > 
> > Different from the CPU SMP system, there is no context switch for
> > the
> > DMA-IOMMU system, so the unification with the CPU asid/vmid cannot
> > be
> > guaranteed. So we need a unique identifier for the address space to
> > establish a communication bridge between the TLBs of different
> > systems.
> > 
> > That is PGD.PPN (for virtualization scenarios: S1/S2.PGD.PPN)
> > 
> > current:
> >  sfence.vma  rs1 = vaddr, rs2 = asid
> >  hfence.vvma rs1 = vaddr, rs2 = asid
> >  hfence.gvma rs1 = gaddr, rs2 = vmid
> > 
> > proposed:
> >  sfence.vma  rs1 = vaddr, rs2 = mode:ppn:asid
> >  hfence.vvma rs1 = vaddr, rs2 = mode:ppn:asid
> >  hfence.gvma rs1 = gaddr, rs2 = mode:ppn:vmid
> > 
> >  mode      - broadcast | local
> >  ppn       - the PPN of the address space of the root page table
> >  vmid/asid - the window identifier of the address space
> > 
> > At the Linux Plumber Conference 2019 RISCV-MC, ref:[1], we've
> > showed two
> > IOMMU examples to explain how it work with hardware.
> > 
> > 1) In a lightweight IOMMU system (up to 64 address spaces), the
> > hardware
> >    could directly convert PGD.PPN into DID (IOMMU ASID)
> > 
> > 2) For the PCI ATS scenario, its IO ASID/VMID encoding space can
> > support
> >    a very large number of address spaces. We use two reverse
> > mapping
> >    tables to let the hardware translate S1/S2.PGD.PPN into IO
> > ASID/VMID.
> > 
> > ASYNC BROADCAST SFENCE.VMA
> > ===========================
> > 
> > To support the high latency broadcast sfence.vma operation in the
> > PCI ATS
> > usage scenario, we modify the sfence.vma from synchronous mode to
> > asynchronous mode. (For simpler implementation, if hardware only
> > implement
> > synchronous mode and software still work in asynchronous mode)
> > 
> > To implement the asynchronous mode, 3 features are added:
> >  1) sstatus:TLBI
> >     A "status bit - TLBI" is added to the sstatus register. The
> > TLBI status
> >     bit indicates if there are still outstanding sfence.vma
> > requests on the
> >     current hart.
> >     Value:
> >       1: sfence.vma requests are not completed.
> >       0: all sfece.vma requests completed, request queue is empty.
> > 
> >  2) sstatus:TLBIC
> >     A "control bits - TLBIC" is added to sstatus register. The
> > TLBIC control
> >     bits are controlled by software.
> >     "Write 1" will trigger the current hart check to see if there
> > are still
> >     outstanding sfence.vma requests. If there are unfinished
> > requests, an
> >     interrupt will be generated when the request is completed,
> > notifying the
> >     software that all of the current sfence.vma requests have been
> > completed.
> >     "Write 0" will cause nothing.
> > 
> >  3) supervisor interrupt register (sip & sie):TLBI finish interrupt
> >     A per-hart interrupt is added to supervisor interrupt
> > registers.
> >     When all sfence.vma requests are completed and sstatus:TLBIC
> > has been
> >     triggered, hart will receive a TLBI finish interrupt. Just like
> > timer,
> >     software and external interrupt's definition in sip & sie.
> > 
> > Fake code:
> > 
> > flush_tlb_page(vma, addr) {
> >     asid = cpu_asid(vma->vm_mm);
> >     ppn = PFN_DOWN(vma->vm_mm->pgd);
> > 
> >     sfence.vma (addr, 1|PPN_OFFSET(ppn)|asid); //1. start request
> > 
> >     while(sstatus:TLBI) if (time_out() > 1ms) break; //2. loop
> > check
> > 
> >     while (sstatus:TLBI) {
> >         ...
> >         set sstatus:TLBIC;
> >         wait_TLBI_finish_interrupt(); //3. wait irq, io_schedule
> >     }
> > }
> > 
> > Here we give 2 level check:
> >  1) loop check sstatus:TLBI, CPU could response Interrupt.
> >  2) set sstatus:TLBIC and wait for irq, CPU schedule out for other
> > task.
> > 
> > ACE-DVM Example
> > ===============
> > 
> > Honestly, "broadcasting addr, asid, vmid, S1/S2.PGD.PPN to
> > interconnects"
> > and "ASYNC SFENCE.VMA" could be implemented by ACE-DVM protocol ref
> > [2].
> > 
> > There are 3 types of transactions in DVM:
> > 
> >  - DVM operation
> >    Send all information to the interconnect, including addr, asid,
> >    S1.PGD.PPN, vmid, S2.PGD.PPN.
> > 
> >  - DVM synchronization
> >    Check that all DVM operations have been completed. If not, it
> > will use
> >    state machine to wait DVM complete requests.
> > 
> >  - DVM complete
> >    Return transaction from components, eg: IOMMU. If hart has
> > received all
> >    DVM completes which are triggered by sfence.vma instructions and
> >    "sstatus:TLBIC" has been set, a TLBI finish interrupt is
> > triggered.
> > 
> > (Actually, we do not need to implement the above functions strictly
> >  according to the ACE specification :P )
> > 
> >  1: https://www.linuxplumbersconf.org/event/4/contributions/307/
> >  2: AMBA AXI and ACE Protocol Specification - Distributed Virtual
> > Memory
> >     Transactions"
> > 
> > Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> > Reviewed-by: Li Feiteng <feiteng_li@c-sky.com>
> > ---
> >  src/hypervisor.tex |  43 ++++++++-------
> >  src/supervisor.tex | 155
> > +++++++++++++++++++++++++++++++++++++++++------------
> >  2 files changed, 143 insertions(+), 55 deletions(-)
> > 
> > diff --git a/src/hypervisor.tex b/src/hypervisor.tex
> > index 47b90b2..3718819 100644
> > --- a/src/hypervisor.tex
> > +++ b/src/hypervisor.tex
> > @@ -1094,15 +1094,15 @@ The hypervisor extension adds two new
> > privileged fence instructions.
> >  \multicolumn{1}{c|}{opcode} \\
> >  \hline
> >  7 & 5 & 5 & 3 & 5 & 7 \\
> > -HFENCE.GVMA & vmid & gaddr & PRIV & 0 & SYSTEM \\
> > -HFENCE.VVMA & asid & vaddr & PRIV & 0 & SYSTEM \\
> > +HFENCE.GVMA & mode:ppn:vmid & gaddr & PRIV & 0 & SYSTEM \\
> > +HFENCE.VVMA & mode:ppn:asid & vaddr & PRIV & 0 & SYSTEM \\
> >  \end{tabular}
> >  \end{center}
> > 
> >  The hypervisor memory-management fence instructions, HFENCE.GVMA
> > and
> >  HFENCE.VVMA, are valid only in HS-mode when {\tt mstatus}.TVM=0,
> > or in M-mode
> >  (irrespective of {\tt mstatus}.TVM).
> > -These instructions perform a function similar to SFENCE.VMA
> > +These instructions perform a function similar to SFENCE.VMA
> > (broadcast/local)
> >  (Section~\ref{sec:sfence.vma}), except applying to the guest-
> > physical
> >  memory-management data structures controlled by CSR {\tt hgatp}
> > (HFENCE.GVMA)
> >  or the VS-level memory-management data structures controlled by
> > CSR {\tt vsatp}
> > @@ -1136,11 +1136,10 @@ An HFENCE.VVMA instruction applies only to
> > a single virtual machine, identified
> >  by the setting of {\tt hgatp}.VMID when HFENCE.VVMA executes.
> >  \end{commentary}
> > 
> > -When {\em rs2}$\neq${\tt x0}, bits XLEN-1:ASIDMAX of the value
> > held in {\em
> > -rs2} are reserved for future use and should be zeroed by software
> > and ignored
> > -by current implementations.
> > -Furthermore, if ASIDLEN~$<$~ASIDMAX, the implementation shall
> > ignore bits
> > -ASIDMAX-1:ASIDLEN of the value held in {\em rs2}.
> > +When {\em rs2}$\neq${\tt x0}, bits contain 3 informations: mode,
> > ppn, asid.
> > +1) mode control HFENCE.VVMA broadcast or not.
> > +2) ppn is the root page talbe's PPN of the asid address space.
> > +3) asid is the identifier of process in virtual machine.
> > 
> >  \begin{commentary}
> >  Simpler implementations of HFENCE.VVMA can ignore the guest
> > virtual address in
> > @@ -1168,11 +1167,10 @@ physical addresses in PMP address registers
> > (Section~\ref{sec:pmp}) and in page
> >  table entries (Sections \ref{sec:sv32}, \ref{sec:sv39},
> > and~\ref{sec:sv48}).
> >  \end{commentary}
> > 
> > -When {\em rs2}$\neq${\tt x0}, bits XLEN-1:VMIDMAX of the value
> > held in {\em
> > -rs2} are reserved for future use and should be zeroed by software
> > and ignored
> > -by current implementations.
> > -Furthermore, if VMIDLEN~$<$~VMIDMAX, the implementation shall
> > ignore bits
> > -VMIDMAX-1:VMIDLEN of the value held in {\em rs2}.
> > +When {\em rs2}$\neq${\tt x0}, bits contain 3 informations: mode,
> > vmid, ppn.
> > +1) mode control HFENCE.GVMA broadcast or not.
> > +2) ppn is the root page talbe's PPN of the vmid address space.
> > +3) vmid is the identifier of virtual machine.
> > 
> >  \begin{commentary}
> >  Simpler implementations of HFENCE.GVMA can ignore the guest
> > physical address in
> > @@ -1567,21 +1565,22 @@ register.
> >  \subsection{Memory-Management Fences}
> > 
> >  The behavior of the SFENCE.VMA instruction is affected by the
> > current
> > -virtualization mode V.  When V=0, the virtual-address argument is
> > an HS-level
> > -virtual address, and the ASID argument is an HS-level ASID.
> > +virtualization mode V.  When V=0, the rs1 argument is an HS-level
> > +virtual address, and the rs2 argument is an HS-level ASID and root
> > page table's PPN.
> >  The instruction orders stores only to HS-level address-translation 
> > structures
> >  with subsequent HS-level address translations.
> > 
> > -When V=1, the virtual-address argument to SFENCE.VMA is a guest
> > virtual
> > -address within the current virtual machine, and the ASID argument
> > is a VS-level
> > -ASID within the current virtual machine.
> > +When V=1, the rs1 argument to SFENCE.VMA is a guest virtual
> > +address within the current virtual machine, and the rs2 argument
> > is a VS-level
> > +ASID and root page table's PPN within the current virtual machine.
> >  The current virtual machine is identified by the VMID field of CSR
> > {\tt hgatp},
> > -and the effective ASID can be considered to be the combination of
> > this VMID
> > -with the VS-level ASID.
> > +and the effective ASID and root page table's PPN can be considered
> > to be the
> > +combination of this VMID and root page table's PPN with the VS-
> > level ASID and
> > +root page table's PPN.
> >  The SFENCE.VMA instruction orders stores only to the VS-level
> >  address-translation structures with subsequent VS-level address
> > translations
> > -for the same virtual machine, i.e., only when {\tt hgatp}.VMID is
> > the same as
> > -when the SFENCE.VMA executed.
> > +for the same virtual machine, i.e., only when {\tt hgatp}.VMID and
> > {\\tt hgatp}.PPN is
> > +the same as when the SFENCE.VMA executed.
> > 
> >  Hypervisor instructions HFENCE.GVMA and HFENCE.VVMA provide
> > additional
> >  memory-management fences to complement SFENCE.VMA.
> > diff --git a/src/supervisor.tex b/src/supervisor.tex
> > index ba3ced5..2877b7a 100644
> > --- a/src/supervisor.tex
> > +++ b/src/supervisor.tex
> > @@ -47,10 +47,12 @@ register keeps track of the processor's current
> > operating state.
> >  \begin{center}
> >  \setlength{\tabcolsep}{4pt}
> >  \scalebox{0.95}{
> > -\begin{tabular}{cWcccccWccccWcc}
> > +\begin{tabular}{cccWcccccWccccWcc}
> >  \\
> >  \instbit{31} &
> > -\instbitrange{30}{20} &
> > +\instbit{30} &
> > +\instbit{29} &
> > +\instbitrange{28}{20} &
> >  \instbit{19} &
> >  \instbit{18} &
> >  \instbit{17} &
> > @@ -66,6 +68,8 @@ register keeps track of the processor's current
> > operating state.
> >  \instbit{0} \\
> >  \hline
> >  \multicolumn{1}{|c|}{SD} &
> > +\multicolumn{1}{|c|}{TLBI} &
> > +\multicolumn{1}{|c|}{TLBIC} &
> >  \multicolumn{1}{c|}{\wpri} &
> >  \multicolumn{1}{c|}{MXR} &
> >  \multicolumn{1}{c|}{SUM} &
> > @@ -82,7 +86,7 @@ register keeps track of the processor's current
> > operating state.
> >  \multicolumn{1}{c|}{\wpri}
> >  \\
> >  \hline
> > -1 & 11 & 1 & 1 & 1 & 2 & 2 & 4 & 1 & 1 & 1 & 1 & 3 & 1 & 1 \\
> > +1 & 1 & 1 & 10 & 1 & 1 & 1 & 2 & 2 & 4 & 1 & 1 & 1 & 1 & 3 & 1 & 1
> > \\
> >  \end{tabular}}
> >  \end{center}
> >  }
> > @@ -95,10 +99,12 @@ register keeps track of the processor's current
> > operating state.
> >  {\footnotesize
> >  \begin{center}
> >  \setlength{\tabcolsep}{4pt}
> > -\begin{tabular}{cMFScccc}
> > +\begin{tabular}{cccMFScccc}
> >  \\
> >  \instbit{SXLEN-1} &
> > -\instbitrange{SXLEN-2}{34} &
> > +\instbit{SXLEN-2} &
> > +\instbit{SXLEN-3} &
> > +\instbitrange{SXLEN-4}{34} &
> >  \instbitrange{33}{32} &
> >  \instbitrange{31}{20} &
> >  \instbit{19} &
> > @@ -107,6 +113,8 @@ register keeps track of the processor's current
> > operating state.
> >   \\
> >  \hline
> >  \multicolumn{1}{|c|}{SD} &
> > +\multicolumn{1}{|c|}{TLBI} &
> > +\multicolumn{1}{|c|}{TLBIC} &
> >  \multicolumn{1}{c|}{\wpri} &
> >  \multicolumn{1}{c|}{UXL[1:0]} &
> >  \multicolumn{1}{c|}{\wpri} &
> > @@ -115,7 +123,7 @@ register keeps track of the processor's current
> > operating state.
> >  \multicolumn{1}{c|}{\wpri} &
> >   \\
> >  \hline
> > -1 & SXLEN-35 & 2 & 12 & 1 & 1 & 1 & \\
> > +1 & 1 & 1 & SXLEN-37 & 2 & 12 & 1 & 1 & 1 & \\
> >  \end{tabular}
> >  \begin{tabular}{cWWFccccWcc}
> >  \\
> > @@ -152,6 +160,17 @@ register keeps track of the processor's
> > current operating state.
> >  \label{sstatusreg}
> >  \end{figure*}
> > 
> > +The TLBI (read-only) bit indicates that any async sfence.vma
> > operations are
> > +still pended on the hart. The value:0 means that there is no
> > sfence.vma
> > +operations pending and value:1 means that there are still
> > sfence.vma operations
> > +pending on the hart.
> > +
> > +When the sstatus:TLBIC bit is written 1, it triggers the hardware
> > to check if
> > +there are any TLB invalidate operations being pended. When all
> > operations are
> > +finished, a TLB Invalidate finish interrupt will be triggered
> > +(see Section~\ref{sipreg}). When the sstatus:TLBIC bit is written
> > 0, it will
> > +cause nothing. Reading sstatus:TLBIC bit will alaways return 0.
> > +
> >  The SPP bit indicates the privilege level at which a hart was
> > executing before
> >  entering supervisor mode.  When a trap is taken, SPP is set to 0
> > if the trap
> >  originated from user mode, or 1 otherwise.  When an SRET
> > instruction
> > @@ -329,8 +348,10 @@ SXLEN-bit read/write register containing
> > interrupt enable bits.
> >  {\footnotesize
> >  \begin{center}
> >  \setlength{\tabcolsep}{4pt}
> > -\begin{tabular}{KcFcFcc}
> > -\instbitrange{SXLEN-1}{10} &
> > +\begin{tabular}{KcFcFcFcc}
> > +\instbitrange{SXLEN-1}{14} &
> > +\instbit{13} &
> > +\instbitrange{12}{10} &
> >  \instbit{9} &
> >  \instbitrange{8}{6} &
> >  \instbit{5} &
> > @@ -339,6 +360,8 @@ SXLEN-bit read/write register containing
> > interrupt enable bits.
> >  \instbit{0} \\
> >  \hline
> >  \multicolumn{1}{|c|}{\wpri} &
> > +\multicolumn{1}{c|}{STLBIP} &
> > +\multicolumn{1}{|c|}{\wpri} &
> >  \multicolumn{1}{c|}{SEIP} &
> >  \multicolumn{1}{c|}{\wpri} &
> >  \multicolumn{1}{c|}{STIP} &
> > @@ -346,7 +369,7 @@ SXLEN-bit read/write register containing
> > interrupt enable bits.
> >  \multicolumn{1}{c|}{SSIP} &
> >  \multicolumn{1}{c|}{\wpri} \\
> >  \hline
> > -SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
> > +SXLEN-14 & 1 & 3 & 1 & 3 & 1 & 3 & 1 & 1 \\
> >  \end{tabular}
> >  \end{center}
> >  }
> > @@ -359,8 +382,10 @@ SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
> >  {\footnotesize
> >  \begin{center}
> >  \setlength{\tabcolsep}{4pt}
> > -\begin{tabular}{KcFcFcc}
> > -\instbitrange{SXLEN-1}{10} &
> > +\begin{tabular}{KcFcFcFcc}
> > +\instbitrange{SXLEN-1}{14} &
> > +\instbit{13} &
> > +\instbitrange{12}{10} &
> >  \instbit{9} &
> >  \instbitrange{8}{6} &
> >  \instbit{5} &
> > @@ -369,6 +394,8 @@ SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
> >  \instbit{0} \\
> >  \hline
> >  \multicolumn{1}{|c|}{\wpri} &
> > +\multicolumn{1}{c|}{STLBIE} &
> > +\multicolumn{1}{|c|}{\wpri} &
> >  \multicolumn{1}{c|}{SEIE} &
> >  \multicolumn{1}{c|}{\wpri} &
> >  \multicolumn{1}{c|}{STIE} &
> > @@ -376,7 +403,7 @@ SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
> >  \multicolumn{1}{c|}{SSIE} &
> >  \multicolumn{1}{c|}{\wpri} \\
> >  \hline
> > -SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
> > +SXLEN-14 & 1 & 3 & 1 & 3 & 1 & 3 & 1 & 1 \\
> >  \end{tabular}
> >  \end{center}
> >  }
> > @@ -410,6 +437,12 @@ when the SEIE bit in the {\tt sie} register is
> > clear.  The implementation
> >  should provide facilities to mask, unmask, and query the cause of
> > external
> >  interrupts.
> > 
> > +A supervisor-level TLB Invalidate finish interrupt is pending if
> > the STLBIP bit
> > +in the {\tt sip} register is set.  Supervisor-level TLB Invalidate
> > finish
> > +interrupts are disabled when the STLBIE bit in the {\tt sie}
> > register is clear.
> > +When hart tlb invalidate operations are finished, hardware will
> > change sstatus:TLBI
> > +bit from 1 to 0 and trigger TLB Invalidate finish interrupt.
> > +
> >  \begin{commentary}
> >  The {\tt sip} and {\tt sie} registers are subsets of the {\tt mip}
> > and {\tt
> >  mie} registers.  Reading any field, or writing any writable field,
> > of {\tt
> > @@ -598,7 +631,9 @@ so is only guaranteed to hold supported
> > exception codes.
> >    1         & 5               & Supervisor timer interrupt \\
> >    1         & 6--8            & {\em Reserved} \\
> >    1         & 9               & Supervisor external interrupt \\
> > -  1         & 10--15          & {\em Reserved} \\
> > +  1         & 10--11          & {\em Reserved} \\
> > +  1         & 12              & Supervisor TLBI finish interrupt
> > \\
> > +  1         & 13--15          & {\em Reserved} \\
> >    1         & $\ge$16         & {\em Available for platform use}
> > \\ \hline
> >    0         & 0               & Instruction address misaligned \\
> >    0         & 1               & Instruction access fault \\
> > @@ -884,7 +919,7 @@ provided.
> >  \multicolumn{1}{c|}{opcode} \\
> >  \hline
> >  7 & 5 & 5 & 3 & 5 & 7 \\
> > -SFENCE.VMA & asid & vaddr & PRIV & 0 & SYSTEM \\
> > +SFENCE.VMA & mode:ppn:asid & vaddr & LOCAL & 0 & SYSTEM \\
> >  \end{tabular}
> >  \end{center}
> > 
> > @@ -899,21 +934,70 @@ from that hart to the memory-management data
> > structures.
> >  Further details on the behavior of this instruction are
> >  described in Section~\ref{virt-control} and Section~\ref{pmp-
> > vmem}.
> > 
> > +SFENCE.VMA is defined as an asynchronous completion instruction,
> > which means
> > +that the TLB operation is not guaranteed to complete when the
> > instruction retires.
> > +Software need check sstatus:TLBI to determine all TLB operations
> > complete.
> > +The sstatus:TLBI described in Section~\ref{sstatus}. When hardware
> > change
> > +sstatus:TLBI bit from 1 to 0, the TLB Invalidate finish interrupt
> > will be
> > +triggered.
> > +
> >  \begin{commentary}
> > -The SFENCE.VMA is used to flush any local hardware caches related
> > to
> > +The SFENCE.VMA is used to flush any local/remote hardware caches
> > related to
> >  address translation.  It is specified as a fence rather than a TLB
> >  flush to provide cleaner semantics with respect to which
> > instructions
> >  are affected by the flush operation and to support a wider variety
> > of
> >  dynamic caching structures and memory-management schemes. 
> > SFENCE.VMA
> >  is also used by higher privilege levels to synchronize page table
> > -writes and the address translation hardware.
> > +writes and the address translation hardware. There is a mode bit
> > to determine
> > +sfence.vma would broadcast on interconnect or not.
> >  \end{commentary}
> > 
> > -SFENCE.VMA orders only the local hart's implicit references to the
> > -memory-management data structures.
> > +\begin{figure}[h!]
> > +{\footnotesize
> > +\begin{center}
> > +\begin{tabular}{c@{}E@{}K}
> > +\instbit{31} &
> > +\instbitrange{30}{9} &
> > +\instbitrange{8}{0} \\
> > +\hline
> > +\multicolumn{1}{|c|}{{\tt MODE}} &
> > +\multicolumn{1}{|c|}{{\tt PPN (root page table)}} &
> > +\multicolumn{1}{|c|}{{\tt ASID}} \\
> > +\hline
> > +1 & 22 & 9 \\
> > +\end{tabular}
> > +\end{center}
> > +}
> > +\vspace{-0.1in}
> > +\caption{RV32 sfence.vma rs2 format.}
> > +\label{rv32satp}
> > +\end{figure}
> > +
> > +\begin{figure}[h!]
> > +{\footnotesize
> > +\begin{center}
> > +\begin{tabular}{@{}S@{}T@{}U}
> > +\instbitrange{63}{60} &
> > +\instbitrange{59}{16} &
> > +\instbitrange{15}{0} \\
> > +\hline
> > +\multicolumn{1}{|c|}{{\tt MODE}} &
> > +\multicolumn{1}{|c|}{{\tt PPN (root page table)}} &
> > +\multicolumn{1}{|c|}{{\tt ASID}} \\
> > +\hline
> > +4 & 44 & 16 \\
> > +\end{tabular}
> > +\end{center}
> > +}
> > +\vspace{-0.1in}
> > +\caption{RV64 sfence.vma rs2 format, for MODE values, only highest
> > bit:63 is
> > +valid and others are reserved.}
> > +\label{rv64satp}
> > +\end{figure}
> > 
> >  \begin{commentary}
> > -Consequently, other harts must be notified separately when the
> > +The mode's highest bit could control sfence.vma behavior with
> > 1:broadcast or 0:local.
> > +If only have mode:local, other harts must be notified separately
> > when the
> >  memory-management data structures have been modified.
> >  One approach is to use 1)
> >  a local data fence to ensure local writes are visible globally,
> > then
> > @@ -928,8 +1012,17 @@ modified for a single address mapping (i.e.,
> > one page or superpage), {\em rs1}
> >  can specify a virtual address within that mapping to effect a
> > translation
> >  fence for that mapping only.  Furthermore, for the common case
> > that the
> >  translation data structures have only been modified for a single
> > address-space
> > -identifier, {\em rs2} can specify the address space.  The behavior
> > of
> > -SFENCE.VMA depends on {\em rs1} and {\em rs2} as follows:
> > +identifier, {\em rs2} can specify the address space with {\tt
> > satp} format
> > +which include asid and root page table's PPN information.
> > +
> > +\begin{commentary}
> > +We use ASID and root page table's PPN to determine address space
> > and the format
> > +stored in rs2 is similar with {\tt satp} described in
> > Section~\ref{sec:satp}.
> > +ASID are used by local harts and root page table's PPN of the asid
> > are used by
> > +other different TLB systems, eg: IOMMU.
> > +\end{commentary}
> > +
> > +The behavior of SFENCE.VMA depends on {\em rs1} and {\em rs2} as
> > follows:
> > 
> >  \begin{itemize}
> >  \item If {\em rs1}={\tt x0} and {\em rs2}={\tt x0}, the fence
> > orders all
> > @@ -939,23 +1032,18 @@ SFENCE.VMA depends on {\em rs1} and {\em
> > rs2} as follows:
> >        all reads and writes made to any level of the page tables,
> > but only
> >        for the address space identified by integer register {\em
> > rs2}.
> >        Accesses to {\em global} mappings (see
> > Section~\ref{sec:translation})
> > -      are not ordered.
> > +      are not ordered. The mode field in rs2 is determine
> > broadcast or local.
> >  \item If {\em rs1}$\neq${\tt x0} and {\em rs2}={\tt x0}, the fence
> > orders
> >        only reads and writes made to the leaf page table entry
> > corresponding
> >        to the virtual address in {\em rs1}, for all address spaces.
> >  \item If {\em rs1}$\neq${\tt x0} and {\em rs2}$\neq${\tt x0}, the
> > fence
> >        orders only reads and writes made to the leaf page table
> > entry
> >        corresponding to the virtual address in {\em rs1}, for the
> > address
> > -      space identified by integer register {\em rs2}.
> > +      space identified by integer register {\em rs2}. The mode
> > field in rs2
> > +      is determine broadcast or local.
> >        Accesses to global mappings are not ordered.
> >  \end{itemize}
> > 
> > -When {\em rs2}$\neq${\tt x0}, bits SXLEN-1:ASIDMAX of the value
> > held in {\em
> > -rs2} are reserved for future use and should be zeroed by software
> > and ignored
> > -by current implementations.  Furthermore, if ASIDLEN~$<$~ASIDMAX,
> > the
> > -implementation shall ignore bits ASIDMAX-1:ASIDLEN of the value
> > held in {\em
> > -rs2}.
> > -
> >  \begin{commentary}
> >  Simpler implementations can ignore the virtual address in {\em
> > rs1} and
> >  the ASID value in {\em rs2} and always perform a global fence.
> > @@ -994,7 +1082,7 @@ can execute the same SFENCE.VMA instruction
> > while a different ASID is loaded
> >  into {\tt satp}, provided the next time {\tt satp} is loaded with
> > the recycled
> >  ASID, it is simultaneously loaded with the new page table.
> > 
> > -\item If the implementation does not provide ASIDs, or software
> > chooses to
> > +\item If the implementation does not provide ASIDs and PPNs, or
> > software chooses to
> >  always use ASID 0, then after every {\tt satp} write, software
> > should execute
> >  SFENCE.VMA with {\em rs1}={\tt x0}.  In the common case that no
> > global
> >  translations have been modified, {\em rs2} should be set to a
> > register other than
> > @@ -1003,13 +1091,14 @@ not flushed.
> > 
> >  \item If software modifies a non-leaf PTE, it should execute
> > SFENCE.VMA with
> >  {\em rs1}={\tt x0}.  If any PTE along the traversal path had its G
> > bit set,
> > -{\em rs2} must be {\tt x0}; otherwise, {\em rs2} should be set to
> > the ASID for
> > -which the translation is being modified.
> > +{\em rs2} must be {\tt x0}; otherwise, {\em rs2} should be set to
> > the ASID and
> > +root page table's PPN for which the translation is being modified.
> > 
> >  \item If software modifies a leaf PTE, it should execute
> > SFENCE.VMA with {\em
> >  rs1} set to a virtual address within the page.  If any PTE along
> > the traversal
> >  path had its G bit set, {\em rs2} must be {\tt x0}; otherwise,
> > {\em rs2}
> > -should be set to the ASID for which the translation is being
> > modified.
> > +should be set to the ASID and root page table's PPN for which the
> > translation
> > +is being modified.
> > 
> >  \item For the special cases of increasing the permissions on a
> > leaf PTE and
> >  changing an invalid PTE to a valid leaf, software may choose to
> > execute


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH v6 3/3] mm: fix double page fault on arm64 if PTE_AF is cleared
From: Jia He @ 2019-09-20  2:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, James Morse,
	Marc Zyngier, Matthew Wilcox, Kirill A. Shutemov,
	linux-arm-kernel, linux-kernel, linux-mm, Suzuki Poulose
  Cc: Ralph Campbell, Jia He, Andrew Morton, Anshuman Khandual, Jun Yao,
	Kaly Xin, Jérôme Glisse, Punit Agrawal, hejianet,
	Thomas Gleixner, Robin Murphy, Alex Van Brunt
In-Reply-To: <20190920022132.149467-1-justin.he@arm.com>

When we tested pmdk unit test [1] vmmalloc_fork TEST1 in arm64 guest, there
will be a double page fault in __copy_from_user_inatomic of cow_user_page.

Below call trace is from arm64 do_page_fault for debugging purpose
[  110.016195] Call trace:
[  110.016826]  do_page_fault+0x5a4/0x690
[  110.017812]  do_mem_abort+0x50/0xb0
[  110.018726]  el1_da+0x20/0xc4
[  110.019492]  __arch_copy_from_user+0x180/0x280
[  110.020646]  do_wp_page+0xb0/0x860
[  110.021517]  __handle_mm_fault+0x994/0x1338
[  110.022606]  handle_mm_fault+0xe8/0x180
[  110.023584]  do_page_fault+0x240/0x690
[  110.024535]  do_mem_abort+0x50/0xb0
[  110.025423]  el0_da+0x20/0x24

The pte info before __copy_from_user_inatomic is (PTE_AF is cleared):
[ffff9b007000] pgd=000000023d4f8003, pud=000000023da9b003, pmd=000000023d4b3003, pte=360000298607bd3

As told by Catalin: "On arm64 without hardware Access Flag, copying from
user will fail because the pte is old and cannot be marked young. So we
always end up with zeroed page after fork() + CoW for pfn mappings. we
don't always have a hardware-managed access flag on arm64."

This patch fix it by calling pte_mkyoung. Also, the parameter is
changed because vmf should be passed to cow_user_page()

Add a WARN_ON_ONCE when __copy_from_user_inatomic() returns error
in case there can be some obscure use-case.(by Kirill)

[1] https://github.com/pmem/pmdk/tree/master/src/test/vmmalloc_fork

Reported-by: Yibo Cai <Yibo.Cai@arm.com>
Signed-off-by: Jia He <justin.he@arm.com>
---
 mm/memory.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 59 insertions(+), 6 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index e2bb51b6242e..7c38c1ce5440 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -118,6 +118,13 @@ int randomize_va_space __read_mostly =
 					2;
 #endif
 
+#ifndef arch_faults_on_old_pte
+static inline bool arch_faults_on_old_pte(void)
+{
+	return false;
+}
+#endif
+
 static int __init disable_randmaps(char *s)
 {
 	randomize_va_space = 0;
@@ -2140,8 +2147,12 @@ static inline int pte_unmap_same(struct mm_struct *mm, pmd_t *pmd,
 	return same;
 }
 
-static inline void cow_user_page(struct page *dst, struct page *src, unsigned long va, struct vm_area_struct *vma)
+static inline int cow_user_page(struct page *dst, struct page *src,
+				struct vm_fault *vmf)
 {
+	struct vm_area_struct *vma = vmf->vma;
+	unsigned long addr = vmf->address;
+
 	debug_dma_assert_idle(src);
 
 	/*
@@ -2151,21 +2162,52 @@ static inline void cow_user_page(struct page *dst, struct page *src, unsigned lo
 	 * fails, we just zero-fill it. Live with it.
 	 */
 	if (unlikely(!src)) {
-		void *kaddr = kmap_atomic(dst);
-		void __user *uaddr = (void __user *)(va & PAGE_MASK);
+		void *kaddr;
+		void __user *uaddr = (void __user *)(addr & PAGE_MASK);
+		pte_t entry;
+
+		/* On architectures with software "accessed" bits, we would
+		 * take a double page fault, so mark it accessed here.
+		 */
+		if (arch_faults_on_old_pte() && !pte_young(vmf->orig_pte)) {
+			spin_lock(vmf->ptl);
+			if (likely(pte_same(*vmf->pte, vmf->orig_pte))) {
+				entry = pte_mkyoung(vmf->orig_pte);
+				if (ptep_set_access_flags(vma, addr,
+							  vmf->pte, entry, 0))
+					update_mmu_cache(vma, addr, vmf->pte);
+			} else {
+				/* Other thread has already handled the fault
+				 * and we don't need to do anything. If it's
+				 * not the case, the fault will be triggered
+				 * again on the same address.
+				 */
+				spin_unlock(vmf->ptl);
+				return -1;
+			}
+			spin_unlock(vmf->ptl);
+		}
 
+		kaddr = kmap_atomic(dst);
 		/*
 		 * This really shouldn't fail, because the page is there
 		 * in the page tables. But it might just be unreadable,
 		 * in which case we just give up and fill the result with
 		 * zeroes.
 		 */
-		if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE))
+		if (__copy_from_user_inatomic(kaddr, uaddr, PAGE_SIZE)) {
+			/* Give a warn in case there can be some obscure
+			 * use-case
+			 */
+			WARN_ON_ONCE(1);
 			clear_page(kaddr);
+		}
 		kunmap_atomic(kaddr);
 		flush_dcache_page(dst);
 	} else
-		copy_user_highpage(dst, src, va, vma);
+		copy_user_highpage(dst, src, addr, vma);
+
+	return 0;
 }
 
 static gfp_t __get_fault_gfp_mask(struct vm_area_struct *vma)
@@ -2318,7 +2360,16 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
 				vmf->address);
 		if (!new_page)
 			goto oom;
-		cow_user_page(new_page, old_page, vmf->address, vma);
+
+		if (cow_user_page(new_page, old_page, vmf)) {
+			/* COW failed, if the fault was solved by other,
+			 * it's fine. If not, userspace would re-fault on
+			 * the same address and we will handle the fault
+			 * from the second attempt.
+			 */
+			put_page(new_page);
+			goto normal;
+		}
 	}
 
 	if (mem_cgroup_try_charge_delay(new_page, mm, GFP_KERNEL, &memcg, false))
@@ -2420,6 +2471,8 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
 		}
 		put_page(old_page);
 	}
+
+normal:
 	return page_copied ? VM_FAULT_WRITE : 0;
 oom_free_new:
 	put_page(new_page);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 2/3] arm64: mm: implement arch_faults_on_old_pte() on arm64
From: Jia He @ 2019-09-20  2:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, James Morse,
	Marc Zyngier, Matthew Wilcox, Kirill A. Shutemov,
	linux-arm-kernel, linux-kernel, linux-mm, Suzuki Poulose
  Cc: Ralph Campbell, Jia He, Andrew Morton, Anshuman Khandual, Jun Yao,
	Kaly Xin, Jérôme Glisse, Punit Agrawal, hejianet,
	Thomas Gleixner, Robin Murphy, Alex Van Brunt
In-Reply-To: <20190920022132.149467-1-justin.he@arm.com>

On arm64 without hardware Access Flag, copying fromuser will fail because
the pte is old and cannot be marked young. So we always end up with zeroed
page after fork() + CoW for pfn mappings. we don't always have a
hardware-managed access flag on arm64.

Hence implement arch_faults_on_old_pte on arm64 to indicate that it might
cause page fault when accessing old pte.

Signed-off-by: Jia He <justin.he@arm.com>
---
 arch/arm64/include/asm/pgtable.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index e09760ece844..4a9939615e41 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -868,6 +868,18 @@ static inline void update_mmu_cache(struct vm_area_struct *vma,
 #define phys_to_ttbr(addr)	(addr)
 #endif
 
+/*
+ * On arm64 without hardware Access Flag, copying fromuser will fail because
+ * the pte is old and cannot be marked young. So we always end up with zeroed
+ * page after fork() + CoW for pfn mappings. we don't always have a
+ * hardware-managed access flag on arm64.
+ */
+static inline bool arch_faults_on_old_pte(void)
+{
+	return !cpu_has_hw_af();
+}
+#define arch_faults_on_old_pte arch_faults_on_old_pte
+
 #endif /* !__ASSEMBLY__ */
 
 #endif /* __ASM_PGTABLE_H */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 1/3] arm64: cpufeature: introduce helper cpu_has_hw_af()
From: Jia He @ 2019-09-20  2:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, James Morse,
	Marc Zyngier, Matthew Wilcox, Kirill A. Shutemov,
	linux-arm-kernel, linux-kernel, linux-mm, Suzuki Poulose
  Cc: Ralph Campbell, Jia He, Andrew Morton, Anshuman Khandual, Jun Yao,
	Kaly Xin, Jérôme Glisse, Punit Agrawal, hejianet,
	Thomas Gleixner, Robin Murphy, Alex Van Brunt
In-Reply-To: <20190920022132.149467-1-justin.he@arm.com>

We unconditionally set the HW_AFDBM capability and only enable it on
CPUs which really have the feature. But sometimes we need to know
whether this cpu has the capability of HW AF. So decouple AF from
DBM by new helper cpu_has_hw_af().

Reported-by: kbuild test robot <lkp@intel.com>
Suggested-by: Suzuki Poulose <Suzuki.Poulose@arm.com>
Signed-off-by: Jia He <justin.he@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index c96ffa4722d3..46caf934ba4e 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -667,6 +667,16 @@ static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange)
 	default: return CONFIG_ARM64_PA_BITS;
 	}
 }
+
+/* Decouple AF from AFDBM. */
+static inline bool cpu_has_hw_af(void)
+{
+	if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM))
+		return read_cpuid(ID_AA64MMFR1_EL1) & 0xf;
+
+	return false;
+}
+
 #endif /* __ASSEMBLY__ */
 
 #endif
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* [PATCH v6 0/3] fix double page fault on arm64
From: Jia He @ 2019-09-20  2:21 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Mark Rutland, James Morse,
	Marc Zyngier, Matthew Wilcox, Kirill A. Shutemov,
	linux-arm-kernel, linux-kernel, linux-mm, Suzuki Poulose
  Cc: Ralph Campbell, Jia He, Andrew Morton, Anshuman Khandual, Jun Yao,
	Kaly Xin, Jérôme Glisse, Punit Agrawal, hejianet,
	Thomas Gleixner, Robin Murphy, Alex Van Brunt

When we tested pmdk unit test vmmalloc_fork TEST1 in arm64 guest, there
will be a double page fault in __copy_from_user_inatomic of cow_user_page.

As told by Catalin: "On arm64 without hardware Access Flag, copying from
user will fail because the pte is old and cannot be marked young. So we
always end up with zeroed page after fork() + CoW for pfn mappings. we
don't always have a hardware-managed access flag on arm64."

Changes
v6: fix error case of returning with spinlock taken (Catalin)
    move kmap_atomic to avoid handling kunmap_atomic
v5: handle the case correctly when !pte_same
    fix kbuild test failed
v4: introduce cpu_has_hw_af (Suzuki)
    bail out if !pte_same (Kirill)
v3: add vmf->ptl lock/unlock (Kirill A. Shutemov)
    add arch_faults_on_old_pte (Matthew, Catalin)
v2: remove FAULT_FLAG_WRITE when setting pte access flag (Catalin)

Jia He (3):
  arm64: cpufeature: introduce helper cpu_has_hw_af()
  arm64: mm: implement arch_faults_on_old_pte() on arm64
  mm: fix double page fault on arm64 if PTE_AF is cleared

 arch/arm64/include/asm/cpufeature.h | 10 +++++
 arch/arm64/include/asm/pgtable.h    | 12 ++++++
 mm/memory.c                         | 65 ++++++++++++++++++++++++++---
 3 files changed, 81 insertions(+), 6 deletions(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v2 04/11] devfreq: exynos-bus: Clean up code
From: Chanwoo Choi @ 2019-09-20  2:22 UTC (permalink / raw)
  To: Artur Świgoń, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-pm, dri-devel
  Cc: Artur Świgoń, b.zolnierkie, sw0312.kim, krzk, inki.dae,
	myungjoo.ham, leonard.crestez, georgi.djakov, m.szyprowski
In-Reply-To: <20190919142236.4071-5-a.swigon@samsung.com>

Hi Artur,

On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> From: Artur Świgoń <a.swigon@partner.samsung.com>
> 
> This patch adds minor improvements to the exynos-bus driver.
> 
> Signed-off-by: Artur Świgoń <a.swigon@partner.samsung.com>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  drivers/devfreq/exynos-bus.c | 66 ++++++++++++++----------------------
>  1 file changed, 25 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 60ad4319fd80..8d44810cac69 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -15,11 +15,10 @@
>  #include <linux/device.h>
>  #include <linux/export.h>
>  #include <linux/module.h>
> -#include <linux/of_device.h>
> +#include <linux/of.h>
>  #include <linux/pm_opp.h>
>  #include <linux/platform_device.h>
>  #include <linux/regulator/consumer.h>
> -#include <linux/slab.h>
>  
>  #define DEFAULT_SATURATION_RATIO	40
>  
> @@ -178,7 +177,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>  	struct device *dev = bus->dev;
>  	struct opp_table *opp_table;
>  	const char *vdd = "vdd";
> -	int i, ret, count, size;
> +	int i, ret, count;
>  
>  	opp_table = dev_pm_opp_set_regulators(dev, &vdd, 1);
>  	if (IS_ERR(opp_table)) {
> @@ -201,8 +200,7 @@ static int exynos_bus_parent_parse_of(struct device_node *np,
>  	}
>  	bus->edev_count = count;
>  
> -	size = sizeof(*bus->edev) * count;
> -	bus->edev = devm_kzalloc(dev, size, GFP_KERNEL);
> +	bus->edev = devm_kcalloc(dev, count, sizeof(*bus->edev), GFP_KERNEL);
>  	if (!bus->edev) {
>  		ret = -ENOMEM;
>  		goto err_regulator;
> @@ -301,10 +299,9 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
>  	profile->exit = exynos_bus_exit;
>  
>  	ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> -	if (!ondemand_data) {
> -		ret = -ENOMEM;
> -		goto err;
> -	}
> +	if (!ondemand_data)
> +		return -ENOMEM;
> +
>  	ondemand_data->upthreshold = 40;
>  	ondemand_data->downdifferential = 5;
>  
> @@ -314,8 +311,7 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
>  						ondemand_data);
>  	if (IS_ERR(bus->devfreq)) {
>  		dev_err(dev, "failed to add devfreq device\n");
> -		ret = PTR_ERR(bus->devfreq);
> -		goto err;
> +		return PTR_ERR(bus->devfreq);
>  	}
>  
>  	/*
> @@ -325,16 +321,13 @@ static int exynos_bus_profile_init(struct exynos_bus *bus,
>  	ret = exynos_bus_enable_edev(bus);
>  	if (ret < 0) {
>  		dev_err(dev, "failed to enable devfreq-event devices\n");
> -		goto err;
> +		return ret;
>  	}
>  
>  	ret = exynos_bus_set_event(bus);
> -	if (ret < 0) {
> +	if (ret < 0)
>  		dev_err(dev, "failed to set event to devfreq-event devices\n");
> -		goto err;

Instead of removing 'goto err', just return err as I commented[1] on v1.
[1] https://lkml.org/lkml/2019/7/26/331

> -	}
>  
> -err:
>  	return ret;

And you just keep 'return ret' or you can change it as 'return 0'.


>  }
>  
> @@ -344,7 +337,6 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
>  	struct device *dev = bus->dev;
>  	struct devfreq_passive_data *passive_data;
>  	struct devfreq *parent_devfreq;
> -	int ret = 0;
>  
>  	/* Initialize the struct profile and governor data for passive device */
>  	profile->target = exynos_bus_target;
> @@ -352,30 +344,26 @@ static int exynos_bus_profile_init_passive(struct exynos_bus *bus,
>  
>  	/* Get the instance of parent devfreq device */
>  	parent_devfreq = devfreq_get_devfreq_by_phandle(dev, 0);
> -	if (IS_ERR(parent_devfreq)) {
> -		ret = -EPROBE_DEFER;
> -		goto err;
> -	}
> +	if (IS_ERR(parent_devfreq))
> +		return -EPROBE_DEFER;
>  
>  	passive_data = devm_kzalloc(dev, sizeof(*passive_data), GFP_KERNEL);
> -	if (!passive_data) {
> -		ret = -ENOMEM;
> -		goto err;
> -	}
> +	if (!passive_data)
> +		return -ENOMEM;
> +
>  	passive_data->parent = parent_devfreq;
>  
>  	/* Add devfreq device for exynos bus with passive governor */
> -	bus->devfreq = devm_devfreq_add_device(dev, profile, DEVFREQ_GOV_PASSIVE,
> +	bus->devfreq = devm_devfreq_add_device(dev, profile,
> +						DEVFREQ_GOV_PASSIVE,
>  						passive_data);
>  	if (IS_ERR(bus->devfreq)) {
>  		dev_err(dev,
>  			"failed to add devfreq dev with passive governor\n");
> -		ret = PTR_ERR(bus->devfreq);
> -		goto err;
> +		return PTR_ERR(bus->devfreq);
>  	}
>  
> -err:
> -	return ret;
> +	return 0;
>  }
>  
>  static int exynos_bus_probe(struct platform_device *pdev)
> @@ -393,18 +381,18 @@ static int exynos_bus_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
> +	bus = devm_kzalloc(dev, sizeof(*bus), GFP_KERNEL);
>  	if (!bus)
>  		return -ENOMEM;
>  	mutex_init(&bus->lock);
> -	bus->dev = &pdev->dev;
> +	bus->dev = dev;
>  	platform_set_drvdata(pdev, bus);
>  
>  	profile = devm_kzalloc(dev, sizeof(*profile), GFP_KERNEL);
>  	if (!profile)
>  		return -ENOMEM;
>  
> -	node = of_parse_phandle(dev->of_node, "devfreq", 0);
> +	node = of_parse_phandle(np, "devfreq", 0);
>  	if (node) {
>  		of_node_put(node);
>  		passive = true;
> @@ -461,12 +449,10 @@ static int exynos_bus_resume(struct device *dev)
>  	int ret;
>  
>  	ret = exynos_bus_enable_edev(bus);
> -	if (ret < 0) {
> +	if (ret < 0)
>  		dev_err(dev, "failed to enable the devfreq-event devices\n");
> -		return ret;

Keep the 'return ret' if error happen as I commented[1] on v1.
[1] https://lkml.org/lkml/2019/7/26/331

> -	}
>  
> -	return 0;
> +	return ret;

And you just keep 'return 0' or you can change it as 'return ret'.

>  }
>  
>  static int exynos_bus_suspend(struct device *dev)
> @@ -475,12 +461,10 @@ static int exynos_bus_suspend(struct device *dev)
>  	int ret;
>  
>  	ret = exynos_bus_disable_edev(bus);
> -	if (ret < 0) {
> +	if (ret < 0)
>  		dev_err(dev, "failed to disable the devfreq-event devices\n");
> -		return ret;

Keep the 'return ret' if error happen as I commented[1] on v1.
[1] https://lkml.org/lkml/2019/7/26/331

> -	}
>  
> -	return 0;
> +	return ret;

And you just keep 'return 0' or you can change it as 'return ret'.

>  }
>  #endif
>  
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v2 01/11] devfreq: exynos-bus: Extract exynos_bus_profile_init()
From: Chanwoo Choi @ 2019-09-20  2:15 UTC (permalink / raw)
  To: Artur Świgoń, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-pm, dri-devel
  Cc: Artur Świgoń, b.zolnierkie, sw0312.kim, krzk, inki.dae,
	myungjoo.ham, leonard.crestez, georgi.djakov, m.szyprowski
In-Reply-To: <20190919142236.4071-2-a.swigon@samsung.com>

Hi,

As I already replied on v1, patch1/2/3 clean-up code
for readability without any behavior changes. 

I think that you better to merge patch1/2/3 to one patch.

On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> From: Artur Świgoń <a.swigon@partner.samsung.com>
> 
> This patch adds a new static function, exynos_bus_profile_init(), extracted
> from exynos_bus_probe().
> 
> Signed-off-by: Artur Świgoń <a.swigon@partner.samsung.com>
> ---
>  drivers/devfreq/exynos-bus.c | 92 +++++++++++++++++++++---------------
>  1 file changed, 53 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/devfreq/exynos-bus.c b/drivers/devfreq/exynos-bus.c
> index 29f422469960..78f38b7fb596 100644
> --- a/drivers/devfreq/exynos-bus.c
> +++ b/drivers/devfreq/exynos-bus.c
> @@ -287,12 +287,62 @@ static int exynos_bus_parse_of(struct device_node *np,
>  	return ret;
>  }
>  
> +static int exynos_bus_profile_init(struct exynos_bus *bus,
> +				   struct devfreq_dev_profile *profile)
> +{
> +	struct device *dev = bus->dev;
> +	struct devfreq_simple_ondemand_data *ondemand_data;
> +	int ret;
> +
> +	/* Initialize the struct profile and governor data for parent device */
> +	profile->polling_ms = 50;
> +	profile->target = exynos_bus_target;
> +	profile->get_dev_status = exynos_bus_get_dev_status;
> +	profile->exit = exynos_bus_exit;
> +
> +	ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> +	if (!ondemand_data) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +	ondemand_data->upthreshold = 40;
> +	ondemand_data->downdifferential = 5;
> +
> +	/* Add devfreq device to monitor and handle the exynos bus */
> +	bus->devfreq = devm_devfreq_add_device(dev, profile,
> +						DEVFREQ_GOV_SIMPLE_ONDEMAND,
> +						ondemand_data);
> +	if (IS_ERR(bus->devfreq)) {
> +		dev_err(dev, "failed to add devfreq device\n");
> +		ret = PTR_ERR(bus->devfreq);
> +		goto err;
> +	}
> +
> +	/*
> +	 * Enable devfreq-event to get raw data which is used to determine
> +	 * current bus load.
> +	 */
> +	ret = exynos_bus_enable_edev(bus);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to enable devfreq-event devices\n");
> +		goto err;
> +	}
> +
> +	ret = exynos_bus_set_event(bus);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to set event to devfreq-event devices\n");
> +		goto err;
> +	}
> +
> +err:
> +	return ret;
> +}
> +
>  static int exynos_bus_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev->of_node, *node;
>  	struct devfreq_dev_profile *profile;
> -	struct devfreq_simple_ondemand_data *ondemand_data;
>  	struct devfreq_passive_data *passive_data;
>  	struct devfreq *parent_devfreq;
>  	struct exynos_bus *bus;
> @@ -334,45 +384,9 @@ static int exynos_bus_probe(struct platform_device *pdev)
>  	if (passive)
>  		goto passive;
>  
> -	/* Initialize the struct profile and governor data for parent device */
> -	profile->polling_ms = 50;
> -	profile->target = exynos_bus_target;
> -	profile->get_dev_status = exynos_bus_get_dev_status;
> -	profile->exit = exynos_bus_exit;
> -
> -	ondemand_data = devm_kzalloc(dev, sizeof(*ondemand_data), GFP_KERNEL);
> -	if (!ondemand_data) {
> -		ret = -ENOMEM;
> +	ret = exynos_bus_profile_init(bus, profile);
> +	if (ret < 0)
>  		goto err;
> -	}
> -	ondemand_data->upthreshold = 40;
> -	ondemand_data->downdifferential = 5;
> -
> -	/* Add devfreq device to monitor and handle the exynos bus */
> -	bus->devfreq = devm_devfreq_add_device(dev, profile,
> -						DEVFREQ_GOV_SIMPLE_ONDEMAND,
> -						ondemand_data);
> -	if (IS_ERR(bus->devfreq)) {
> -		dev_err(dev, "failed to add devfreq device\n");
> -		ret = PTR_ERR(bus->devfreq);
> -		goto err;
> -	}
> -
> -	/*
> -	 * Enable devfreq-event to get raw data which is used to determine
> -	 * current bus load.
> -	 */
> -	ret = exynos_bus_enable_edev(bus);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to enable devfreq-event devices\n");
> -		goto err;
> -	}
> -
> -	ret = exynos_bus_set_event(bus);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to set event to devfreq-event devices\n");
> -		goto err;
> -	}
>  
>  	goto out;
>  passive:
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v2 00/11] Simple QoS for exynos-bus driver using interconnect
From: Chanwoo Choi @ 2019-09-20  2:14 UTC (permalink / raw)
  To: Artur Świgoń, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-pm, dri-devel
  Cc: b.zolnierkie, sw0312.kim, krzk, inki.dae, myungjoo.ham,
	cpgs (cpgs@samsung.com), leonard.crestez, georgi.djakov,
	m.szyprowski
In-Reply-To: <fe6d026d-71b5-438d-6932-6a2234fc72c2@samsung.com>

Hi Artur,

I tried to just build this patch on mainline kernel or linux-next.
But, when I applied them, merge conflict happens. You didn't develop
them on latest version. Please rebase them based on latest mainline kernel.

On 19. 9. 20. 오전 10:07, Chanwoo Choi wrote:
> Hi Artur,
> 
> On v1, I mentioned that we need to discuss how to change
> the v2 for this. But, I have not received any reply from you on v1.
> And, without your reply from v1, you just send v2.
> 
> I think that it is not proper development sequence.
> I have spent many times to review your patches
> and also I'll review your patches. You have to take care
> the reply of reviewer and and keep the basic rule
> of mailing contribution for discussion.
> 
> On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
>> The following patchset adds interconnect[1][2] framework support to the
>> exynos-bus devfreq driver. Extending the devfreq driver with interconnect
>> capabilities started as a response to the issue referenced in [3]. The
>> patches can be subdivided into four logical groups:
>>
>> (a) Refactoring the existing devfreq driver in order to improve readability
>> and accommodate for adding new code (patches 01--04/11).
>>
>> (b) Tweaking the interconnect framework to support the exynos-bus use case
>> (patches 05--07/11). Exporting of_icc_get_from_provider() allows us to
>> avoid hardcoding every single graph edge in the DT or driver source, and
>> relaxing the requirement contained in that function removes the need to
>> provide dummy node IDs in the DT. Adjusting the logic in
>> apply_constraints() (drivers/interconnect/core.c) accounts for the fact
>> that every bus is a separate entity and therefore a separate interconnect
>> provider, albeit constituting a part of a larger hierarchy.
>>
>> (c) Implementing interconnect providers in the exynos-bus devfreq driver
>> and adding required DT properties for one selected platform, namely
>> Exynos4412 (patches 08--09/11). Due to the fact that this aims to be a
>> generic driver for various Exynos SoCs, node IDs are generated dynamically
>> rather than hardcoded. This has been determined to be a simpler approach,
>> but depends on changes described in (b).
>>
>> (d) Implementing a sample interconnect consumer for exynos-mixer targeted
>> at the issue referenced in [3], again with DT info only for Exynos4412
>> (patches 10--11/11).
>>
>> Integration of devfreq and interconnect functionalities is achieved by
>> using dev_pm_qos_*() API[5]. All new code works equally well when
>> CONFIG_INTERCONNECT is 'n' (as in exynos_defconfig) in which case all
>> interconnect API functions are no-ops.
>>
>> This patchset depends on [5].
>>
>> --- Changes since v1 [6]:
>> * Rebase on [4] (coupled regulators).
>> * Rebase on [5] (dev_pm_qos for devfreq).
>> * Use dev_pm_qos_*() API[5] instead of overriding frequency in
>>   exynos_bus_target().
>> * Use IDR for node ID allocation.
>> * Avoid goto in functions extracted in patches 01 & 02 (cf. patch 04).
>> * Reverse order of multiplication and division in
>>   mixer_set_memory_bandwidth() (patch 11) to avoid integer overflow.
>>
>> ---
>> Artur Świgoń
>> Samsung R&D Institute Poland
>> Samsung Electronics
>>
>> ---
>> References:
>> [1] Documentation/interconnect/interconnect.rst
>> [2] Documentation/devicetree/bindings/interconnect/interconnect.txt
>> [3] https://patchwork.kernel.org/patch/10861757/ (original issue)
>> [4] https://patchwork.kernel.org/cover/11083663/ (coupled regulators; merged)
>> [5] https://patchwork.kernel.org/cover/11149497/ (dev_pm_qos for devfreq)
>> [6] https://patchwork.kernel.org/cover/11054417/ (v1 of this RFC)
>>
>> Artur Świgoń (10):
>>   devfreq: exynos-bus: Extract exynos_bus_profile_init()
>>   devfreq: exynos-bus: Extract exynos_bus_profile_init_passive()
>>   devfreq: exynos-bus: Change goto-based logic to if-else logic
>>   devfreq: exynos-bus: Clean up code
>>   interconnect: Export of_icc_get_from_provider()
>>   interconnect: Relax requirement in of_icc_get_from_provider()
>>   interconnect: Relax condition in apply_constraints()
>>   arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412
>>   devfreq: exynos-bus: Add interconnect functionality to exynos-bus
>>   arm: dts: exynos: Add interconnects to Exynos4412 mixer
>>
>> Marek Szyprowski (1):
>>   drm: exynos: mixer: Add interconnect support
>>
>>  .../boot/dts/exynos4412-odroid-common.dtsi    |   1 +
>>  arch/arm/boot/dts/exynos4412.dtsi             |  10 +
>>  drivers/devfreq/exynos-bus.c                  | 319 +++++++++++++-----
>>  drivers/gpu/drm/exynos/exynos_mixer.c         |  71 +++-
>>  drivers/interconnect/core.c                   |  12 +-
>>  include/linux/interconnect-provider.h         |   6 +
>>  6 files changed, 327 insertions(+), 92 deletions(-)
>>
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* RE: [PATCH V2 1/2] clk: imx8mm: Move 1443X/1416X PLL clock structure to common place
From: Anson Huang @ 2019-09-20  1:27 UTC (permalink / raw)
  To: mturquette@baylibre.com, sboyd@kernel.org, shawnguo@kernel.org,
	s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
	Leonard Crestez, Abel Vesa, Peng Fan, Jacky Bai, Fancy Fang,
	S.j. Wang, Aisheng Dong, sfr@canb.auug.org.au,
	l.stach@pengutronix.de, linux-clk@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
  Cc: dl-linux-imx
In-Reply-To: <1567776846-6373-1-git-send-email-Anson.Huang@nxp.com>

Gentle ping...

> Subject: [PATCH V2 1/2] clk: imx8mm: Move 1443X/1416X PLL clock structure
> to common place
> 
> Many i.MX8M SoCs use same 1443X/1416X PLL, such as i.MX8MM, i.MX8MN
> and later i.MX8M SoCs, moving these PLL definitions to pll14xx driver can
> save a lot of duplicated code on each platform.
> 
> Meanwhile, no need to define PLL clock structure for every module which
> uses same type of PLL, e.g., audio/video/dram use 1443X PLL,
> arm/gpu/vpu/sys use 1416X PLL, define 2 PLL clock structure for each group
> is enough.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V1:
> 	- Move 1443X/1416X PLL clock table/structure to pll14xx driver.
> ---
>  drivers/clk/imx/clk-imx8mm.c  | 87 +++++--------------------------------------
>  drivers/clk/imx/clk-pll14xx.c | 30 +++++++++++++++
>  drivers/clk/imx/clk.h         |  3 ++
>  3 files changed, 43 insertions(+), 77 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
> index 2758e3f..9649250 100644
> --- a/drivers/clk/imx/clk-imx8mm.c
> +++ b/drivers/clk/imx/clk-imx8mm.c
> @@ -26,73 +26,6 @@ static u32 share_count_disp;  static u32
> share_count_pdm;  static u32 share_count_nand;
> 
> -static const struct imx_pll14xx_rate_table imx8mm_pll1416x_tbl[] = {
> -	PLL_1416X_RATE(1800000000U, 225, 3, 0),
> -	PLL_1416X_RATE(1600000000U, 200, 3, 0),
> -	PLL_1416X_RATE(1200000000U, 300, 3, 1),
> -	PLL_1416X_RATE(1000000000U, 250, 3, 1),
> -	PLL_1416X_RATE(800000000U,  200, 3, 1),
> -	PLL_1416X_RATE(750000000U,  250, 2, 2),
> -	PLL_1416X_RATE(700000000U,  350, 3, 2),
> -	PLL_1416X_RATE(600000000U,  300, 3, 2),
> -};
> -
> -static const struct imx_pll14xx_rate_table imx8mm_audiopll_tbl[] = {
> -	PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
> -	PLL_1443X_RATE(361267200U, 361, 3, 3, 17511),
> -};
> -
> -static const struct imx_pll14xx_rate_table imx8mm_videopll_tbl[] = {
> -	PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
> -	PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
> -};
> -
> -static const struct imx_pll14xx_rate_table imx8mm_drampll_tbl[] = {
> -	PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
> -};
> -
> -static struct imx_pll14xx_clk imx8mm_audio_pll = {
> -		.type = PLL_1443X,
> -		.rate_table = imx8mm_audiopll_tbl,
> -		.rate_count = ARRAY_SIZE(imx8mm_audiopll_tbl),
> -};
> -
> -static struct imx_pll14xx_clk imx8mm_video_pll = {
> -		.type = PLL_1443X,
> -		.rate_table = imx8mm_videopll_tbl,
> -		.rate_count = ARRAY_SIZE(imx8mm_videopll_tbl),
> -};
> -
> -static struct imx_pll14xx_clk imx8mm_dram_pll = {
> -		.type = PLL_1443X,
> -		.rate_table = imx8mm_drampll_tbl,
> -		.rate_count = ARRAY_SIZE(imx8mm_drampll_tbl),
> -};
> -
> -static struct imx_pll14xx_clk imx8mm_arm_pll = {
> -		.type = PLL_1416X,
> -		.rate_table = imx8mm_pll1416x_tbl,
> -		.rate_count = ARRAY_SIZE(imx8mm_pll1416x_tbl),
> -};
> -
> -static struct imx_pll14xx_clk imx8mm_gpu_pll = {
> -		.type = PLL_1416X,
> -		.rate_table = imx8mm_pll1416x_tbl,
> -		.rate_count = ARRAY_SIZE(imx8mm_pll1416x_tbl),
> -};
> -
> -static struct imx_pll14xx_clk imx8mm_vpu_pll = {
> -		.type = PLL_1416X,
> -		.rate_table = imx8mm_pll1416x_tbl,
> -		.rate_count = ARRAY_SIZE(imx8mm_pll1416x_tbl),
> -};
> -
> -static struct imx_pll14xx_clk imx8mm_sys_pll = {
> -		.type = PLL_1416X,
> -		.rate_table = imx8mm_pll1416x_tbl,
> -		.rate_count = ARRAY_SIZE(imx8mm_pll1416x_tbl),
> -};
> -
>  static const char *pll_ref_sels[] = { "osc_24m", "dummy", "dummy",
> "dummy", };  static const char *audio_pll1_bypass_sels[] = {"audio_pll1",
> "audio_pll1_ref_sel", };  static const char *audio_pll2_bypass_sels[] =
> {"audio_pll2", "audio_pll2_ref_sel", }; @@ -396,16 +329,16 @@ static int
> imx8mm_clocks_probe(struct platform_device *pdev)
>  	clks[IMX8MM_SYS_PLL2_REF_SEL] = imx_clk_mux("sys_pll2_ref_sel",
> base + 0x104, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
>  	clks[IMX8MM_SYS_PLL3_REF_SEL] = imx_clk_mux("sys_pll3_ref_sel",
> base + 0x114, 0, 2, pll_ref_sels, ARRAY_SIZE(pll_ref_sels));
> 
> -	clks[IMX8MM_AUDIO_PLL1] = imx_clk_pll14xx("audio_pll1",
> "audio_pll1_ref_sel", base, &imx8mm_audio_pll);
> -	clks[IMX8MM_AUDIO_PLL2] = imx_clk_pll14xx("audio_pll2",
> "audio_pll2_ref_sel", base + 0x14, &imx8mm_audio_pll);
> -	clks[IMX8MM_VIDEO_PLL1] = imx_clk_pll14xx("video_pll1",
> "video_pll1_ref_sel", base + 0x28, &imx8mm_video_pll);
> -	clks[IMX8MM_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
> "dram_pll_ref_sel", base + 0x50, &imx8mm_dram_pll);
> -	clks[IMX8MM_GPU_PLL] = imx_clk_pll14xx("gpu_pll",
> "gpu_pll_ref_sel", base + 0x64, &imx8mm_gpu_pll);
> -	clks[IMX8MM_VPU_PLL] = imx_clk_pll14xx("vpu_pll",
> "vpu_pll_ref_sel", base + 0x74, &imx8mm_vpu_pll);
> -	clks[IMX8MM_ARM_PLL] = imx_clk_pll14xx("arm_pll",
> "arm_pll_ref_sel", base + 0x84, &imx8mm_arm_pll);
> -	clks[IMX8MM_SYS_PLL1] = imx_clk_pll14xx("sys_pll1",
> "sys_pll1_ref_sel", base + 0x94, &imx8mm_sys_pll);
> -	clks[IMX8MM_SYS_PLL2] = imx_clk_pll14xx("sys_pll2",
> "sys_pll2_ref_sel", base + 0x104, &imx8mm_sys_pll);
> -	clks[IMX8MM_SYS_PLL3] = imx_clk_pll14xx("sys_pll3",
> "sys_pll3_ref_sel", base + 0x114, &imx8mm_sys_pll);
> +	clks[IMX8MM_AUDIO_PLL1] = imx_clk_pll14xx("audio_pll1",
> "audio_pll1_ref_sel", base, &imx_1443x_pll);
> +	clks[IMX8MM_AUDIO_PLL2] = imx_clk_pll14xx("audio_pll2",
> "audio_pll2_ref_sel", base + 0x14, &imx_1443x_pll);
> +	clks[IMX8MM_VIDEO_PLL1] = imx_clk_pll14xx("video_pll1",
> "video_pll1_ref_sel", base + 0x28, &imx_1443x_pll);
> +	clks[IMX8MM_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
> "dram_pll_ref_sel", base + 0x50, &imx_1443x_pll);
> +	clks[IMX8MM_GPU_PLL] = imx_clk_pll14xx("gpu_pll",
> "gpu_pll_ref_sel", base + 0x64, &imx_1416x_pll);
> +	clks[IMX8MM_VPU_PLL] = imx_clk_pll14xx("vpu_pll",
> "vpu_pll_ref_sel", base + 0x74, &imx_1416x_pll);
> +	clks[IMX8MM_ARM_PLL] = imx_clk_pll14xx("arm_pll",
> "arm_pll_ref_sel", base + 0x84, &imx_1416x_pll);
> +	clks[IMX8MM_SYS_PLL1] = imx_clk_pll14xx("sys_pll1",
> "sys_pll1_ref_sel", base + 0x94, &imx_1416x_pll);
> +	clks[IMX8MM_SYS_PLL2] = imx_clk_pll14xx("sys_pll2",
> "sys_pll2_ref_sel", base + 0x104, &imx_1416x_pll);
> +	clks[IMX8MM_SYS_PLL3] = imx_clk_pll14xx("sys_pll3",
> +"sys_pll3_ref_sel", base + 0x114, &imx_1416x_pll);
> 
>  	/* PLL bypass out */
>  	clks[IMX8MM_AUDIO_PLL1_BYPASS] =
> imx_clk_mux_flags("audio_pll1_bypass", base, 4, 1, audio_pll1_bypass_sels,
> ARRAY_SIZE(audio_pll1_bypass_sels), CLK_SET_RATE_PARENT); diff --git
> a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c index
> b721302..4a61743 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -41,6 +41,36 @@ struct clk_pll14xx {
> 
>  #define to_clk_pll14xx(_hw) container_of(_hw, struct clk_pll14xx, hw)
> 
> +const struct imx_pll14xx_rate_table imx_pll1416x_tbl[] = {
> +	PLL_1416X_RATE(1800000000U, 225, 3, 0),
> +	PLL_1416X_RATE(1600000000U, 200, 3, 0),
> +	PLL_1416X_RATE(1200000000U, 300, 3, 1),
> +	PLL_1416X_RATE(1000000000U, 250, 3, 1),
> +	PLL_1416X_RATE(800000000U,  200, 3, 1),
> +	PLL_1416X_RATE(750000000U,  250, 2, 2),
> +	PLL_1416X_RATE(700000000U,  350, 3, 2),
> +	PLL_1416X_RATE(600000000U,  300, 3, 2), };
> +
> +const struct imx_pll14xx_rate_table imx_pll1443x_tbl[] = {
> +	PLL_1443X_RATE(650000000U, 325, 3, 2, 0),
> +	PLL_1443X_RATE(594000000U, 198, 2, 2, 0),
> +	PLL_1443X_RATE(393216000U, 262, 2, 3, 9437),
> +	PLL_1443X_RATE(361267200U, 361, 3, 3, 17511), };
> +
> +struct imx_pll14xx_clk imx_1443x_pll = {
> +	.type = PLL_1443X,
> +	.rate_table = imx_pll1443x_tbl,
> +	.rate_count = ARRAY_SIZE(imx_pll1443x_tbl), };
> +
> +struct imx_pll14xx_clk imx_1416x_pll = {
> +	.type = PLL_1416X,
> +	.rate_table = imx_pll1416x_tbl,
> +	.rate_count = ARRAY_SIZE(imx_pll1416x_tbl), };
> +
>  static const struct imx_pll14xx_rate_table *imx_get_pll_settings(
>  		struct clk_pll14xx *pll, unsigned long rate)  { diff --git
> a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index f7a389a..bc5bb6a
> 100644
> --- a/drivers/clk/imx/clk.h
> +++ b/drivers/clk/imx/clk.h
> @@ -50,6 +50,9 @@ struct imx_pll14xx_clk {
>  	int flags;
>  };
> 
> +extern struct imx_pll14xx_clk imx_1416x_pll; extern struct
> +imx_pll14xx_clk imx_1443x_pll;
> +
>  #define imx_clk_cpu(name, parent_name, div, mux, pll, step) \
>  	imx_clk_hw_cpu(name, parent_name, div, mux, pll, step)->clk
> 
> --
> 2.7.4

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH 1/2] soc: ti: big cleanup of Kconfig file
From: santosh.shilimkar @ 2019-09-20  1:14 UTC (permalink / raw)
  To: Randy Dunlap, LKML, LAK
  Cc: Dave Gerlach, Tony Lindgren, Keerthy, Sandeep Nair,
	Santosh Shilimkar, Olof Johansson
In-Reply-To: <8437a1f9-18f2-dd03-4fea-de5ba71f25c9@infradead.org>

On 9/19/19 3:33 PM, Randy Dunlap wrote:
> From: Randy Dunlap <rdunlap@infradead.org>
> 
> Cleanup drivers/soc/ti/Kconfig:
> - delete duplicate words
> - end sentences with '.'
> - fix typos/spellos
> - Subsystem is one word
> - capitalize acronyms
> - reflow lines to be <= 80 columns
> 
> Fixes: 41f93af900a2 ("soc: ti: add Keystone Navigator QMSS driver")
> Fixes: 88139ed03058 ("soc: ti: add Keystone Navigator DMA support")
> Fixes: afe761f8d3e9 ("soc: ti: Add pm33xx driver for basic suspend support")
> Fixes: 5a99ae0092fe ("soc: ti: pm33xx: AM437X: Add rtc_only with ddr in self-refresh support")
> Fixes: a869b7b30dac ("soc: ti: Add Support for AM654 SoC config option")
> Fixes: cff377f7897a ("soc: ti: Add Support for J721E SoC config option")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Olof Johansson <olof@lixom.net>
> Cc: Santosh Shilimkar <ssantosh@kernel.org>
> Cc: Sandeep Nair <sandeep_n@ti.com>
> Cc: Dave Gerlach <d-gerlach@ti.com>
> Cc: Keerthy <j-keerthy@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> @Santosh: MAINTAINERS says that you maintain drivers/soc/ti/*,
> but there is more that Keystone-related code in that subdirectory
> now... just in case you want to update that info.
>
Yes am aware there more drivers and so far I have been taking
care of everything in drivers/soc/ti/*

>   drivers/soc/ti/Kconfig |   20 ++++++++++----------
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
Patch looks fine to me. Do you want me to pick this up ?


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* RE: [PATCH v5 1/3] arm64: cpufeature: introduce helper cpu_has_hw_af()
From: Justin He (Arm Technology China) @ 2019-09-20  1:14 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Mark Rutland, Kaly Xin (Arm Technology China), Ralph Campbell,
	Andrew Morton, Suzuki Poulose, Marc Zyngier, Anshuman Khandual,
	linux-kernel@vger.kernel.org, Matthew Wilcox, linux-mm@kvack.org,
	Jérôme Glisse, James Morse,
	linux-arm-kernel@lists.infradead.org, Punit Agrawal,
	hejianet@gmail.com, Thomas Gleixner, Will Deacon, Alex Van Brunt,
	Kirill A. Shutemov, Robin Murphy
In-Reply-To: <20190919163644.GD6472@arrakis.emea.arm.com>


Hi Catalin
> -----Original Message-----
> From: Catalin Marinas <catalin.marinas@arm.com>
> Sent: 2019年9月20日 0:37
> To: Justin He (Arm Technology China) <Justin.He@arm.com>
> Cc: Will Deacon <will@kernel.org>; Mark Rutland
> <Mark.Rutland@arm.com>; James Morse <James.Morse@arm.com>; Marc
> Zyngier <maz@kernel.org>; Matthew Wilcox <willy@infradead.org>; Kirill A.
> Shutemov <kirill.shutemov@linux.intel.com>; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> mm@kvack.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Punit
> Agrawal <punitagrawal@gmail.com>; Anshuman Khandual
> <Anshuman.Khandual@arm.com>; Alex Van Brunt
> <avanbrunt@nvidia.com>; Robin Murphy <Robin.Murphy@arm.com>;
> Thomas Gleixner <tglx@linutronix.de>; Andrew Morton <akpm@linux-
> foundation.org>; Jérôme Glisse <jglisse@redhat.com>; Ralph Campbell
> <rcampbell@nvidia.com>; hejianet@gmail.com; Kaly Xin (Arm Technology
> China) <Kaly.Xin@arm.com>
> Subject: Re: [PATCH v5 1/3] arm64: cpufeature: introduce helper
> cpu_has_hw_af()
>
> On Fri, Sep 20, 2019 at 12:12:02AM +0800, Jia He wrote:
> > diff --git a/arch/arm64/kernel/cpufeature.c
> b/arch/arm64/kernel/cpufeature.c
> > index b1fdc486aed8..fb0e9425d286 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -1141,6 +1141,16 @@ static bool has_hw_dbm(const struct
> arm64_cpu_capabilities *cap,
> >     return true;
> >  }
> >
> > +/* Decouple AF from AFDBM. */
> > +bool cpu_has_hw_af(void)
> > +{
> > +   return (read_cpuid(ID_AA64MMFR1_EL1) & 0xf);
> > +}
> > +#else /* CONFIG_ARM64_HW_AFDBM */
> > +bool cpu_has_hw_af(void)
> > +{
> > +   return false;
> > +}
> >  #endif
>
> Please place this function in cpufeature.h directly, no need for an
> additional function call. Something like:
>
> static inline bool cpu_has_hw_af(void)
> {
>       if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM))
>               return read_cpuid(ID_AA64MMFR1_EL1) & 0xf;
>       return false;
> }
>
Ok, thanks

--
Cheers,
Justin (Jia He)


> --
> Catalin
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* RE: [PATCH v5 3/3] mm: fix double page fault on arm64 if PTE_AF is cleared
From: Justin He (Arm Technology China) @ 2019-09-20  1:13 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Mark Rutland, Kaly Xin (Arm Technology China), Ralph Campbell,
	Andrew Morton, Suzuki Poulose, Marc Zyngier, Anshuman Khandual,
	linux-kernel@vger.kernel.org, Matthew Wilcox, linux-mm@kvack.org,
	Jérôme Glisse, James Morse,
	linux-arm-kernel@lists.infradead.org, Punit Agrawal,
	hejianet@gmail.com, Thomas Gleixner, Will Deacon, Alex Van Brunt,
	Kirill A. Shutemov, Robin Murphy
In-Reply-To: <20190919164206.GE6472@arrakis.emea.arm.com>

Hi Catalin

> -----Original Message-----
> From: Catalin Marinas <catalin.marinas@arm.com>
> Sent: 2019年9月20日 0:42
> To: Justin He (Arm Technology China) <Justin.He@arm.com>
> Cc: Will Deacon <will@kernel.org>; Mark Rutland
> <Mark.Rutland@arm.com>; James Morse <James.Morse@arm.com>; Marc
> Zyngier <maz@kernel.org>; Matthew Wilcox <willy@infradead.org>; Kirill A.
> Shutemov <kirill.shutemov@linux.intel.com>; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
> mm@kvack.org; Suzuki Poulose <Suzuki.Poulose@arm.com>; Punit
> Agrawal <punitagrawal@gmail.com>; Anshuman Khandual
> <Anshuman.Khandual@arm.com>; Alex Van Brunt
> <avanbrunt@nvidia.com>; Robin Murphy <Robin.Murphy@arm.com>;
> Thomas Gleixner <tglx@linutronix.de>; Andrew Morton <akpm@linux-
> foundation.org>; Jérôme Glisse <jglisse@redhat.com>; Ralph Campbell
> <rcampbell@nvidia.com>; hejianet@gmail.com; Kaly Xin (Arm Technology
> China) <Kaly.Xin@arm.com>
> Subject: Re: [PATCH v5 3/3] mm: fix double page fault on arm64 if PTE_AF
> is cleared
>
> On Fri, Sep 20, 2019 at 12:12:04AM +0800, Jia He wrote:
> > @@ -2152,7 +2163,29 @@ static inline void cow_user_page(struct page
> *dst, struct page *src, unsigned lo
> >      */
> >     if (unlikely(!src)) {
> >             void *kaddr = kmap_atomic(dst);
> > -           void __user *uaddr = (void __user *)(va & PAGE_MASK);
> > +           void __user *uaddr = (void __user *)(addr & PAGE_MASK);
> > +           pte_t entry;
> > +
> > +           /* On architectures with software "accessed" bits, we would
> > +            * take a double page fault, so mark it accessed here.
> > +            */
> > +           if (arch_faults_on_old_pte() && !pte_young(vmf->orig_pte))
> {
> > +                   spin_lock(vmf->ptl);
> > +                   if (likely(pte_same(*vmf->pte, vmf->orig_pte))) {
> > +                           entry = pte_mkyoung(vmf->orig_pte);
> > +                           if (ptep_set_access_flags(vma, addr,
> > +                                                     vmf->pte, entry, 0))
> > +                                   update_mmu_cache(vma, addr, vmf-
> >pte);
> > +                   } else {
> > +                           /* Other thread has already handled the
> fault
> > +                            * and we don't need to do anything. If it's
> > +                            * not the case, the fault will be triggered
> > +                            * again on the same address.
> > +                            */
> > +                           return -1;
> > +                   }
> > +                   spin_unlock(vmf->ptl);
>
> Returning with the spinlock held doesn't normally go very well ;).
Yes, my bad. Will fix asap

--
Cheers,
Justin (Jia He)


IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v2 00/11] Simple QoS for exynos-bus driver using interconnect
From: Chanwoo Choi @ 2019-09-20  1:07 UTC (permalink / raw)
  To: Artur Świgoń, devicetree, linux-arm-kernel,
	linux-samsung-soc, linux-kernel, linux-pm, dri-devel
  Cc: b.zolnierkie, sw0312.kim, krzk, inki.dae, myungjoo.ham,
	cpgs (cpgs@samsung.com), leonard.crestez, georgi.djakov,
	m.szyprowski
In-Reply-To: <20190919142236.4071-1-a.swigon@samsung.com>

Hi Artur,

On v1, I mentioned that we need to discuss how to change
the v2 for this. But, I have not received any reply from you on v1.
And, without your reply from v1, you just send v2.

I think that it is not proper development sequence.
I have spent many times to review your patches
and also I'll review your patches. You have to take care
the reply of reviewer and and keep the basic rule
of mailing contribution for discussion.

On 19. 9. 19. 오후 11:22, Artur Świgoń wrote:
> The following patchset adds interconnect[1][2] framework support to the
> exynos-bus devfreq driver. Extending the devfreq driver with interconnect
> capabilities started as a response to the issue referenced in [3]. The
> patches can be subdivided into four logical groups:
> 
> (a) Refactoring the existing devfreq driver in order to improve readability
> and accommodate for adding new code (patches 01--04/11).
> 
> (b) Tweaking the interconnect framework to support the exynos-bus use case
> (patches 05--07/11). Exporting of_icc_get_from_provider() allows us to
> avoid hardcoding every single graph edge in the DT or driver source, and
> relaxing the requirement contained in that function removes the need to
> provide dummy node IDs in the DT. Adjusting the logic in
> apply_constraints() (drivers/interconnect/core.c) accounts for the fact
> that every bus is a separate entity and therefore a separate interconnect
> provider, albeit constituting a part of a larger hierarchy.
> 
> (c) Implementing interconnect providers in the exynos-bus devfreq driver
> and adding required DT properties for one selected platform, namely
> Exynos4412 (patches 08--09/11). Due to the fact that this aims to be a
> generic driver for various Exynos SoCs, node IDs are generated dynamically
> rather than hardcoded. This has been determined to be a simpler approach,
> but depends on changes described in (b).
> 
> (d) Implementing a sample interconnect consumer for exynos-mixer targeted
> at the issue referenced in [3], again with DT info only for Exynos4412
> (patches 10--11/11).
> 
> Integration of devfreq and interconnect functionalities is achieved by
> using dev_pm_qos_*() API[5]. All new code works equally well when
> CONFIG_INTERCONNECT is 'n' (as in exynos_defconfig) in which case all
> interconnect API functions are no-ops.
> 
> This patchset depends on [5].
> 
> --- Changes since v1 [6]:
> * Rebase on [4] (coupled regulators).
> * Rebase on [5] (dev_pm_qos for devfreq).
> * Use dev_pm_qos_*() API[5] instead of overriding frequency in
>   exynos_bus_target().
> * Use IDR for node ID allocation.
> * Avoid goto in functions extracted in patches 01 & 02 (cf. patch 04).
> * Reverse order of multiplication and division in
>   mixer_set_memory_bandwidth() (patch 11) to avoid integer overflow.
> 
> ---
> Artur Świgoń
> Samsung R&D Institute Poland
> Samsung Electronics
> 
> ---
> References:
> [1] Documentation/interconnect/interconnect.rst
> [2] Documentation/devicetree/bindings/interconnect/interconnect.txt
> [3] https://patchwork.kernel.org/patch/10861757/ (original issue)
> [4] https://patchwork.kernel.org/cover/11083663/ (coupled regulators; merged)
> [5] https://patchwork.kernel.org/cover/11149497/ (dev_pm_qos for devfreq)
> [6] https://patchwork.kernel.org/cover/11054417/ (v1 of this RFC)
> 
> Artur Świgoń (10):
>   devfreq: exynos-bus: Extract exynos_bus_profile_init()
>   devfreq: exynos-bus: Extract exynos_bus_profile_init_passive()
>   devfreq: exynos-bus: Change goto-based logic to if-else logic
>   devfreq: exynos-bus: Clean up code
>   interconnect: Export of_icc_get_from_provider()
>   interconnect: Relax requirement in of_icc_get_from_provider()
>   interconnect: Relax condition in apply_constraints()
>   arm: dts: exynos: Add parents and #interconnect-cells to Exynos4412
>   devfreq: exynos-bus: Add interconnect functionality to exynos-bus
>   arm: dts: exynos: Add interconnects to Exynos4412 mixer
> 
> Marek Szyprowski (1):
>   drm: exynos: mixer: Add interconnect support
> 
>  .../boot/dts/exynos4412-odroid-common.dtsi    |   1 +
>  arch/arm/boot/dts/exynos4412.dtsi             |  10 +
>  drivers/devfreq/exynos-bus.c                  | 319 +++++++++++++-----
>  drivers/gpu/drm/exynos/exynos_mixer.c         |  71 +++-
>  drivers/interconnect/core.c                   |  12 +-
>  include/linux/interconnect-provider.h         |   6 +
>  6 files changed, 327 insertions(+), 92 deletions(-)
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [RFC PATCH v1 1/1] Add support for arm64 to carry ima measurement log in kexec_file_load
From: Prakhar Srivastava @ 2019-09-20  0:32 UTC (permalink / raw)
  To: Thiago Jung Bauermann, Mimi Zohar
  Cc: mark.rutland, jean-philippe, arnd, yamada.masahiro, sboyd,
	catalin.marinas, kexec, linux-kernel, takahiro.akashi,
	kristina.martsenko, duwe, linux-arm-kernel, james.morse,
	linux-integrity, tglx, allison
In-Reply-To: <871rwd2ay8.fsf@morokweng.localdomain>



On 9/18/2019 8:59 PM, Thiago Jung Bauermann wrote:
> Mimi Zohar <zohar@linux.ibm.com> writes:
>
>> On Wed, 2019-09-18 at 10:15 -0400, Mimi Zohar wrote:
>>
>>>> +	uint64_t tmp_start, tmp_end;
>>>> +
>>>> +	propStart = of_find_property(of_chosen, "linux,ima-kexec-buffer",
>>>> +				     NULL);
>>>> +	if (propStart) {
>>>> +		tmp_start = fdt64_to_cpu(*((const fdt64_t *) propStart));
>>>> +		ret = of_remove_property(of_chosen, propStart);
>>>> +		if (!ret) {
>>>> +			return ret;
>>>> +		}
>>>> +
>>>> +		propEnd = of_find_property(of_chosen,
>>>> +					   "linux,ima-kexec-buffer-end", NULL);
>>>> +		if (!propEnd) {
>>>> +			return -EINVAL;
>>>> +		}
>>>> +
>>>> +		tmp_end = fdt64_to_cpu(*((const fdt64_t *) propEnd));
>>>> +
>>>> +		ret = of_remove_property(of_chosen, propEnd);
>>>> +		if (!ret) {
>>>> +			return ret;
>>>> +		}
>>> There seems to be quite a bit of code duplication in this function and
>>> in ima_get_kexec_buffer().  It could probably be cleaned up with some
>>> refactoring.
>> Sorry, my mistake.  One calls of_get_property(), while the other calls
>> of_find_property().
> of_get_property() is a thin wrapper around of_find_property(), so if
> that's the only difference I think they can still be merged.

I will move to using of_get_property and see if i can reduce the code 
further.

Also address other comments by Mimi in my next iteration.

Thanks,

Prakhar Srivastava


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [tech-privileged] [RFC PATCH V1] riscv-privileged: Add broadcast mode to sfence.vma
From: Guo Ren @ 2019-09-20  0:13 UTC (permalink / raw)
  To: Andrew Waterman
  Cc: julien.thierry, Catalin Marinas, Palmer Dabbelt, Will Deacon,
	Atish Patra, Julien Grall, gary, linux-riscv, kvmarm,
	Jean-Philippe Brucker, linux-csky, Mike Rapoport, Guo Ren, benh,
	jacob.jun.pan, tech-privileged, Marc Zyngier, linux-arm-kernel,
	feiteng_li, Anup Patel, Linux Kernel Mailing List, iommu, dwmw2
In-Reply-To: <CA++6G0AzDs7w3gjjT4tDZjWBiMPf4Bvd36Ak0xdzfcJdPriKiw@mail.gmail.com>

Hi,

On Fri, Sep 20, 2019 at 12:10 AM Andrew Waterman <andrew@sifive.com> wrote:
>
> This needs to be discussed and debated at length; proposing edits to the spec at this stage is putting the cart before the horse!
Agree :)

>
> We shouldn’t change the definition of the existing SFENCE.VMA instruction to accomplish this. It’s also not abundantly clear to me that this should be an instruction:
If you implement sfence.vma as current define, it also could work with
new mechanism, they are compatible.

> TLB shootdown looks more like MMIO.
Per-CPU MMIO ? I the proposal, every hart only takes care of its own request.




>
> On Thu, Sep 19, 2019 at 5:36 AM Guo Ren <guoren@kernel.org> wrote:
>>
>> From: Guo Ren <ren_guo@c-sky.com>
>>
>> The patch is for https://github.com/riscv/riscv-isa-manual
>>
>> The proposal has been talked in LPC-2019 RISC-V MC ref [1]. Here is the
>> formal patch.
>>
>> Introduction
>> ============
>>
>> Using the Hardware TLB broadcast invalidation instruction to maintain the
>> system TLB is a good choice and it'll simplify the system software design.
>> The proposal hopes to add a broadcast mode to the sfence.vma in the
>> riscv-privilege specification. To support the sfence.vma broadcast mode,
>> there are two modification introduced below:
>>
>>  1) Add PGD.PPN (root page table's PPN) as the unique identifier of the
>>     address space in addition to asid/vmid. Compared to the dynamically
>>     changed asid/vmid, PGD.PPN is fixed throughout the address space life
>>     cycle. This feature enables uniform address space identification
>>     between different TLB systems (actually, it's difficult to unify the
>>     asid/vmid between the CPU system and the IOMMU system, because their
>>     mechanisms are different)
>>
>>  2) Modify the definition of the sfence.vma instruction from synchronous
>>     mode to asynchronous mode, which means that the completion of the TLB
>>     operation is not guaranteed when the sfence.vma instruction retires.
>>     It needs to be completed by checking the flag bit on the hart. The
>>     sfence.vma request finish can notify the software by generating an
>>     interrupt. This function alleviates the large delay of TLB invalidation
>>     in the PCI ATS system.
>>
>> Add S1/S2.PGD.PPN for ASID/VMID
>> ===============================
>>
>> PGD is global directory (defined in linux) and PPN is page physical number
>> (defined in riscv-spec). PGD.PNN corresponds to the root page table pointer
>> of the address space, i.e. mm->pgd (linux concept).
>>
>> In CPU/IOMMU TLB, we use asid/vmid to distinguish the address space of
>> process or virtual machine. Due to the limitation of id encoding, it can
>> only represent a part(window) of the address space. S1/S2.PGD.PPN are the
>> root page table's PPNs of the address spaces and S1/S2.PGD.PPN are the
>> unique identifier of the address spaces.
>>
>> For the CPU SMP system, you can use context switch to perform the necessary
>> software mechanism to ensure that the asid/vmid on all harts is consistent
>> (please refer to the arm64 asid mechanism). In this way, the TLB broadcast
>> invalidation instruction can determine the address space processed on all
>> harts by asid/vmid.
>>
>> Different from the CPU SMP system, there is no context switch for the
>> DMA-IOMMU system, so the unification with the CPU asid/vmid cannot be
>> guaranteed. So we need a unique identifier for the address space to
>> establish a communication bridge between the TLBs of different systems.
>>
>> That is PGD.PPN (for virtualization scenarios: S1/S2.PGD.PPN)
>>
>> current:
>>  sfence.vma  rs1 = vaddr, rs2 = asid
>>  hfence.vvma rs1 = vaddr, rs2 = asid
>>  hfence.gvma rs1 = gaddr, rs2 = vmid
>>
>> proposed:
>>  sfence.vma  rs1 = vaddr, rs2 = mode:ppn:asid
>>  hfence.vvma rs1 = vaddr, rs2 = mode:ppn:asid
>>  hfence.gvma rs1 = gaddr, rs2 = mode:ppn:vmid
>>
>>  mode      - broadcast | local
>>  ppn       - the PPN of the address space of the root page table
>>  vmid/asid - the window identifier of the address space
>>
>> At the Linux Plumber Conference 2019 RISCV-MC, ref:[1], we've showed two
>> IOMMU examples to explain how it work with hardware.
>>
>> 1) In a lightweight IOMMU system (up to 64 address spaces), the hardware
>>    could directly convert PGD.PPN into DID (IOMMU ASID)
>>
>> 2) For the PCI ATS scenario, its IO ASID/VMID encoding space can support
>>    a very large number of address spaces. We use two reverse mapping
>>    tables to let the hardware translate S1/S2.PGD.PPN into IO ASID/VMID.
>>
>> ASYNC BROADCAST SFENCE.VMA
>> ===========================
>>
>> To support the high latency broadcast sfence.vma operation in the PCI ATS
>> usage scenario, we modify the sfence.vma from synchronous mode to
>> asynchronous mode. (For simpler implementation, if hardware only implement
>> synchronous mode and software still work in asynchronous mode)
>>
>> To implement the asynchronous mode, 3 features are added:
>>  1) sstatus:TLBI
>>     A "status bit - TLBI" is added to the sstatus register. The TLBI status
>>     bit indicates if there are still outstanding sfence.vma requests on the
>>     current hart.
>>     Value:
>>       1: sfence.vma requests are not completed.
>>       0: all sfece.vma requests completed, request queue is empty.
>>
>>  2) sstatus:TLBIC
>>     A "control bits - TLBIC" is added to sstatus register. The TLBIC control
>>     bits are controlled by software.
>>     "Write 1" will trigger the current hart check to see if there are still
>>     outstanding sfence.vma requests. If there are unfinished requests, an
>>     interrupt will be generated when the request is completed, notifying the
>>     software that all of the current sfence.vma requests have been completed.
>>     "Write 0" will cause nothing.
>>
>>  3) supervisor interrupt register (sip & sie):TLBI finish interrupt
>>     A per-hart interrupt is added to supervisor interrupt registers.
>>     When all sfence.vma requests are completed and sstatus:TLBIC has been
>>     triggered, hart will receive a TLBI finish interrupt. Just like timer,
>>     software and external interrupt's definition in sip & sie.
>>
>> Fake code:
>>
>> flush_tlb_page(vma, addr) {
>>     asid = cpu_asid(vma->vm_mm);
>>     ppn = PFN_DOWN(vma->vm_mm->pgd);
>>
>>     sfence.vma (addr, 1|PPN_OFFSET(ppn)|asid); //1. start request
>>
>>     while(sstatus:TLBI) if (time_out() > 1ms) break; //2. loop check
>>
>>     while (sstatus:TLBI) {
>>         ...
>>         set sstatus:TLBIC;
>>         wait_TLBI_finish_interrupt(); //3. wait irq, io_schedule
>>     }
>> }
>>
>> Here we give 2 level check:
>>  1) loop check sstatus:TLBI, CPU could response Interrupt.
>>  2) set sstatus:TLBIC and wait for irq, CPU schedule out for other task.
>>
>> ACE-DVM Example
>> ===============
>>
>> Honestly, "broadcasting addr, asid, vmid, S1/S2.PGD.PPN to interconnects"
>> and "ASYNC SFENCE.VMA" could be implemented by ACE-DVM protocol ref [2].
>>
>> There are 3 types of transactions in DVM:
>>
>>  - DVM operation
>>    Send all information to the interconnect, including addr, asid,
>>    S1.PGD.PPN, vmid, S2.PGD.PPN.
>>
>>  - DVM synchronization
>>    Check that all DVM operations have been completed. If not, it will use
>>    state machine to wait DVM complete requests.
>>
>>  - DVM complete
>>    Return transaction from components, eg: IOMMU. If hart has received all
>>    DVM completes which are triggered by sfence.vma instructions and
>>    "sstatus:TLBIC" has been set, a TLBI finish interrupt is triggered.
>>
>> (Actually, we do not need to implement the above functions strictly
>>  according to the ACE specification :P )
>>
>>  1: https://www.linuxplumbersconf.org/event/4/contributions/307/
>>  2: AMBA AXI and ACE Protocol Specification - Distributed Virtual Memory
>>     Transactions"
>>
>> Signed-off-by: Guo Ren <ren_guo@c-sky.com>
>> Reviewed-by: Li Feiteng <feiteng_li@c-sky.com>
>> ---
>>  src/hypervisor.tex |  43 ++++++++-------
>>  src/supervisor.tex | 155 +++++++++++++++++++++++++++++++++++++++++------------
>>  2 files changed, 143 insertions(+), 55 deletions(-)
>>
>> diff --git a/src/hypervisor.tex b/src/hypervisor.tex
>> index 47b90b2..3718819 100644
>> --- a/src/hypervisor.tex
>> +++ b/src/hypervisor.tex
>> @@ -1094,15 +1094,15 @@ The hypervisor extension adds two new privileged fence instructions.
>>  \multicolumn{1}{c|}{opcode} \\
>>  \hline
>>  7 & 5 & 5 & 3 & 5 & 7 \\
>> -HFENCE.GVMA & vmid & gaddr & PRIV & 0 & SYSTEM \\
>> -HFENCE.VVMA & asid & vaddr & PRIV & 0 & SYSTEM \\
>> +HFENCE.GVMA & mode:ppn:vmid & gaddr & PRIV & 0 & SYSTEM \\
>> +HFENCE.VVMA & mode:ppn:asid & vaddr & PRIV & 0 & SYSTEM \\
>>  \end{tabular}
>>  \end{center}
>>
>>  The hypervisor memory-management fence instructions, HFENCE.GVMA and
>>  HFENCE.VVMA, are valid only in HS-mode when {\tt mstatus}.TVM=0, or in M-mode
>>  (irrespective of {\tt mstatus}.TVM).
>> -These instructions perform a function similar to SFENCE.VMA
>> +These instructions perform a function similar to SFENCE.VMA (broadcast/local)
>>  (Section~\ref{sec:sfence.vma}), except applying to the guest-physical
>>  memory-management data structures controlled by CSR {\tt hgatp} (HFENCE.GVMA)
>>  or the VS-level memory-management data structures controlled by CSR {\tt vsatp}
>> @@ -1136,11 +1136,10 @@ An HFENCE.VVMA instruction applies only to a single virtual machine, identified
>>  by the setting of {\tt hgatp}.VMID when HFENCE.VVMA executes.
>>  \end{commentary}
>>
>> -When {\em rs2}$\neq${\tt x0}, bits XLEN-1:ASIDMAX of the value held in {\em
>> -rs2} are reserved for future use and should be zeroed by software and ignored
>> -by current implementations.
>> -Furthermore, if ASIDLEN~$<$~ASIDMAX, the implementation shall ignore bits
>> -ASIDMAX-1:ASIDLEN of the value held in {\em rs2}.
>> +When {\em rs2}$\neq${\tt x0}, bits contain 3 informations: mode, ppn, asid.
>> +1) mode control HFENCE.VVMA broadcast or not.
>> +2) ppn is the root page talbe's PPN of the asid address space.
>> +3) asid is the identifier of process in virtual machine.
>>
>>  \begin{commentary}
>>  Simpler implementations of HFENCE.VVMA can ignore the guest virtual address in
>> @@ -1168,11 +1167,10 @@ physical addresses in PMP address registers (Section~\ref{sec:pmp}) and in page
>>  table entries (Sections \ref{sec:sv32}, \ref{sec:sv39}, and~\ref{sec:sv48}).
>>  \end{commentary}
>>
>> -When {\em rs2}$\neq${\tt x0}, bits XLEN-1:VMIDMAX of the value held in {\em
>> -rs2} are reserved for future use and should be zeroed by software and ignored
>> -by current implementations.
>> -Furthermore, if VMIDLEN~$<$~VMIDMAX, the implementation shall ignore bits
>> -VMIDMAX-1:VMIDLEN of the value held in {\em rs2}.
>> +When {\em rs2}$\neq${\tt x0}, bits contain 3 informations: mode, vmid, ppn.
>> +1) mode control HFENCE.GVMA broadcast or not.
>> +2) ppn is the root page talbe's PPN of the vmid address space.
>> +3) vmid is the identifier of virtual machine.
>>
>>  \begin{commentary}
>>  Simpler implementations of HFENCE.GVMA can ignore the guest physical address in
>> @@ -1567,21 +1565,22 @@ register.
>>  \subsection{Memory-Management Fences}
>>
>>  The behavior of the SFENCE.VMA instruction is affected by the current
>> -virtualization mode V.  When V=0, the virtual-address argument is an HS-level
>> -virtual address, and the ASID argument is an HS-level ASID.
>> +virtualization mode V.  When V=0, the rs1 argument is an HS-level
>> +virtual address, and the rs2 argument is an HS-level ASID and root page table's PPN.
>>  The instruction orders stores only to HS-level address-translation structures
>>  with subsequent HS-level address translations.
>>
>> -When V=1, the virtual-address argument to SFENCE.VMA is a guest virtual
>> -address within the current virtual machine, and the ASID argument is a VS-level
>> -ASID within the current virtual machine.
>> +When V=1, the rs1 argument to SFENCE.VMA is a guest virtual
>> +address within the current virtual machine, and the rs2 argument is a VS-level
>> +ASID and root page table's PPN within the current virtual machine.
>>  The current virtual machine is identified by the VMID field of CSR {\tt hgatp},
>> -and the effective ASID can be considered to be the combination of this VMID
>> -with the VS-level ASID.
>> +and the effective ASID and root page table's PPN can be considered to be the
>> +combination of this VMID and root page table's PPN with the VS-level ASID and
>> +root page table's PPN.
>>  The SFENCE.VMA instruction orders stores only to the VS-level
>>  address-translation structures with subsequent VS-level address translations
>> -for the same virtual machine, i.e., only when {\tt hgatp}.VMID is the same as
>> -when the SFENCE.VMA executed.
>> +for the same virtual machine, i.e., only when {\tt hgatp}.VMID and {\\tt hgatp}.PPN is
>> +the same as when the SFENCE.VMA executed.
>>
>>  Hypervisor instructions HFENCE.GVMA and HFENCE.VVMA provide additional
>>  memory-management fences to complement SFENCE.VMA.
>> diff --git a/src/supervisor.tex b/src/supervisor.tex
>> index ba3ced5..2877b7a 100644
>> --- a/src/supervisor.tex
>> +++ b/src/supervisor.tex
>> @@ -47,10 +47,12 @@ register keeps track of the processor's current operating state.
>>  \begin{center}
>>  \setlength{\tabcolsep}{4pt}
>>  \scalebox{0.95}{
>> -\begin{tabular}{cWcccccWccccWcc}
>> +\begin{tabular}{cccWcccccWccccWcc}
>>  \\
>>  \instbit{31} &
>> -\instbitrange{30}{20} &
>> +\instbit{30} &
>> +\instbit{29} &
>> +\instbitrange{28}{20} &
>>  \instbit{19} &
>>  \instbit{18} &
>>  \instbit{17} &
>> @@ -66,6 +68,8 @@ register keeps track of the processor's current operating state.
>>  \instbit{0} \\
>>  \hline
>>  \multicolumn{1}{|c|}{SD} &
>> +\multicolumn{1}{|c|}{TLBI} &
>> +\multicolumn{1}{|c|}{TLBIC} &
>>  \multicolumn{1}{c|}{\wpri} &
>>  \multicolumn{1}{c|}{MXR} &
>>  \multicolumn{1}{c|}{SUM} &
>> @@ -82,7 +86,7 @@ register keeps track of the processor's current operating state.
>>  \multicolumn{1}{c|}{\wpri}
>>  \\
>>  \hline
>> -1 & 11 & 1 & 1 & 1 & 2 & 2 & 4 & 1 & 1 & 1 & 1 & 3 & 1 & 1 \\
>> +1 & 1 & 1 & 10 & 1 & 1 & 1 & 2 & 2 & 4 & 1 & 1 & 1 & 1 & 3 & 1 & 1 \\
>>  \end{tabular}}
>>  \end{center}
>>  }
>> @@ -95,10 +99,12 @@ register keeps track of the processor's current operating state.
>>  {\footnotesize
>>  \begin{center}
>>  \setlength{\tabcolsep}{4pt}
>> -\begin{tabular}{cMFScccc}
>> +\begin{tabular}{cccMFScccc}
>>  \\
>>  \instbit{SXLEN-1} &
>> -\instbitrange{SXLEN-2}{34} &
>> +\instbit{SXLEN-2} &
>> +\instbit{SXLEN-3} &
>> +\instbitrange{SXLEN-4}{34} &
>>  \instbitrange{33}{32} &
>>  \instbitrange{31}{20} &
>>  \instbit{19} &
>> @@ -107,6 +113,8 @@ register keeps track of the processor's current operating state.
>>   \\
>>  \hline
>>  \multicolumn{1}{|c|}{SD} &
>> +\multicolumn{1}{|c|}{TLBI} &
>> +\multicolumn{1}{|c|}{TLBIC} &
>>  \multicolumn{1}{c|}{\wpri} &
>>  \multicolumn{1}{c|}{UXL[1:0]} &
>>  \multicolumn{1}{c|}{\wpri} &
>> @@ -115,7 +123,7 @@ register keeps track of the processor's current operating state.
>>  \multicolumn{1}{c|}{\wpri} &
>>   \\
>>  \hline
>> -1 & SXLEN-35 & 2 & 12 & 1 & 1 & 1 & \\
>> +1 & 1 & 1 & SXLEN-37 & 2 & 12 & 1 & 1 & 1 & \\
>>  \end{tabular}
>>  \begin{tabular}{cWWFccccWcc}
>>  \\
>> @@ -152,6 +160,17 @@ register keeps track of the processor's current operating state.
>>  \label{sstatusreg}
>>  \end{figure*}
>>
>> +The TLBI (read-only) bit indicates that any async sfence.vma operations are
>> +still pended on the hart. The value:0 means that there is no sfence.vma
>> +operations pending and value:1 means that there are still sfence.vma operations
>> +pending on the hart.
>> +
>> +When the sstatus:TLBIC bit is written 1, it triggers the hardware to check if
>> +there are any TLB invalidate operations being pended. When all operations are
>> +finished, a TLB Invalidate finish interrupt will be triggered
>> +(see Section~\ref{sipreg}). When the sstatus:TLBIC bit is written 0, it will
>> +cause nothing. Reading sstatus:TLBIC bit will alaways return 0.
>> +
>>  The SPP bit indicates the privilege level at which a hart was executing before
>>  entering supervisor mode.  When a trap is taken, SPP is set to 0 if the trap
>>  originated from user mode, or 1 otherwise.  When an SRET instruction
>> @@ -329,8 +348,10 @@ SXLEN-bit read/write register containing interrupt enable bits.
>>  {\footnotesize
>>  \begin{center}
>>  \setlength{\tabcolsep}{4pt}
>> -\begin{tabular}{KcFcFcc}
>> -\instbitrange{SXLEN-1}{10} &
>> +\begin{tabular}{KcFcFcFcc}
>> +\instbitrange{SXLEN-1}{14} &
>> +\instbit{13} &
>> +\instbitrange{12}{10} &
>>  \instbit{9} &
>>  \instbitrange{8}{6} &
>>  \instbit{5} &
>> @@ -339,6 +360,8 @@ SXLEN-bit read/write register containing interrupt enable bits.
>>  \instbit{0} \\
>>  \hline
>>  \multicolumn{1}{|c|}{\wpri} &
>> +\multicolumn{1}{c|}{STLBIP} &
>> +\multicolumn{1}{|c|}{\wpri} &
>>  \multicolumn{1}{c|}{SEIP} &
>>  \multicolumn{1}{c|}{\wpri} &
>>  \multicolumn{1}{c|}{STIP} &
>> @@ -346,7 +369,7 @@ SXLEN-bit read/write register containing interrupt enable bits.
>>  \multicolumn{1}{c|}{SSIP} &
>>  \multicolumn{1}{c|}{\wpri} \\
>>  \hline
>> -SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
>> +SXLEN-14 & 1 & 3 & 1 & 3 & 1 & 3 & 1 & 1 \\
>>  \end{tabular}
>>  \end{center}
>>  }
>> @@ -359,8 +382,10 @@ SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
>>  {\footnotesize
>>  \begin{center}
>>  \setlength{\tabcolsep}{4pt}
>> -\begin{tabular}{KcFcFcc}
>> -\instbitrange{SXLEN-1}{10} &
>> +\begin{tabular}{KcFcFcFcc}
>> +\instbitrange{SXLEN-1}{14} &
>> +\instbit{13} &
>> +\instbitrange{12}{10} &
>>  \instbit{9} &
>>  \instbitrange{8}{6} &
>>  \instbit{5} &
>> @@ -369,6 +394,8 @@ SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
>>  \instbit{0} \\
>>  \hline
>>  \multicolumn{1}{|c|}{\wpri} &
>> +\multicolumn{1}{c|}{STLBIE} &
>> +\multicolumn{1}{|c|}{\wpri} &
>>  \multicolumn{1}{c|}{SEIE} &
>>  \multicolumn{1}{c|}{\wpri} &
>>  \multicolumn{1}{c|}{STIE} &
>> @@ -376,7 +403,7 @@ SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
>>  \multicolumn{1}{c|}{SSIE} &
>>  \multicolumn{1}{c|}{\wpri} \\
>>  \hline
>> -SXLEN-10 & 1 & 3 & 1 & 3 & 1 & 1 \\
>> +SXLEN-14 & 1 & 3 & 1 & 3 & 1 & 3 & 1 & 1 \\
>>  \end{tabular}
>>  \end{center}
>>  }
>> @@ -410,6 +437,12 @@ when the SEIE bit in the {\tt sie} register is clear.  The implementation
>>  should provide facilities to mask, unmask, and query the cause of external
>>  interrupts.
>>
>> +A supervisor-level TLB Invalidate finish interrupt is pending if the STLBIP bit
>> +in the {\tt sip} register is set.  Supervisor-level TLB Invalidate finish
>> +interrupts are disabled when the STLBIE bit in the {\tt sie} register is clear.
>> +When hart tlb invalidate operations are finished, hardware will change sstatus:TLBI
>> +bit from 1 to 0 and trigger TLB Invalidate finish interrupt.
>> +
>>  \begin{commentary}
>>  The {\tt sip} and {\tt sie} registers are subsets of the {\tt mip} and {\tt
>>  mie} registers.  Reading any field, or writing any writable field, of {\tt
>> @@ -598,7 +631,9 @@ so is only guaranteed to hold supported exception codes.
>>    1         & 5               & Supervisor timer interrupt \\
>>    1         & 6--8            & {\em Reserved} \\
>>    1         & 9               & Supervisor external interrupt \\
>> -  1         & 10--15          & {\em Reserved} \\
>> +  1         & 10--11          & {\em Reserved} \\
>> +  1         & 12              & Supervisor TLBI finish interrupt \\
>> +  1         & 13--15          & {\em Reserved} \\
>>    1         & $\ge$16         & {\em Available for platform use} \\ \hline
>>    0         & 0               & Instruction address misaligned \\
>>    0         & 1               & Instruction access fault \\
>> @@ -884,7 +919,7 @@ provided.
>>  \multicolumn{1}{c|}{opcode} \\
>>  \hline
>>  7 & 5 & 5 & 3 & 5 & 7 \\
>> -SFENCE.VMA & asid & vaddr & PRIV & 0 & SYSTEM \\
>> +SFENCE.VMA & mode:ppn:asid & vaddr & LOCAL & 0 & SYSTEM \\
>>  \end{tabular}
>>  \end{center}
>>
>> @@ -899,21 +934,70 @@ from that hart to the memory-management data structures.
>>  Further details on the behavior of this instruction are
>>  described in Section~\ref{virt-control} and Section~\ref{pmp-vmem}.
>>
>> +SFENCE.VMA is defined as an asynchronous completion instruction, which means
>> +that the TLB operation is not guaranteed to complete when the instruction retires.
>> +Software need check sstatus:TLBI to determine all TLB operations complete.
>> +The sstatus:TLBI described in Section~\ref{sstatus}. When hardware change
>> +sstatus:TLBI bit from 1 to 0, the TLB Invalidate finish interrupt will be
>> +triggered.
>> +
>>  \begin{commentary}
>> -The SFENCE.VMA is used to flush any local hardware caches related to
>> +The SFENCE.VMA is used to flush any local/remote hardware caches related to
>>  address translation.  It is specified as a fence rather than a TLB
>>  flush to provide cleaner semantics with respect to which instructions
>>  are affected by the flush operation and to support a wider variety of
>>  dynamic caching structures and memory-management schemes.  SFENCE.VMA
>>  is also used by higher privilege levels to synchronize page table
>> -writes and the address translation hardware.
>> +writes and the address translation hardware. There is a mode bit to determine
>> +sfence.vma would broadcast on interconnect or not.
>>  \end{commentary}
>>
>> -SFENCE.VMA orders only the local hart's implicit references to the
>> -memory-management data structures.
>> +\begin{figure}[h!]
>> +{\footnotesize
>> +\begin{center}
>> +\begin{tabular}{c@{}E@{}K}
>> +\instbit{31} &
>> +\instbitrange{30}{9} &
>> +\instbitrange{8}{0} \\
>> +\hline
>> +\multicolumn{1}{|c|}{{\tt MODE}} &
>> +\multicolumn{1}{|c|}{{\tt PPN (root page table)}} &
>> +\multicolumn{1}{|c|}{{\tt ASID}} \\
>> +\hline
>> +1 & 22 & 9 \\
>> +\end{tabular}
>> +\end{center}
>> +}
>> +\vspace{-0.1in}
>> +\caption{RV32 sfence.vma rs2 format.}
>> +\label{rv32satp}
>> +\end{figure}
>> +
>> +\begin{figure}[h!]
>> +{\footnotesize
>> +\begin{center}
>> +\begin{tabular}{@{}S@{}T@{}U}
>> +\instbitrange{63}{60} &
>> +\instbitrange{59}{16} &
>> +\instbitrange{15}{0} \\
>> +\hline
>> +\multicolumn{1}{|c|}{{\tt MODE}} &
>> +\multicolumn{1}{|c|}{{\tt PPN (root page table)}} &
>> +\multicolumn{1}{|c|}{{\tt ASID}} \\
>> +\hline
>> +4 & 44 & 16 \\
>> +\end{tabular}
>> +\end{center}
>> +}
>> +\vspace{-0.1in}
>> +\caption{RV64 sfence.vma rs2 format, for MODE values, only highest bit:63 is
>> +valid and others are reserved.}
>> +\label{rv64satp}
>> +\end{figure}
>>
>>  \begin{commentary}
>> -Consequently, other harts must be notified separately when the
>> +The mode's highest bit could control sfence.vma behavior with 1:broadcast or 0:local.
>> +If only have mode:local, other harts must be notified separately when the
>>  memory-management data structures have been modified.
>>  One approach is to use 1)
>>  a local data fence to ensure local writes are visible globally, then
>> @@ -928,8 +1012,17 @@ modified for a single address mapping (i.e., one page or superpage), {\em rs1}
>>  can specify a virtual address within that mapping to effect a translation
>>  fence for that mapping only.  Furthermore, for the common case that the
>>  translation data structures have only been modified for a single address-space
>> -identifier, {\em rs2} can specify the address space.  The behavior of
>> -SFENCE.VMA depends on {\em rs1} and {\em rs2} as follows:
>> +identifier, {\em rs2} can specify the address space with {\tt satp} format
>> +which include asid and root page table's PPN information.
>> +
>> +\begin{commentary}
>> +We use ASID and root page table's PPN to determine address space and the format
>> +stored in rs2 is similar with {\tt satp} described in Section~\ref{sec:satp}.
>> +ASID are used by local harts and root page table's PPN of the asid are used by
>> +other different TLB systems, eg: IOMMU.
>> +\end{commentary}
>> +
>> +The behavior of SFENCE.VMA depends on {\em rs1} and {\em rs2} as follows:
>>
>>  \begin{itemize}
>>  \item If {\em rs1}={\tt x0} and {\em rs2}={\tt x0}, the fence orders all
>> @@ -939,23 +1032,18 @@ SFENCE.VMA depends on {\em rs1} and {\em rs2} as follows:
>>        all reads and writes made to any level of the page tables, but only
>>        for the address space identified by integer register {\em rs2}.
>>        Accesses to {\em global} mappings (see Section~\ref{sec:translation})
>> -      are not ordered.
>> +      are not ordered. The mode field in rs2 is determine broadcast or local.
>>  \item If {\em rs1}$\neq${\tt x0} and {\em rs2}={\tt x0}, the fence orders
>>        only reads and writes made to the leaf page table entry corresponding
>>        to the virtual address in {\em rs1}, for all address spaces.
>>  \item If {\em rs1}$\neq${\tt x0} and {\em rs2}$\neq${\tt x0}, the fence
>>        orders only reads and writes made to the leaf page table entry
>>        corresponding to the virtual address in {\em rs1}, for the address
>> -      space identified by integer register {\em rs2}.
>> +      space identified by integer register {\em rs2}. The mode field in rs2
>> +      is determine broadcast or local.
>>        Accesses to global mappings are not ordered.
>>  \end{itemize}
>>
>> -When {\em rs2}$\neq${\tt x0}, bits SXLEN-1:ASIDMAX of the value held in {\em
>> -rs2} are reserved for future use and should be zeroed by software and ignored
>> -by current implementations.  Furthermore, if ASIDLEN~$<$~ASIDMAX, the
>> -implementation shall ignore bits ASIDMAX-1:ASIDLEN of the value held in {\em
>> -rs2}.
>> -
>>  \begin{commentary}
>>  Simpler implementations can ignore the virtual address in {\em rs1} and
>>  the ASID value in {\em rs2} and always perform a global fence.
>> @@ -994,7 +1082,7 @@ can execute the same SFENCE.VMA instruction while a different ASID is loaded
>>  into {\tt satp}, provided the next time {\tt satp} is loaded with the recycled
>>  ASID, it is simultaneously loaded with the new page table.
>>
>> -\item If the implementation does not provide ASIDs, or software chooses to
>> +\item If the implementation does not provide ASIDs and PPNs, or software chooses to
>>  always use ASID 0, then after every {\tt satp} write, software should execute
>>  SFENCE.VMA with {\em rs1}={\tt x0}.  In the common case that no global
>>  translations have been modified, {\em rs2} should be set to a register other than
>> @@ -1003,13 +1091,14 @@ not flushed.
>>
>>  \item If software modifies a non-leaf PTE, it should execute SFENCE.VMA with
>>  {\em rs1}={\tt x0}.  If any PTE along the traversal path had its G bit set,
>> -{\em rs2} must be {\tt x0}; otherwise, {\em rs2} should be set to the ASID for
>> -which the translation is being modified.
>> +{\em rs2} must be {\tt x0}; otherwise, {\em rs2} should be set to the ASID and
>> +root page table's PPN for which the translation is being modified.
>>
>>  \item If software modifies a leaf PTE, it should execute SFENCE.VMA with {\em
>>  rs1} set to a virtual address within the page.  If any PTE along the traversal
>>  path had its G bit set, {\em rs2} must be {\tt x0}; otherwise, {\em rs2}
>> -should be set to the ASID for which the translation is being modified.
>> +should be set to the ASID and root page table's PPN for which the translation
>> +is being modified.
>>
>>  \item For the special cases of increasing the permissions on a leaf PTE and
>>  changing an invalid PTE to a valid leaf, software may choose to execute
>> --
>> 2.7.4
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>>
>> View/Reply Online (#810): https://lists.riscv.org/g/tech-privileged/message/810
>> Mute This Topic: https://lists.riscv.org/mt/34198986/1677273
>> Group Owner: tech-privileged+owner@lists.riscv.org
>> Unsubscribe: https://lists.riscv.org/g/tech-privileged/unsub  [andrew@sifive.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>


-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH RFC 11/14] arm64: Move the ASID allocator code in a separate file
From: Guo Ren @ 2019-09-20  0:07 UTC (permalink / raw)
  To: Jean-Philippe Brucker
  Cc: aou, Linux Kernel Mailing List, Arnd Bergmann, suzuki.poulose,
	Marc Zyngier, Catalin Marinas, Palmer Dabbelt, christoffer.dall,
	iommu, Mike Rapoport, Anup Patel, Atish Patra, Julien Grall,
	james.morse, gary, Paul Walmsley, linux-riscv, Will Deacon,
	kvmarm, linux-arm-kernel
In-Reply-To: <20190919151844.GG1013538@lophozonia>

On Thu, Sep 19, 2019 at 11:18 PM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:

>
> The SMMU does support PCI Virtual Function - an hypervisor can assign a
> VF to a guest, and let that guest partition the VF into smaller contexts
> by using PASID.  What it can't support is assigning partitions of a PCI
> function (VF or PF) to multiple Virtual Machines, since there is a
> single S2 PGD per function (in the Stream Table Entry), rather than one
> S2 PGD per PASID context.
>
In my concept, the two sentences "The SMMU does support PCI Virtual
Functio" v.s. "What it can't support is assigning partitions of a PCI
function (VF or PF) to multiple Virtual Machines" are conflict and I
don't want to play naming game :)

-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 2/2] soc: ti: move 2 driver config options into the TI SOC drivers menu
From: Randy Dunlap @ 2019-09-19 22:33 UTC (permalink / raw)
  To: LKML, LAK
  Cc: Olof Johansson, Nishanth Menon, Benjamin Fair, Tony Lindgren,
	Tero Kristo

From: Randy Dunlap <rdunlap@infradead.org>

Move the AM654 and J721E SOC config options inside the "TI SOC drivers"
menu with the other TI SOC drivers.

Fixes: a869b7b30dac ("soc: ti: Add Support for AM654 SoC config option")
Fixes: cff377f7897a ("soc: ti: Add Support for J721E SoC config option")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Olof Johansson <olof@lixom.net>
Cc: Nishanth Menon <nm@ti.com>
Cc: Benjamin Fair <b-fair@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Tero Kristo <t-kristo@ti.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
 drivers/soc/ti/Kconfig |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

--- lnx-53.orig/drivers/soc/ti/Kconfig
+++ lnx-53/drivers/soc/ti/Kconfig
@@ -1,4 +1,12 @@
 # SPDX-License-Identifier: GPL-2.0-only
+
+# TI SOC drivers
+#
+menuconfig SOC_TI
+	bool "TI SOC drivers support"
+
+if SOC_TI
+
 # 64-bit ARM SoCs from TI
 if ARM64
 
@@ -14,17 +22,9 @@ config ARCH_K3_J721E_SOC
 	help
 	  Enable support for TI's J721E SoC Family.
 
-endif
+endif # ARCH_K3
 
-endif
-
-#
-# TI SOC drivers
-#
-menuconfig SOC_TI
-	bool "TI SOC drivers support"
-
-if SOC_TI
+endif # ARM64
 
 config KEYSTONE_NAVIGATOR_QMSS
 	tristate "Keystone Queue Manager Subsystem"



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH 1/2] soc: ti: big cleanup of Kconfig file
From: Randy Dunlap @ 2019-09-19 22:33 UTC (permalink / raw)
  To: LKML, LAK
  Cc: Dave Gerlach, Tony Lindgren, Keerthy, Sandeep Nair,
	Santosh Shilimkar, Olof Johansson

From: Randy Dunlap <rdunlap@infradead.org>

Cleanup drivers/soc/ti/Kconfig:
- delete duplicate words
- end sentences with '.'
- fix typos/spellos
- Subsystem is one word
- capitalize acronyms
- reflow lines to be <= 80 columns

Fixes: 41f93af900a2 ("soc: ti: add Keystone Navigator QMSS driver")
Fixes: 88139ed03058 ("soc: ti: add Keystone Navigator DMA support")
Fixes: afe761f8d3e9 ("soc: ti: Add pm33xx driver for basic suspend support")
Fixes: 5a99ae0092fe ("soc: ti: pm33xx: AM437X: Add rtc_only with ddr in self-refresh support")
Fixes: a869b7b30dac ("soc: ti: Add Support for AM654 SoC config option")
Fixes: cff377f7897a ("soc: ti: Add Support for J721E SoC config option")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Olof Johansson <olof@lixom.net>
Cc: Santosh Shilimkar <ssantosh@kernel.org>
Cc: Sandeep Nair <sandeep_n@ti.com>
Cc: Dave Gerlach <d-gerlach@ti.com>
Cc: Keerthy <j-keerthy@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
@Santosh: MAINTAINERS says that you maintain drivers/soc/ti/*,
but there is more that Keystone-related code in that subdirectory
now... just in case you want to update that info.

 drivers/soc/ti/Kconfig |   20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

--- lnx-53.orig/drivers/soc/ti/Kconfig
+++ lnx-53/drivers/soc/ti/Kconfig
@@ -7,12 +7,12 @@ if ARCH_K3
 config ARCH_K3_AM6_SOC
 	bool "K3 AM6 SoC"
 	help
-	  Enable support for TI's AM6 SoC Family support
+	  Enable support for TI's AM6 SoC Family.
 
 config ARCH_K3_J721E_SOC
 	bool "K3 J721E SoC"
 	help
-	  Enable support for TI's J721E SoC Family support
+	  Enable support for TI's J721E SoC Family.
 
 endif
 
@@ -27,7 +27,7 @@ menuconfig SOC_TI
 if SOC_TI
 
 config KEYSTONE_NAVIGATOR_QMSS
-	tristate "Keystone Queue Manager Sub System"
+	tristate "Keystone Queue Manager Subsystem"
 	depends on ARCH_KEYSTONE
 	help
 	  Say y here to support the Keystone multicore Navigator Queue
@@ -42,9 +42,9 @@ config KEYSTONE_NAVIGATOR_DMA
 	tristate "TI Keystone Navigator Packet DMA support"
 	depends on ARCH_KEYSTONE
 	help
-	  Say y tp enable support for the Keystone Navigator Packet DMA on
-	  on Keystone family of devices. It sets up the dma channels for the
-	  Queue Manager Sub System.
+	  Say y to enable support for the Keystone Navigator Packet DMA on
+	  on Keystone family of devices. It sets up the DMA channels for the
+	  Queue Manager Subsystem.
 
 	  If unsure, say N.
 
@@ -53,10 +53,10 @@ config AMX3_PM
 	depends on SOC_AM33XX || SOC_AM43XX
 	depends on WKUP_M3_IPC && TI_EMIF_SRAM && SRAM && RTC_DRV_OMAP
 	help
-	  Enable power management on AM335x and AM437x. Required for suspend to mem
-	  and standby states on both AM335x and AM437x platforms and for deeper cpuidle
-	  c-states on AM335x. Also required for rtc and ddr in self-refresh low
-	  power mode on AM437x platforms.
+	  Enable power management on AM335x and AM437x. Required for suspend
+	  to mem and standby states on both AM335x and AM437x platforms and
+	  for deeper cpuidle c-states on AM335x. Also required for RTC and
+	  DDR in self-refresh low power mode on AM437x platforms.
 
 config WKUP_M3_IPC
 	tristate "TI AMx3 Wkup-M3 IPC Driver"


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v2] clk: imx7ulp: remove IMX7ULP_CLK_MIPI_PLL clock
From: Stephen Boyd @ 2019-09-19 22:17 UTC (permalink / raw)
  To: shawnguo@kernel.org, Fancy Fang
  Cc: mturquette@baylibre.com, linux-kernel@vger.kernel.org,
	linux-clk@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
	s.hauer@pengutronix.de, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190919030912.16957-2-chen.fang@nxp.com>

Quoting Fancy Fang (2019-09-18 20:10:48)
> diff --git a/include/dt-bindings/clock/imx7ulp-clock.h b/include/dt-bindings/clock/imx7ulp-clock.h
> index 6f66f9005c81..a39b0c40cb41 100644
> --- a/include/dt-bindings/clock/imx7ulp-clock.h
> +++ b/include/dt-bindings/clock/imx7ulp-clock.h
> @@ -49,7 +49,6 @@
>  #define IMX7ULP_CLK_NIC1_DIV           36
>  #define IMX7ULP_CLK_NIC1_BUS_DIV       37
>  #define IMX7ULP_CLK_NIC1_EXT_DIV       38
> -#define IMX7ULP_CLK_MIPI_PLL           39

You can't remove this. Just add a comment like /* unused */ or
something to indicate this shouldn't be used.

>  #define IMX7ULP_CLK_SIRC               40
>  #define IMX7ULP_CLK_SOSC_BUS_CLK       41
>  #define IMX7ULP_CLK_FIRC_BUS_CLK       42
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [GIT PULL] Mailbox changes for v5.4
From: pr-tracker-bot @ 2019-09-19 21:30 UTC (permalink / raw)
  To: Jassi Brar; +Cc: Linus Torvalds, Linux Kernel Mailing List, linux-arm-kernel
In-Reply-To: <CABb+yY2AFK4G8i765--h0D7h1xcsrhSP2fKzWmcza9OcrdT22g@mail.gmail.com>

The pull request you sent on Wed, 18 Sep 2019 11:00:28 -0500:

> git://git.linaro.org/landing-teams/working/fujitsu/integration.git tags/mailbox-v5.4

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/b682242f6012dddf81ef94b7ce5d2ec5ac8f8047

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] ARM: dts: rockchip: Add cpu id to rk3288 efuse node
From: Douglas Anderson @ 2019-09-19 21:26 UTC (permalink / raw)
  To: Heiko Stuebner
  Cc: Mark Rutland, devicetree, Douglas Anderson, Rob Herring,
	linux-kernel, linux-rockchip, mka, linux-arm-kernel

This just adds in another field of what's stored in the e-fuse on
rk3288.  Though I can't personally promise that every rk3288 out there
has the CPU ID stored in the eFuse at this location, there is some
evidence that it is correct:
- This matches what was in the Chrome OS 3.14 branch (see
  EFUSE_CHIP_UID_OFFSET and EFUSE_CHIP_UID_LEN) for rk3288.
- The upstream rk3399 dts file has this same data at the same offset
  and with the same length, indiciating that this is likely common for
  several modern Rockchip SoCs.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 arch/arm/boot/dts/rk3288.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index cc893e154fe5..415b48fc3ce8 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -1391,6 +1391,9 @@
 		clocks = <&cru PCLK_EFUSE256>;
 		clock-names = "pclk_efuse";
 
+		cpu_id: cpu-id@7 {
+			reg = <0x07 0x10>;
+		};
 		cpu_leakage: cpu_leakage@17 {
 			reg = <0x17 0x1>;
 		};
-- 
2.23.0.351.gc4317032e6-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related

* Re: mt76x2e hardware restart
From: Oleksandr Natalenko @ 2019-09-19 21:22 UTC (permalink / raw)
  To: linux-mediatek
  Cc: Ryder Lee, Stanislaw Gruszka, netdev, linux-wireless,
	linux-kernel, Matthias Brugger, linux-arm-kernel, Roy Luo,
	Lorenzo Bianconi, Lorenzo Bianconi, David S. Miller, Kalle Valo,
	Felix Fietkau
In-Reply-To: <deaafa7a3e9ea2111ebb5106430849c6@natalenko.name>

On 19.09.2019 18:24, Oleksandr Natalenko wrote:
> [  +9,979664] mt76x2e 0000:01:00.0: Firmware Version: 0.0.00
> [  +0,000014] mt76x2e 0000:01:00.0: Build: 1
> [  +0,000010] mt76x2e 0000:01:00.0: Build Time: 201507311614____
> [  +0,018017] mt76x2e 0000:01:00.0: Firmware running!
> [  +0,001101] ieee80211 phy4: Hardware restart was requested

IIUC, this happens due to watchdog. I think the following applies.

Watchdog is started here:

=== mt76x02_util.c
130 void mt76x02_init_device(struct mt76x02_dev *dev)
131 {
...
155         INIT_DELAYED_WORK(&dev->wdt_work, mt76x02_wdt_work);
===

It checks for TX hang here:

=== mt76x02_mmio.c
557 void mt76x02_wdt_work(struct work_struct *work)
558 {
...
562     mt76x02_check_tx_hang(dev);
===

Conditions:

=== mt76x02_mmio.c
530 static void mt76x02_check_tx_hang(struct mt76x02_dev *dev)
531 {
532     if (mt76x02_tx_hang(dev)) {
533         if (++dev->tx_hang_check >= MT_TX_HANG_TH)
534             goto restart;
535     } else {
536         dev->tx_hang_check = 0;
537     }
538
539     if (dev->mcu_timeout)
540         goto restart;
541
542     return;
543
544 restart:
545     mt76x02_watchdog_reset(dev);
===

Actual check:

=== mt76x02_mmio.c
367 static bool mt76x02_tx_hang(struct mt76x02_dev *dev)
368 {
369     u32 dma_idx, prev_dma_idx;
370     struct mt76_queue *q;
371     int i;
372
373     for (i = 0; i < 4; i++) {
374         q = dev->mt76.q_tx[i].q;
375
376         if (!q->queued)
377             continue;
378
379         prev_dma_idx = dev->mt76.tx_dma_idx[i];
380         dma_idx = readl(&q->regs->dma_idx);
381         dev->mt76.tx_dma_idx[i] = dma_idx;
382
383         if (prev_dma_idx == dma_idx)
384             break;
385     }
386
387     return i < 4;
388 }
===

(I don't quite understand what it does here; why 4? does each device 
have 4 queues? maybe, my does not? I guess this is where watchdog is 
triggered, though, because otherwise I'd see mcu_timeout message like 
"MCU message %d (seq %d) timed out\n")

Once it detects TX hang, the reset is triggered:

=== mt76x02_mmio.c
446 static void mt76x02_watchdog_reset(struct mt76x02_dev *dev)
447 {
...
485     if (restart)
486         mt76_mcu_restart(dev);
===

mt76_mcu_restart() is just a define for this series here:

=== mt76.h
555 #define mt76_mcu_restart(dev, ...)  
(dev)->mt76.mcu_ops->mcu_restart(&((dev)->mt76))
===

Actual OP:

=== mt76x2/pci_mcu.c
188 int mt76x2_mcu_init(struct mt76x02_dev *dev)
189 {
190     static const struct mt76_mcu_ops mt76x2_mcu_ops = {
191         .mcu_restart = mt76pci_mcu_restart,
192         .mcu_send_msg = mt76x02_mcu_msg_send,
193     };
===

This triggers loading the firmware:

=== mt76x2/pci_mcu.c
168 static int
169 mt76pci_mcu_restart(struct mt76_dev *mdev)
170 {
...
179     ret = mt76pci_load_firmware(dev);
===

which does the printout I observe:

=== mt76x2/pci_mcu.c
  91 static int
  92 mt76pci_load_firmware(struct mt76x02_dev *dev)
  93 {
...
156     dev_info(dev->mt76.dev, "Firmware running!\n");
===

Too bad it doesn't show the actual watchdog message, IOW, why the reset 
happens. I guess I will have to insert some pr_infos here and there.

Does it make sense? Any ideas why this can happen?

More info on the device during boot:

===
[  +0,333233] mt76x2e 0000:01:00.0: enabling device (0000 -> 0002)
[  +0,000571] mt76x2e 0000:01:00.0: ASIC revision: 76120044
[  +0,017806] mt76x2e 0000:01:00.0: ROM patch build: 20141115060606a
===

-- 
   Oleksandr Natalenko (post-factum)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH v4 3/5] locking/qspinlock: Introduce CNA into the slow path of qspinlock
From: Waiman Long @ 2019-09-19 20:54 UTC (permalink / raw)
  To: Alex Kogan
  Cc: linux-arch, guohanjun, arnd, peterz, dave.dice, jglauber, x86,
	will.deacon, linux, linux-kernel, rahul.x.yadav, mingo, bp, hpa,
	steven.sistare, tglx, daniel.m.jordan, linux-arm-kernel
In-Reply-To: <87B87982-670F-4F12-9EE0-DC89A059FAEC@oracle.com>

On 9/19/19 11:55 AM, Alex Kogan wrote:
>>> +/*
>>> + * cna_try_find_next - scan the main waiting queue looking for the first
>>> + * thread running on the same NUMA node as the lock holder. If found (call it
>>> + * thread T), move all threads in the main queue between the lock holder and
>>> + * T to the end of the secondary queue and return T; otherwise, return NULL.
>>> + *
>>> + * Schematically, this may look like the following (nn stands for numa_node and
>>> + * et stands for encoded_tail).
>>> + *
>>> + *     when cna_try_find_next() is called (the secondary queue is empty):
>>> + *
>>> + *  A+------------+   B+--------+   C+--------+   T+--------+
>>> + *   |mcs:next    | -> |mcs:next| -> |mcs:next| -> |mcs:next| -> NULL
>>> + *   |mcs:locked=1|    |cna:nn=0|    |cna:nn=2|    |cna:nn=1|
>>> + *   |cna:nn=1    |    +--------+    +--------+    +--------+
>>> + *   +----------- +
>>> + *
>>> + *     when cna_try_find_next() returns (the secondary queue contains B and C):
>>> + *
>>> + *  A+----------------+    T+--------+
>>> + *   |mcs:next        | ->  |mcs:next| -> NULL
>>> + *   |mcs:locked=B.et | -+  |cna:nn=1|
>>> + *   |cna:nn=1        |  |  +--------+
>>> + *   +--------------- +  |
>>> + *                       |
>>> + *                       +->  B+--------+   C+--------+
>>> + *                             |mcs:next| -> |mcs:next|
>>> + *                             |cna:nn=0|    |cna:nn=2|
>>> + *                             |cna:tail| -> +--------+
>>> + *                             +--------+
>>> + *
>>> + * The worst case complexity of the scan is O(n), where n is the number
>>> + * of current waiters. However, the fast path, which is expected to be the
>>> + * common case, is O(1).
>>> + */
>>> +static struct mcs_spinlock *cna_try_find_next(struct mcs_spinlock *node,
>>> +					      struct mcs_spinlock *next)
>>> +{
>>> +	struct cna_node *cn = (struct cna_node *)node;
>>> +	struct cna_node *cni = (struct cna_node *)next;
>>> +	struct cna_node *first, *last = NULL;
>>> +	int my_numa_node = cn->numa_node;
>>> +
>>> +	/* fast path: immediate successor is on the same NUMA node */
>>> +	if (cni->numa_node == my_numa_node)
>>> +		return next;
>>> +
>>> +	/* find any next waiter on 'our' NUMA node */
>>> +	for (first = cni;
>>> +	     cni && cni->numa_node != my_numa_node;
>>> +	     last = cni, cni = (struct cna_node *)READ_ONCE(cni->mcs.next))
>>> +		;
>>> +
>>> +	/* if found, splice any skipped waiters onto the secondary queue */
>>> +	if (cni && last)
>>> +		cna_splice_tail(cn, first, last);
>>> +
>>> +	return (struct mcs_spinlock *)cni;
>>> +}
>> At the Linux Plumbers Conference last week, Will has raised the concern
>> about the latency of the O(1) cna_try_find_next() operation that will
>> add to the lock hold time.
> While the worst case complexity of the scan is O(n), I _think it can be proven
> that the amortized complexity is O(1). For intuition, consider a two-node 
> system with N threads total. In the worst case scenario, the scan will go 
> over N/2 threads running on a different node. If the scan ultimately “fails”
> (no thread from the lock holder’s node is found), the lock will be passed
> to the first thread from a different node and then between all those N/2 threads,
> with a scan of just one node for the next N/2 - 1 passes. Otherwise, those 
> N/2 threads will be moved to the secondary queue. On the next lock handover, 
> we pass the lock either to the next thread in the main queue (as it has to be 
> from our node) or to the first node in the secondary queue. In both cases, we 
> scan just one node, and in the latter case, we have again N/2 - 1 passes with 
> a scan of just one node each.
I agree that it should not be a problem for a 2-socket. For larger SMP
systems with 8, 16 or even 32 sockets, it can be an issue as those
systems are also more likely to have more lock contention and hence
longer wait queues.
>> One way to hide some of the latency is to do
>> a pre-scan before acquiring the lock. The CNA code could override the
>> pv_wait_head_or_lock() function to call cna_try_find_next() as a
>> pre-scan and return 0. What do you think?
> This is certainly possible, but I do not think it would completely eliminate 
> the worst case scenario. It will probably make it even less likely, but at 
> the same time, we will reduce the chance of actually finding a thread from the
> same node (that may enter the main queue while we wait for the owner & pending 
> to go away).

When I said prescan, I mean to move the front queue entries that are
from non-local nodes to the secondary queue before acquiring the lock.
After acquiring the lock, you can repeat the scan in case the prescan
didn't find any local node queue entry. Yes, we will need to do the
similar operation twice.

Yes, it does not eliminate the worst case scenario, but it should help
in reducing the average lock hold time.

Of course, the probabilistic (or deterministic) check to go to the next
local node entry or to the secondary queue should be done before
pre-scan so that we won't waste the effort.

Cheers,
Longman


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox