On Thu, Nov 8, 2012 at 1:57 PM, Anton Romanov <theli.ua@xxxxxxxxx> wrote:
>
>
>
> On Thu, Nov 8, 2012 at 2:28 PM, Christoffer Dall
> <c.dall@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>>
>> On Thu, Nov 8, 2012 at 1:14 PM, Anton Romanov <theli.ua@xxxxxxxxx> wrote:
>> > On Wed, Nov 7, 2012 at 6:11 PM, Marc Zyngier <marc.zyngier@xxxxxxx>
>> > wrote:
>> >>
>> >> On 07/11/12 16:04, Anton Romanov wrote:
>> >> > What is the proper way for removing mapping created by
>> >> > kvm_phys_addr_ioremap so that the same guest address could be
>> >> > remapped
>> >> > to different physical one?
>> >>
>> >> So far, we only have static mappings (we don't expect the hardware to
>> >> be
>> >> dynamically removed).
>> >>
>> >> To support this, you'll have to carefully remove the mapping from
>> >> stage-2 and flush the corresponding TLBs after synchronizing the CPUs.
>> >> Patches welcome.
>> >>
>> > Is the following enough or should I do that differently before
>> > submitting
>> > patch?
>>
>> you can send a patch using git to get it correctly formatted, but I
>> would like to see motivation and upstream use cases before merging
>> this code.
>
> I was asking about the code, not about formatting :)
>>
>> >
>> > This is how it currently works for me (TLB is invalidated in
>> > stage2_clear_pte
>>
>> > if needed):
>> >
>> > /**
>> > * @brief - unmap a device range from guest IPA
>>
>> brief?
>>
>> > *
>> > * @kvm: The KVM pointer
>> > * @guest_ipa: The IPA at which to insert the mapping
>> > * @size: The size of the mapping
>> > */
>> > void kvm_phys_addr_unmap(struct kvm *kvm, phys_addr_t guest_ipa,
>> > unsigned long size)
>> > {
>> > phys_addr_t addr, end;
>> >
>> > end = (guest_ipa + size + PAGE_SIZE - 1) & PAGE_MASK;
>> >
>> > for (addr = guest_ipa; addr < end; addr += PAGE_SIZE) {
>> > spin_lock(&kvm->mmu_lock);
>> > stage2_clear_pte(kvm, addr);
>> > spin_unlock(&kvm->mmu_lock);
>> > }
>> >
>> > }
>> >
>>
>> why do you need to explicitly unmap first? Can't you just redirect the
>> mappings if you wish calling ste_stage2_pte with a new physical
>> address and possibly tweak it so it doesn't error out for existing io
>> mappings?
>>
> I don't need to explicitly unmap. Remapping works fine for me too.
> However I felt that
> if (iomap && pte_present(*pte))
> return -EFAULT;
> wasn't sure about the reasoning behind that check.
the use case is to avoid unintentional overlapping IO regions. You
could change the iomap flag to be something like overlap_check.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
