Re: [PATCH 3/3] mini-os: mm: convert set_readonly() to use walk_pt()

[Jürgen Groß <jgross@xxxxxxxx>]

On 31.07.24 23:37, Samuel Thibault wrote:
> Juergen Gross, le mer. 31 juil. 2024 15:00:26 +0200, a ecrit:
> > +static int set_readonly_func(unsigned long va, unsigned int lvl, bool is_leaf,
> > +                             pgentry_t *pte, void *par)
> > +{
> > +    struct set_readonly_par *ro = par;
> >   
> > +    mmu_updates[ro->count].ptr = virt_to_mach(pte);
> > +    mmu_updates[ro->count].val = *pte & ~_PAGE_RW;
> > +    ro->count++;
> > +
> > +    if ( (ro->count == L1_PAGETABLE_ENTRIES ||
> > +          va + 2 * PAGE_SIZE > ro->etext) &&
> > +         HYPERVISOR_mmu_update(mmu_updates, ro->count, NULL, DOMID_SELF) < 0 )
> > +    {
> > +        printk("ERROR: set_readonly(): PTE could not be updated\n");
> > +        do_exit();
> > +    }
> Don't we also want to set ro->count to 0?
Oh, indeed. Thanks for catching this.

> And assert that it is 0 after calling walk_pt in set_readonly, to make
> sure the va + 2 * PAGE_SIZE > ro->etext test did work properly
> (personally I would have rather made set_readonly call a last
> HYPERVISOR_mmu_update in case ro->count is not 0, which looks more
> robust that a quite magic-looking va + 2 * PAGE_SIZE > ro->etext test)
I think you are right. I'll do that.


Juergen