On 08/11/12 13:24, Anton Romanov wrote: > On Thu, Nov 8, 2012 at 3:03 PM, Christoffer Dall > <c.dall@xxxxxxxxxxxxxxxxxxxxxx <mailto:c.dall@xxxxxxxxxxxxxxxxxxxxxx>> > wrote: > > On Thu, Nov 8, 2012 at 1:57 PM, Anton Romanov <theli.ua@xxxxxxxxx > <mailto:theli.ua@xxxxxxxxx>> wrote: > > > > > > > > On Thu, Nov 8, 2012 at 2:28 PM, Christoffer Dall > > <c.dall@xxxxxxxxxxxxxxxxxxxxxx > <mailto:c.dall@xxxxxxxxxxxxxxxxxxxxxx>> wrote: > >> > >> On Thu, Nov 8, 2012 at 1:14 PM, Anton Romanov <theli.ua@xxxxxxxxx > <mailto:theli.ua@xxxxxxxxx>> wrote: > >> > On Wed, Nov 7, 2012 at 6:11 PM, Marc Zyngier > <marc.zyngier@xxxxxxx <mailto: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. > > I see. Thanks. Will take that into account. I'd also like to see a compelling use case for such a feature. M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
