From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 14FA52C07 for ; Tue, 23 Aug 2016 14:22:26 +0200 (CEST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga105.fm.intel.com with ESMTP; 23 Aug 2016 05:22:26 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,565,1464678000"; d="scan'208";a="1018996095" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.67.162]) by orsmga001.jf.intel.com with ESMTP; 23 Aug 2016 05:22:25 -0700 Date: Tue, 23 Aug 2016 20:32:11 +0800 From: Yuanhan Liu To: Maxime Coquelin Cc: dev@dpdk.org Message-ID: <20160823123211.GK30752@yliu-dev.sh.intel.com> References: <1471939839-29778-1-git-send-email-yuanhan.liu@linux.intel.com> <1471939839-29778-3-git-send-email-yuanhan.liu@linux.intel.com> <13f37c6e-b389-a758-81cd-861db7337e1f@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <13f37c6e-b389-a758-81cd-861db7337e1f@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH 2/6] vhost: get guest/host physical address mappings X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Aug 2016 12:22:28 -0000 On Tue, Aug 23, 2016 at 11:58:42AM +0200, Maxime Coquelin wrote: > > > >+/* Convert guest physical address to host physical address */ > >+static inline phys_addr_t __attribute__((always_inline)) > >+gpa_to_hpa(struct virtio_net *dev, uint64_t gpa, uint64_t size) > >+{ > >+ uint32_t i; > >+ struct guest_page *page; > >+ > >+ for (i = 0; i < dev->nr_guest_pages; i++) { > >+ page = &dev->guest_pages[i]; > >+ > >+ if (gpa >= page->guest_phys_addr && > >+ gpa + size < page->guest_phys_addr + page->size) { > Shouldn't be '<=' here? Oops, you are right. > >+ return gpa - page->guest_phys_addr + > >+ page->host_phys_addr; > >+ } > >+ } > >+ > >+ return 0; > >+} > >+ > > struct virtio_net_device_ops const *notify_ops; > > struct virtio_net *get_device(int vid); > > > >diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > >index d2071fd..045d4f0 100644 > >--- a/lib/librte_vhost/vhost_user.c > >+++ b/lib/librte_vhost/vhost_user.c > >@@ -372,6 +372,81 @@ vhost_user_set_vring_base(struct virtio_net *dev, > > return 0; > > } > > > >+static void > >+add_one_guest_page(struct virtio_net *dev, uint64_t guest_phys_addr, > >+ uint64_t host_phys_addr, uint64_t size) > >+{ > >+ struct guest_page *page; > >+ > >+ if (dev->nr_guest_pages == dev->max_guest_pages) { > >+ dev->max_guest_pages *= 2; > >+ dev->guest_pages = realloc(dev->guest_pages, > >+ dev->max_guest_pages * sizeof(*page)); > > Maybe realloc return could be checked? Yes, I should have done that. Besides, I also forgot to free it at somewhere. Will fix it. > > >+ } > >+ > >+ page = &dev->guest_pages[dev->nr_guest_pages++]; > >+ page->guest_phys_addr = guest_phys_addr; > >+ page->host_phys_addr = host_phys_addr; > >+ page->size = size; > >+} > >+ > >+static void > >+add_guest_pages(struct virtio_net *dev, struct virtio_memory_region *reg, > >+ uint64_t page_size) > >+{ > >+ uint64_t reg_size = reg->size; > >+ uint64_t host_user_addr = reg->host_user_addr; > >+ uint64_t guest_phys_addr = reg->guest_phys_addr; > >+ uint64_t host_phys_addr; > >+ uint64_t size; > >+ uint32_t pre_read; > >+ > >+ pre_read = *((uint32_t *)(uintptr_t)host_user_addr); > >+ host_phys_addr = rte_mem_virt2phy((void *)(uintptr_t)host_user_addr); > >+ size = page_size - (guest_phys_addr & (page_size - 1)); > >+ size = RTE_MIN(size, reg_size); > >+ > >+ add_one_guest_page(dev, guest_phys_addr, host_phys_addr, size); > >+ host_user_addr += size; > >+ guest_phys_addr += size; > >+ reg_size -= size; > >+ > >+ while (reg_size > 0) { > >+ pre_read += *((uint32_t *)(uintptr_t)host_user_addr); > >+ host_phys_addr = rte_mem_virt2phy((void *)(uintptr_t)host_user_addr); > >+ add_one_guest_page(dev, guest_phys_addr, host_phys_addr, page_size); > >+ > >+ host_user_addr += page_size; > >+ guest_phys_addr += page_size; > >+ reg_size -= page_size; > >+ } > >+ > >+ /* FIXME */ > >+ RTE_LOG(INFO, VHOST_CONFIG, ":: %u ::\n", pre_read); > For my information, what is the purpose of pre_read? Again, I put a FIXME here, but I forgot to add some explanation. Here is the thing: the read will make sure the kernel populate the corresponding PTE entry, so that rte_mem_virt2phy() will return proper physical address, otherwise, invalid value is returned. I can't simply do the read but do not actually reference/consume it. Otherwise, the compiler will treat it as some noops and remove it. An ugly RTE_LOG will make sure the read operation is not eliminated. I'm seeking a more proper way to achieve that. Maybe I can add a new field in virtio_net structure and store it there. Or, do you have better ideas? --yliu