| Message ID | 1478262745-47028-2-git-send-email-christophe.milard@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Nov 4, 2016 at 7:32 AM, Christophe Milard < christophe.milard@linaro.org> wrote: > The function _odp_ishmphy_getphy() is added to query for physical > addresses (given a virtual address) > This function is meant to be used to populate the physical address > of packet segments which are to be used by drivers (without iommu). > The function _odp_ishmphy_can_getphy(), also added, return true if > _odp_ishmphy_getphy() is able to works (as it requires specific > permission) > > Signed-off-by: Christophe Milard <christophe.milard@linaro.org> > --- > platform/linux-generic/_ishmphy.c | 82 > ++++++++++++++++++++++ > platform/linux-generic/include/_ishmphy_internal.h | 14 ++++ > 2 files changed, 96 insertions(+) > > diff --git a/platform/linux-generic/_ishmphy.c b/platform/linux-generic/_ > ishmphy.c > index 2b2d100..beb213c 100644 > --- a/platform/linux-generic/_ishmphy.c > +++ b/platform/linux-generic/_ishmphy.c > @@ -29,6 +29,7 @@ > #include <fcntl.h> > #include <sys/types.h> > #include <sys/wait.h> > +#include <inttypes.h> > #include <_ishmphy_internal.h> > > static void *common_va_address; > @@ -38,6 +39,8 @@ static uint64_t common_va_len; > #define MAP_ANONYMOUS MAP_ANON > #endif > > +#define PAGEMAP_FILE "/proc/self/pagemap" > + > /* Book some virtual address space > * This function is called at odp_init_global() time to pre-book some > * virtual address space inherited by all odpthreads (i.e. descendant > @@ -183,3 +186,82 @@ int _odp_ishmphy_unmap(void *start, uint64_t len, int > flags) > ODP_ERR("_ishmphy_free failure: %s\n", strerror(errno)); > return ret; > } > + > +/* > + * Get physical address from virtual address addr. > + */ > +phys_addr_t _odp_ishmphy_getphy(const void *addr) > +{ > + int page_sz; > + int fd; > + off_t offset; > + int read_bytes; > + uint64_t page; > + phys_addr_t phys_addr; > + > + /* get normal page sizes: */ > + page_sz = odp_sys_page_size(); > + > + /* read 8 bytes (uint64_t) at position N*8, where N is > addr/page_sz */ > + fd = open(PAGEMAP_FILE, O_RDONLY); > + if (fd < 0) { > + ODP_ERR("cannot open " PAGEMAP_FILE ": %s\n", > + strerror(errno)); > + return PHYS_ADDR_INVALID; > + } > + > + offset = ((unsigned long)addr / page_sz) * sizeof(uint64_t); > + if (lseek(fd, offset, SEEK_SET) == (off_t)-1) { > + ODP_ERR("cannot seek " PAGEMAP_FILE ": %s\n", > + strerror(errno)); > + close(fd); > + return PHYS_ADDR_INVALID; > + } > + > + read_bytes = read(fd, &page, sizeof(uint64_t)); > + close(fd); > + if (read_bytes < 0) { > + ODP_ERR("cannot read " PAGEMAP_FILE ": %s\n", > + strerror(errno)); + return PHYS_ADDR_INVALID; > + } else if (read_bytes != sizeof(uint64_t)) { > + ODP_ERR("read %d bytes from " PAGEMAP_FILE " " > + "but expected %d:\n", > + read_bytes, sizeof(uint64_t)); + return PHYS_ADDR_INVALID; > + } > + > + /* some kernel return PFN zero when permission is denied: */ > + if (!(page & 0x7fffffffffffffULL)) > + return PHYS_ADDR_INVALID; + > + /* > + * the pfn (page frame number) are bits 0-54 (see > + * pagemap.txt in linux Documentation) > + */ > + phys_addr = ((page & 0x7fffffffffffffULL) * page_sz) > + + ((unsigned long)addr % page_sz); > + > + return phys_addr; > +} > + > +/* > + * check if physical address are readable > + * return true if physical addresses can be retrieved. > + * Just do a test to see if it works > + */ > +int _odp_ishmphy_can_getphy(void) > Would this function be better exposed as a return param from a capability API? In addition to knowing whether you can get physical addresses you probably want to know whether you need to worry about them. Also, I thought we had decided that we were going to require the presence of an IOMMU. Has that changed? > +{ > + int block_index; > + phys_addr_t phy; > + > + /* allocate a block, locked in memory and try to grab its phy > address */ > + block_index = _odp_ishm_reserve(NULL, 10, -1, 0, _ODP_ISHM_LOCK, > 0); > + phy = _odp_ishmphy_getphy((void *)_odp_ishm_address(block_index)); > + _odp_ishm_free_by_index(block_index); > + > + if (phy == PHYS_ADDR_INVALID) > + return 0; > + > + return 1; > +} > diff --git a/platform/linux-generic/include/_ishmphy_internal.h > b/platform/linux-generic/include/_ishmphy_internal.h > index 4fe560f..31b154b 100644 > --- a/platform/linux-generic/include/_ishmphy_internal.h > +++ b/platform/linux-generic/include/_ishmphy_internal.h > @@ -13,11 +13,25 @@ extern "C" { > > #include <stdint.h> > > +typedef uint64_t phys_addr_t; /* Physical address definition. */ > +#define PHYS_ADDR_INVALID ((phys_addr_t)-1) > Since we already have a strong typing mechanism in place, why not use it? typedef ODP_HANDLE_T(phys_addr_t); #define PHYS_ADDR_INVALID _odp_cast_scalar(phys_addr_t, -1); Since you need to extract an actual address from this I'd just add a new helper macro to odp/api/plat/strong_types.h: #define _odp_typeptr(handle) ((void *)(uintptr_t)(handle)) and you should be good to go. This has the added benefit that it should work on 32-bit systems as well as 64-bit systems. + > void *_odp_ishmphy_book_va(uintptr_t len, intptr_t align); > int _odp_ishmphy_unbook_va(void); > void *_odp_ishmphy_map(int fd, void *start, uint64_t size, int flags); > int _odp_ishmphy_unmap(void *start, uint64_t len, int flags); > > +/* > + * check if physical address are readable > + * return true if physical addresses can be retrieved. > + */ > +int _odp_ishmphy_can_getphy(void); > + > +/* > + * Get physical address from virtual address addr. > + */ > +phys_addr_t _odp_ishmphy_getphy(const void *addr); > + > #ifdef __cplusplus > } > #endif > -- > 2.7.4 > >
Hi Bill. This patch adds only _ishmphy functions. It does not affect any API at all (at this stage). Petri will need it to provide the physical address of each segment being allocated. So odp_packet.c will call this function, I guess (internally, in the linux ODP implementation). Petri wants to do the segment stuff, so he will design the segment_allocate() segemnt free(), segment_concatenate()... functions. But the segments must contain the physical address, so this is needed. Yes, we decided to go for IOMMU, but that was by the time the focus was a real NIC drivers, such as PCI boards. IOMMUs are physically located between the IO buss and the physical memory... By changing the focus from real NIC to virtio, IOMMUs became kind of useless (as the virtio driver just handles memory transfer): Think the case of a guest trying to reach a host switch via virtio... there is not even any HW involved in that...An IOMMU placed between the physical memory and some IO bus (such as PCI) won't be of much help. There are kernel discussions for homogenizing the IO interfaces (using IOMMU simulations of all sorts), but these are just discussions. Nothing exist in the kernel at this stage as far as I know. (I'd be happy to be wrong there, but I had a discussion with some virtualisation guy at LAS, and he said I was right, sadly.) Hence, sadly, the need for physical memory knowledge for virtio. Thanks for you interrest! Christophe. On 4 November 2016 at 13:28, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > > On Fri, Nov 4, 2016 at 7:32 AM, Christophe Milard > <christophe.milard@linaro.org> wrote: >> >> The function _odp_ishmphy_getphy() is added to query for physical >> addresses (given a virtual address) >> This function is meant to be used to populate the physical address >> of packet segments which are to be used by drivers (without iommu). >> The function _odp_ishmphy_can_getphy(), also added, return true if >> _odp_ishmphy_getphy() is able to works (as it requires specific >> permission) >> >> Signed-off-by: Christophe Milard <christophe.milard@linaro.org> >> --- >> platform/linux-generic/_ishmphy.c | 82 >> ++++++++++++++++++++++ >> platform/linux-generic/include/_ishmphy_internal.h | 14 ++++ >> 2 files changed, 96 insertions(+) >> >> diff --git a/platform/linux-generic/_ishmphy.c >> b/platform/linux-generic/_ishmphy.c >> index 2b2d100..beb213c 100644 >> --- a/platform/linux-generic/_ishmphy.c >> +++ b/platform/linux-generic/_ishmphy.c >> @@ -29,6 +29,7 @@ >> #include <fcntl.h> >> #include <sys/types.h> >> #include <sys/wait.h> >> +#include <inttypes.h> >> #include <_ishmphy_internal.h> >> >> static void *common_va_address; >> @@ -38,6 +39,8 @@ static uint64_t common_va_len; >> #define MAP_ANONYMOUS MAP_ANON >> #endif >> >> +#define PAGEMAP_FILE "/proc/self/pagemap" >> + >> /* Book some virtual address space >> * This function is called at odp_init_global() time to pre-book some >> * virtual address space inherited by all odpthreads (i.e. descendant >> @@ -183,3 +186,82 @@ int _odp_ishmphy_unmap(void *start, uint64_t len, int >> flags) >> ODP_ERR("_ishmphy_free failure: %s\n", strerror(errno)); >> return ret; >> } >> + >> +/* >> + * Get physical address from virtual address addr. >> + */ >> +phys_addr_t _odp_ishmphy_getphy(const void *addr) >> +{ >> + int page_sz; >> + int fd; >> + off_t offset; >> + int read_bytes; >> + uint64_t page; >> + phys_addr_t phys_addr; >> + >> + /* get normal page sizes: */ >> + page_sz = odp_sys_page_size(); >> + >> + /* read 8 bytes (uint64_t) at position N*8, where N is >> addr/page_sz */ >> + fd = open(PAGEMAP_FILE, O_RDONLY); >> + if (fd < 0) { >> + ODP_ERR("cannot open " PAGEMAP_FILE ": %s\n", >> + strerror(errno)); >> + return PHYS_ADDR_INVALID; >> + } >> + >> + offset = ((unsigned long)addr / page_sz) * sizeof(uint64_t); >> + if (lseek(fd, offset, SEEK_SET) == (off_t)-1) { >> + ODP_ERR("cannot seek " PAGEMAP_FILE ": %s\n", >> + strerror(errno)); >> + close(fd); >> + return PHYS_ADDR_INVALID; >> + } >> + >> + read_bytes = read(fd, &page, sizeof(uint64_t)); >> + close(fd); >> + if (read_bytes < 0) { >> + ODP_ERR("cannot read " PAGEMAP_FILE ": %s\n", >> + strerror(errno)); >> >> + return PHYS_ADDR_INVALID; >> + } else if (read_bytes != sizeof(uint64_t)) { >> + ODP_ERR("read %d bytes from " PAGEMAP_FILE " " >> + "but expected %d:\n", >> + read_bytes, sizeof(uint64_t)); >> >> + return PHYS_ADDR_INVALID; >> + } >> + >> + /* some kernel return PFN zero when permission is denied: */ >> + if (!(page & 0x7fffffffffffffULL)) >> + return PHYS_ADDR_INVALID; >> >> + >> + /* >> + * the pfn (page frame number) are bits 0-54 (see >> + * pagemap.txt in linux Documentation) >> + */ >> + phys_addr = ((page & 0x7fffffffffffffULL) * page_sz) >> + + ((unsigned long)addr % page_sz); >> + >> + return phys_addr; >> +} >> + >> +/* >> + * check if physical address are readable >> + * return true if physical addresses can be retrieved. >> + * Just do a test to see if it works >> + */ >> +int _odp_ishmphy_can_getphy(void) > > > Would this function be better exposed as a return param from a capability > API? In addition to knowing whether you can get physical addresses you > probably want to know whether you need to worry about them. Also, I thought > we had decided that we were going to require the presence of an IOMMU. Has > that changed? > >> >> +{ >> + int block_index; >> + phys_addr_t phy; >> + >> + /* allocate a block, locked in memory and try to grab its phy >> address */ >> + block_index = _odp_ishm_reserve(NULL, 10, -1, 0, _ODP_ISHM_LOCK, >> 0); >> + phy = _odp_ishmphy_getphy((void *)_odp_ishm_address(block_index)); >> + _odp_ishm_free_by_index(block_index); >> + >> + if (phy == PHYS_ADDR_INVALID) >> + return 0; >> + >> + return 1; >> +} >> diff --git a/platform/linux-generic/include/_ishmphy_internal.h >> b/platform/linux-generic/include/_ishmphy_internal.h >> index 4fe560f..31b154b 100644 >> --- a/platform/linux-generic/include/_ishmphy_internal.h >> +++ b/platform/linux-generic/include/_ishmphy_internal.h >> @@ -13,11 +13,25 @@ extern "C" { >> >> #include <stdint.h> >> >> +typedef uint64_t phys_addr_t; /* Physical address definition. */ >> +#define PHYS_ADDR_INVALID ((phys_addr_t)-1) > > > Since we already have a strong typing mechanism in place, why not use it? > > typedef ODP_HANDLE_T(phys_addr_t); > #define PHYS_ADDR_INVALID _odp_cast_scalar(phys_addr_t, -1); > > Since you need to extract an actual address from this I'd just add a new > helper macro to odp/api/plat/strong_types.h: > > #define _odp_typeptr(handle) ((void *)(uintptr_t)(handle)) > > and you should be good to go. > > This has the added benefit that it should work on 32-bit systems as well as > 64-bit systems. > >> + >> void *_odp_ishmphy_book_va(uintptr_t len, intptr_t align); >> int _odp_ishmphy_unbook_va(void); >> void *_odp_ishmphy_map(int fd, void *start, uint64_t size, int flags); >> int _odp_ishmphy_unmap(void *start, uint64_t len, int flags); >> >> +/* >> + * check if physical address are readable >> + * return true if physical addresses can be retrieved. >> + */ >> +int _odp_ishmphy_can_getphy(void); >> + >> +/* >> + * Get physical address from virtual address addr. >> + */ >> +phys_addr_t _odp_ishmphy_getphy(const void *addr); >> + >> #ifdef __cplusplus >> } >> #endif >> -- >> 2.7.4 >> >
diff --git a/platform/linux-generic/_ishmphy.c b/platform/linux-generic/_ishmphy.c index 2b2d100..beb213c 100644 --- a/platform/linux-generic/_ishmphy.c +++ b/platform/linux-generic/_ishmphy.c @@ -29,6 +29,7 @@ #include <fcntl.h> #include <sys/types.h> #include <sys/wait.h> +#include <inttypes.h> #include <_ishmphy_internal.h> static void *common_va_address; @@ -38,6 +39,8 @@ static uint64_t common_va_len; #define MAP_ANONYMOUS MAP_ANON #endif +#define PAGEMAP_FILE "/proc/self/pagemap" + /* Book some virtual address space * This function is called at odp_init_global() time to pre-book some * virtual address space inherited by all odpthreads (i.e. descendant @@ -183,3 +186,82 @@ int _odp_ishmphy_unmap(void *start, uint64_t len, int flags) ODP_ERR("_ishmphy_free failure: %s\n", strerror(errno)); return ret; } + +/* + * Get physical address from virtual address addr. + */ +phys_addr_t _odp_ishmphy_getphy(const void *addr) +{ + int page_sz; + int fd; + off_t offset; + int read_bytes; + uint64_t page; + phys_addr_t phys_addr; + + /* get normal page sizes: */ + page_sz = odp_sys_page_size(); + + /* read 8 bytes (uint64_t) at position N*8, where N is addr/page_sz */ + fd = open(PAGEMAP_FILE, O_RDONLY); + if (fd < 0) { + ODP_ERR("cannot open " PAGEMAP_FILE ": %s\n", + strerror(errno)); + return PHYS_ADDR_INVALID; + } + + offset = ((unsigned long)addr / page_sz) * sizeof(uint64_t); + if (lseek(fd, offset, SEEK_SET) == (off_t)-1) { + ODP_ERR("cannot seek " PAGEMAP_FILE ": %s\n", + strerror(errno)); + close(fd); + return PHYS_ADDR_INVALID; + } + + read_bytes = read(fd, &page, sizeof(uint64_t)); + close(fd); + if (read_bytes < 0) { + ODP_ERR("cannot read " PAGEMAP_FILE ": %s\n", + strerror(errno)); + return PHYS_ADDR_INVALID; + } else if (read_bytes != sizeof(uint64_t)) { + ODP_ERR("read %d bytes from " PAGEMAP_FILE " " + "but expected %d:\n", + read_bytes, sizeof(uint64_t)); + return PHYS_ADDR_INVALID; + } + + /* some kernel return PFN zero when permission is denied: */ + if (!(page & 0x7fffffffffffffULL)) + return PHYS_ADDR_INVALID; + + /* + * the pfn (page frame number) are bits 0-54 (see + * pagemap.txt in linux Documentation) + */ + phys_addr = ((page & 0x7fffffffffffffULL) * page_sz) + + ((unsigned long)addr % page_sz); + + return phys_addr; +} + +/* + * check if physical address are readable + * return true if physical addresses can be retrieved. + * Just do a test to see if it works + */ +int _odp_ishmphy_can_getphy(void) +{ + int block_index; + phys_addr_t phy; + + /* allocate a block, locked in memory and try to grab its phy address */ + block_index = _odp_ishm_reserve(NULL, 10, -1, 0, _ODP_ISHM_LOCK, 0); + phy = _odp_ishmphy_getphy((void *)_odp_ishm_address(block_index)); + _odp_ishm_free_by_index(block_index); + + if (phy == PHYS_ADDR_INVALID) + return 0; + + return 1; +} diff --git a/platform/linux-generic/include/_ishmphy_internal.h b/platform/linux-generic/include/_ishmphy_internal.h index 4fe560f..31b154b 100644 --- a/platform/linux-generic/include/_ishmphy_internal.h +++ b/platform/linux-generic/include/_ishmphy_internal.h @@ -13,11 +13,25 @@ extern "C" { #include <stdint.h> +typedef uint64_t phys_addr_t; /* Physical address definition. */ +#define PHYS_ADDR_INVALID ((phys_addr_t)-1) + void *_odp_ishmphy_book_va(uintptr_t len, intptr_t align); int _odp_ishmphy_unbook_va(void); void *_odp_ishmphy_map(int fd, void *start, uint64_t size, int flags); int _odp_ishmphy_unmap(void *start, uint64_t len, int flags); +/* + * check if physical address are readable + * return true if physical addresses can be retrieved. + */ +int _odp_ishmphy_can_getphy(void); + +/* + * Get physical address from virtual address addr. + */ +phys_addr_t _odp_ishmphy_getphy(const void *addr); + #ifdef __cplusplus } #endif
The function _odp_ishmphy_getphy() is added to query for physical addresses (given a virtual address) This function is meant to be used to populate the physical address of packet segments which are to be used by drivers (without iommu). The function _odp_ishmphy_can_getphy(), also added, return true if _odp_ishmphy_getphy() is able to works (as it requires specific permission) Signed-off-by: Christophe Milard <christophe.milard@linaro.org> --- platform/linux-generic/_ishmphy.c | 82 ++++++++++++++++++++++ platform/linux-generic/include/_ishmphy_internal.h | 14 ++++ 2 files changed, 96 insertions(+) -- 2.7.4