RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen mi...@neli.hopto.org wrote: I look at it this way: it prevents the need for another layer of indirection: translating component tag to a destid. The destid alone is not enough. You will need an entire rio_dev object for that device anyway. Why no relation? My experience is that during debugging it's useful to have the destid directly at hand, it's just very practical. (Otherwise any drawing of a random network would need two identification numbers per drawn node: the component tag (true identification), and destid since that's what everyone uses to identify a device, what needs to programmed into the LUTs of a switch, identification in sysfs, etc.). I think we are mixing two things together here. I understand your idea but do not see how it prevents me from having one common set of access coordinates for RIO devices (the starting point of our discussion). Regardless of an implementation, having a way that ensures unified identification of switches by all processor boards is better than the current mainline implementation. Methods of forming a component tag may differ but still serve the same purpose. Personally I prefer to avoid any link of device identification to the destid because it may not be as intuitive as it seems for large systems with hot-plug. I will discuss this with some of RIO TWG guys to get their opinion on the best approach. I will make a patch that defines fields of component tag, probably just one for now - identification field. This will ensure that any method used to assign component tag (id part of it) will be compatible with RIO spec part 8 error management. As for switch identification, at this moment I still prefer replacing rswitch-switchid with ID portion of the component tag because it is very simple and does not require changes to enumeration algorithm. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Bounine, Alexandre wrote: Micha Nelissen mi...@neli.hopto.org wrote: I look at it this way: it prevents the need for another layer of indirection: translating component tag to a destid. The destid alone is not enough. You will need an entire rio_dev object for that device anyway. ?? I did not say a rio_dev object is not needed; on the contrary, I do have it. In various parts of the enumeration and routing algorithm, it would need to lookup the tag to find the destid, if the destid is in the tag then this lookup is not needed. I think we are mixing two things together here. I understand your idea but do not see how it prevents me from having one common set of access coordinates for RIO devices (the starting point of our discussion). I'm trying to argue we do not want redundant identification in the first place unless absolutely necessary. Regardless of an implementation, having a way that ensures unified identification of switches by all processor boards is better than the current mainline implementation. Well, we both know the current mainline implementation is easily improved to unique identification I proposed. Therefore this statement is misleading. Methods of forming a component tag may differ but still serve the same purpose. Personally I prefer to avoid any link of device identification to the destid because it may not be as intuitive as it seems for large systems with hot-plug. I will discuss this with some of RIO TWG guys to get their opinion on the best approach. Please do, please elaborate: may not be as intuitive ... to what? for implementation of ...? I will make a patch that defines fields of component tag, probably just one for now - identification field. This will ensure that any method used to assign component tag (id part of it) will be compatible with RIO spec part 8 error management. Again slightly misleading information. Any unique identification satisfies this requirement. To prove my point: are you going to recycle the component tags as well in case of hot-swaps, just like the destids? As for switch identification, at this moment I still prefer replacing rswitch-switchid with ID portion of the component tag because it is very simple and does not require changes to enumeration algorithm. Yes, I agree switchid = tag, that's what I use also. Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen mi...@neli.hopto.org wrote: In various parts of the enumeration and routing algorithm, it would need to lookup the tag to find the destid, if the destid is in the tag then this lookup is not needed. Let's keep this discussion within limits of the current implementation. Existing method of forming CT may be replaced when that change is justified by use. Any method that may be implemented later is good if it ensures unique identification of RIO devices. At this moment, we just need to define CT format now to avoid future conflicts of enumeration methods. Well, we both know the current mainline implementation is easily improved to unique identification I proposed. Therefore this statement is misleading. Your improvement will require assigning separate destID to any switch that has no endpoints attached. I prefer to keep changes to enumeration for later patches. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen mi...@neli.hopto.org wrote: Bounine, Alexandre wrote: If we will need to identify the same physical switch by different processors we may use the component tag which now is unique for every device. Yes, identification is the point. I think it might be confusing to have a destid *and* a component tag id which are slightly different. The destid is unambiguous (if you know whether the device is a switch or endpoint) so I think it makes sense to use that if possible. The component tag is the way to identify a RIO device (switch or endpoint). 1. it is defined by RIO spec as a register existing in both types of devices (this provides a universal access to the identification information by any processor). 2. the Error Management specification already uses the CT as a device identifier. 3. the CT value is large enough to be unique for max number of endpoints in the large system and any reasonable number of switches. BTW, I am planning to provide a patch that defines CT fields to ensure future compatibility. The destid does not exists as a HW element of switches and therefore cannot be used as a universal identification token. This actually gives me another idea: instead of using global next_switchid counter make rswitch-switchid = component_tag and switches in sysfs will look identical for every processor (or just get rid of rswitch-switchid and use component_tag directly for switches). I still prefer the destid as the single identification id. As I answered above, destid cannot be used as a universal identification token - it is a routing element. The destID has register in endpoints only to provide a packet filtering. In your patch you allocate individual destid for switches. This method has two problems: 1. The destid for the switch needs an additional mechanism to share it among processors in the RIO network, 2. It takes ID value away from the pool of available IDs, what will reduce number of IDs that can be assigned to endpoints. (NOTE: I am actually working on destID assignment scheme that will recycle destID in case of hot-swap events, i.e. if device is extracted its destID will be returned to the pool of available IDs and may be reused later for device insertion). The only case when assigning individual destid to the switch is justified is an empty switch - one without any endpoints attached to it. But that destid should be assigned to an endpoint as soon as it is attached to that switch. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Bounine, Alexandre wrote: Micha Nelissen mi...@neli.hopto.org wrote: rid of rswitch-switchid and use component_tag directly for switches). I still prefer the destid as the single identification id. In your patch you allocate individual destid for switches. This method has two problems: 1. The destid for the switch needs an additional mechanism to share it among processors in the RIO network, ? See comment for 2) 2. It takes ID value away from the pool of available IDs, what will It does not take an ID away, it shares it with a connected endpoint to that switch. The tag uses one extra bit to identify the device as a switch instead of an endpoint. This provides the information to unambiguously identify a switch from an endpoint. Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen mi...@neli.hopto.org wrote: Bounine, Alexandre wrote: 1. The destid for the switch needs an additional mechanism to share it among processors in the RIO network, ? See comment for 2) 2. It takes ID value away from the pool of available IDs, what will It does not take an ID away, it shares it with a connected endpoint to that switch. The tag uses one extra bit to identify the device as a switch instead of an endpoint. This provides the information to unambiguously identify a switch from an endpoint. OK taking away #2. But do not see how it justifies storing two values of destid. And you have just confirmed using CT for unique identification. We simply have differences in interpretation of CT: you are using component tag to pass unique identification and I am using CT as a unique identification. I prefer not to assume any relationship between routing information and the component tag. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Bounine, Alexandre wrote: Micha Nelissen mi...@neli.hopto.org wrote: that switch. The tag uses one extra bit to identify the device as a switch instead of an endpoint. This provides the information to unambiguously identify a switch from an endpoint. OK taking away #2. But do not see how it justifies storing two values of destid. I look at it this way: it prevents the need for another layer of indirection: translating component tag to a destid. And you have just confirmed using CT for unique identification. That's correct, but I never said (intended to say) I didn't. We simply have differences in interpretation of CT: you are using component tag to pass unique identification and I am using CT as a unique identification. I prefer not to assume any relationship between routing information and the component tag. Why no relation? My experience is that during debugging it's useful to have the destid directly at hand, it's just very practical. (Otherwise any drawing of a random network would need two identification numbers per drawn node: the component tag (true identification), and destid since that's what everyone uses to identify a device, what needs to programmed into the LUTs of a switch, identification in sysfs, etc.). Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen mi...@neli.hopto.org wrote: Alexandre Bounine wrote: 1. Using one storage location common for switches and endpoints eliminates unnecessary device type checks during maintenance access operations. While destination IDs and hop counts have different meaning for endpoints and switches, this does not prevent us from storing them in the primary RIO device structure (rio_dev) for both types. How can you say this? The two variables have different meanings, this logically implies you can't merge them. So how do you say 'this does not prevent us from ...' without providing a reason? Looks like I formulated it bad - better would be: they have different interpretation by hardware but logically in RapidIO they have single role - destid/hopcount are a device coordinates in the RIO network used to access that device. 2. Convert RIO switch device structures (rio_dev + rio_switch) into single allocation unit. This change is based on the fact that RIO switches are using common RIO device objects anyway. Allocating RIO switch objects as RIO devices with added space for switch information simplifies handling of RIO switch device objects. I still don't think that's a good idea because the rdev-rswitch pointer can be defined to point to the switch that a given rio_dev is connected to. This is useful for quick lookups. How else can to know to which switch a given device is connected? rdev-rswitch is not a pointer to the entire switch device object - it is a pointer to the switch specific extension associated with given rio_dev (if applicable). There is no other role for rdev-rswitch. Why would you keep a pointer to device data extension instead of the pointer to attached device object itself? BTW, I have back and forward links added in previous patches and only one link that may be added later is a forward link from mport to the attached rio_dev (ptr to rio_switch will not work here because it can be switchless connection). But this reference has to be added into rio_mport. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Bounine, Alexandre wrote: Micha Nelissen mi...@neli.hopto.org wrote: Alexandre Bounine wrote: How can you say this? The two variables have different meanings, this logically implies you can't merge them. So how do you say 'this does not prevent us from ...' without providing a reason? Looks like I formulated it bad - better would be: they have different interpretation by hardware but logically in RapidIO they have single role - destid/hopcount are a device coordinates in the RIO network used to access that device. They are logically different as well (for a non-host). rswitch-destid with hopcount is the way to reach that switch. rswitch-rdev-destid should be the id associated with a given switch, so that every (processor) device can agree what id some switch has. For a non-host, the path to reach a switch may use a different id than the switch itself has; it's just the id by which it was discovered. However, it's possible to fix that by fixing the id+hopcount once the switch is found using the path with its own id: then you know the right hopcount. can be defined to point to the switch that a given rio_dev is connected to. This is useful for quick lookups. How else can to know to which switch a given device is connected? rdev-rswitch is not a pointer to the entire switch device object - it is a pointer to the switch specific extension associated with given rio_dev (if applicable). There is no other role for rdev-rswitch. I know this, it doesn't answer my question. Why would you keep a pointer to device data extension instead of the pointer to attached device object itself? There is no particular reason, but this is a useful way to define the fields that are there. My point is, now that you remove the pointer field, that information (to which switch is a particular device connected) cannot be stored in this way, so do you have an alternative proposal for that? Maybe add a new field. BTW, I have back and forward links added in previous patches and only one link that may be added later is a forward link from mport to the attached rio_dev (ptr to rio_switch will not work here because it can be switchless connection). But this reference has to be added into rio_mport. Possible, but I suggest to put it in the rio_net: fields rdev_host, and rdev_self. You can see it in the patch I sent you. Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Micha Nelissen mi...@neli.hopto.org wrote: Bounine, Alexandre wrote: Looks like I formulated it bad - better would be: they have different interpretation by hardware but logically in RapidIO they have single role - destid/hopcount are a device coordinates in the RIO network used to access that device. They are logically different as well (for a non-host). rswitch-destid with hopcount is the way to reach that switch. OK. This is moved to rdev-destid now to make access unified with endpoints. rswitch-rdev-destid should be the id associated with a given switch, so that every (processor) device can agree what id some switch has. For a non-host, the path to reach a switch may use a different id than the switch itself has; it's just the id by which it was discovered. However, it's possible to fix that by fixing the id+hopcount once the switch is found using the path with its own id: then you know the right hopcount. I have got an impression that we are discussing slightly different implementations here. The suggested role of rswitch-rdev-destid is not clear to me. I agree that destid and hopcount for switch will be different for every processor. There is nothing wrong with it because a switch physically does not have its own ID. If we will need to identify the same physical switch by different processors we may use the component tag which now is unique for every device. This actually gives me another idea: instead of using global next_switchid counter make rswitch-switchid = component_tag and switches in sysfs will look identical for every processor (or just get rid of rswitch-switchid and use component_tag directly for switches). can be defined to point to the switch that a given rio_dev is connected to. This is useful for quick lookups. How else can to know to which switch a given device is connected? rdev-rswitch is not a pointer to the entire switch device object - it is a pointer to the switch specific extension associated with given rio_dev (if applicable). There is no other role for rdev-rswitch. I know this, it doesn't answer my question. Why would you keep a pointer to device data extension instead of the pointer to attached device object itself? There is no particular reason, but this is a useful way to define the fields that are there. My point is, now that you remove the pointer field, that information (to which switch is a particular device connected) cannot be stored in this way, so do you have an alternative proposal for that? Maybe add a new field. See my comment below ;). BTW, I have back and forward links added in previous patches and only one link that may be added later is a forward link from mport to the attached rio_dev (ptr to rio_switch will not work here because it can be switchless connection). But this reference has to be added into rio_mport. Possible, but I suggest to put it in the rio_net: fields rdev_host, and rdev_self. You can see it in the patch I sent you. Yes, we may rework rio_net that way and use some good things from there. Alex. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Bounine, Alexandre wrote: Micha Nelissen mi...@neli.hopto.org wrote: rswitch-rdev-destid should be the id associated with a given switch, so that every (processor) device can agree what id some switch has. If we will need to identify the same physical switch by different processors we may use the component tag which now is unique for every device. Yes, identification is the point. I think it might be confusing to have a destid *and* a component tag id which are slightly different. The destid is unambiguous (if you know whether the device is a switch or endpoint) so I think it makes sense to use that if possible. This actually gives me another idea: instead of using global next_switchid counter make rswitch-switchid = component_tag and switches in sysfs will look identical for every processor (or just get rid of rswitch-switchid and use component_tag directly for switches). I still prefer the destid as the single identification id. Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH -mm 0/2] RapidIO: Changes to handling of RIO switches
Alexandre Bounine wrote: 1. Using one storage location common for switches and endpoints eliminates unnecessary device type checks during maintenance access operations. While destination IDs and hop counts have different meaning for endpoints and switches, this does not prevent us from storing them in the primary RIO device structure (rio_dev) for both types. How can you say this? The two variables have different meanings, this logically implies you can't merge them. So how do you say 'this does not prevent us from ...' without providing a reason? 2. Convert RIO switch device structures (rio_dev + rio_switch) into single allocation unit. This change is based on the fact that RIO switches are using common RIO device objects anyway. Allocating RIO switch objects as RIO devices with added space for switch information simplifies handling of RIO switch device objects. I still don't think that's a good idea because the rdev-rswitch pointer can be defined to point to the switch that a given rio_dev is connected to. This is useful for quick lookups. How else can to know to which switch a given device is connected? Micha ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev