Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
On Wed, 13 Feb 2019, Rich Persaud wrote: > On Feb 4, 2019, at 13:22, Stefano Stabellini wrote: > > On Mon, 4 Feb 2019, Roger Pau Monné wrote: > Yes, v7 was sent to address Jan and Julien's review > comments in parallel > > with our ongoing discussion on v5 macros. v7 also provided > a checkpoint > > for Argo testers to maximize test coverage as the series > converges into > > a Xen 4.12 merge candidate for Juergen. It addressed: > > > - Jan's v6 review comments > > - Julien's v1 review comment > > - most of your xen-devel and offline review comments > > > I think it will benefit the community to give this review in > public, > > so other reviewers know whats going on. IMO getting this private > > review makes it harder for me (as a reviewer) to know the > motivation > > of some of the changes between versions, and likely also makes it > > harder for you since you have to keep track of comments from > multiple > > sources on different channels. > > > There is one more reason to require public comments which I have only > learned recently: for safety certifications we need to keep a record of > all review comments and patches that address them for traceability. > > > Do you mean: > > (A) all _merged_ patches and their review comments > > or > > (B) all comments and patches (merged or not) that address them > > i.e. would the certification process be seeking traceability of > safety-impacting patches (code, scenario A) or decisions > (including decisions to leave code unchanged, scenario B)? I meant (A), however, I don't know specifically if anything from (B) is also required. > If you mean (B), would we need an update to the Xen Security Problem Response > Process [1]? e.g. public archive of all comments > from pre-disclosure discussion, along with content hashes stored immutably? > Rich > > [1] https://www.xenproject.org/security-policy.html I don't think the archives of the pre-disclosure security discussions need to be public, but they probably need to be available.___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
> On Feb 4, 2019, at 05:07, Roger Pau Monné wrote: > >> On Sun, Feb 03, 2019 at 10:04:29AM -0800, Christopher Clark wrote: >> On Thu, Jan 31, 2019 at 5:39 AM Roger Pau Monné >> wrote: >> >> On Wed, Jan 30, 2019 at 08:05:30PM -0800, Christopher Clark wrote: >> On Tue, Jan 22, 2019 at 6:19 AM Roger Pau Monné >> wrote: >> >> On Mon, Jan 21, 2019 at 01:59:40AM -0800, Christopher Clark wrote: >> Version five of this patch series: >> >> * Changes are primarily addressing feedback from the v4 series reviews. >> Many points noted on the invididual commit posts. >> >> * Critical sections have been shrunk, with allocations and frees >> pulled outside where possible, reordering logic within hypercall ops. >> >> * A new ring hash function implemented, derived from the djb2 string >> hash function. >> >> * Flags returned by the notify op have been simplified. >> >> * Now uses a single argo boot parameter, taking a list: >> - top level boolean to enable/disable Argo >> - mac-permissive option to enable/disable wildcard rings >> - command line doc edit: no "CONFIG_ARGO" but refers to build config >> >> * Switched to use the standard list data structures used by Xen's >> common code. > > AFAIK this was not requested by any reviewer, so I wonder why you made > such change. The more that you open coded some of the list_ macros > instead of just doing a s/hlist_/list_/ replacement. > I'm fine with using list instead of hlist, At your request, v7 replaces open coding with Xen's list macros. The hlist macros were not used by any of the common code in Xen. > but I don't understand why > you decided to open code list_for_each and list_for_each_safe instead > of using the macros provided by Xen. Is there an issue with such > macros? As discussed offline: - Using Xen's list macros will expedite Argo's merge for Xen 4.12 - List macros in Xen list.h originated in Linux list.h and have diverged - OpenXT has use cases for measured launch and nested virtualization, which influence downstream performance and security requirements for Argo and Xen - OpenXT can temporarily patch Xen 4.12 for downstream use > I've made a couple of minor comments, but I think the current status > is good, and fixing those minor comments is going to be trivial. Ack, thanks. Hopefully v7 looks good. >>> >>> As a note, the common flow of interactions usually involves the >>> contributor replying to the comments made by the reviewer in order to >>> try to reach an agreement before sending a new version. >> >> Yes, v7 was sent to address Jan and Julien's review comments in parallel >> with our ongoing discussion on v5 macros. v7 also provided a checkpoint >> for Argo testers to maximize test coverage as the series converges into >> a Xen 4.12 merge candidate for Juergen. It addressed: >> >> - Jan's v6 review comments >> - Julien's v1 review comment >> - most of your xen-devel and offline review comments > > I think it will benefit the community to give this review in public, > so other reviewers know whats going on. IMO getting this private > review makes it harder for me (as a reviewer) to know the motivation > of some of the changes between versions, and likely also makes it > harder for you since you have to keep track of comments from multiple > sources on different channels. > > Is there anything that prevents those people from making the review > comments publicly on xen-devel? > > We should very much try to fix that so everyone can make review > comments on the public mailing list. I've advocated for open-source principles in several large organizations. At XenSource and Citrix, we created organizational separation between the OSS Xen dev team and product teams. I don't know if that structure remains today, but it was once helpful in reducing conflict between public OSS and private product roadmaps. The separation between server and client Xen product teams was less ideal, which eventually lead to OpenXT. Six years after v4v was posted to xen-devel, Xen Argo is the first step to possible reunification, a small chance at reversal, via public open-source, of architectural and resource fragmentation that took place privately. Like QubesOS, OpenXT (and predecessor Citrix XenClient) development is spread across many open-source projects, including Xen, enabling user workflows that balance hardware-assisted security with usability. Spanning ecosystems, OpenXT is: - unbundling OSS capabilities, e.g. TrenchBoot and coreboot for launch integrity - moving code upstream (Argo, stubdom, blktap, Qemu, OpenEmbedded meta-virt) - refactoring for peer & downstream derivatives, on client devices and beyond To achieve this cross-community integration, we work with many stakeholders
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
> On Feb 4, 2019, at 13:22, Stefano Stabellini wrote: > > On Mon, 4 Feb 2019, Roger Pau Monné wrote: >>> Yes, v7 was sent to address Jan and Julien's review comments in parallel >>> with our ongoing discussion on v5 macros. v7 also provided a checkpoint >>> for Argo testers to maximize test coverage as the series converges into >>> a Xen 4.12 merge candidate for Juergen. It addressed: >>> >>> - Jan's v6 review comments >>> - Julien's v1 review comment >>> - most of your xen-devel and offline review comments >> >> I think it will benefit the community to give this review in public, >> so other reviewers know whats going on. IMO getting this private >> review makes it harder for me (as a reviewer) to know the motivation >> of some of the changes between versions, and likely also makes it >> harder for you since you have to keep track of comments from multiple >> sources on different channels. > > There is one more reason to require public comments which I have only > learned recently: for safety certifications we need to keep a record of > all review comments and patches that address them for traceability. Do you mean: (A) all _merged_ patches and their review comments or (B) all comments and patches (merged or not) that address them i.e. would the certification process be seeking traceability of safety-impacting patches (code, scenario A) or decisions (including decisions to leave code unchanged, scenario B)? If you mean (B), would we need an update to the Xen Security Problem Response Process [1]? e.g. public archive of all comments from pre-disclosure discussion, along with content hashes stored immutably? Rich [1] https://www.xenproject.org/security-policy.html ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
Wrong 'To:' field in the previous email, sorry. Gentle ping on the questions below. On Mon, Feb 04, 2019 at 11:07:12AM +0100, Roger Pau Monné wrote: > On Sun, Feb 03, 2019 at 10:04:29AM -0800, Christopher Clark wrote: > > On Thu, Jan 31, 2019 at 5:39 AM Roger Pau Monné > > wrote: > > > > > > On Wed, Jan 30, 2019 at 08:05:30PM -0800, Christopher Clark wrote: > > > > On Tue, Jan 22, 2019 at 6:19 AM Roger Pau Monné > > > > wrote: > > > > > > > > > > On Mon, Jan 21, 2019 at 01:59:40AM -0800, Christopher Clark wrote: > > > > As discussed offline: > > > > > > > > - Using Xen's list macros will expedite Argo's merge for Xen 4.12 > > > > - List macros in Xen list.h originated in Linux list.h and have diverged > > > > - OpenXT has use cases for measured launch and nested virtualization, > > > > which influence downstream performance and security requirements for > > > > Argo and Xen > > FWIW, I don't see the connection between nested virtualization or > measured launch and the list macros. I think a little bit more context > would be helpful here in order to understand the issue. > > > > > - OpenXT can temporarily patch Xen 4.12 for downstream use > > Patching the macros for OpenXT is perfectly fine, but it would be > better to understand and fix the problem upstream if possible. > > How are you patching the macros? > > What are you trying to achieve by patching them? > > Thanks, Roger. > > ___ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
Gentle ping on the questions below. On Mon, Feb 04, 2019 at 11:07:12AM +0100, Roger Pau Monné wrote: > On Sun, Feb 03, 2019 at 10:04:29AM -0800, Christopher Clark wrote: > > On Thu, Jan 31, 2019 at 5:39 AM Roger Pau Monné > > wrote: > > > > > > On Wed, Jan 30, 2019 at 08:05:30PM -0800, Christopher Clark wrote: > > > > On Tue, Jan 22, 2019 at 6:19 AM Roger Pau Monné > > > > wrote: > > > > > > > > > > On Mon, Jan 21, 2019 at 01:59:40AM -0800, Christopher Clark wrote: > > > > As discussed offline: > > > > > > > > - Using Xen's list macros will expedite Argo's merge for Xen 4.12 > > > > - List macros in Xen list.h originated in Linux list.h and have diverged > > > > - OpenXT has use cases for measured launch and nested virtualization, > > > > which influence downstream performance and security requirements for > > > > Argo and Xen > > FWIW, I don't see the connection between nested virtualization or > measured launch and the list macros. I think a little bit more context > would be helpful here in order to understand the issue. > > > > > - OpenXT can temporarily patch Xen 4.12 for downstream use > > Patching the macros for OpenXT is perfectly fine, but it would be > better to understand and fix the problem upstream if possible. > > How are you patching the macros? > > What are you trying to achieve by patching them? > > Thanks, Roger. > > ___ > Xen-devel mailing list > Xen-devel@lists.xenproject.org > https://lists.xenproject.org/mailman/listinfo/xen-devel ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
On Mon, 4 Feb 2019, Roger Pau Monné wrote: > > Yes, v7 was sent to address Jan and Julien's review comments in parallel > > with our ongoing discussion on v5 macros. v7 also provided a checkpoint > > for Argo testers to maximize test coverage as the series converges into > > a Xen 4.12 merge candidate for Juergen. It addressed: > > > > - Jan's v6 review comments > > - Julien's v1 review comment > > - most of your xen-devel and offline review comments > > I think it will benefit the community to give this review in public, > so other reviewers know whats going on. IMO getting this private > review makes it harder for me (as a reviewer) to know the motivation > of some of the changes between versions, and likely also makes it > harder for you since you have to keep track of comments from multiple > sources on different channels. There is one more reason to require public comments which I have only learned recently: for safety certifications we need to keep a record of all review comments and patches that address them for traceability.___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
On Sun, Feb 03, 2019 at 10:04:29AM -0800, Christopher Clark wrote: > On Thu, Jan 31, 2019 at 5:39 AM Roger Pau Monné wrote: > > > > On Wed, Jan 30, 2019 at 08:05:30PM -0800, Christopher Clark wrote: > > > On Tue, Jan 22, 2019 at 6:19 AM Roger Pau Monné > > > wrote: > > > > > > > > On Mon, Jan 21, 2019 at 01:59:40AM -0800, Christopher Clark wrote: > > > > > Version five of this patch series: > > > > > > > > > > * Changes are primarily addressing feedback from the v4 series > > > > > reviews. > > > > > Many points noted on the invididual commit posts. > > > > > > > > > > * Critical sections have been shrunk, with allocations and frees > > > > > pulled outside where possible, reordering logic within hypercall > > > > > ops. > > > > > > > > > > * A new ring hash function implemented, derived from the djb2 string > > > > > hash function. > > > > > > > > > > * Flags returned by the notify op have been simplified. > > > > > > > > > > * Now uses a single argo boot parameter, taking a list: > > > > > - top level boolean to enable/disable Argo > > > > > - mac-permissive option to enable/disable wildcard rings > > > > > - command line doc edit: no "CONFIG_ARGO" but refers to build config > > > > > > > > > > * Switched to use the standard list data structures used by Xen's > > > > > common code. > > > > > > > > AFAIK this was not requested by any reviewer, so I wonder why you made > > > > such change. The more that you open coded some of the list_ macros > > > > instead of just doing a s/hlist_/list_/ replacement. > > > > I'm fine with using list instead of hlist, > > > > > > At your request, v7 replaces open coding with Xen's list macros. The > > > hlist macros were not used by any of the common code in Xen. > > > > > > > but I don't understand why > > > > you decided to open code list_for_each and list_for_each_safe instead > > > > of using the macros provided by Xen. Is there an issue with such > > > > macros? > > > > > > As discussed offline: > > > > > > - Using Xen's list macros will expedite Argo's merge for Xen 4.12 > > > - List macros in Xen list.h originated in Linux list.h and have diverged > > > - OpenXT has use cases for measured launch and nested virtualization, > > > which influence downstream performance and security requirements for > > > Argo and Xen > > > - OpenXT can temporarily patch Xen 4.12 for downstream use > > > > > > > I've made a couple of minor comments, but I think the current status > > > > is good, and fixing those minor comments is going to be trivial. > > > > > > Ack, thanks. Hopefully v7 looks good. > > > > As a note, the common flow of interactions usually involves the > > contributor replying to the comments made by the reviewer in order to > > try to reach an agreement before sending a new version. > > Yes, v7 was sent to address Jan and Julien's review comments in parallel > with our ongoing discussion on v5 macros. v7 also provided a checkpoint > for Argo testers to maximize test coverage as the series converges into > a Xen 4.12 merge candidate for Juergen. It addressed: > > - Jan's v6 review comments > - Julien's v1 review comment > - most of your xen-devel and offline review comments I think it will benefit the community to give this review in public, so other reviewers know whats going on. IMO getting this private review makes it harder for me (as a reviewer) to know the motivation of some of the changes between versions, and likely also makes it harder for you since you have to keep track of comments from multiple sources on different channels. Is there anything that prevents those people from making the review comments publicly on xen-devel? We should very much try to fix that so everyone can make review comments on the public mailing list. > > There are comments from v5 that haven't been fixed in v7 > > (the mask usage and list_first_entry_or_null for example) > > and the reply to the reviewer's comment was sent at the same time as > > v7, leaving no time for further discussion (and for reaching an > > agreement suitable to both parties) before sending v7. > > Code changes from our ongoing discussion will be addressed in v8. A > proposal to address mask usage has been put forward in the parallel > thread. Your proposed usage of list_first_entry_or_null will be made in > v8, subject to the previous offline discussion about list macros > (duplicated here for convenience): > > > > As discussed offline: > > > > > > - Using Xen's list macros will expedite Argo's merge for Xen 4.12 > > > - List macros in Xen list.h originated in Linux list.h and have diverged > > > - OpenXT has use cases for measured launch and nested virtualization, > > > which influence downstream performance and security requirements for > > > Argo and Xen FWIW, I don't see the connection between nested virtualization or measured launch and the list macros. I think a little bit more context would be helpful here in order to understand the issue. > > > - OpenXT can tempo
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
On Thu, Jan 31, 2019 at 5:39 AM Roger Pau Monné wrote: > > On Wed, Jan 30, 2019 at 08:05:30PM -0800, Christopher Clark wrote: > > On Tue, Jan 22, 2019 at 6:19 AM Roger Pau Monné > > wrote: > > > > > > On Mon, Jan 21, 2019 at 01:59:40AM -0800, Christopher Clark wrote: > > > > Version five of this patch series: > > > > > > > > * Changes are primarily addressing feedback from the v4 series reviews. > > > > Many points noted on the invididual commit posts. > > > > > > > > * Critical sections have been shrunk, with allocations and frees > > > > pulled outside where possible, reordering logic within hypercall ops. > > > > > > > > * A new ring hash function implemented, derived from the djb2 string > > > > hash function. > > > > > > > > * Flags returned by the notify op have been simplified. > > > > > > > > * Now uses a single argo boot parameter, taking a list: > > > > - top level boolean to enable/disable Argo > > > > - mac-permissive option to enable/disable wildcard rings > > > > - command line doc edit: no "CONFIG_ARGO" but refers to build config > > > > > > > > * Switched to use the standard list data structures used by Xen's > > > > common code. > > > > > > AFAIK this was not requested by any reviewer, so I wonder why you made > > > such change. The more that you open coded some of the list_ macros > > > instead of just doing a s/hlist_/list_/ replacement. > > > I'm fine with using list instead of hlist, > > > > At your request, v7 replaces open coding with Xen's list macros. The > > hlist macros were not used by any of the common code in Xen. > > > > > but I don't understand why > > > you decided to open code list_for_each and list_for_each_safe instead > > > of using the macros provided by Xen. Is there an issue with such > > > macros? > > > > As discussed offline: > > > > - Using Xen's list macros will expedite Argo's merge for Xen 4.12 > > - List macros in Xen list.h originated in Linux list.h and have diverged > > - OpenXT has use cases for measured launch and nested virtualization, > > which influence downstream performance and security requirements for > > Argo and Xen > > - OpenXT can temporarily patch Xen 4.12 for downstream use > > > > > I've made a couple of minor comments, but I think the current status > > > is good, and fixing those minor comments is going to be trivial. > > > > Ack, thanks. Hopefully v7 looks good. > > As a note, the common flow of interactions usually involves the > contributor replying to the comments made by the reviewer in order to > try to reach an agreement before sending a new version. Yes, v7 was sent to address Jan and Julien's review comments in parallel with our ongoing discussion on v5 macros. v7 also provided a checkpoint for Argo testers to maximize test coverage as the series converges into a Xen 4.12 merge candidate for Juergen. It addressed: - Jan's v6 review comments - Julien's v1 review comment - most of your xen-devel and offline review comments > There are comments from v5 that haven't been fixed in v7 > (the mask usage and list_first_entry_or_null for example) > and the reply to the reviewer's comment was sent at the same time as > v7, leaving no time for further discussion (and for reaching an > agreement suitable to both parties) before sending v7. Code changes from our ongoing discussion will be addressed in v8. A proposal to address mask usage has been put forward in the parallel thread. Your proposed usage of list_first_entry_or_null will be made in v8, subject to the previous offline discussion about list macros (duplicated here for convenience): > > As discussed offline: > > > > - Using Xen's list macros will expedite Argo's merge for Xen 4.12 > > - List macros in Xen list.h originated in Linux list.h and have diverged > > - OpenXT has use cases for measured launch and nested virtualization, > > which influence downstream performance and security requirements for > > Argo and Xen > > - OpenXT can temporarily patch Xen 4.12 for downstream use Christopher ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
On Wed, Jan 30, 2019 at 08:05:30PM -0800, Christopher Clark wrote: > On Tue, Jan 22, 2019 at 6:19 AM Roger Pau Monné wrote: > > > > On Mon, Jan 21, 2019 at 01:59:40AM -0800, Christopher Clark wrote: > > > Version five of this patch series: > > > > > > * Changes are primarily addressing feedback from the v4 series reviews. > > > Many points noted on the invididual commit posts. > > > > > > * Critical sections have been shrunk, with allocations and frees > > > pulled outside where possible, reordering logic within hypercall ops. > > > > > > * A new ring hash function implemented, derived from the djb2 string > > > hash function. > > > > > > * Flags returned by the notify op have been simplified. > > > > > > * Now uses a single argo boot parameter, taking a list: > > > - top level boolean to enable/disable Argo > > > - mac-permissive option to enable/disable wildcard rings > > > - command line doc edit: no "CONFIG_ARGO" but refers to build config > > > > > > * Switched to use the standard list data structures used by Xen's > > > common code. > > > > AFAIK this was not requested by any reviewer, so I wonder why you made > > such change. The more that you open coded some of the list_ macros > > instead of just doing a s/hlist_/list_/ replacement. > > I'm fine with using list instead of hlist, > > At your request, v7 replaces open coding with Xen's list macros. The > hlist macros were not used by any of the common code in Xen. > > > but I don't understand why > > you decided to open code list_for_each and list_for_each_safe instead > > of using the macros provided by Xen. Is there an issue with such > > macros? > > As discussed offline: > > - Using Xen's list macros will expedite Argo's merge for Xen 4.12 > - List macros in Xen list.h originated in Linux list.h and have diverged > - OpenXT has use cases for measured launch and nested virtualization, > which influence downstream performance and security requirements for > Argo and Xen > - OpenXT can temporarily patch Xen 4.12 for downstream use > > > I've made a couple of minor comments, but I think the current status > > is good, and fixing those minor comments is going to be trivial. > > Ack, thanks. Hopefully v7 looks good. As a note, the common flow of interactions usually involves the contributor replying to the comments made by the reviewer in order to try to reach an agreement before sending a new version. There are comments from v5 that haven't been fixed in v7 (the mask usage and list_first_entry_or_null for example) and the reply to the reviewer's comment was sent at the same time as v7, leaving no time for further discussion (and for reaching an agreement suitable to both parties) before sending v7. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
On Tue, Jan 22, 2019 at 6:19 AM Roger Pau Monné wrote: > > On Mon, Jan 21, 2019 at 01:59:40AM -0800, Christopher Clark wrote: > > Version five of this patch series: > > > > * Changes are primarily addressing feedback from the v4 series reviews. > > Many points noted on the invididual commit posts. > > > > * Critical sections have been shrunk, with allocations and frees > > pulled outside where possible, reordering logic within hypercall ops. > > > > * A new ring hash function implemented, derived from the djb2 string > > hash function. > > > > * Flags returned by the notify op have been simplified. > > > > * Now uses a single argo boot parameter, taking a list: > > - top level boolean to enable/disable Argo > > - mac-permissive option to enable/disable wildcard rings > > - command line doc edit: no "CONFIG_ARGO" but refers to build config > > > > * Switched to use the standard list data structures used by Xen's > > common code. > > AFAIK this was not requested by any reviewer, so I wonder why you made > such change. The more that you open coded some of the list_ macros > instead of just doing a s/hlist_/list_/ replacement. > I'm fine with using list instead of hlist, At your request, v7 replaces open coding with Xen's list macros. The hlist macros were not used by any of the common code in Xen. > but I don't understand why > you decided to open code list_for_each and list_for_each_safe instead > of using the macros provided by Xen. Is there an issue with such > macros? As discussed offline: - Using Xen's list macros will expedite Argo's merge for Xen 4.12 - List macros in Xen list.h originated in Linux list.h and have diverged - OpenXT has use cases for measured launch and nested virtualization, which influence downstream performance and security requirements for Argo and Xen - OpenXT can temporarily patch Xen 4.12 for downstream use > I've made a couple of minor comments, but I think the current status > is good, and fixing those minor comments is going to be trivial. Ack, thanks. Hopefully v7 looks good. Christopher ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
On Mon, Jan 21, 2019 at 01:59:40AM -0800, Christopher Clark wrote: > Version five of this patch series: > > * Changes are primarily addressing feedback from the v4 series reviews. > Many points noted on the invididual commit posts. > > * Critical sections have been shrunk, with allocations and frees > pulled outside where possible, reordering logic within hypercall ops. > > * A new ring hash function implemented, derived from the djb2 string > hash function. > > * Flags returned by the notify op have been simplified. > > * Now uses a single argo boot parameter, taking a list: > - top level boolean to enable/disable Argo > - mac-permissive option to enable/disable wildcard rings > - command line doc edit: no "CONFIG_ARGO" but refers to build config > > * Switched to use the standard list data structures used by Xen's > common code. AFAIK this was not requested by any reviewer, so I wonder why you made such change. The more that you open coded some of the list_ macros instead of just doing a s/hlist_/list_/ replacement. I'm fine with using list instead of hlist, but I don't understand why you decided to open code list_for_each and list_for_each_safe instead of using the macros provided by Xen. Is there an issue with such macros? I've made a couple of minor comments, but I think the current status is good, and fixing those minor comments is going to be trivial. Roger. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH v5 00/15] Argo: hypervisor-mediated interdomain communication
Version five of this patch series: * Changes are primarily addressing feedback from the v4 series reviews. Many points noted on the invididual commit posts. * Critical sections have been shrunk, with allocations and frees pulled outside where possible, reordering logic within hypercall ops. * A new ring hash function implemented, derived from the djb2 string hash function. * Flags returned by the notify op have been simplified. * Now uses a single argo boot parameter, taking a list: - top level boolean to enable/disable Argo - mac-permissive option to enable/disable wildcard rings - command line doc edit: no "CONFIG_ARGO" but refers to build config * Switched to use the standard list data structures used by Xen's common code. * Further removal of uses of fixed-width types. * Added a new patch to add Argo to the MAINTAINERS file. Christopher Clark (15): argo: Introduce the Kconfig option to govern inclusion of Argo argo: introduce the argo_op hypercall boilerplate argo: define argo_dprintk for subsystem debugging argo: init, destroy and soft-reset, with enable command line opt errno: add POSIX error codes EMSGSIZE, ECONNREFUSED to the ABI xen/arm: introduce guest_handle_for_field() argo: implement the register op argo: implement the unregister op argo: implement the sendv op; evtchn: expose send_guest_global_virq argo: implement the notify op xsm, argo: XSM control for argo register xsm, argo: XSM control for argo message send operation xsm, argo: XSM control for any access to argo by a domain xsm, argo: notify: don't describe rings that cannot be sent to MAINTAINERS: add new section for Argo and self as maintainer MAINTAINERS |8 + docs/misc/xen-command-line.pandoc| 22 + tools/flask/policy/modules/guest_features.te |7 + xen/arch/x86/guest/hypercall_page.S |2 +- xen/arch/x86/hvm/hypercall.c |3 + xen/arch/x86/hypercall.c |3 + xen/arch/x86/pv/hypercall.c |3 + xen/common/Kconfig | 19 + xen/common/Makefile |3 +- xen/common/argo.c| 2281 ++ xen/common/compat/argo.c | 62 + xen/common/domain.c |9 + xen/common/event_channel.c |2 +- xen/include/Makefile |1 + xen/include/asm-arm/guest_access.h |3 + xen/include/public/argo.h| 280 xen/include/public/errno.h |2 + xen/include/public/xen.h |4 +- xen/include/xen/argo.h | 44 + xen/include/xen/event.h |7 + xen/include/xen/hypercall.h |9 + xen/include/xen/sched.h |5 + xen/include/xlat.lst |8 + xen/include/xsm/dummy.h | 25 + xen/include/xsm/xsm.h| 31 + xen/xsm/dummy.c |6 + xen/xsm/flask/hooks.c| 41 +- xen/xsm/flask/policy/access_vectors | 16 + xen/xsm/flask/policy/security_classes|1 + 29 files changed, 2899 insertions(+), 8 deletions(-) create mode 100644 xen/common/argo.c create mode 100644 xen/common/compat/argo.c create mode 100644 xen/include/public/argo.h create mode 100644 xen/include/xen/argo.h -- 2.7.4 ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel