lkml.org 
[lkml]   [2021]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 04/16] KVM: arm64: Add MMIO checking infrastructure
    On Tuesday 20 Jul 2021 at 14:15:56 (+0100), Marc Zyngier wrote:
    > On Tue, 20 Jul 2021 12:13:20 +0100,
    > Quentin Perret <qperret@google.com> wrote:
    > >
    > > On Thursday 15 Jul 2021 at 17:31:47 (+0100), Marc Zyngier wrote:
    > > > +struct s2_walk_data {
    > > > + kvm_pte_t pteval;
    > > > + u32 level;
    > > > +};
    > > > +
    > > > +static int s2_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
    > > > + enum kvm_pgtable_walk_flags flag, void * const arg)
    > > > +{
    > > > + struct s2_walk_data *data = arg;
    > > > +
    > > > + data->level = level;
    > > > + data->pteval = *ptep;
    > > > + return 0;
    > > > +}
    > > > +
    > > > +/* Assumes mmu_lock taken */
    > > > +static bool __check_ioguard_page(struct kvm_vcpu *vcpu, gpa_t ipa)
    > > > +{
    > > > + struct s2_walk_data data;
    > > > + struct kvm_pgtable_walker walker = {
    > > > + .cb = s2_walker,
    > > > + .flags = KVM_PGTABLE_WALK_LEAF,
    > > > + .arg = &data,
    > > > + };
    > > > +
    > > > + kvm_pgtable_walk(vcpu->arch.hw_mmu->pgt, ALIGN_DOWN(ipa, PAGE_SIZE),
    > > > + PAGE_SIZE, &walker);
    > > > +
    > > > + /* Must be a PAGE_SIZE mapping with our annotation */
    > > > + return (BIT(ARM64_HW_PGTABLE_LEVEL_SHIFT(data.level)) == PAGE_SIZE &&
    > > > + data.pteval == MMIO_NOTE);
    > >
    > > Nit: you could do this check in the walker directly and check the return
    > > value of kvm_pgtable_walk() instead. That would allow to get rid of
    > > struct s2_walk_data.
    > >
    > > Also, though the compiler might be able to optimize, maybe simplify the
    > > level check to level == (KVM_PGTABLE_MAX_LEVELS - 1)?
    >
    > Yup, all good points. I guess I could do the same in my other series
    > that parses the userspace PT to extract the level.

    Well, actually, let me take that back. I think something like you have
    would be useful, but in pgtable.c directly and re-usable for stage-1 and
    stage-2 walks. Maybe something like the below (totally untested)?

    I could use such a walker in several places as well in the memory
    ownership series:

    - following the idea of [1], I could remove the
    kvm_pgtable_stage2_find_range() function entirely;

    - [2] defines 2 custom walkers that do nothing but walk host stage-2
    and hyp stage-1 page-tables to check permissions and such -- they
    could be removed/re-implemented easily as well.

    And you seem to need something similar here, so clearly there is a need.
    WDYT?

    Thanks,
    Quentin

    [1] https://lore.kernel.org/kvmarm/20210719104735.3681732-3-qperret@google.com/
    [2] https://lore.kernel.org/kvmarm/20210719104735.3681732-14-qperret@google.com/

    diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
    index e0ae57dca827..bd6d26f27e1a 100644
    --- a/arch/arm64/kvm/hyp/pgtable.c
    +++ b/arch/arm64/kvm/hyp/pgtable.c
    @@ -357,6 +357,38 @@ int kvm_pgtable_walk(struct kvm_pgtable *pgt, u64 addr, u64 size,
    return _kvm_pgtable_walk(&walk_data);
    }

    +struct get_leaf_data {
    + kvm_pte_t *ptep;
    + u32 *level;
    +};
    +
    +static int get_leaf_walker(u64 addr, u64 end, u32 level, kvm_pte_t *ptep,
    + enum kvm_pgtable_walk_flags flag, void * const arg)
    +{
    + struct get_leaf_data *data = arg;
    +
    + *(data->ptep) = *ptep;
    + *(data->level) = level;
    +
    + return 0;
    +}
    +
    +int kvm_pgtable_get_leaf(struct kvm_pgtable *pgt, u64 addr, kvm_pte_t *ptep,
    + u32 *level)
    +{
    + struct get_leaf_data data = {
    + .ptep = ptep,
    + .level = level,
    + };
    + struct kvm_pgtable_walker __get_leaf_walker = {
    + .cb = get_leaf_walker,
    + .flags = KVM_PGTABLE_WALK_LEAF,
    + .arg = &data,
    + };
    +
    + return kvm_pgtable_walk(pgt, addr, PAGE_SIZE, &__get_leaf_walker);
    +}
    +
    struct hyp_map_data {
    u64 phys;
    kvm_pte_t attr;
    \
     
     \ /
      Last update: 2021-07-20 17:56    [W:4.580 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site