ikwzm / udmabuf

User space mappable dma buffer device driver for Linux.
BSD 2-Clause "Simplified" License
560 stars 168 forks source link

Use pgprot_noncached for ARM64 #28

Closed iasakura closed 5 years ago

iasakura commented 5 years ago

はじめまして、お世話になっております。

Ultra96 上で udmabuf を用いて 72 byte のデータを udmabuf に配置しその後 AXI DMA を用いて PL に送信してみたのですが、最初の 64 byte のみ送信され、残りの値が 0 になってしまいました。 以下のコードで再現します。

#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <assert.h>
#include <fcntl.h>
#include <sys/mman.h>
#include <unistd.h>

int main(int argc, char *argv[]) {
    int fd;
    char attr[1024];

    if ((fd = open("/dev/udmabuf0", O_RDWR | O_SYNC)) == -1) {
        printf("cannot open udmabuf0\n");
        exit(-1);
    }

    uint32_t size = 0;
    {
        int fd = 0;
        if ((fd  = open("/sys/class/udmabuf/udmabuf0/size", O_RDONLY)) == -1) {
            fprintf(stderr, "cannot get size\n");
            exit(-1);
        }
        read(fd, attr, 1024);
        sscanf(attr, "%u", &size);
        close(fd);
    }

    uint32_t phys_addr = 0;
    {
        int fd = 0;
        if ((fd  = open("/sys/class/udmabuf/udmabuf0/phys_addr", O_RDONLY)) == -1) {
            fprintf(stderr, "cannot get phys_addr\n");
            exit(-1);
        }
        read(fd, attr, 1024);
        sscanf(attr, "%x", &phys_addr);
        close(fd);
    }

    uint8_t *ptr = (uint8_t*)mmap(NULL, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
    if (ptr == NULL) {
        fprintf(stderr, "cannot mmap physically-backed mempool\n");
        exit(-1);
    }

    // Truncate offset to a multiple of the page size, or mmap will fail.
    size_t pagesize = sysconf(_SC_PAGE_SIZE);
    off_t page_base = (phys_addr / pagesize) * pagesize;
    off_t page_offset = phys_addr - page_base;

    unsigned char *mem = NULL;
    { 
        int fd = open("/dev/mem", O_RDWR);
        mem = mmap(NULL, page_offset + SIZE, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, page_base);
        if (mem == MAP_FAILED) {
            perror("Can't map memory");
            exit(-1);
        }
    }
    uint8_t arr[SIZE];
    for (int i = 0; i < SIZE; ++i) {
        arr[i] = 1;
    }
    int size_in_bytes = atoi(argv[1]);
    memcpy(ptr, arr, size_in_bytes);
    printf("%d\n", mem[IDX]);
    return 0;
}

以下のようにして実行できます。

gcc -DSIZE=72 -DIDX=64 -g -o udmabuf_test udmabuf_test.c
./udmabuf_test 72
0

1で埋められた配列 arr を udmabuf で確保したメモリに memcpy した後、/dev/mem を通じて phys_addr+64 にアクセスしているのですが、0が返ってきてしまいます。(0~63 にアクセスしたときは正しく1が返ってきます。)

この PR の変更を適用することで解決したのですが、もし正しい修正ならマージしていただけますでしょうか。

よろしくお願いいたします。

ikwzm commented 5 years ago

issue ありがとうございます。

私も当初、pgprot_noncached(vm_page_prot) にしていたのですが、ARM64 の場合、pgprot_noncached(vm_page_prot) だと memset() で Bus error が起きました。 当時の状況を再現すると次のようになります。

root@debian-fpga:~/work/u-dma-buf# insmod u-dma-buf.ko udmabuf0=0x100000
[ 1503.464874] u-dma-buf u-dma-buf.0: DMA mask not set
[ 1503.472404] u-dma-buf udmabuf0: driver version = 2.1.0-rc2
[ 1503.477913] u-dma-buf udmabuf0: major number   = 239
[ 1503.482876] u-dma-buf udmabuf0: minor number   = 0
[ 1503.487662] u-dma-buf udmabuf0: phys address   = 0x0000000070100000
[ 1503.493931] u-dma-buf udmabuf0: buffer size    = 1048576
[ 1503.499240] u-dma-buf udmabuf0: dma device     = u-dma-buf.0
[ 1503.504893] u-dma-buf udmabuf0: dma coherent   = 0
[ 1503.509682] u-dma-buf u-dma-buf.0: driver installed.
root@debian-fpga:~/work/u-dma-buf# ./u-dma-buf_test
phys_addr=0x70100000
size=1048576
check_buf()
sync_mode=0, O_SYNC=0, time = 0.340671 sec
sync_mode=0, O_SYNC=1, time = 0.340689 sec
sync_mode=1, O_SYNC=0, time = 0.340718 sec
sync_mode=1, O_SYNC=1, time = 2.287233 sec
sync_mode=2, O_SYNC=0, time = 0.340636 sec
sync_mode=2, O_SYNC=1, time = 2.284903 sec
sync_mode=3, O_SYNC=0, time = 0.340641 sec
sync_mode=3, O_SYNC=1, time = 2.285236 sec
sync_mode=4, O_SYNC=0, time = 0.340651 sec
sync_mode=4, O_SYNC=1, time = 0.340708 sec
sync_mode=5, O_SYNC=0, time = 2.286918 sec
sync_mode=5, O_SYNC=1, time = 2.286774 sec
sync_mode=6, O_SYNC=0, time = 2.284642 sec
sync_mode=6, O_SYNC=1, time = 2.285348 sec
sync_mode=7, O_SYNC=0, time = 2.285135 sec
sync_mode=7, O_SYNC=1, time = 2.288342 sec
clear_buf()
sync_mode=0, O_SYNC=0, time = 0.026720 sec
sync_mode=0, O_SYNC=1, time = 0.026641 sec
sync_mode=1, O_SYNC=0, time = 0.026642 sec
Bus error
root@debian-fpga:~/work/u-dma-buf#

ここでの u-dma-buf_test は、https://github.com/ikwzm/udmabuf の含まれている u-dma-buf_test.c をコンパイルしたものです。

この問題は pgprot_noncached(vm_page_prot) を pgprot_writecombine(vm_page_prot) に変更することで治りました。変更後の u-dma-buf_test 実行結果は次の通りです。

root@debian-fpga:~/work/u-dma-buf# insmod u-dma-buf.ko udmabuf0=0x100000
[ 1871.450164] u-dma-buf u-dma-buf.0: DMA mask not set
[ 1871.458503] u-dma-buf udmabuf0: driver version = 2.1.0-rc2
[ 1871.463998] u-dma-buf udmabuf0: major number   = 238
[ 1871.468970] u-dma-buf udmabuf0: minor number   = 0
[ 1871.473763] u-dma-buf udmabuf0: phys address   = 0x0000000070100000
[ 1871.480032] u-dma-buf udmabuf0: buffer size    = 1048576
[ 1871.485341] u-dma-buf udmabuf0: dma device     = u-dma-buf.0
[ 1871.490993] u-dma-buf udmabuf0: dma coherent   = 0
[ 1871.495782] u-dma-buf u-dma-buf.0: driver installed.
root@debian-fpga:~/work/u-dma-buf# ./u-dma-buf_test
phys_addr=0x70100000
size=1048576
check_buf()
sync_mode=0, O_SYNC=0, time = 0.340644 sec
sync_mode=0, O_SYNC=1, time = 0.340684 sec
sync_mode=1, O_SYNC=0, time = 0.340817 sec
sync_mode=1, O_SYNC=1, time = 2.284729 sec
sync_mode=2, O_SYNC=0, time = 0.340670 sec
sync_mode=2, O_SYNC=1, time = 2.284785 sec
sync_mode=3, O_SYNC=0, time = 0.340598 sec
sync_mode=3, O_SYNC=1, time = 2.284776 sec
sync_mode=4, O_SYNC=0, time = 0.340752 sec
sync_mode=4, O_SYNC=1, time = 0.340691 sec
sync_mode=5, O_SYNC=0, time = 2.284722 sec
sync_mode=5, O_SYNC=1, time = 2.284562 sec
sync_mode=6, O_SYNC=0, time = 2.284552 sec
sync_mode=6, O_SYNC=1, time = 2.284584 sec
sync_mode=7, O_SYNC=0, time = 2.284632 sec
sync_mode=7, O_SYNC=1, time = 2.284620 sec
clear_buf()
sync_mode=0, O_SYNC=0, time = 0.019320 sec
sync_mode=0, O_SYNC=1, time = 0.019328 sec
sync_mode=1, O_SYNC=0, time = 0.019312 sec
sync_mode=1, O_SYNC=1, time = 0.026633 sec
sync_mode=2, O_SYNC=0, time = 0.019315 sec
sync_mode=2, O_SYNC=1, time = 0.026621 sec
sync_mode=3, O_SYNC=0, time = 0.019306 sec
sync_mode=3, O_SYNC=1, time = 0.026632 sec
sync_mode=4, O_SYNC=0, time = 0.019307 sec
sync_mode=4, O_SYNC=1, time = 0.019317 sec
sync_mode=5, O_SYNC=0, time = 0.026609 sec
sync_mode=5, O_SYNC=1, time = 0.026621 sec
sync_mode=6, O_SYNC=0, time = 0.026615 sec
sync_mode=6, O_SYNC=1, time = 0.026624 sec
sync_mode=7, O_SYNC=0, time = 0.026605 sec
sync_mode=7, O_SYNC=1, time = 0.026623 sec

ですので、pgprot_noncached に変更するのは見送らせてください。

しかし、ご指摘の通り、問題があるのはわかりました。 write_combine は CPUからの書き込み要求をキャッシュラインサイズ(ARM64 では 64Byte)単位でをマージしてからメモリに書き込む方法なので、ご指摘のプログラムのようにキャッシュライン単位じゃない書き込み(72Byte書き込み)の後、すぐに読み出すとおかしくなるのかもしれません。 ですが、これって、デバイスドライバレベルで修正できるものなのかわかりません。。。

ikwzm commented 5 years ago

前言撤回します

あなたの pull request をマージさせてもらいました。ありがとうございます。

マージした理由は、Readme.md や Readme.ja.md にも書きましたが、キャッシュコヒーレンシの問題は発症した場合とても分かりにくくデバッグが困難だからです。それに悩むくらいなら、バスエラーを発生させた方がまだよいだろうと判断しました。

もしバスエラーが発生した場合は、O_SYNC によるキャッシュ無効化ではなく、ハードウェアによるキャッシュコヒーレンシ維持か、sync_for_cpu、sync_for_device によるマニュアルでのキャッシュコヒーレンシ維持を使うことを検討してください。

この度は、貴重な pull request ありがとうございました。今後ともよろしくお願いします。

iasakura commented 5 years ago

レビュー & マージありがとうございました! なるほど、memset でバスエラーが生じる問題があったのですね…。こちらでも再現させてみて、もしなにか分かったらまた報告させていただこうと思います。