Message ID | 20200825184836.1282371-1-alistair.francis@wdc.com |
---|---|
State | New |
Headers | show |
On Tue, Aug 25, 2020 at 2:24 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 25 Aug 2020 at 20:01, Alistair Francis <alistair.francis@wdc.com> wrote: > > > > The following changes since commit 7774e403f2ac58b3e87bfe8d2f77676501ba893e: > > > > Merge remote-tracking branch 'remotes/kraxel/tags/fixes-20200825-pull-request' into staging (2020-08-25 10:54:51 +0100) > > > > are available in the Git repository at: > > > > git@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200825 > > > > for you to fetch changes up to e39a8320b088dd5efc9ebaafe387e52b3d962665: > > > > target/riscv: Support the Virtual Instruction fault (2020-08-25 09:11:36 -0700) > > > > ---------------------------------------------------------------- > > This pull request first adds support for multi-socket NUMA RISC-V > > machines. The Spike and Virt machines both support NUMA sockets. > > > > This PR also updates the current experimental Hypervisor support to the > > v0.6.1 spec. > > > > ---------------------------------------------------------------- > > The hypervisor related patches don't seem to have any > reviewed-by tags, which seems a shame for a fairly significant > chunk of work. Is there really nobody who can review them > for you ? Unfortunately not. They have been on the list since April and haven't received any feedback. There isn't a lot of people reviewing the RISC-V patches unfortunately. Alistair > > thanks > -- PMM
On Tue, 25 Aug 2020 at 20:01, Alistair Francis <alistair.francis@wdc.com> wrote: > > The following changes since commit 7774e403f2ac58b3e87bfe8d2f77676501ba893e: > > Merge remote-tracking branch 'remotes/kraxel/tags/fixes-20200825-pull-request' into staging (2020-08-25 10:54:51 +0100) > > are available in the Git repository at: > > git@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200825 > > for you to fetch changes up to e39a8320b088dd5efc9ebaafe387e52b3d962665: > > target/riscv: Support the Virtual Instruction fault (2020-08-25 09:11:36 -0700) > > ---------------------------------------------------------------- > This pull request first adds support for multi-socket NUMA RISC-V > machines. The Spike and Virt machines both support NUMA sockets. > > This PR also updates the current experimental Hypervisor support to the > v0.6.1 spec. > > ---------------------------------------------------------------- The hypervisor related patches don't seem to have any reviewed-by tags, which seems a shame for a fairly significant chunk of work. Is there really nobody who can review them for you ? thanks -- PMM
On Tue, 25 Aug 2020 at 22:32, Alistair Francis <alistair23@gmail.com> wrote: > > On Tue, Aug 25, 2020 at 2:24 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > The hypervisor related patches don't seem to have any > > reviewed-by tags, which seems a shame for a fairly significant > > chunk of work. Is there really nobody who can review them > > for you ? > > Unfortunately not. They have been on the list since April and haven't > received any feedback. > > There isn't a lot of people reviewing the RISC-V patches unfortunately. :-( I'd hoped it was a more active target than that. -- PMM
On Tue, Aug 25, 2020 at 2:50 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 25 Aug 2020 at 22:32, Alistair Francis <alistair23@gmail.com> wrote: > > > > On Tue, Aug 25, 2020 at 2:24 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > > The hypervisor related patches don't seem to have any > > > reviewed-by tags, which seems a shame for a fairly significant > > > chunk of work. Is there really nobody who can review them > > > for you ? > > > > Unfortunately not. They have been on the list since April and haven't > > received any feedback. > > > > There isn't a lot of people reviewing the RISC-V patches unfortunately. > > :-( I'd hoped it was a more active target than that. There are lots of active contributors, we are just short on reviewers. Richard and Philippe review patches and some of the RISC-V patches get reviewed by the RISC-V community. The main problem (which is a common problem in open source) is that large technical patch series just get ignored. Alistair > > -- PMM
On Wed, Aug 26, 2020 at 6:41 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Tue, Aug 25, 2020 at 2:50 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > > > On Tue, 25 Aug 2020 at 22:32, Alistair Francis <alistair23@gmail.com> wrote: > > > > > > On Tue, Aug 25, 2020 at 2:24 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > > > The hypervisor related patches don't seem to have any > > > > reviewed-by tags, which seems a shame for a fairly significant > > > > chunk of work. Is there really nobody who can review them > > > > for you ? > > > > > > Unfortunately not. They have been on the list since April and haven't > > > received any feedback. > > > > > > There isn't a lot of people reviewing the RISC-V patches unfortunately. > > > > :-( I'd hoped it was a more active target than that. > > There are lots of active contributors, we are just short on reviewers. > > Richard and Philippe review patches and some of the RISC-V patches get > reviewed by the RISC-V community. The main problem (which is a common > problem in open source) is that large technical patch series just get > ignored. Yep, I am only comfortable reviewing patches which I have confidence in. Right now I am not working on any H- or V - extension for RISC-V so I cannot contribute to any review of these large numbers of H- or V- extension related patches. Sorry! Regards, Bin
On Wed, 26 Aug 2020 at 04:21, Bin Meng <bmeng.cn@gmail.com> wrote: > On Wed, Aug 26, 2020 at 6:41 AM Alistair Francis <alistair23@gmail.com> wrote: > > Richard and Philippe review patches and some of the RISC-V patches get > > reviewed by the RISC-V community. The main problem (which is a common > > problem in open source) is that large technical patch series just get > > ignored. > > Yep, I am only comfortable reviewing patches which I have confidence > in. Right now I am not working on any H- or V - extension for RISC-V > so I cannot contribute to any review of these large numbers of H- or > V- extension related patches. Sorry! So, everybody has a ton of work they need to do and only a limited amount of time they might have for code review, so it's important to prioritise. But I would encourage you, and other people contributing to RISC-V parts of QEMU, to at least sometimes review changes that are a little bit out of your "comfort zone" if nobody else seems to be doing so. Review can find bugs, areas that are confusing or need comments, etc, even without a thorough knowledge of the relevant spec. (In fact, not knowing the spec can help in identifying where explanatory comments can help the reader!) And for the project it means we have more people who at least have some idea of what that bit of code is doing. Review that is limited to "this code seems to make sense but I haven't checked it against the spec" is better than patches getting no review at all, I think. And it's a good way to build your knowledge of the codebase and the architecture over time. thanks -- PMM
On Tue, 25 Aug 2020 at 20:01, Alistair Francis <alistair.francis@wdc.com> wrote: > > The following changes since commit 7774e403f2ac58b3e87bfe8d2f77676501ba893e: > > Merge remote-tracking branch 'remotes/kraxel/tags/fixes-20200825-pull-request' into staging (2020-08-25 10:54:51 +0100) > > are available in the Git repository at: > > git@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200825 > > for you to fetch changes up to e39a8320b088dd5efc9ebaafe387e52b3d962665: > > target/riscv: Support the Virtual Instruction fault (2020-08-25 09:11:36 -0700) > > ---------------------------------------------------------------- > This pull request first adds support for multi-socket NUMA RISC-V > machines. The Spike and Virt machines both support NUMA sockets. > > This PR also updates the current experimental Hypervisor support to the > v0.6.1 spec. > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2 for any user-visible changes. -- PMM
Hi Peter, On Wed, Aug 26, 2020 at 5:25 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Wed, 26 Aug 2020 at 04:21, Bin Meng <bmeng.cn@gmail.com> wrote: > > On Wed, Aug 26, 2020 at 6:41 AM Alistair Francis <alistair23@gmail.com> wrote: > > > Richard and Philippe review patches and some of the RISC-V patches get > > > reviewed by the RISC-V community. The main problem (which is a common > > > problem in open source) is that large technical patch series just get > > > ignored. > > > > Yep, I am only comfortable reviewing patches which I have confidence > > in. Right now I am not working on any H- or V - extension for RISC-V > > so I cannot contribute to any review of these large numbers of H- or > > V- extension related patches. Sorry! > > So, everybody has a ton of work they need to do and only a limited > amount of time they might have for code review, so it's important to > prioritise. But I would encourage you, and other people contributing > to RISC-V parts of QEMU, to at least sometimes review changes that are > a little bit out of your "comfort zone" if nobody else seems to be > doing so. Review can find bugs, areas that are confusing or need > comments, etc, even without a thorough knowledge of the relevant spec. > (In fact, not knowing the spec can help in identifying where > explanatory comments can help the reader!) And for the project it means > we have more people who at least have some idea of what that bit of code > is doing. Review that is limited to "this code seems to make sense but > I haven't checked it against the spec" is better than patches getting > no review at all, I think. And it's a good way to build your knowledge > of the codebase and the architecture over time. Agree. I really wanted to spend more time on this project but like you said it's priorities. One thing I do not understand is that according to MAINTAINTERS there are 4 custodians for the RISC-V maintenance work but it looks to me so far only Alistair is actively reviewing patches. I know Palmer used to review patches but if it's only one person that might be some issues. At least MAINTAINTERS can cross-review, and we have 4 there. Regards, Bin
On Wed, Aug 26, 2020 at 3:06 AM Bin Meng <bmeng.cn@gmail.com> wrote: > > Hi Peter, > > On Wed, Aug 26, 2020 at 5:25 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > > > On Wed, 26 Aug 2020 at 04:21, Bin Meng <bmeng.cn@gmail.com> wrote: > > > On Wed, Aug 26, 2020 at 6:41 AM Alistair Francis <alistair23@gmail.com> wrote: > > > > Richard and Philippe review patches and some of the RISC-V patches get > > > > reviewed by the RISC-V community. The main problem (which is a common > > > > problem in open source) is that large technical patch series just get > > > > ignored. > > > > > > Yep, I am only comfortable reviewing patches which I have confidence > > > in. Right now I am not working on any H- or V - extension for RISC-V > > > so I cannot contribute to any review of these large numbers of H- or > > > V- extension related patches. Sorry! > > > > So, everybody has a ton of work they need to do and only a limited > > amount of time they might have for code review, so it's important to > > prioritise. But I would encourage you, and other people contributing > > to RISC-V parts of QEMU, to at least sometimes review changes that are > > a little bit out of your "comfort zone" if nobody else seems to be > > doing so. Review can find bugs, areas that are confusing or need > > comments, etc, even without a thorough knowledge of the relevant spec. > > (In fact, not knowing the spec can help in identifying where > > explanatory comments can help the reader!) And for the project it means > > we have more people who at least have some idea of what that bit of code > > is doing. Review that is limited to "this code seems to make sense but > > I haven't checked it against the spec" is better than patches getting > > no review at all, I think. And it's a good way to build your knowledge > > of the codebase and the architecture over time. > > Agree. I really wanted to spend more time on this project but like you > said it's priorities. > > One thing I do not understand is that according to MAINTAINTERS there > are 4 custodians for the RISC-V maintenance work but it looks to me so > far only Alistair is actively reviewing patches. I know Palmer used to > review patches but if it's only one person that might be some issues. > At least MAINTAINTERS can cross-review, and we have 4 there. Yeah, most of the people in the RISC-V MAINTAINERS file are inactive. Besides Palmer and myself I haven't seen an email from anyone. Alistair > > Regards, > Bin
On 2020/8/26 6:30, Alistair Francis wrote: > On Tue, Aug 25, 2020 at 2:50 PM Peter Maydell <peter.maydell@linaro.org> wrote: >> On Tue, 25 Aug 2020 at 22:32, Alistair Francis <alistair23@gmail.com> wrote: >>> On Tue, Aug 25, 2020 at 2:24 PM Peter Maydell <peter.maydell@linaro.org> wrote: >>>> The hypervisor related patches don't seem to have any >>>> reviewed-by tags, which seems a shame for a fairly significant >>>> chunk of work. Is there really nobody who can review them >>>> for you ? >>> Unfortunately not. They have been on the list since April and haven't >>> received any feedback. >>> >>> There isn't a lot of people reviewing the RISC-V patches unfortunately. >> :-( I'd hoped it was a more active target than that. > There are lots of active contributors, we are just short on reviewers. > > Richard and Philippe review patches and some of the RISC-V patches get > reviewed by the RISC-V community. The main problem (which is a common > problem in open source) is that large technical patch series just get > ignored. Hi Alistair, It's really a pity. I will review every patch that CC me in no more than a week if no other people reviewed this patch. So if there too many patches, just ease to CC me. Best Regards, Zhiwei > > Alistair > >> -- PMM
On Sat, Aug 29, 2020 at 8:50 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote: > > > > On 2020/8/26 6:30, Alistair Francis wrote: > > On Tue, Aug 25, 2020 at 2:50 PM Peter Maydell <peter.maydell@linaro.org> wrote: > >> On Tue, 25 Aug 2020 at 22:32, Alistair Francis <alistair23@gmail.com> wrote: > >>> On Tue, Aug 25, 2020 at 2:24 PM Peter Maydell <peter.maydell@linaro.org> wrote: > >>>> The hypervisor related patches don't seem to have any > >>>> reviewed-by tags, which seems a shame for a fairly significant > >>>> chunk of work. Is there really nobody who can review them > >>>> for you ? > >>> Unfortunately not. They have been on the list since April and haven't > >>> received any feedback. > >>> > >>> There isn't a lot of people reviewing the RISC-V patches unfortunately. > >> :-( I'd hoped it was a more active target than that. > > There are lots of active contributors, we are just short on reviewers. > > > > Richard and Philippe review patches and some of the RISC-V patches get > > reviewed by the RISC-V community. The main problem (which is a common > > problem in open source) is that large technical patch series just get > > ignored. > Hi Alistair, > > It's really a pity. > > I will review every patch that CC me in no more than a week if no other > people reviewed this patch. > > So if there too many patches, just ease to CC me. Thanks! If you want to review more it's also a good idea to sign up to the RISC-V QEMU mailing list. That way you can keep an eye on all patches and start with reviewing ones that are interesting to you. Alistair > > > Best Regards, > Zhiwei > > > > Alistair > > > >> -- PMM >