Hi Stefano,
On 27/07/2016 19:25, Stefano Stabellini wrote:
On Wed, 27 Jul 2016, Julien Grall wrote:
Hi Stefano,
On 26/07/16 23:28, Stefano Stabellini wrote:
On Wed, 20 Jul 2016, Julien Grall wrote:
@@ -411,7 +411,7 @@ static int p2m_create_table(struct domain *d, lpae_t
*entry,
if ( splitting )
{
p2m_type_t t = entry->p2m.type;
- unsigned long base_pfn = entry->p2m.base;
+ mfn_t mfn = _mfn(entry->p2m.base);
int i;
/*
@@ -420,8 +420,9 @@ static int p2m_create_table(struct domain *d, lpae_t
*entry,
*/
for ( i=0 ; i < LPAE_ENTRIES; i++ )
{
- pte = mfn_to_p2m_entry(base_pfn +
(i<<(level_shift-LPAE_SHIFT)),
- MATTR_MEM, t, p2m->default_access);
+ pte = mfn_to_p2m_entry(mfn, MATTR_MEM, t,
p2m->default_access);
+
+ mfn = mfn_add(mfn, 1UL << (level_shift - LPAE_SHIFT));
Should we be incrementing mfn before calling mfn_to_p2m_entry?
No. The base of the superpage is mfn, after splitting the first entry will be
equal to the base, the second entry base + level_size...
I understand what the patch is doing now, I confused "1" with "i" :-)
The patch is OK. It might be more obvious as the following:
for ( i=0 ; i < LPAE_ENTRIES; i++ )
{
pte = mfn_to_p2m_entry(mfn_add(mfn, (i<<(level_shift-LPAE_SHIFT))),
MATTR_MEM, t, p2m->default_access);
However it's just a matter of taste, so I'll let you choose the way you
prefer.
I wanted to avoid shifting "i" at each loop (which should save an
instruction). However, as it seems to be confusing, I will use your
suggestion.
Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
Thank you!
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel