Ack. Will have that in v2. Hao On Mon, Jul 20, 2020 at 7:37 PM Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > On Mon, Jul 20, 2020 at 1:28 PM Hao Luo <haoluo@xxxxxxxxxx> wrote: > > > > > > > > This should ideally look like a real global variable extern: > > > > > > extern const struct rq runqueues __ksym; > > > > > > > > > But that's the case for non-per-cpu variables. You didn't seem to > > > address per-CPU variables in this patch set. How did you intend to > > > handle that? We should look at a possible BPF helper to access such > > > variables as well and how the verifier will prevent direct memory > > > accesses for such variables. > > > > > > We should have some BPF helper that accepts per-CPU PTR_TO_BTF_ID, and > > > returns PTR_TO_BTF_ID, but adjusted to desired CPU. And verifier > > > ideally would allow direct memory access on that resulting > > > PTR_TO_BTF_ID, but not on per-CPU one. Not sure yet how this should > > > look like, but the verifier probably needs to know that variable > > > itself is per-cpu, no? > > > > > > > Yes, that's what I was unclear about, so I don't have that part in > > this patchset. But your explanation helped me organize my thoughts. :) > > > > Actually, the verifier can tell whether a var is percpu from the > > DATASEC, since we have encoded "percpu" DATASEC in btf. I think the > > following should work: > > > > We may introduce a new PTR_TO_BTF_VAR_ID. In ld_imm, libbpf replaces > > ksyms with btf_id. The btf id points to a KIND_VAR. If the pointed VAR > > is found in the "percpu" DATASEC, dst_reg is set to PTR_TO_BTF_VAR_ID; > > otherwise, it will be a PTR_TO_BTF_ID. For PTR_TO_BTF_VAR_ID, > > reg->btf_id is the id of the VAR. For PTR_TO_BTF_ID, reg->btf_id is > > the id of the actual kernel type. The verifier would reject direct > > memory access on PTR_TO_BTF_VAR_ID, but the new BPF helper can convert > > a PTR_TO_BTF_VAR_ID to PTR_TO_BTF_ID. > > Sounds good to me as a plan, except that PTR_TO_BTF_VAR_ID is a > misleading name. It's always a variable. The per-CPU part is crucial, > though, so maybe something like PTR_TO_PERCPU_BTF_ID? > > > > > Hao