Re: [Xen-devel] help -- A question about ' XENFEAT_auto_translated_physmap':

[Jan Beulich]

>>> On 10.08.16 at 15:19,  wrote:
> On August 09, 2016 9:02 PM, < jbeul...@suse.com > wrote:
>> >>> On 09.08.16 at 14:36,  wrote:
>> > Hi Jan,
>> >
>> > A question about ' XENFEAT_auto_translated_physmap':
>> >
>> > In linux code, in arch/x86/xen/mmu.c,
>>
>> I assume you know that I'm not a maintainer of the Linux code.
>>
>> >  __xen_pgd_walk()
>> > {
>> > 
>> >
>> > if (xen_feature(XENFEAT_auto_translated_physmap))
>> > return 0;
>> > 
>> > }
>> >
>> >
>> >
>> > Why xen_feature(XENFEAT_auto_translated_physmap) is true, then return
>> > directly?
>> > If not return directly, is there any potential risk?
>>
>> Well, the function is specifically there for operations (pinning/unpinning) 
> which are required only for the not-auto-translated case.
>> Why would anyone want to traverse a page table tree just to do nothing on 
> each of the entries?
> 
> Jan, thank you!!
> As I am struggling with a dom0 crash, the kernel is old, 3.0.X..
> Now there is a crash in 
> 
> [] _pin_lock+0x165/0x2a0<*crash*
> 
> unable to handle kernel paging request at 8b1021826000

Which appears to have pretty little to do with the original question,
as here you appear to be dealing with a forward ported XenoLinux
kernel.

> static void _pin_lock(struct mm_struct *mm, int lock)
> {
> 171 pgd_t *pgd = mm->pgd;
> 172 unsigned g;
> 173
> 174 for (g = 0; g <= ((TASK_SIZE_MAX-1) / PGDIR_SIZE); g++, 
> pgd++) {
> 175 pud_t *pud;
> 176 unsigned u;
> 177
> 178 if (pgd_none(*pgd))
> 179 continue;
> 180 pud = pud_offset(pgd, 0);
> 181 for (u = 0; u < PTRS_PER_PUD; u++, pud++) {
> 182 pmd_t *pmd;
> 183 unsigned m;
> 184
> 185 if (pud_none(*pud))
> 186 continue;
> 187 pmd = pmd_offset(pud, 0);
> 188 for (m = 0; m < PTRS_PER_PMD; m++, pmd++) 
> {
> 189 spinlock_t *ptl;
> 190
> 191 if (pmd_none(*pmd))  
> <-*crash*

Well, fine, it crashes here. But without further detail I don't think
you really expect anyone to be able to help (at the first glance
this looks like a follow-on effect from some earlier problem, as
presumably you've hit a ballooned out page, which obviously
shouldn't be referenced by any page tables)? And for this old a
kernel, and a non-upstream one, such a discussion presumably
doesn't belong on this list anyway. If this is a SUSE kernel, and
if the kernel is up-to-date wrt the distro it comes from, then either
open a ticket with our support folks, or (less reasonable, as there
never was an openSUSE version with a 3.0 based kernel) provide
much greater detail on a list like opensuse-virt...@opensuse.org.

Jan

> 192 continue;
> 193 ptl = pte_lockptr(0, pmd);
> 194 if (lock)
> 195 spin_lock(ptl);
> 196 else
> 197 spin_unlock(ptl);
> 198 }
> 199 }
> 200 }
> 201 }
> 202 #endif
> }
> 
> 
> 
> Quan




___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] help -- A question about ' XENFEAT_auto_translated_physmap':

[Xuquan (Euler)]

On August 09, 2016 9:02 PM, < jbeul...@suse.com > wrote:
> >>> On 09.08.16 at 14:36,  wrote:
> > Hi Jan,
> >
> > A question about ' XENFEAT_auto_translated_physmap':
> >
> > In linux code, in arch/x86/xen/mmu.c,
>
> I assume you know that I'm not a maintainer of the Linux code.
>
> >  __xen_pgd_walk()
> > {
> > 
> >
> > if (xen_feature(XENFEAT_auto_translated_physmap))
> > return 0;
> > 
> > }
> >
> >
> >
> > Why xen_feature(XENFEAT_auto_translated_physmap) is true, then return
> > directly?
> > If not return directly, is there any potential risk?
>
> Well, the function is specifically there for operations (pinning/unpinning) 
> which are required only for the not-auto-translated case.
> Why would anyone want to traverse a page table tree just to do nothing on 
> each of the entries?

Jan, thank you!!
As I am struggling with a dom0 crash, the kernel is old, 3.0.X..
Now there is a crash in 

[] _pin_lock+0x165/0x2a0<*crash*

unable to handle kernel paging request at 8b1021826000

