| Message ID | 1367331446-28030-5-git-send-email-marc.zyngier@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, > not the size of the PGD. > > Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE > is equal to 1. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > arch/arm/kvm/mmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > index b978ebe..09ece5c 100644 > --- a/arch/arm/kvm/mmu.c > +++ b/arch/arm/kvm/mmu.c > @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > return -ENOMEM; > > /* stage-2 pgd must be aligned to its size */ > - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); > + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); I think the define is broken and should be fixed and/or renamed instead. > > memset(pgd, 0, PTRS_PER_S2_PGD * sizeof(pgd_t)); > kvm_clean_pgd(pgd); > -- > 1.8.2.1 > > > > _______________________________________________ > kvmarm mailing list > kvmarm@lists.cs.columbia.edu > https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30/04/13 18:40, Christoffer Dall wrote: > On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >> S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, >> not the size of the PGD. >> >> Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE >> is equal to 1. >> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- >> arch/arm/kvm/mmu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >> index b978ebe..09ece5c 100644 >> --- a/arch/arm/kvm/mmu.c >> +++ b/arch/arm/kvm/mmu.c >> @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) >> return -ENOMEM; >> >> /* stage-2 pgd must be aligned to its size */ >> - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); >> + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); > > I think the define is broken and should be fixed and/or renamed instead. To be fair, I wouldn't mind the whole check to be dropped altogether. If __get_free_pages doesn't give you an aligned allocation, the whole kernel is going the way of the dodo, and we shouldn't really care... Your call, really. M.
On Tue, Apr 30, 2013 at 06:52:24PM +0100, Marc Zyngier wrote: > On 30/04/13 18:40, Christoffer Dall wrote: > > On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: > >> S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, > >> not the size of the PGD. > >> > >> Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE > >> is equal to 1. > >> > >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > >> --- > >> arch/arm/kvm/mmu.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c > >> index b978ebe..09ece5c 100644 > >> --- a/arch/arm/kvm/mmu.c > >> +++ b/arch/arm/kvm/mmu.c > >> @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) > >> return -ENOMEM; > >> > >> /* stage-2 pgd must be aligned to its size */ > >> - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); > >> + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); > > > > I think the define is broken and should be fixed and/or renamed instead. > > To be fair, I wouldn't mind the whole check to be dropped altogether. If > __get_free_pages doesn't give you an aligned allocation, the whole > kernel is going the way of the dodo, and we shouldn't really care... > > Your call, really. > Yeah, this is from way back when, where the whole thing was different and we were debugging all sorts of issues, so we can get rid of both the check and the define in stead. Want to take care of it as part of your series? Thanks, -Christoffer -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30/04/13 18:56, Christoffer Dall wrote: > On Tue, Apr 30, 2013 at 06:52:24PM +0100, Marc Zyngier wrote: >> On 30/04/13 18:40, Christoffer Dall wrote: >>> On Tue, Apr 30, 2013 at 7:17 AM, Marc Zyngier <marc.zyngier@arm.com> wrote: >>>> S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, >>>> not the size of the PGD. >>>> >>>> Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE >>>> is equal to 1. >>>> >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >>>> --- >>>> arch/arm/kvm/mmu.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c >>>> index b978ebe..09ece5c 100644 >>>> --- a/arch/arm/kvm/mmu.c >>>> +++ b/arch/arm/kvm/mmu.c >>>> @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) >>>> return -ENOMEM; >>>> >>>> /* stage-2 pgd must be aligned to its size */ >>>> - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); >>>> + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); >>> >>> I think the define is broken and should be fixed and/or renamed instead. >> >> To be fair, I wouldn't mind the whole check to be dropped altogether. If >> __get_free_pages doesn't give you an aligned allocation, the whole >> kernel is going the way of the dodo, and we shouldn't really care... >> >> Your call, really. >> > Yeah, this is from way back when, where the whole thing was different > and we were debugging all sorts of issues, so we can get rid of both the > check and the define in stead. Want to take care of it as part of your > series? Sure. I'll kill it when I repost the series. M.
diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c index b978ebe..09ece5c 100644 --- a/arch/arm/kvm/mmu.c +++ b/arch/arm/kvm/mmu.c @@ -377,7 +377,7 @@ int kvm_alloc_stage2_pgd(struct kvm *kvm) return -ENOMEM; /* stage-2 pgd must be aligned to its size */ - VM_BUG_ON((unsigned long)pgd & (S2_PGD_SIZE - 1)); + VM_BUG_ON((unsigned long)pgd & (PAGE_SIZE * S2_PGD_SIZE - 1)); memset(pgd, 0, PTRS_PER_S2_PGD * sizeof(pgd_t)); kvm_clean_pgd(pgd);
S2_PGD_SIZE describe the number of pages a used by a stage-2 PGD, not the size of the PGD. Fix the VM_BUG_ON() call that doesn't check much when S2_PGD_SIZE is equal to 1. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- arch/arm/kvm/mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)