[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 04/19] xen/arm: mm: Allow other mapping size in xen_pt_update_entry()
Hi Stefano,
On 05/04/2022 21:46, Stefano Stabellini wrote:
On Sat, 2 Apr 2022, Julien Grall wrote:
On 02/04/2022 00:35, Stefano Stabellini wrote:
+/* Return the level where mapping should be done */
+static int xen_pt_mapping_level(unsigned long vfn, mfn_t mfn, unsigned
long nr,
+ unsigned int flags)
+{
+ unsigned int level;
+ unsigned long mask;
Shouldn't mask be 64-bit on aarch32?
The 3 variables we will use (mfn, vfn, nr) are unsigned long. So it is fine to
define the mask as unsigned long.
Good point
+}
+
static DEFINE_SPINLOCK(xen_pt_lock);
static int xen_pt_update(unsigned long virt,
mfn_t mfn,
- unsigned long nr_mfns,
+ const unsigned long nr_mfns,
Why const? nr_mfns is an unsigned long so it is passed as value: it
couldn't change the caller's parameter anyway. Just curious.
Because nr_mfns is used to flush the TLBs. In the original I made the mistake
to decrement the variable and only discovered later on when the TLB contained
the wrong entry.
Such bug tends to be very subtle and it is hard to find the root cause. So
better mark the variable const to avoid any surprise.
The short version of what I wrote is in the commit message. I can write a
small comment in the code if you want.
No, that's fine. Thanks for the explanation.
I thought about it and decided to add a comment. This will avoid someone
to remove the 'const'.
Cheers,
--
Julien Grall
|