static void _pin_lock(struct mm_struct *mm, int lock)
{
171 pgd_t *pgd = mm->pgd;
172 unsigned g;
173
174 for (g = 0; g <= ((TASK_SIZE_MAX-1) / PGDIR_SIZE); g++, 
pgd++) {
175 pud_t *pud;
176 unsigned u;
177
178 if (pgd_none(*pgd))
179 continue;
180 pud = pud_offset(pgd, 0);
181 for (u = 0; u < PTRS_PER_PUD; u++, pud++) {
182 pmd_t *pmd;
183 unsigned m;
184
185 if (pud_none(*pud))
186 continue;
187 pmd = pmd_offset(pud, 0);
188 for (m = 0; m < PTRS_PER_PMD; m++, pmd++) {
189 spinlock_t *ptl;
190
191 if (pmd_none(*pmd))  
<-*crash*
192 continue;
193 ptl = pte_lockptr(0, pmd);
194 if (lock)
195 spin_lock(ptl);
196 else
197 spin_unlock(ptl);
198 }
199 }
200 }
201 }
202 #endif
}



Quan




___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] help -- A question about ' XENFEAT_auto_translated_physmap':

[Jan Beulich]

>>> On 09.08.16 at 14:36,  wrote:
> Hi Jan,
> 
> A question about ' XENFEAT_auto_translated_physmap':
> 
> In linux code, in arch/x86/xen/mmu.c,

I assume you know that I'm not a maintainer of the Linux code.

>  __xen_pgd_walk()
> {
> 
> 
> if (xen_feature(XENFEAT_auto_translated_physmap))
> return 0;
> 
> }
> 
> 
> 
> Why xen_feature(XENFEAT_auto_translated_physmap) is true, then return 
> directly?
> If not return directly, is there any potential risk?

Well, the function is specifically there for operations (pinning/unpinning)
which are required only for the not-auto-translated case. Why would
anyone want to traverse a page table tree just to do nothing on each
of the entries?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] help -- A question about ' XENFEAT_auto_translated_physmap':

[Xuquan (Euler)]

Hi Jan,

A question about ' XENFEAT_auto_translated_physmap':

In linux code, in arch/x86/xen/mmu.c,

 __xen_pgd_walk()
{


if (xen_feature(XENFEAT_auto_translated_physmap))
return 0;

}



Why xen_feature(XENFEAT_auto_translated_physmap) is true, then return directly?
If not return directly, is there any potential risk?






Detail for  __xen_pgd_walk():

static int __xen_pgd_walk(struct mm_struct *mm, pgd_t *pgd,
  int (*func)(struct mm_struct *mm, struct page *,
  enum pt_level),
  unsigned long limit)
{
int flush = 0;
unsigned hole_low, hole_high;
unsigned pgdidx_limit, pudidx_limit, pmdidx_limit;
unsigned pgdidx, pudidx, pmdidx;

/* The limit is the last byte to be touched */
limit--;
BUG_ON(limit >= FIXADDR_TOP);

if (xen_feature(XENFEAT_auto_translated_physmap))
return 0;

/*
 * 64-bit has a great big hole in the middle of the address
 * space, which contains the Xen mappings.  On 32-bit these
 * will end up making a zero-sized hole and so is a no-op.
 */
hole_low = pgd_index(USER_LIMIT);
hole_high = pgd_index(PAGE_OFFSET);

pgdidx_limit = pgd_index(limit);
#if PTRS_PER_PUD > 1
pudidx_limit = pud_index(limit);
#else
pudidx_limit = 0;
#endif
#if PTRS_PER_PMD > 1
pmdidx_limit = pmd_index(limit);
#else
pmdidx_limit = 0;
#endif

for (pgdidx = 0; pgdidx <= pgdidx_limit; pgdidx++) {
pud_t *pud;

if (pgdidx >= hole_low && pgdidx < hole_high)
continue;

if (!pgd_val(pgd[pgdidx]))
continue;

pud = pud_offset([pgdidx], 0);

if (PTRS_PER_PUD > 1) /* not folded */
flush |= (*func)(mm, virt_to_page(pud), PT_PUD);

for (pudidx = 0; pudidx < PTRS_PER_PUD; pudidx++) {
pmd_t *pmd;

if (pgdidx == pgdidx_limit &&
pudidx > pudidx_limit)
goto out;

if (pud_none(pud[pudidx]))
continue;

pmd = pmd_offset([pudidx], 0);

if (PTRS_PER_PMD > 1) /* not folded */
flush |= (*func)(mm, virt_to_page(pmd), PT_PMD);

for (pmdidx = 0; pmdidx < PTRS_PER_PMD; pmdidx++) {
struct page *pte;

if (pgdidx == pgdidx_limit &&
pudidx == pudidx_limit &&
pmdidx > pmdidx_limit)
goto out;

if (pmd_none(pmd[pmdidx]))
continue;

pte = pmd_page(pmd[pmdidx]);
flush |= (*func)(mm, pte, PT_PTE);
}
}
}

out:
/* Do the top level last, so that the callbacks can use it as
   a cue to do final things like tlb flushes. */
flush |= (*func)(mm, virt_to_page(pgd), PT_PGD);

return flush;
}



Quan






___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel